Beniamino Galvani authored Jun 27, 2022 and Thomas Haller committed Dec 20, 2022

Since kernel 5.18 there is a stricter validation [1][2] on the tos
field of routing rules, that must not include ECN bits.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f55fbb6afb8d701e3185e31e73f5ea9503a66744
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a410a0cf98854a698a519bfbeb604145da384c0e

Fixes the following failure:

  >>> src/core/platform/tests/test-route-linux
  >>> ...
  # NetworkManager-MESSAGE: <warn>  [1656321515.6604] platform-linux: do-add-rule: failure 22 (Invalid argument - Invalid dsfield (tos): ECN bits must be 0)
  >>> failing... errno=-22, rule=[routing-rule,0x13d6e80,1,+alive,+visible; [6] 0: from all tos 0xff fwmark 0x4/0 suppress_prefixlen -459579276 action-214 protocol 255]
  >>> existing rule: * [routing-rule,0x13d71e0,2,+alive,+visible; [6] 0: from all sport 65534 lookup 10009 suppress_prefixlen 0 none]
  >>> existing rule:   [routing-rule,0x13d7280,2,+alive,+visible; [4] 0: from all fwmark 0/0x9a7e9992 ipproto 255 suppress_prefixlen 0 realms 0x00000008 none protocol 71]
  >>> existing rule:   [routing-rule,0x13d7320,2,+alive,+visible; [6] 598928157: from all suppress_prefixlen 0 none]
  >>> existing rule:   [routing-rule,0x13d73c0,2,+alive,+visible; [4] 0: from 192.192.5.200/8 lookup 254 suppress_prefixlen 0 none protocol 9]
  >>> existing rule:   [routing-rule,0x13d7460,2,+alive,+visible; [4] 0: from all ipproto 3 suppress_prefixlen 0 realms 0xffffffff none protocol 5]
  >>> existing rule:   [routing-rule,0x13d7500,2,+alive,+visible; [4] 0: from all fwmark 0x1/0 lookup 254 suppress_prefixlen 0 action-124 protocol 4]
  >>> existing rule:   [routing-rule,0x13d75a0,2,+alive,+visible; [4] 0: from all suppress_prefixlen 0 action-109]
  0:      from all fwmark 0/0x9a7e9992 ipproto ipproto-255 realms 8 none proto 71
  0:      from 192.192.5.200/8 lookup main suppress_prefixlength 0 none proto ra
  0:      from all ipproto ggp realms 65535/65535 none proto 5
  0:      from all fwmark 0x1/0 lookup main suppress_prefixlength 0 124 proto static
  0:      from all 109
  0:      from all sport 65534 lookup 10009 suppress_prefixlength 0 none
  598928157:      from all none
  Bail out! nm:ERROR:../src/core/platform/tests/test-route.c:1787:test_rule: assertion failed (r == 0): (-22 == 0)

Fixes: 5ae2431b ('platform/tests: add tests for handling policy routing rules')
(cherry picked from commit bf9a2bab)
(cherry picked from commit 09b0014a)
(cherry picked from commit e1266b3b)

Beniamino Galvani authored Sep 07, 2022 and Fernando Fernández Mancera committed Dec 16, 2022

If there is no carrier on a device, don't try to resolve the hostname
on it. Instead, subscribe to carrier change notifications and retry
again once carrier goes up.

https://bugzilla.redhat.com/show_bug.cgi?id=2118817
!1402
(cherry picked from commit e3cf5083)
(cherry picked from commit 1673e3f0)
(cherry picked from commit 69e66102)
(cherry picked from commit a8f8ca01)

https://bugzilla.redhat.com/show_bug.cgi?id=2110000

!1343

(cherry picked from commit 7f40eb1b)

(cherry picked from commit 14633422)

(cherry picked from commit c8341aa3)

(cherry picked from commit 8b9a12e1)

If the MAC changes there is the possibility that the DHCP client will
not be able to renew the address because it uses the old MAC as
CHADDR. Depending on the implementation, the DHCP server might use
CHADDR (so, the old address) as the destination MAC for DHCP replies,
and those packets will be lost.

To avoid this problem, restart the DHCP client when the MAC changes.

