We need to support systems where there are hundereds of thousand routes
(e.g. BGP software).
For that, we will ignore routes that have a rtm_protocol value which was
certainly not created by NetworkManager. Before we had a BPF filer to
filter them out, but that had issues and is removed for now. Still,
don't put those routes in the platform cache and ignore them early.
Note that we still deserialize the RTM_NEWROUTE message to a NMPObject,
and don't shortcut earlier. The reason is that we should still call
delayed_action_refresh_all_in_progress() and handle the RTM_GETROUTE
response, even if the route has a different protocol. So we error out
later, shortly before putting the object in the cache. This means we
will malloc() a NMPObject and initialize it, but that is probably cheap,
compared to the first problem that the process already had to wake up
and read the netlink socket.
This restores the effective behavior of the BPF filter, albeit with a
higher overhead, as the route is rejected later. The important part for
now is that we stick to the behavior of not caching certain routes of a
certain protocol. If that can in the future be optimized (e.g. by a new
BPF filter), then we should do that. But for now that would be only a
future performance improvement, which requires new profiling first and
that has not the highest priority. Note that not caching certain routes
already should reduce the largest part of the overhead that those routes
brings. Whether this form is sufficient to reach the expected
performance goals, needs to be measured in the future.
The socket BPF filter operates on the entire message (skb). One netlink
message (for RTM_NEWROUTE) usually contains a list of routes (struct
nlmsghdr), but the BPF filter was only looking at the first route to decide
whether to throw away the entire message. That is wrong.
This causes that we miss routes. It also means that the response to our
RTM_GETROUTE dump request could be filtered and we poll/wait (unsuccessfully)
for it:
<trace> [1641829930.4111] platform-linux: netlink: read: wait for ACK for sequence number 26...
To get this right, the BPF filter would have to look at all routes in the
list to find out whether there are any we want to receive (and if
there are any, to pass the entire message, regardless that is might also
contain routes we want to ignore). As BPF does not support loops, that
is not trivial. Arguably, we still could do something where we look at a
bounded, unrolled number of routes, in the hope that there are not more
routes in one message that we support. The problem is that this BPF
filter is supposed to filter out massive amounts of routes, so most
messages will be filled to the brim with routes and we would have to
expect a high number of routes in the RTM_NEWROUTE that need checking.
We could still ignore routes in _new_from_nl_route(), to enter the
cache. That means, we would catch them later than with the BPF filter,
but still before a lot of the overhead of handling them. That probably
will be done next, but for now revert the commit.
This reverts commit e9ca5583e5.
https://bugzilla.redhat.com/show_bug.cgi?id=2037411
The force-commit flag is used to force the commit of an address or a
route from DHCP/RA even when it was removed from platform externally
(for example because it expired). Routes generated from the l3cd
should also have the flag set.
Without this, NM properly re-adds the DHCP address after the lease is
lost and obtained again, but fails to add the prefix-route.
Fixes: 2838b1c5e8 ('core: track force-commit flag for l3cd and platform objects')
https://bugzilla.redhat.com/show_bug.cgi?id=2033991https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1049
Try to debug a hang in platform code, presumably during poll().
This logging seems useful for debugging this particular issue,
but it might be useful in general.
Routing daemons can add a large amount of routes to the
system. Currently NM receives netlink notifications for all those
routes and exposes them on D-Bus. With many routes, the daemon becomes
increasingly slow and uses a lot of memory.
The rtm_protocol field of the route indicates the source of the
route. From /usr/include/linux/rtnetlink.h, the allowed values are:
#define RTPROT_UNSPEC 0
#define RTPROT_REDIRECT 1 /* Route installed by ICMP redirects;
not used by current IPv4 */
#define RTPROT_KERNEL 2 /* Route installed by kernel */
#define RTPROT_BOOT 3 /* Route installed during boot */
#define RTPROT_STATIC 4 /* Route installed by administrator */
/* Values of protocol >= RTPROT_STATIC are not interpreted by kernel;
they are just passed from user and back as is.
It will be used by hypothetical multiple routing daemons.
Note that protocol values should be standardized in order to
avoid conflicts.
*/
#define RTPROT_GATED 8 /* Apparently, GateD */
#define RTPROT_RA 9 /* RDISC/ND router advertisements */
#define RTPROT_MRT 10 /* Merit MRT */
#define RTPROT_ZEBRA 11 /* Zebra */
#define RTPROT_BIRD 12 /* BIRD */
#define RTPROT_DNROUTED 13 /* DECnet routing daemon */
#define RTPROT_XORP 14 /* XORP */
#define RTPROT_NTK 15 /* Netsukuku */
#define RTPROT_DHCP 16 /* DHCP client */
#define RTPROT_MROUTED 17 /* Multicast daemon */
#define RTPROT_KEEPALIVED 18 /* Keepalived daemon */
#define RTPROT_BABEL 42 /* Babel daemon */
#define RTPROT_OPENR 99 /* Open Routing (Open/R) Routes */
#define RTPROT_BGP 186 /* BGP Routes */
#define RTPROT_ISIS 187 /* ISIS Routes */
#define RTPROT_OSPF 188 /* OSPF Routes */
#define RTPROT_RIP 189 /* RIP Routes */
#define RTPROT_EIGRP 192 /* EIGRP Routes */
Since NM uses only values <= RTPROT_STATIC, plus RTPROT_RA and
RTPROT_DHCP, add a BPF filter to the netlink socket to discard
notifications for other route types.
https://bugzilla.redhat.com/show_bug.cgi?id=1861527https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1038
We use clang-format for automatic formatting of our source files.
Since clang-format is actively maintained software, the actual
formatting depends on the used version of clang-format. That is
unfortunate and painful, but really unavoidable unless clang-format
would be strictly bug-compatible.
So the version that we must use is from the current Fedora release, which
is also tested by our gitlab-ci. Previously, we were using Fedora 34 with
clang-tools-extra-12.0.1-1.fc34.x86_64.
As Fedora 35 comes along, we need to update our formatting as Fedora 35
comes with version "13.0.0~rc1-1.fc35".
An alternative would be to freeze on version 12, but that has different
problems (like, it's cumbersome to rebuild clang 12 on Fedora 35 and it
would be cumbersome for our developers which are on Fedora 35 to use a
clang that they cannot easily install).
The (differently painful) solution is to reformat from time to time, as we
switch to a new Fedora (and thus clang) version.
Usually we would expect that such a reformatting brings minor changes.
But this time, the changes are huge. That is mentioned in the release
notes [1] as
Makes PointerAligment: Right working with AlignConsecutiveDeclarations. (Fixes https://llvm.org/PR27353)
[1] https://releases.llvm.org/13.0.0/tools/clang/docs/ReleaseNotes.html#clang-format
Problem: if l3cfg commits an address and routes from DHCP, when the
address expires those objects are removed automatically. NM tracks the
objects as missing as if the user removed them. This is to prevent
l3cfg to committing them again. If the lease if renewed, l3cfg should
be allowed to commit those objects again.
Introduce a l3cd flag to indicate that it should be force-committed
once, and propagate this flag to platform objects. In this way, l3cfg
can avoid committing again objects that are removed externally, but it
can commit them when the l3cd changes.
Fixes-test: @bridge_down_to_l2_only
Completely rework IP configuration in the daemon. Use NML3Cfg as layer 3
manager for the IP configuration of an interface. Use NML3ConfigData as
pieces of configuration that the various components collect and
configure. NMDevice is managing most of the IP configuration at a higher
level, that is, it starts DHCP and other IP methods. Rework the state
handling there.
This is a huge rework of how NetworkManager daemon handles IP
configuration. Some fallout is to be expected.
It appears the patch deletes many lines of code. That is not accurate, because
you also have to count the files `src/core/nm-l3*`, which were unused previously.
Co-authored-by: Beniamino Galvani <bgalvani@redhat.com>
This is an accessor to the peer_address field. It should work
both for const and non-const arguments.
Similar like strchr() casts the constness away, we also need to
do that here.
Introduce a construct-only property for platform objects to enable or
disable the caching of tc objects. When disabled, the netlink socket
doesn't receive netlink events for tc objects, and objects are never
added to the cache. This commit doesn't change behavior yet.
Update nm_platform_qdisc_sync() and nm_platform_tfilter_sync() to
avoid looking into the platform cache, so that we no longer require to
keep tc and qdiscs in the cache.
There is no API in kernel to retrieve tc objects only for a specific
interface, so NM had to receive all tc events, even for unmanaged
interfaces. This could cause high CPU usage in some scenarios with
many objects.
Instead, try to delete root qdiscs and filters and then add the known
ones.
Also, combine the two functions together since they are related. In
particular, removing all qdiscs also removes all attached filters.
NMPlatformIPAddress, NMPlatformIP4Address and NMPlatformIP6Address are supposed
to have a common first part, which is address family agnostic. For that, the
is the macro __NMPlatformIPAddress_COMMON which defines the first fields.
Something similar is also done for routes and object types that have an ifindex.
Anyway, __NMPlatformIPAddress_COMMON used to have a bitfield as last element.
In particular NMPlatformIP4Address then has a bitfield as first IPv4 specific
field. With this it's not clear to me that the alignment is guaranteed
to be the same for all structs.
Avoid the trailing bitfield at __NMPlatformIPAddress_COMMON to workaround
this potential problem.
This flag is only relevant for IPv4. That is, because the way we do
ACD/DAD is fundamentally different between IPv4 and IPv6. For IPv4, we
use libn-acd while IPv6 we configure the address in kernel and wait for
the tentative flag to go away.
NMPlatformIP{Address,Route} are mainly the structs that we receive via
netlink and get cached in the NMPlatform cache.
However, the same structures are also used by the upper layers to track
which addresses to add.
Add a flag to addresses and routes, for a certain behavior, relevant
during NML3Cfg commit. The idea is that during commits for NML3Cfg of
type NM_L3_CFG_COMMIT_TYPE_ASSUME, no new addresses are added that
are not already configured. In some cases, we want to override that,
and need a flag to track that. More about that later.
We have a copy of a few linux user space headers in `src/linux-headers`.
The idea is that we want to use recent kernel API, and not depend on the
kernel UAPI headers installed on the build system (and not need to
workaround that).
However, we may not be able to simply compile them, because they too
have dependencies. For example,
../src/linux-headers/ethtool.h:1389:2: error: implicit declaration of function '__KERNEL_DIV_ROUND_UP' [-Werror=implicit-function-declaration]
__u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
^
As workaround, don't include headers from "linux-headers" directly,
but only include the new "libnm-std-aux/nm-linux-compat.h" adapter
header, which tries to solve these incompatibilities.
Fixes: 34d48d2596 ('platform: clear all BASE types when setting advertised modes for ethernet autoneg')
Disabling autoneg is not supported for Gigabit ethernet. But it seems that
ixgbe also doesn't honor
ethtool -s enp5s0f0 speed 100 duplex full autoneg off
As a workaround, when we disable autoneg then always set the advertised
modes too. I think (hope) that should not have a bad effect otherwise,
but seems most sensible for ixgbe.
While NMUtilsIPv6IfaceId is only 8 bytes large, it seems unidiomatic to
pass the plain struct around.
With a "const NMUtilsIPv6IfaceId *" argument it is more clear what the
meaning of this is.
Change to use pointers.
The preference for IPv6 routes was added in kernel v4.1,
22 June 2015. It is even in latest RHEL7 kernels.
Drop trying to be compatible with such old kernels.
We use extended IFA_FLAGS for IFA_F_MANAGETEMPADDR (IPv6) and
IFA_F_NOPREFIXROUTE (IPv4 and IPv6).
These flags for IPv4 were added to kernel 3.14, 30 March, 2014.
The flag for IPv4 was added to kernel 4.4, 11 January 2016.
Even latest RHEL-7 kernels have backport for IFA_F_NOPREFIXROUTE
for IPv4 (rh#1221311).
Drop this. The backward compatibility code paths are likely broken
anyway, and add considerable complexity.
This is supported since kernel 3.17, dated 5 October, 2014. Drop the backward
compatibility for that.
It's very hard to sensibly support a mode where we set the interface up,
but prevent kernel from enabling IPv6. We would hack around that by disabling
IPv6 altogether.
But these code paths are not tested and likely make no sense. And it's hard
to implement a sensible behavior in this case anyway.
The term "user_ipv6ll" is confusing and not something somebody familiar
with kernel or `ip -d link` would understand.
Also, it maps a boolean to addr-gen-mode "none" or "eui64", although
there are 2 more address generation modes in kernel.
Don't abstract the underlying API, and name things as they are in
kernel.
It can be convenient to track a NMPObject in form of their down-cast
pointers. When doing that, it's useful to have clear/ref-set helpers
that operate on such pointers and up-cast first.
With LTO builds, it assumes that the assertion-failed code-paths
can be reached, and thus a warning gets emitted:
In function nmp_cache_lookup,
inlined from nm_platform_lookup at src/libnm-platform/nm-platform.c:3377:12,
inlined from nm_platform_lookup_object at ./src/libnm-platform/nmp-object.h:975:12:
src/libnm-platform/nmp-object.h:742:46: error: lookup.cache_id_type may be used uninitialized [-Werror=maybe-uninitialized]
742 | return nmp_cache_lookup_all(cache, lookup->cache_id_type, &lookup->selector_obj);
| ^
./src/libnm-platform/nmp-object.h: In function nm_platform_lookup_object:
./src/libnm-platform/nmp-object.h:972:15: note: lookup declared here
972 | NMPLookup lookup;
| ^
Add and call the new `nm_platform_link_get_permanent_address()` to
obtain `l_perm_address` via netlink or lookup via ethtool if kernel
does not expose the `IFLA_PERM_ADDRESS`.
And call the new `nm_platform_link_get_permanent_address()` in the unit
tests.
https://bugzilla.redhat.com/show_bug.cgi?id=1987286
Signed-off-by: Wen Liang <liangwen12year@gmail.com>
Replace the arguments "buf+length" of
`nm_platform_link_get_permanent_address()` with "NMPLinkAddress *out_addr"
Signed-off-by: Wen Liang <liangwen12year@gmail.com>
