Background: when router sends router advertisement (RA) message,
NetworkManager processes it and passes data to a lower system layer.
Currently there is a problem that NetworkManager adds one second to both
valid lifetime and preferred lifetime. This happens because of the
algorithm in nm_ndisc_data_to_l3cd() function.
Let's look at an example: let current timestamp be 100450, so now_sec
variable is 100. At this moment RA message was received from the router.
The IPv6 address' valid lifetime is 200 seconds (for example), so
expiration timestamp (ndisc_addr->expiry_msec) is 300450. But after the
_nm_ndisc_lifetime_from_expiry() call, NMPlatformIP6Address lifetime
becomes 201 ((300450-(100*1000)+999)/1000). Which is wrong.
This commit fixes this behaviour by replacing
nm_utils_get_monotonic_timestamp_sec() with
nm_utils_get_monotonic_timestamp_msec() so that timestamps are
calculated more precisely.
Related issue: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1464
Merge request: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1863
(cherry picked from commit 14e7220f5f)
Depending on the type of challenge used in the 2FA authentication, the
user input doesn't need to be hidden and sometimes it's even undesired
(it makes more difficult to enter the text).
Allow to VPN plugins to indicate that a secret that is being requested
is a 2FA challenge with ECHO mode enabled:
- When using auth dialog: accept a new option "ForceEcho" that can be
set to TRUE to enable ECHO.
- When using the fallback method: recognize the prefix
"x-dynamic-challenge-echo". This indicate both that ECHO should be enabled
and that this is a 2FA challenge (see previous commit).
The correct way to enable echo mode from VPN plugins is doing both
things: pass the hint prefixed with "x-dynamic-challenge-echo" and add the
option "ForceEcho=true" for the auth dialog.
An attempt to support ECHO mode from NM-openvpn was made by passing
"IsSecret=false", but it didn't work because nm-secret-agent-simple
ignores returned values for which "IsSecret=false". It's not a good idea
to start accepting them because we could break other plugins, and anyway
the challenge response is actually a secret, so it is better to keep it
as such and add this new "ForceEcho" option.
This is backwards compatible because existing plugins were not using the
tag nor the auth dialog option. Withouth them, the previous behaviour is
preserved. On the contrary, plugins that want to use this new feature
will need to bump their NM version dependency because old daemons will
not handle correctly the prefix tag.
Secret agents will need to be updated to check secret->force_echo if
they want to support this feature. Until they update, the only drawback
is that ECHO mode will be ignored and the user's input will be hidden.
Updated nmcli and nmtui to support ECHO mode.
(cherry picked from commit 2ab56e82d4)
Clients using nm-secret-agent-simple always asked for some default VPN
secrets, which are dependent on the VPN service, when the auth dialog
can't be used and the fallback method is used instead.
When using 2FA this has to be avoided in the 2nd step because those
default secrets were already requested and validated in the 1st step.
Fix it by adding a new "x-dynamic-challenge" prefix tag that can be used
in the hints received from the VPN plugin. This tag indicates that we
are in the 2nd step of a 2FA authentication. This way we know that we
don't have to request the default secrets this time. Note that the tag
name doesn't explicitly mention VPNs so it can be reused for other type
of connections in the future.
As the default secrets were requested always unconditionally when using
the fallback method, there is no possible workaround to this problem
that avoids having to change libnm-client.
The change is backwards compatible because VPN plugins were not using
the tag and the previous behaviour does not change if the tag is not
used. However, VPN plugins that want to properly support 2FA
aunthentication will need to bump the NM version dependency because
old daemons won't handle properly a hint with the new prefix tag.
Finally, move the macro that defines the "x-vpn-message:" tag in a public
header so it is more visible for users. It has been renamed and prefixed
with the NM_ namespace so it shouldn't collide with macros defined in
the VPN plugins.
(cherry picked from commit c5f46bae43)
Probably not all drivers and devices return all parameters. Set them to
"unknown" if they are missing and let the caller to decide what to do.
In our case, if the sriov setting has a value different to "preserve" it
will try to set it (and will probably fail). But if the missing
parameter is set to "preserve" in the sriov setting we can continue,
just ignoring it.
(cherry picked from commit 7346c5b556)
If sriov_totalvfs file doesn't exist we don't need to consider it a
fatal failure. Try to create the required number of VFs as we were doing
before.
Note: at least netdevsim doesn't have sriov_totalvfs file, I don't know
if there are real drivers that neither has it.
(cherry picked from commit 27eaf34fcf)
Set these parameters according to the values set in the new properties
sriov.eswitch-inline-mode and sriov.eswitch-encap-mode.
The number of parameters related to SR-IOV was becoming too big.
Refactor to group them in a NMPlatformSriovParams struct and pass it
around.
(cherry picked from commit 4669f01eb0)
It is not safe to change the eswitch mode when there are VFs already
created: it often fails, or even worse, doesn't fail immediatelly but
there are later problems with the VFs.
What is supposed to be well tested in all drivers is to change the
eswitch mode with no VFs created, and then create the VFs, so let's set
num_vfs=0 before changing the eswitch mode.
As we want to change num_vfs asynchronously in a separate thread, we
need to do a multi-step process with callbacks each time that a step
finish (before it was just set num_vfs asynchronously and invoke the
callback when it's done).
This makes link_set_sriov_params_async to become even larger and more
complex than it already was. Refactor it to make it cleaner and easier
to follow, and hopefully less error prone, and implement that multi-step
process.
(cherry picked from commit 770340627b)
Add property to allow changing the eswitch mode between legacy SRIOV and
switchdev. Allow also to set "preserve" to prevent NM from modifying the
eswitch mode.
(cherry picked from commit c61c87f8a6)
Add support for Devlink, which is just another family of Generic Netlink
like nl80211. Implement get_eswitch_mode and set_eswitch_mode to allow
changing between legacy SRIOV and switchdev modes.
Devlink's purpose is to allow querying and configuring stuff related to
a piece of hardware but not to any of the usual Linux device classes.
For example, nowadays the Smart NICs normally allow to change the
eswitch mode per PF, because their hardware implements one eswitch per
PF, but future models might have a single eswitch for all the physical
and virtual ports of the NIC allowing more advanced bridge offloads.
Regarding the above example, for the moment we only support PCI network
devices with the "one eswitch per PF" model. The reason is that currently
NM only knows about netdevs so dealing with "devlink devices" that
doesn't map 1-1 with a netdev would require new mechanisms to understand
what they are and their relation with the netdevs that NM manage. We
will deal with that use cases when they arise and we have more
information about the right way to support them.
(cherry picked from commit f31d29bbb7)
When the lease is lost, NM tries to get a new by restarting the DHCP
transaction. However, it doesn't delete the existing l3cds (one from
the DHCP client with flag ONLY_FOR_ACD, the other from
NMDevice). Therefore, the l3cfg still tracks the ACD state of the
address as "external-removed", and when NM gets the same address via
DHCP, ACD is considered as failed; as a consequence, NM sends a
DECLINE message to the server.
Moreover, the l3cd added by NMDevice for DHCP has a zero ACD timeout,
and so it's not possible to do ACD again on the same address.
Remove those l3cds when the lease expires, so that any ACD state is
cleared and DHCP can perform ACD again.
Fixes: 240ec7f891 ('dhcp: implement ACD (address collision detection) for DHCPv4')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1853
(cherry picked from commit a80fef9f37)
The comparison checking for MAC address equality had previously been flipped around.
Fixes: b084ad7f2b ('libnm-core: canonicalize hardware addresses in settings')
(cherry picked from commit 641e717797)
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
(cherry picked from commit 5f7a027f59)
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()')
(cherry picked from commit e4b154e1b0)
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>
(cherry picked from commit 02c34d538c)
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.
(cherry picked from commit 7a031eef5d)
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.
(cherry picked from commit 62c1745f62)
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);
| ^~~~~~~~~~~~
(cherry picked from commit 63ab0d926d)
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));
| ^
(cherry picked from commit 5715feebe7)
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.
(cherry picked from commit d8b33e2a97)
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.
(cherry picked from commit fcd907e062)
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.
(cherry picked from commit 2f543f1154)
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
(cherry picked from commit 7e7d3a7981)
To embrace inclusive language, deprecate the NMSettingConnection
autoconnect-slaves property and introduce autoconnect-ports property.
(cherry picked from commit 194455660d)
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.
(cherry picked from commit b90dd247be)
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.
(cherry picked from commit 2f1b599fe3)
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.