https://bugzilla.redhat.com/show_bug.cgi?id=2110000
(cherry picked from commit 905adabd)
(cherry picked from commit 5a49a2f6)
(cherry picked from commit d0fb3fbf)
(cherry picked from commit 59a52510)

(cherry picked from commit 6cd69fde)
(cherry picked from commit 2f8e4e2b)
(cherry picked from commit 8011d0b3)
(cherry picked from commit 3d02df80)

During the deactivation of ovs interfaces, ovsdb receives the command to
remove the interface but for OVS system ports the device won't
disappear.

When reconnecting, ovsdb will update first the status and it will notice
that the OVS system interface was removed and it will set the status as
DEACTIVATING. This is incorrect if the status is already DEACTIVATING,
DISCONNECTED, UNMANAGED or UNAVAILABLE because it will block the
activation of the interface.

https://bugzilla.redhat.com/show_bug.cgi?id=2080236
(cherry picked from commit bc6e28e5)
(cherry picked from commit 19613a8d)
(cherry picked from commit 45c0cde9)

If we have a lease and we get a NAK renewing/rebinding it, the lease
is lost.

Without this, probe->current_lease remains set and after the next
DISCOVER/OFFER round, any call to n_dhcp4_client_lease_select() will
fail at:

        if (lease->probe->current_lease)
                return -ENOTRECOVERABLE;

As in:

 [5325.1313] dhcp4 (veth0): send REQUEST of 172.25.1.200 to 255.255.255.255
 [5325.1434] dhcp4 (veth0): received NACK from 172.25.1.1
 [5325.1435] dhcp4 (veth0): client event 3 (RETRACTED)
 [5325.1436] dhcp4 (veth0): send DISCOVER to 255.255.255.255
 [5325.1641] dhcp4 (veth0): received OFFER of 172.25.1.200 from 172.25.1.1
 [5325.1641] dhcp4 (veth0): client event (OFFER)
 [5325.1641] dhcp4 (veth0): selecting lease failed: -131 (ENOTRECOVERABLE)

Upstream: https://github.com/nettools/n-dhcp4/pull/33
Upstream: https://github.com/nettools/n-dhcp4/commit/e4af93228e3772bbb443ec1237252e6a2f3e3dd7

#993

Fixes: e43b1791 ('Merge commit 'e23b3c9c' as 'shared/n-dhcp4'')

(cherry picked from commit e141cd45)
(cherry picked from commit e056a68d)
(cherry picked from commit 6636c792)

Thomas Haller authored Feb 10, 2022 and Beniamino Galvani committed Jun 08, 2022

Add support for IPv6 multipath routes, by treating them as single-hop
routes. Otherwise, we can easily end up with an inconsistent platform
cache.

Background:
-----------

Routes are hard. We have NMPlatform which is a cache of netlink objects.
That means, we have a hash table and we cache objects based on some
identity (nmp_object_id_equal()). So those objects must have some immutable,
indistinguishable properties that determine whether an object is the
same or a different one.

For routes and routing rules, this identifying property is basically a subset
of the attributes (but not all!). That makes it very hard, because tomorrow
kernel could add an attribute that becomes part of the identity, and NetworkManager
wouldn't recognize it, resulting in cache inconsistency by wrongly
thinking two different routes are one and the same. Anyway.

The other point is that we rely on netlink events to maintain the cache.
So when we receive a RTM_NEWROUTE we add the object to the cache, and
delete it upon RTM_DELROUTE. When you do `ip route replace`, kernel
might replace a (different!) route, but only send one RTM_NEWROUTE message.
We handle that by somehow finding the route that was replaced/deleted. It's
ugly. Did I say, that routes are hard?

Also, for IPv4 routes, multipath attributes are just a part of the
routes identity. That is, you add two different routes that only differ
by their multipath list, and then kernel does as you would expect.
NetworkManager does not support IPv4 multihop routes and just ignores
them.
Also, a multipath route can have next hops on different interfaces,
which goes against our current assumption, that an NMPlatformIP4Route
has an interface (or no interface, in case of blackhole routes). That
makes it hard to meaningfully support IPv4 routes. But we probably don't
have to, because we can just pretend that such routes don't exist and
our cache stays consistent (at least, until somebody calls `ip route
replace` *sigh*).

