Commit graph

15807 commits

Author SHA1 Message Date
Lubomir Rintel
1dc8b7f477 device: wait for carrier even if it wasn't us who brought the device IFF_UP
The devices generally need to be IFF_UP and wait a little before the
carrier detection is reliable. Some devices, actually need to wait
more than a little -- r8169 needs up to 5 seconds.

For this reason, we delay startup complete while the carrier is down
after we bring the device up. We do this so that we don't reject
activations due to carrier down until we're sure it's really down.
This works well as long as it's us who brought the device up.

If we're restarting the daemon, the device is going to be already up
when we start up the daemon for the second time. There's, however, a
slim chance that the device was brought down and up very shortly before
the restart and therefore the carrier reporting is still not reliable.
As a matter of fact, we bring the devices down and back up on some
occassions, such as when enslaving to a team device.

Therefore, the following events in quick succession cause trouble:

  # nmcli con up team-slave-eth0
  [20099.205355] Generic FE-GE Realtek PHY r8169-0-300:00: attached PHY driver (mii_bus:phy_addr=r8169-0-300:00, irq=MAC)
  [20099.365641] nm-team: Port device eth0 added
  [20099.370728] r8169 0000:03:00.0 eth0: Link is Down
  [20099.436631] nm-team: Port device eth0 removed
  [20099.463422] Generic FE-GE Realtek PHY r8169-0-300:00: attached PHY driver (mii_bus:phy_addr=r8169-0-300:00, irq=MAC)
  [20099.628505] r8169 0000:03:00.0 eth0: Link is Down
  [20099.669425] Generic FE-GE Realtek PHY r8169-0-300:00: attached PHY driver (mii_bus:phy_addr=r8169-0-300:00, irq=MAC)
  [20099.833457] r8169 0000:03:00.0 eth0: Link is Down
  [20099.838471] nm-team: Port device eth0 added

The device has been brought down, enslaved and brought up.
"Link is Down" indicates carrier not being detected.

  Connection successfully activated (D-Bus active path: /org/freedesktop/NetworkManager/ActiveConnection/7)
  # systemctl restart NetworkManager

Now NM sees the device being up, but carrier down.

  # nmcli con up testeth0
  Error: Connection activation failed: No suitable device found for this connection (...).

Activation failed, because eth0 carrier still appears down.

  # [20102.943464] r8169 0000:03:00.0 eth0: Link is Up - 1Gbps/Full - flow control rx/tx

Now it's up, but the party is already over. Shiet.

Let's wait whenever the device reaches unavailable state, whether we
bring it up at that point or not.

Fixes-test: @restart_L2_only_lacp

https://bugzilla.redhat.com/show_bug.cgi?id=2092361
2022-07-29 00:03:18 +02:00
Thomas Haller
5806db7f4e
glib-aux: replace nm_ip4_addr_is_localhost() by nm_utils_ip4_address_is_loopback()
This was duplicated.
2022-07-28 13:08:31 +02:00
Thomas Haller
fdebcfa0a0
glib-aux: use different name for local variables in nm_g_array_index_p() macro
This doesn't use NM_UNIQ_T() to create a truly unique name.

Instead, it avoids "_arr" as local variable name, which other
macros also use. By choosing a different name, we can nest such
macro calls, without getting a "-Wshadow" warning.
2022-07-28 13:08:10 +02:00
Thomas Haller
8153b3ff0c
std-aux: use unique local variable in NM_IN_SET() macro
If you do:

  nm_assert_addr_family(NMP_OBJECT_CAST_MPTCP_ADDR(obj)->addr_family));

then there are two nested NM_IN_SET() macro invocations. Once,
NMP_OBJECT_CAST_MPTCP_ADDR() checks that the object type is one of
a few selected (using NM_IN_SET()). Then, that is passed to
nm_assert_addr_family(), which checks NM_IN_SET(addr_family, AF_INET,
AF_INET6).

