GKeyfile changed something about how to handle invalid escape sequences.
As we don't want to test GKeyfile (per-se), just adjust to test to not
hit the problem.
This would fail with glib2-2.79.1-1.fc40:
# ./tools/run-nm-test.sh -m src/core/tests/config/test-config -p /config/set-values
TAP version 13
# random seed: R02Sb8afff1ec38ca5a1b7713e8c40eb4f56
# Start of config tests
# GLib-GIO-DEBUG: _g_io_module_get_default: Found default implementation local (GLocalVfs) for ?gio-vfs?
# (src/core/tests/config/test-config.c:1107) invalid value in config-data .intern.with-whitespace.key2 = (null) (instead of " b c\, d ")
./tools/run-nm-test.sh: line 307: 245847 Trace/breakpoint trap (core dumped) "${NMTST_DBUS_RUN_SESSION[@]}" "$TEST" "${TEST_ARGV[@]}"
exec "src/core/tests/config/test-config" failed with exit code 133
Upstream systemd fixed this compiler warning. What really needs to be
done, is re-importing the upstream code.
In the meantime, suppress the warning that hits on GCC 14.
This is a temporary workaround!
See-also: fdd84270df
When doing reapply on linux bridge interface, NetworkManager will reset
the VLAN filtering and default PVID which cause PVID been readded to all
bridge ports regardless they are managed by NetworkManager.
This is because Linux kernel will re-add PVID to bridge port upon the
changes of bridge default-pvid value.
To fix the issue, this patch introduce netlink parsing code for
`vlan_filtering` and `default_pvid` of NMPlatformLnkBridge, and use that
to compare desired VLAN filtering settings, skip the reset of VLAN
filter if `default_pvid` and `vlan_filtering` are unchanged.
Signed-off-by: Gris Ge <fge@redhat.com>
Older gcc versions don't like this. The _Pragam() itself is to workaround a
-Wnonnull-compare warning with gcc 14.
After all, we use compiler warning extensively. They are our linters and have
necessarily false positives. To make them usable across a wide range of
compilers, is a constant effort.
Here is another one.
The error:
./src/libnm-std-aux/nm-std-aux.h: In function ‘nm_utils_addr_family_other’:
./src/libnm-std-aux/nm-std-aux.h:230:36: error: expected expression before ‘#pragma’
230 | #define NM_PRAGMA_DIAGNOSTICS_PUSH _Pragma("GCC diagnostic push")
| ^~~~~~~
./src/libnm-std-aux/nm-std-aux.h:232:5: note: in expansion of macro ‘NM_PRAGMA_DIAGNOSTICS_PUSH’
232 | NM_PRAGMA_DIAGNOSTICS_PUSH _Pragma(_NM_PRAGMA_WARNING_DO(warning))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
./src/libnm-std-aux/nm-std-aux.h:291:9: note: in expansion of macro ‘NM_PRAGMA_WARNING_DISABLE’
291 | NM_PRAGMA_WARNING_DISABLE("-Wnonnull-compare"); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~
./src/libnm-std-aux/nm-std-aux.h:953:9: note: in expansion of macro ‘nm_assert’
953 | nm_assert(true || NM_UNIQ_T(xx, uniq) == (x)); \
| ^~~~~~~~~
./src/libnm-std-aux/nm-std-aux.h:961:27: note: in expansion of macro ‘_NM_IN_SET’
961 | #define NM_IN_SET(x, ...) _NM_IN_SET(NM_UNIQ, ||, typeof(x), x, __VA_ARGS__)
| ^~~~~~~~~~
./src/libnm-std-aux/nm-std-aux.h:1493:15: note: in expansion of macro ‘NM_IN_SET’
1493 | nm_assert(NM_IN_SET((addr_family), NM_AF_INET, NM_AF_INET6))
| ^~~~~~~~~
./src/libnm-std-aux/nm-std-aux.h:1502:9: note: in expansion of macro ‘nm_assert_addr_family’
1502 | nm_assert_addr_family(NM_UNIQ_T(_addr_family, uniq)); \
| ^~~~~~~~~~~~~~~~~~~~~
./src/libnm-std-aux/nm-std-aux.h:1510:33: note: in expansion of macro ‘_NM_IS_IPv4’
1510 | #define NM_IS_IPv4(addr_family) _NM_IS_IPv4(NM_UNIQ, addr_family)
| ^~~~~~~~~~~
./src/libnm-std-aux/nm-std-aux.h:1515:12: note: in expansion of macro ‘NM_IS_IPv4’
1515 | return NM_IS_IPv4(addr_family) ? NM_AF_INET6 : NM_AF_INET;
| ^~~~~~~~~~
Fixes: 62c1745f62 ('std-aux: suppress "-Wnonnull-compare" warning in nm_assert()')
With a static array, we indicate that the argument must not be NULL.
Gcc-14.0.1-0.2.fc40 now warns against that:
CC src/libnm-base/libnm_base_la-nm-base.lo
In file included from ../src/libnm-std-aux/nm-default-std.h:102,
from ../src/libnm-glib-aux/nm-default-glib.h:11,
from ../src/libnm-glib-aux/nm-default-glib-i18n-lib.h:13,
from ../src/libnm-base/nm-base.c:3:
../src/libnm-base/nm-base.c: In function 'nm_net_devname_infiniband':
../src/libnm-std-aux/nm-std-aux.h:191:12: error: 'nonnull' argument 'name' compared to NULL [-Werror=nonnull-compare]
191 | if (expr) \
| ^
../src/libnm-std-aux/nm-std-aux.h:202:27: note: in expansion of macro '_NM_BOOLEAN_EXPR_IMPL'
202 | _NM_BOOLEAN_EXPR_IMPL(NM_UNIQ, expr))
| ^~~~~~~~~~~~~~~~~~~~~
../src/libnm-glib-aux/nm-macros-internal.h:1693:31: note: in expansion of macro 'NM_BOOLEAN_EXPR'
1693 | #define _G_BOOLEAN_EXPR(expr) NM_BOOLEAN_EXPR(expr)
| ^~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gmacros.h:1244:43: note: in expansion of macro '_G_BOOLEAN_EXPR'
1244 | #define G_LIKELY(expr) (__builtin_expect (_G_BOOLEAN_EXPR(expr), 1))
| ^~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gmessages.h:656:9: note: in expansion of macro 'G_LIKELY'
656 | if (G_LIKELY (expr)) \
| ^~~~~~~~
../src/libnm-base/nm-base.c:57:5: note: in expansion of macro 'g_return_val_if_fail'
57 | g_return_val_if_fail(name, NULL);
| ^~~~~~~~~~~~~~~~~~~~
../src/libnm-core-impl/nm-setting-wireguard.c: In function '_nm_wireguard_peer_set_public_key_bin':
../src/libnm-core-impl/nm-setting-wireguard.c:316:8: error: 'nonnull' argument 'public_key' compared to NULL [-Werror=nonnull-compare]
316 | if (!public_key)
| ^
Convert these checks to an nm_assert() to suppress the warning.
When we use a "static" array declarator to a function, we understand
and tell the compiler that the argument must not be NULL.
But now gcc-14.0.1-0.2.fc40 starts warning about NULL checks for
such arguments.
static void foo(char args[static 10]) {
nm_assert(args);
sprintf(args, "hi");
}
Granted, the compiler is right, and we know that this condition is not
supposed to be violated. A logical thing would be just to drop the
assertion.
Instead, suppress "-Wnonnull-compare" warnings inside a nm_assert(). An
nm_assert() is more than a run time check, it's an additional
self-documenting code of the invariants.
It's fine to assert for something that is true. Actually, all the
conditions that we assert against, hold. The compiler telling us that
the condition that we assert against is valid, is not useful.
Otherwise, gcc-14.0.1-0.2.fc40 warns:
../src/libnm-core-impl/nm-utils.c: In function _nm_utils_strstrdictkey_create:
../src/libnm-core-impl/nm-utils.c:5076:16: error: allocation of insufficient size '1' for type 'NMUtilsStrStrDictKey' {aka 'struct _NMUtilsStrStrDictKey'} with size '2' [-Werror=alloc-size]
5076 | return g_malloc0(1);
| ^~~~~~~~~~~~
gcc-14.0.1-0.2.fc40 warns:
CC src/libnm-core-impl/libnm_core_impl_la-nm-setting-team.lo
../src/libnm-core-impl/nm-setting-team.c: In function nm_team_link_watcher_new_ethtool:
../src/libnm-core-impl/nm-setting-team.c:127:13: error: allocation of insufficient size 16 for type NMTeamLinkWatcher with size 48 [-Werror=alloc-size]
127 | watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
| ^
Section 4.9 of RFC 4594 specifies that DHCP should use the standard
(CS0 = 0) service class. Section 3.2 says that class CS6 is for
"transmitting packets between network devices (routers) that require
control (routing) information to be exchanged between nodes", listing
"OSPF, BGP, ISIS, RIP" as examples of such traffic. Furthermore, it
says that:
User traffic is not allowed to use this service class. By user
traffic, we mean packet flows that originate from user-controlled
end points that are connected to the network.
Indeed, we got reports of some Cisco switches dropping DHCP packets
because of the CS6 marking.
For these reasons, change the default value to the recommended one,
CS0.
Currently the internal DHCP client sets traffic class "CS6" in the DS
field of the IP header for outgoing packets.
dhclient sets the field according to the definition of TOS (RFC 1349),
which was was deprecated in 1998 by RFC 2474 in favor of DSCP.
Introduce a new property IPvX.dhcp-dscp (currently valid only for
IPv4) to specify a custom DSCP value for DHCP backends that support it
(currently, only the internal one).
Define the default value to CS0, because:
- section 4.9 of RFC 4594 specifies that DHCP should use the standard
(CS0 = 0) service class;
- section 3.2 says that class CS6 is for "transmitting packets
between network devices (routers) that require control (routing)
information to be exchanged between nodes", listing "OSPF, BGP,
ISIS, RIP" as examples of such traffic. Furthermore, it says that:
User traffic is not allowed to use this service class. By user
traffic, we mean packet flows that originate from user-controlled
end points that are connected to the network.
- we got reports of some Cisco switches dropping DHCP packets because
of the CS6 marking.
The client currently always sets the DSCP value in the DS field
(formerly known as "TOS") to CS6. Some network equipment drops packets
with such DSCP value; provide a way to change it.
This commit introduces the display of the global metered state in the
`nmcli general` command output.
Key changes:
- Parse and display the global metered state
Previously, input fields for peer attributes such as 'Public Key' were
not pre-populated with the existing settings of the peer. This was due
to the WireGuard peer editor class not setting its peer property during
object construction, as the necessary flag was absent. This commit
addresses and fixes this issue.
Closes#1443https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1851
In order to make _nm_setting_property_define_direct_enum more flexible,
this patch is introducing property_type argument to it. When set to NULL
it will set property_type to nm_sett_info_propert_type_direct_enum.
Defining the wrong from_dbus/to_dbus functions is something not
probable. The unit test is just getting in the way of those who knows
what they do and force contributors to change the same thing in multiple
places.
It's simply not valid to read the ref-count without an atomic.
The compiler might optimize out the assignment to "r" and read the
_ref_count field multiple times. Thereby, we might at first appear
to be larger than > 1, and later pass 1 to compare-and-exchange.
We need an atomic get here.
Fixes: 19d4027824 ('refstr: inline nm_ref_string_{ref,unref}()')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1847
This property indicates that a non-null strv array is expected, and
an empty strv array should be returned instead of NULL if it hadn't
been created yet.
The comparison checking for MAC address equality had previously been flipped around.
Fixes: b084ad7f2b ('libnm-core: canonicalize hardware addresses in settings')
The default in systemd-resolved is nowadays "yes". In any case, since
the setting is configurable systemd-resolved, don't describe it in the
manual page.
Instead, clarify the behavior and try to improve the documentation.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1467
As done for IPv4 in commit 3b145a33b7 ('vpn-connections: allow the
plugin to specify route preferred src'), allow VPN plugins to return a
preferred source address for routes. Still accept the old format.
OpenShift MetalLB team requests to configure additional routes
whenever the nodes does not have a configured IP address or route for
the subnet in which MetalLB issues addresses.
Note in linux network stack, it does not matter what interface you add
the address on a node (for example, loopback), the kernel is always
processing arp-requests and sending arp-replies to any of them, this
behavior is considered correct and, moreover, it is widely used in the a
dynamic environment as Kubernetes.
https://issues.redhat.com/browse/RHEL-5098https://gitlab.freedesktop.org/NetworkManager/NetworkManager-ci/-/merge_requests/1587
The decision to configure or not configure routes without addresses only
related to what method is configured - DHCP and non-DHCP cases. For DHCP
case, the deamon waits until addresses appear first before configuring
the static routes to preserve the behavior mentioned in
https://bugzilla.redhat.com/show_bug.cgi?id=2102212, otherwise, the
daemon can configure the routes immediately for non-DHCP case.
"nm_setting_hsr_get_port1" is new API and verify() already enforces that
the strings are not empty. The flag is redundant.
Also drop it from a few other places, where it's redundant.
Most properties don't accept empty strings and reject them during
verify().
All _nm_setting_property_define_direct_mac_address() call
nm_utils_hwaddr_valid() on the string, which rejects empty strings.
Clear the .direct_string_allow_empty flag for those. The usage of the
flag is misleading.
Most string properties should not accept empty strings. Add a generic
way to reject them during verify.
Add a new flag NMSettInfoProperty.direct_string_allow_empty.
Note that properties must opt-in to allow empty values. Since all
existing properties didn't have this check (but hopefully re-implemented
it in verify()), all existing properties get this flag set to TRUE.
The main point here it that new properties get the strict check by
default.
We should also review existing uses of direct_string_allow_empty,
whether the flag can be cleared. This can be done if verify() already
enforces a non-empty string, or if we accept to break behavior by
tightening up the check.
Current verifications happens by implementing NMSetting's verify().
Add code for a second step of validation, that can operate based on the
known type.
The use case will be to reject empty strings.
To embrace inclusive language, deprecate the NMSettingConnection
slave-type property and introduce port-type property.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
When deleting a profile, the confirmation dialog shows "Cancel" and
"Delete" buttons. ESC key should do nothing, but in some distributions
like Debian and Ubuntu newt has a downstream patch that enables it (see
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=584098).
In that case, when pressing ESC the return value of the dialog is not
"Cancel" (1) or "Delete" (2), but the "otherwise" value (0). Fix it by
not checking if "Cancel" is pressed. Instead, check if "Delete" was
pressed, and continue deleting only in that case.
Also, fix the doc comment that incorrectly says that the dialog returns
0/1 for the buttons, it is 1/2.
If a generated connection matches a connection that uses interface name
as controller, we need to drop the existing value from the settings to
avoid conflicts. Therefore, both of them need to be dropped; controller
and master.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1833
Fixes: 3e4a2ebb3c ('all: use the new NMSettingConnection Controller property')
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