Not so for IPv6. When you add (`ip route append`) an IPv6 route that is
identical to an existing route -- except their multipath attribute -- then it
behaves as if the existing route was modified and the result is the
merged route with more next-hops. Note that in this case kernel will
only send a RTM_NEWROUTE message with the full multipath list. If we
would treat the multipath list as part of the route's identity, this
would be as if kernel deleted one routes and created a different one (the
merged one), but only sending one notification. That's a bit similar to
what happens during `ip route replace`, but it would be nightmare to
find out which route was thereby replaced.
Likewise, when you delete a route, then kernel will "subtract" the
next-hop and sent a RTM_DELROUTE notification only about the next-hop that
was deleted. To handle that, you would have to find the full multihop
route, and replace it with the remainder after the subtraction.

NetworkManager so far ignored IPv6 routes with more than one next-hop, this
means you can start with one single-hop route (that NetworkManger sees
and has in the platform cache). Then you create a similar route (only
differing by the next-hop). Kernel will merge the routes, but not notify
NetworkManager that the single-hop route is not longer a single-hop
route. This can easily cause a cache inconsistency and subtle bugs. For
IPv6 we MUST handle multihop routes.

Kernels behavior makes little sense, if you expect that routes have an
immutable identity and want to get notifications about addition/removal.
We can however make sense by it by pretending that all IPv6 routes are
single-hop! With only the twist that a single RTM_NEWROUTE notification
might notify about multiple routes at the same time. This is what the
patch does.

The Patch
---------

Now one RTM_NEWROUTE message can contain multiple IPv6 routes
(NMPObject). That would mean that nmp_object_new_from_nl() needs to
return a list of objects. But it's not implemented that way. Instead,
we still call nmp_object_new_from_nl(), and the parsing code can
indicate that there is something more, indicating the caller to call
nmp_object_new_from_nl() again in a loop to fetch more objects.

In practice, I think all RTM_DELROUTE messages for IPv6 routes are
single-hop. Still, we implement it to handle also multi-hop messages the
same way.

Note that we just parse the netlink message again from scratch. The alternative
would be to parse the first object once, and then clone the object and
only update the next-hop. That would be more efficient, but probably
harder to understand/implement.

https://bugzilla.redhat.com/show_bug.cgi?id=1837254#c20
(cherry picked from commit dac12a8d)
(cherry picked from commit 698cf109)

Thomas Haller authored Feb 03, 2022 and Beniamino Galvani committed Jun 08, 2022

The variable with this purpose is usually called "IS_IPv4".

It's upper case, because usually this is a const variable, and because
it reminds of the NM_IS_IPv4(addr_family) macro. That letter case
is unusual, but it makes sense to me for the special purpose that this
variable has.

Anyway. The naming of this variable is a different point. Let's
use the variable name that is consistent and widely used.

(cherry picked from commit 8085c012)
(cherry picked from commit eec32669)

Thomas Haller authored Feb 10, 2022 and Beniamino Galvani committed Jun 08, 2022

To parse the RTA_MULTIHOP message, "policy" is not right (which is used
to parse the overall message). Instead, we don't really have a special
policy that we should use.

This was not a severe issue, because the allocated buffer (with
G_N_ELEMENTS(policy) elements) was larger than need be. And apparently,
using the wrong policy also didn't cause us to reject important
messages.

(cherry picked from commit 997d7293)
(cherry picked from commit 21b19780)

Fixes this error:

  checking whether more special flags are required for pthreads... no
  checking for PTHREAD_PRIO_INHERIT... yes
  ./configure: line 30294: ,as_fn_error: command not found
  checking for a Python interpreter with version >= 3... python
  checking for python... /usr/bin/python

Fixes: 3affccf2 ('tests: fix undefined references to pthread')
(cherry picked from commit a8284b1d)

Recent python-black (22.0) dropped support for Python 2 and thus fail
for those files. Make the examples Python3 compatible.

(cherry picked from commit 95e6a0a6)
(cherry picked from commit 2e4d1e8d)

Beniamino Galvani authored Mar 10, 2022 and Fernando Fernández Mancera committed Mar 24, 2022