In general, it's easy to end up in a situation like this.

And it mostly works just fine. The only problem was that NM_IN_SET()
uses an internal, local variable "_x". The compiler will emit a very
verbose failure about the shadowed variable:

  ./src/libnm-std-aux/nm-std-aux.h:802:14: error: declaration of '_x' shadows a previous local [-Werror=shadow]
    802 |         type _x = (x);                                                \

NM_UNIQ_T() exists for this purpose. Use it. NM_IN_SET() is
popular enough to warrant a special treatment to avoid this pitfall.
2022-07-28 13:07:50 +02:00
Thomas Haller
6501f741fc
std-aux: add argument to "op" parameter for NM_VA_ARGS_FOREACH() macro
This will be needed to pass "NM_UNIQ" to the op macro, to construct
a NM_UNIQ_T() unique name.
2022-07-28 13:06:08 +02:00
Thomas Haller
5c54bad89d
std-aux: add nm_memeq() helper 2022-07-28 11:06:53 +02:00
Thomas Haller
60404836d8
std-aux: add c_list_insert_sorted()
The strength of CList is of course to use it as a stack of queue,
and only append/remove from the front/tail.

However, since this is an intrusive list, it can also be useful to
just use it to track elements, and -- when necessary -- sort them
via c_list_sort().

If we have a sorted list, we might want to insert a new element
honoring the sort order. This function achieves that.
2022-07-28 11:05:14 +02:00
Beniamino Galvani
0cd3ffa7e9 build: fix compilation
Fixes: dbf29c5450 ('platform: fix build with musl libc')
2022-07-27 19:51:56 +02:00
Beniamino Galvani
dbf29c5450 platform: fix build with musl libc
Don't mix <net/ethernet.h> and <linux/if_ether.h>.