I got a report of a scenario where multiple servers reply to a REQUEST
in SELECTING, and all servers send NAKs except the one which sent the
offer, which replies with a ACK. In that scenario, n-dhcp4 is not able
to obtain a lease because it restarts from INIT as soon as the first
NAK is received. For comparison, dhclient can get a lease because it
ignores all NAKs in SELECTING.

Arguably, the network is misconfigured there, but it would be great if
n-dhcp4 could still work in such scenario.

According to RFC 2131, ACK and NAK messages from server must contain a
server-id option. The RFC doesn't explicitly say that the client
should check the option, but I think it's a reasonable thing to do, at
least for NAKs.

This patch stores the server-id of the REQUEST in SELECTING, and
compares it with the server-id from NAKs, to discard other servers'
replies.

!1144
(cherry picked from commit 118561e2)
(cherry picked from commit 3abfdbab)

  In function 'nm_uuid_unparse',
      inlined from 'nm_uuid_generate_from_string_str' at src/libnm-glib-aux/nm-uuid.c:393:12,
      inlined from 'nm_uuid_generate_from_strings.constprop' at src/libnm-glib-aux/nm-uuid.c:430:16:
  src/libnm-glib-aux/nm-uuid.h:37:12: error: 'uuid' may be used uninitialized [-Werror=maybe-uninitialized]
     37 |     return nm_uuid_unparse_case(uuid, out_str, FALSE);
        |            ^
  src/libnm-glib-aux/nm-uuid.c: In function 'nm_uuid_generate_from_strings.constprop':
  src/libnm-glib-aux/nm-uuid.c:20:1: note: by argument 1 of type 'const struct NMUuid *' to 'nm_uuid_unparse_case.constprop' declared here
     20 | nm_uuid_unparse_case(const NMUuid *uuid, char out_str[static 37], gboolean upper_case)
        | ^
  src/libnm-glib-aux/nm-uuid.c:390:12: note: 'uuid' declared here
    390 |     NMUuid uuid;
        |            ^
  lto1: all warnings being treated as errors

The problem are code paths with failed g_return*() assertions. Being in
a bad state already, they don't bother to ensure proper return values,
and with LTO the compiler might think there are valid code paths wrongly
handled. Work around.

(cherry picked from commit cb9ca679)

The compiler is often adament to warn about maybe-uninitialized.

(cherry picked from commit 3dd854eb)

(cherry picked from commit 87bce61b)

  In function '_nm_auto_g_free',
      inlined from 'test_tc_config_tfilter_matchall_mirred' at src/libnm-core-impl/tests/test-setting.c:2955:24:
  ./src/libnm-glib-aux/nm-macros-internal.h:58:1: error: 'str' may be used uninitialized [-Werror=maybe-uninitialized]
     58 | NM_AUTO_DEFINE_FCN_VOID0(void *, _nm_auto_g_free, g_free);
        | ^
  src/libnm-core-impl/tests/test-setting.c: In function 'test_tc_config_tfilter_matchall_mirred':
  src/libnm-core-impl/tests/test-setting.c:2955:24: note: 'str' was declared here
   2955 |     gs_free char      *str;
        |                        ^
  lto1: all warnings being treated as errors
  lto-wrapper: fatal error: gcc returned 1 exit status

(cherry picked from commit 6f0e22a6)

Fix comparison with a NULL string.

The only current caller is nm-supplicant-interface.c, and this bug
causes a missing cleanup of the CurrentBSS property on disconnect.

Fixes: ac8c3a71 ('glib-aux: improve nm_ref_string_equals_str() to work for non-C-strings')

https://bugzilla.redhat.com/show_bug.cgi?id=1983735
!1073
(cherry picked from commit b755539a)

Thomas Haller authored Jan 10, 2022 and Beniamino Galvani committed Jan 13, 2022

Fixes: d074ffc8 ('platform: refactor completing netlink responses in event_handler_read_netlink()')
(cherry picked from commit 65cdbd35)

Thomas Haller authored Jan 11, 2022 and Beniamino Galvani committed Jan 12, 2022

Interestingly, on RHEL9 I suddenly get a linker error:

  libtool: link: gcc -Wall -Werror -Wextra -Wdeclaration-after-statement -Wfloat-equal -Wformat-nonliteral -Wformat-security -Wimplicit-function-declaration -Winit-self -Wlogical-op -Wmissing-declarations -Wmissing-include-dirs -Wmissing-prototypes -Wpointer-arith -Wshadow -Wshift-negative-value -Wstrict-prototypes -Wundef -Wvla -Wno-duplicate-decl-specifier -Wno-format-truncation -Wno-format-y2k -Wno-missing-field-initializers -Wno-pragmas -Wno-sign-compare -Wno-unknown-pragmas -Wno-unused-parameter -Wno-array-bounds -Wunused-value -Wcast-function-type -Wimplicit-fallthrough -fno-strict-aliasing -fdata-sections -ffunction-sections -Wl,--gc-sections -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64-v2 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Wl,--version-script=./linker-script-binary.ver -Wl,-z -Wl,relro -Wl,--as-needed -Wl,-z -Wl,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -o src/nm-online/.libs/nm-online src/nm-online/nm_online-nm-online.o -Wl,--export-dynamic -pthread  src/libnm-client-impl/.libs/libnm.so src/libnm-client-aux-extern/.libs/libnm-client-aux-extern.a /root/NetworkManager/contrib/fedora/rpm/NetworkManager.20220111-121006.2X0NXg/BUILD/NetworkManager-1.35.3/src/libnm-client-impl/.libs/libnm.so -lgnutls -ludev src/libnm-glib-aux/.libs/libnm-glib-aux.a src/libnm-std-aux/.libs/libnm-std-aux.a src/c-siphash/.libs/libc-siphash.a -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lglib-2.0 -pthread
  /usr/bin/ld: src/libnm-glib-aux/.libs/libnm-glib-aux.a(libnm_glib_aux_la-nm-time-utils.o): in function `_t_init_global_state':
  /root/NetworkManager/contrib/fedora/rpm/NetworkManager.20220111-121006.2X0NXg/BUILD/NetworkManager-1.35.3/src/libnm-glib-aux/nm-time-utils.c:73: undefined reference to `_nm_utils_monotonic_timestamp_initialized'
  collect2: error: ld returned 1 exit status
  make[2]: *** [Makefile:11437: src/nm-online/nm-online] Error 1

The linker is right. Fix it.

(cherry picked from commit f21340ab)

Ana Cabral authored Jan 07, 2022 and Beniamino Galvani committed Jan 12, 2022

NMSettingOvsDpdk does not have a verify() implementation that would prevent
the devargs property from being NULL. We must thus anticipate and handle
a NULL value.

Fixes: ae415212 ('ovs/ovsdb: add support for setting dpdk devargs option')
(cherry picked from commit d6395f7e)

Thomas Haller authored Jan 11, 2022 and Beniamino Galvani committed Jan 12, 2022

We need to first free "priv->bzobjs", which then will unlink all bzobjs
from the lists. The assert needs to go after.

https://bugzilla.redhat.com/show_bug.cgi?id=2028427

Fixes: 4154d961 ('bluetooth: refactor BlueZ handling and let NMBluezManager cache ObjectManager data')
(cherry picked from commit d5f917e7)

The name "nm-sudo" reminds of the "sudo" tool, and this is a bit
confusing because it's not related. Rename the service to
"nm-priv-helper", which stands for "NM privileged helper".

!938
(cherry picked from commit d68ab6b8)