Fixes the following build error with musl libc:

  In file included from /usr/include/net/ethernet.h:10,
                   from ../src/libnm-platform/nm-linux-platform.c:17:
  /usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
    115 | struct ethhdr {
        |        ^~~~~~
  In file included from ../src/linux-headers/ethtool.h:19,
                   from ../src/libnm-std-aux/nm-linux-compat.h:22,
                   from ../src/libnm-platform/nm-linux-platform.c:10:
  /usr/include/linux/if_ether.h:169:8: note: originally defined here
    169 | struct ethhdr {
        |        ^~~~~~

Fixes: dc98ab807c ('platform: include "linux-headers" via "libnm-std-aux/nm-linux-compat.h"')
2022-07-27 18:46:01 +02:00
Thomas Haller
0b9384187b
platform: fix identity for MPTCP NMPObject to honor port 2022-07-26 13:09:20 +02:00
Thomas Haller
766349879e
platform/trivial: add code comments for NMPGlobalTracker 2022-07-26 13:09:19 +02:00
Thomas Haller
d3c9bb4666
platform: rename file "nmp-route-manager.[hc]" to "nmp-global-tracker.[hc]" 2022-07-26 12:45:55 +02:00
Thomas Haller
bf248e0400
platform: rename NMPRouteManager to NMPGlobalTracker
NetworkManager primarily manages interfaces in an independent fashion.
That means, whenever possible, we want to have a interface specific
view. In many cases, the underlying kernel API also supports that view.
For example, when configuring IP addresses or unicast routes, we do so
per interfaces and don't need a holistic view.

However, that is not always sufficient. For routing rules and certain
route types (blackhole, unreachable, etc), we need a system wide view
of all the objects in the network namespace.

Originally, NMPRulesManager was added to track routing rules. Then, it
was extended to also track certain route types, and the API was renamed to
NMPRouteManager.

This will also be used to track MPTCP addresses.

So rename again, to give it a general name that is suitable for what it
does. Still, the name is not great (suggestion welcome), but it should
cover the purpose of the API well enough. And it's the best I came
up with.

Rename.
2022-07-26 12:43:44 +02:00
Thomas Haller
e466ad62e5
platform: set the scope for IPv4 loopback address to "host"
For IPv6, kernel does not accept the ifa_scope parameter and always
determines the scope based on the address itself.

For IPv4, it honors whatever scope the user sets via netlink.
NetworkManager does not allow to directly configure the address
scope, but autodetects it.

Use nm_platform_ip4_address_get_scope() for detecting the scopt.

This also fixes the issue that to detect loopback addresses 127.0.0.0/8
and use scope "host".

Try:

  $ nmcli device modify "$IFACE" +ipv4.addresses 127.0.0.5/8
2022-07-26 12:28:05 +02:00
Thomas Haller
e25b7a579e
platform: add nm_platform_ip{4,6,}_address_get_scope() helper 2022-07-26 12:28:05 +02:00
Thomas Haller
dc98ab807c
platform: include "linux-headers" via "libnm-std-aux/nm-linux-compat.h"
We have our own copy of linux kernel headers, and we must never
directly include the corresponding versions from the system.

Avoid that, by only including the clones via "libnm-std-aux/nm-linux-compat.h"
and by including the compat wrapper header before other system headers.
2022-07-26 12:28:04 +02:00
Thomas Haller
b3f60d891f
platform: assert for success genlmsg_put() in _nl802154_alloc_msg() 2022-07-26 12:28:04 +02:00
Thomas Haller
9c3e0846ee
platform/trivial: add blank line after variable declaration in NLA_PUT_TYPE() 2022-07-26 12:28:04 +02:00
Thomas Haller
1f7db38dd7
glib-aux: add nm_ip_addr_cmp_for_sort() helper for sorting IP addresses
It's similar to nm_ip_addr_cmp(), but it can be used as an argument
to g_qsort_with_data() to sort a list of NMIPAddr (or in_addr_t or
struct in6_addr).

The address family needs to be given as user-data.
2022-07-26 12:28:03 +02:00
Thomas Haller
76d63c66d7
glib-aux: add nm_utils_ip4_address_is_loopback() 2022-07-26 12:28:03 +02:00
Thomas Haller
51625013d4
glib-aux: add code comment to nm_ip_addr_set() about using it for NMIPAddr argument 2022-07-26 12:27:58 +02:00
Beniamino Galvani
2c70fef12e bridge: don't reset vlan filtering parameters on external connections
Fixes: 96fab7b462 ('all: add vlan-filtering and vlan-default-pvid bridge properties')

https://bugzilla.redhat.com/show_bug.cgi?id=2107647
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1305
2022-07-26 09:00:43 +02:00
Beniamino Galvani
e35f2494f8 supplicant: increase the PMK lifetime for WPA-EAP
By default, wpa_supplicant sets these parameters according to the
802.11 standard:

  dot11RSNAConfigPMKLifetime = 43200 seconds (12 hours)
  dot11RSNAConfigPMKReauthThreshold = 70%

With these, the supplicant triggers a new EAP authentication every 8
hours and 24 minutes. If the network uses one-time secrets, the
reauthentication fails and the supplicant disconnects. It doesn't seem
desirable that the client starts a reauthentication so early; bump the
lifetime to a week.

Currently, due to a bug, the new value is ignored by wpa_supplicant
when set via D-Bus. This patch needs the fix at [1], not yet merged.

[1] http://lists.infradead.org/pipermail/hostap/2022-July/040664.html

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1306
2022-07-26 08:48:45 +02:00
Lubomir Rintel
8e8fed433f bridge: add reapply support
We're able to reapply all properties in the bridge setting, aside from
"mac-address" which is used for matching the device.

https://bugzilla.redhat.com/show_bug.cgi?id=2092762
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1296
2022-07-25 13:42:50 +02:00
Lubomir Rintel
b2a6d9d2aa manager: recreate virtual devices on "nmcli net on"
"nmcli networking off" brings down all connections, resulting in virtual
devices disappearing:

  # nmcli c add type dummy ifname dummy0
  # nmcli networking off
  # nmcli networking on
  # nmcli d show dummy0
  Error: Device 'dummy0' not found.

Attempt to recreate them all upon bringing the networking back up.

Fixes-test: @ovs_cloned_mac_set_on_iface

https://bugzilla.redhat.com/show_bug.cgi?id=2093175
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1292
2022-07-25 13:41:22 +02:00
Lubomir Rintel
4d0227f3fa manager: make "nmcli net off" wait for deactivations
The current behavior of "nmcli networking off" is that it starts
disconnecting the devices, but doesn't wait for them to actually
come down.

That is not too helpful: the user never knows when the network is
actually disconnected.

Some users, notably the NetworkManager-CI test suite, seem to expect the
devices are all disconnected after the command finishes. Even worse,
it immediately proceeds activating the connections:

  @ovs_cloned_mac_set_on_iface
  ...
  * Execute "nmcli networking off && nmcli networking on"

This results in pure utter chaos. In particular, the slave connections
sometimes refuse to activate after "nmcli networking on", because the
master connections are still getting disconnected in response to
preceding "nmcli networking off".

Let's make Enable(FALSE) and Sleep(TRUE) block until none of the devices
are expected to go down.

Note that this makes those call also return when Enable(TRUE) and
Sleep(FALSE) is issued in meanwhile. Therefore a return from
Enable(FALSE) doesn't necessarily imply the networking is disabled.
This is a feature, not a bug -- the actual manager state is available in
the "state" property.

Fixes-test: @ovs_cloned_mac_set_on_iface

https://bugzilla.redhat.com/show_bug.cgi?id=2093175
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1292
2022-07-25 13:40:43 +02:00
Thomas Haller
077d6503ad
c-stdaux: re-import git-subtree for 'src/c-stdaux'
git subtree pull --prefix src/c-stdaux git@github.com:c-util/c-stdaux.git main --squash
2022-07-25 10:29:09 +02:00
Thomas Haller
c13df3e4dc
modules: patch meson subprojects to use local dependencies
Usually we want no difference between the upstream project that we fork
via git-subtree, and our copy. However, for the subprojects, we need to
patch them. Do it.

If you know a better way, that allows to overwriting the subprojects
please send a patch.
2022-07-25 10:27:33 +02:00
Thomas Haller
5077018ff4
dhcp: fix EXTENDED DHCP event to accept lease for dhclient plugin
n-dhcp4 only supports calling ACCEPT during the GRANTED state.
Not during a EXTENDED event. So usually, we would not want
to call accept in that case.

And we didn't. During EXTENDED event, we would usually skip ACD (because
it's either not enabled or we already passed ACD for the current address).
In that case, in _nm_dhcp_client_notify() we hit the line

     if (client_event_type == NM_DHCP_CLIENT_EVENT_TYPE_BOUND && priv->l3cd_curr
         && nm_l3_config_data_get_num_addresses(priv->l3cd_curr, priv->config.addr_family) > 0)
         priv->l3cfg_notify.wait_dhcp_commit = TRUE;
     else
         priv->l3cfg_notify.wait_dhcp_commit = FALSE;

and would not set `wait_dhpc_commit`. That means, we never called _dhcp_client_accept().
For nettools, that doesn't really matter because calling ACCEPT during EXTENDED
is invalid anyway. However, for dhclient that is fatal because we wouldn't reply the
D-Bus request from nm-dhcp-helper. The helper times out after 60 seconds and dhclient
would misbehave.

We need to fix that by also calling _dhcp_client_accept() in the case when we don't
need to wait (the EXTENDED case).

However, previously _dhcp_client_accept() was rather peculiar and didn't like to be
called in an unexpected state. Relax that. Now, when calling accept in an unexpected
state, just do nothing and signal success. That frees the caller from the complexity
to understand when they must/must not call accept.

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

Fixes: 156d84217c ('dhcp/dhclient: implement accept/decline (ACD) for dhclient plugin')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1308
2022-07-22 13:13:27 +02:00
Thomas Haller
7c04f57320
c-stdaux: re-import git-subtree for 'src/c-stdaux'
git subtree pull --prefix src/c-stdaux git@github.com:c-util/c-stdaux.git main --squash
2022-07-22 12:10:21 +02:00
Thomas Haller
21539f1159 Squashed 'src/c-stdaux/' changes from da7209900ef0..ddd666b76654
ddd666b76654 c-stdaux.h: don't include <stdatomic.h>

git-subtree-dir: src/c-stdaux
git-subtree-split: ddd666b766548a8703ba845c3d58a21bdeaf5f2e
2022-07-22 12:10:00 +02:00
Thomas Haller
9f3057b08e
libnm: update HTTP reference to documentation in code comment 2022-07-22 10:17:26 +02:00
Thomas Haller
c8f3d50329
libnmc/trivial: rename _device_state_to_string() function
_device_state_externally_to_string() is a better name for what it does.
2022-07-21 22:03:33 +02:00
Thomas Haller
465df6f432
libnm: fix annotations for nm_conn_wireguard_import()
Fixes: f00e747beb ('libnm-client: Add public nm_conn_wireguard_import() func')
2022-07-21 17:22:34 +02:00
Wen Liang
bd84ae4dc5 platform: add the a_no_auto_noprefixroute flag
`nm_platform_ip_address_sync()` likes to add IFA_F_NOPREFIXROUTE flag
for all addresses, regardless of `a_ifi_flags` property. By setting this
boolean, that automatism can be suppressed, and the noprefixroute flag
does not get added automatically.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1307
2022-07-21 09:09:03 -04:00
Christian Glombek
f00e747beb
libnm-client: Add public nm_conn_wireguard_import() func
This commit moves the `nm_vpn_wireguard_import()` function
implementation from `libnmc-base` to `libnm-client-impl`, renaming it to
`nm_conn_wireguard_import()`.

A new `nm_conn_utils` header file is added in `libnm-client-public`.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1031

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1299
2022-07-21 14:53:26 +02:00
Thomas Haller
2fb2a83090
platform: fix compile error in event_seq_check()
Some compiler versions don't like this. Workaround.

  src/libnm-platform/nm-linux-platform.c: In function event_seq_check:
  src/libnm-platform/nm-linux-platform.c:7254:1: error: label at end of compound statement
   out:
   ^~~

Fixes: 3d4906a3da ('platform: add genl socket support for events and genl family')
2022-07-20 21:07:01 +02:00
Thomas Haller
c391162a81
platform: print genl family ID in hex format
That seems common. It's also done by genl-ctrl-list and
iproute2's genl tool.

Also, use avoid the leading zeros (0x1c instead of 0x001c).
iproute2's genl tool does the former, libnl3's genl-ctrl-list
does the latter.
2022-07-20 14:42:25 +02:00
Thomas Haller
e9f6e0abe1
platform: try harder to get the genl family ID by getting it synchronously
We now cache the family ID for generic netlink protocols. However,
when we for example create a wireguard interface, the kernel module
might just get autoloaded. At this point, we didn't know the family ID
yet.

We already made an effort, that if the family ID is unknown during
nm_platform_genl_get_family_id(), we would try to poll the genl socket
in the hope there is a relevant event there. However, polling the socket
also means to potentially emit all signals for any change that happen.
We don't want that, if we currently are already polling the socket.

Instead, fallback to synchronously get the family ID.

  $ sudo rmmod wireguard \
    ./tools/run-nm-test.sh -m src/core/platform/tests/test-link-linux -p /link/software/detect/wireguard/1/external

Fixes: 3d4906a3da ('platform: add genl socket support for events and genl family')
2022-07-20 14:33:51 +02:00
Thomas Haller
1a0c8772b0
platform: add NMPlatformMptcpAddr object
An NMPObject is hashable, can be compared and printed. That is useful.
Make an NMPObject for MPTCP addresses. It will hold the content of
MPTCP_PM_ATTR_ADDR netlink attribute. But like other NMPObject types it
will also be used to represent the data as NetworkManager tracks it.
2022-07-20 10:25:47 +02:00
Thomas Haller
be4b775585
platform: avoid logging plain pointer values in "nmp-object.c" 2022-07-20 10:25:46 +02:00
Thomas Haller
543a22af0a
platform: drop unnecessary cmd_plobj_id_copy implementations
The default implementation just uses memcpy() of the public part. That
is just what we want. No need to implement those functions.
2022-07-20 10:25:46 +02:00
Thomas Haller
a242d41cc0
platform: improve nmp_object_stackinit_id() for types that don't implement cmd_plobj_id_copy()
An object type that doesn't implement cmd_plobj_id_copy(), either:

- implements cmd_obj_copy(), but then we cannot copy the ID only
  to a stack instance, because that cannot track ownership.
  This is a bug in the caller. We cannot use stackinit for an
  object of a type that is not a plain old data (in C++ terms).

- fallback to plain memcpy(). That is in line with nmp_object_clone().
  and nmp_object_copy().
2022-07-20 10:24:45 +02:00
Thomas Haller
52c8ee2c9d
platform: drop detecting address scope in _nl_msg_new_address()
All callers explicitly set a scope, like they should. Drop guessing
the scope. Also, use the proper integer type.
2022-07-20 10:23:05 +02:00
Thomas Haller
45ca7dfaf3
platform/netlink: add NLA_S32 enum value for attribute type 2022-07-20 10:23:05 +02:00
Thomas Haller
8d0b30b243
linux-headers: include "mptcp.h" kernel header
Taken from v5.18, 4b0986a3613c ('Linux 5.18') from 20220522.
2022-07-20 10:23:04 +02:00
Thomas Haller
22f6ac039d
glib-aux: use max_align_t in NM_HASH_COMBINE_VALS() macro
The _nm_alignas() exists to choose a suitable alignment. Since it's
on the stack, it has (almost) no overhead to just use the maximum
alignment.

That's why gint64 was chosen before. But that isn't the largest
alignment. We rely on C11 already, and we also always include <stddef.h>.
So use max_align_t instead.
2022-07-20 10:21:44 +02:00
Thomas Haller
c4992c75f7
std-aux: use nm_memcmp() in NM_CMP_DIRECT_MEMCMP()
NM_CMP_DIRECT_MEMCMP() gets called by NM_CMP_FIELD_MEMCMP_LEN().
For example, if you want to compare a NMIPAddr, it seems sensible
to call

  NM_CMP_FIELD_MEMCMP_LEN(obj1, obj2, addr, nm_utils_addr_family_to_size(obj1->addr_family));

Granted, nm_utils_addr_family_to_size() asserts that addr_family is
either AF_INET or AF_INET6. However, if the assertion fails, we don't
want yet another undefined behavior here and do the sensible thing
about n zero.

In general, for a low-level function that uses memcmp(), it's non
obvious to ensure that the caller does not accidentally invoke undefined
behavior. nm_memcmp() avoids that.
2022-07-20 10:15:28 +02:00
Thomas Haller
2ebc3ac7d9
std-aux: add nm_memcmp() helper 2022-07-20 10:15:23 +02:00
Beniamino Galvani
93372e8100 ovs: fail device only when it's activating
It doesn't make sense to fail a device that is not activating.

Especially, if the device was in state UNMANAGED, it would enter state
FAILED (and then DISCONNECTED) or ACTIVATED (when external or
assumed); both are wrong.

https://bugzilla.redhat.com/show_bug.cgi?id=2077950
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1302
2022-07-19 14:02:24 +02:00