It's not clear how this could happen, but it did:

  #0  _g_log_abort (breakpoint=1) at gmessages.c:580
  #0  0x00007f4e782c5895 in _g_log_abort (breakpoint=1) at gmessages.c:580
  #1  0x00007f4e782c6b98 in g_logv (log_domain=0x558436ef1520 "nm", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7ffd5b20b0c0) at gmessages.c:1391
  #2  0x00007f4e782c6d63 in g_log (log_domain=log_domain@entry=0x558436ef1520 "nm", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7f4e78313620 "%s: assertion '%s' failed") at gmessages.c:1432
  #3  0x00007f4e782c759d in g_return_if_fail_warning (log_domain=log_domain@entry=0x558436ef1520 "nm", pretty_function=pretty_function@entry=0x558436e49820 <__func__.43636> "nm_ip6_config_reset_addresses_ndisc", expression=expression@entry=0x558436e48b00 "priv->ifindex > 0") at gmessages.c:2809
  #4  0x0000558436bc47ca in nm_ip6_config_reset_addresses_ndisc (self=0x5584385cc190 [NMIP6Config], addresses=0x5584385952a0, addresses_n=1, plen=plen@entry=64 '@', ifa_flags=ifa_flags@entry=768) at src/core/nm-ip6-config.c:1468
  #5  0x0000558436d32e50 in ndisc_config_changed (ndisc=<optimized out>, rdata=0x55843856e4d0, changed_int=159, self=0x5584385c00f0 [NMDeviceOvsInterface]) at src/core/devices/nm-device.c:10838
  #6  0x00007f4e7323b09e in ffi_call_unix64 () at ../src/x86/unix64.S:76
  #7  0x00007f4e7323aa4f in ffi_call (cif=cif@entry=0x7ffd5b20b550, fn=fn@entry=0x558436d32a30 <ndisc_config_changed>, rvalue=<optimized out>, avalue=avalue@entry=0x7ffd5b20b460) at ../src/x86/ffi64.c:525
  #8  0x00007f4e787a0386 in g_cclosure_marshal_generic_va (closure=<optimized out>, return_value=<optimized out>, instance=<optimized out>, args_list=<optimized out>, marshal_data=<optimized out>, n_params=<optimized out>, param_types=<optimized out>) at gclosure.c:1604
  #9  0x00007f4e7879f616 in _g_closure_invoke_va (closure=0x55843850b200, return_value=0x0, instance=0x55843856e5d0, args=0x7ffd5b20b800, n_params=2, param_types=0x558438495e50) at gclosure.c:867
  #10 0x00007f4e787bba9c in g_signal_emit_valist (instance=0x55843856e5d0, signal_id=<optimized out>, detail=0, var_args=var_args@entry=0x7ffd5b20b800) at gsignal.c:3301
  #11 0x00007f4e787bc093 in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3448
  #12 0x0000558436ddf04b in check_timestamps (ndisc=ndisc@entry=0x55843856e5d0 [NMLndpNDisc], now_msec=now_msec@entry=15132, changed=changed@entry=(NM_NDISC_CONFIG_DHCP_LEVEL | NM_NDISC_CONFIG_GATEWAYS | NM_NDISC_CONFIG_ADDRESSES | NM_NDISC_CONFIG_ROUTES | NM_NDISC_CONFIG_DNS_SERVERS | NM_NDISC_CONFIG_MTU)) at src/core/ndisc/nm-ndisc.c:1539
  #13 0x0000558436de08d0 in nm_ndisc_ra_received (ndisc=ndisc@entry=0x55843856e5d0 [NMLndpNDisc], now_msec=now_msec@entry=15132, changed=changed@entry=(NM_NDISC_CONFIG_DHCP_LEVEL | NM_NDISC_CONFIG_GATEWAYS | NM_NDISC_CONFIG_ADDRESSES | NM_NDISC_CONFIG_ROUTES | NM_NDISC_CONFIG_DNS_SERVERS | NM_NDISC_CONFIG_MTU)) at src/core/ndisc/nm-ndisc.c:1556
  #14 0x0000558436dd8d50 in receive_ra (ndp=<optimized out>, msg=0x5584385e77c0, user_data=<optimized out>) at src/core/ndisc/nm-lndp-ndisc.c:333
  #15 0x00007f4e794718a3 in ndp_call_handlers (msg=0x5584385e77c0, ndp=0x5584384db840) at libndp.c:1993
  #16 0x00007f4e794718a3 in ndp_sock_recv (ndp=0x5584384db840) at libndp.c:1871
  #17 0x00007f4e794718a3 in ndp_call_eventfd_handler (ndp=ndp@entry=0x5584384db840) at libndp.c:2097
  #18 0x00007f4e7947199f in ndp_callall_eventfd_handler (ndp=0x5584384db840) at libndp.c:2126
  #19 0x0000558436dda229 in event_ready (fd=<optimized out>, condition=<optimized out>, user_data=<optimized out>) at src/core/ndisc/nm-lndp-ndisc.c:588
  #20 0x00007f4e782bf95d in g_main_dispatch (context=0x558438409a40) at gmain.c:3193
  #21 0x00007f4e782bf95d in g_main_context_dispatch (context=context@entry=0x558438409a40) at gmain.c:3873
  #22 0x00007f4e782bfd18 in g_main_context_iterate (context=0x558438409a40, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3946
  #23 0x00007f4e782c0042 in g_main_loop_run (loop=0x5584383e5150) at gmain.c:4142

Above is a stack trace of commit af00e39d ('libnm: add NMIPAddress
and NMIPRoute dups backported symbols from 1.30.8').

As workaround, ignore the ndisc signal, if we currently don't have an ifindex.
Also, recreate the NMIP6Config instances, if the ifindex doesn't match
(or we don't have one).

This workaround is probably good enough for the stable branch, as the
code on main (1.35+) was heavily reworked and the fix does not apply
there.

https://bugzilla.redhat.com/show_bug.cgi?id=2013266#c1

!1058

When the device gets realized, similar to the situation that the device
 is unmanaged by platform-init, if the device is still unmanaged by
parent and we clear the assume state. Then, when the device becomes
managed, NM is not able to properly assume the device using the UUID.

Therefore, we should not clear the assume state if the device has only
the NM_UNMANAGED_PLATFORM_INIT or the NM_UNMANAGED_PARENT flag set
in the unmanaged flags.

The previous commit 3c4450aa ('core: don't reset assume state too
early') did something similar for NM_UNMANAGED_PLATFORM_INIT flag only.

(cherry picked from commit 87674740)

  $ nmcli connection modify dummy1 ethtool.feature-rx a
  (process:3077356): GLib-WARNING **: GError set over the top of a previous GError or uninitialized memory.
  This indicates a bug in someone's code. You must ensure an error is NULL before it's set.
  The overwriting error message was: 'a' is not valid; use 'on', 'off', or 'ignore'
  Error: failed to modify ethtool.feature-rx: 'a' is not valid; use [true, yes, on], [false, no, off] or [unknown].

Fixes: e5b46aa3 ('cli: use nmc_string_to_ternary() to parse ternary in _set_fcn_ethtool()')
(cherry picked from commit 25e705c3)

Nacho Barrientos authored Nov 29, 2021 and Beniamino Galvani committed Nov 30, 2021

I've seen this in `NetworkManager-1.34.0-0.3.el8.x86_64` (latest in CentOS
Stream 8 at the time of writing this message) which does not use the latest
Systemd but probably the code base is the same (see
https://github.com/NetworkManager/NetworkManager/commit/51f93e00a23fbd09f5ad96da6290bf4ca737d46a).

Before the patch:

```
libsystemd: eth0: DHCPv6 client: T1 expires in 34y 3w 6d 45min 31s
libsystemd: eth0: DHCPv6 client: T2 expires in 54y 5month 3w 3d 23h 20min 35s
```

After the patch:

```
libsystemd: eth0: DHCPv6 client: T1 expires in 3d 7h 58min 3s
libsystemd: eth0: DHCPv6 client: T2 expires in 5d 2h 26min 50s
```

same box (x86_64 system) and same DHCPv6 server.

This regression has likely been introduced by 8a8955507af363c31297bbc5df79852db4ad39d6.

See-also: https://github.com/systemd/systemd/pull/21558

https://bugzilla.redhat.com/show_bug.cgi?id=2027267
#863
(cherry picked f...

!938 (comment 1163013)
(cherry picked from commit 5e0764cf)

spec: Update for backwards compatibility

See merge request !1024

!1012

!1002

"direct" properties are the latest preferred way to implement GObject
base properties. That way, the property meta data tracks the
"direct_type" and the offset where to find the data in the struct.

That way, we can automatically

- initialize the default values
- free during finalize
- implement get_property()/set_property()

Also, the other settings operations (compare, to/from D-Bus) are
implemented more efficiently and don't need to go through
g_object_get_property()/GValue API.

Certain properties need to release memory when destroying the NMSetting.
For "direct" properties, we have all the information we need to do that
generically in the NMSetting base class. In practice, this only concerns
string properties.

See _finalize_direct() in "nm-setting.c".

However, if the NMSetting base class takes care of freeing the strings,
then the subclasses must not also unref the variable (to avoid double free).
Previously, subclasses had to opt-in for the base class to indicate that
they are fine with that.

Now, let the base class always handle it. We only need to make sure that
classes that implement direct string properties don't also try to free
the values during destruction.