When using VRF devices we must pre-generate dependent local
routes in the VRF's table otherwise they will be incorrectly added
to the local table instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1857133
Fixes: a199cd2a7d ('core: add dependent local routes configured by kernel')
(cherry picked from commit d342af1925)
Keep priv->sriov.pending set during the callback set so that it
becomes possible to insert a new operation from the callback itself.
(cherry picked from commit 74ccda8a71)
When dispose() is called, there can't be any pending operation because
they keep a reference to the device. Instead, there can be a a queued
operation not yet executed. Destroy it.
(cherry picked from commit 6fcb077a98)
NM will now sync all tables when a connection has specified
at least 1 local route in 'ipv[4|6].routes' to correctly
reconcile local routes when reapplying connections on a device.
If the connection has no local routes only the main table will be
taken into account preserving the previous NM's behaviour.
https://bugzilla.redhat.com/show_bug.cgi?id=1821787
(cherry picked from commit c5496f7372)
NetworkManager can't control the name of the PPP interface name
created by pppd; so it has to wait for the interface to appear and
then rename it. This happens in nm_device_take_over_link() called by
nm-device-ppp.c:ppp_ifindex_set() when pppd tells NM the ifindex of
the interface that was created.
However, sometimes the initial interface name is already correct, for
example when the connection.interface-name is ppp0 and this is the
first PPP interface created.
When this happens, nm_device_update_from_platform_link() is called on
the NMDevicePPP and it sets the device ifindex. Later, when pppd
notifies NM, nm_device_take_over_link() fails because the ifindex is
already set:
nm_device_take_over_link: assertion 'priv->ifindex <= 0' failed
Make nm_device_take_over_link() more robust to cope with this
situation.
https://bugzilla.redhat.com/show_bug.cgi?id=1849386
(cherry picked from commit 75bc21c4cf)
nm_device_cleanup() can be called when the device no longer has an
ifindex. In such case, don't try to reset the MAC address as that
would lead to an assertion failure.
(cherry picked from commit 77b6ce7d04)
When the instance is not running (after creation or after stop), there
is no need to keep the GHashTable around.
Create it when needed (during start) and clear it during stop. This
makes it slightly cheaper to keep a NMLldpListener instance around,
if it's currently not running.
NMDevice already keeps the NMLldpListener around, even after stopping
it. It's not clear whether the instance will be started again, so also
clear the GHashTable. Also, one effect is that if you initially were in
a network with many LLDP neibors, after stop and start, the GHashTable
now gets recreated and may not need to allocate a large internal array
as before.
When a device is not marked as unmanaged, but also not actively managed
by NetworkManager, then NetworkManager will generate an in-memory
profile to represent the active state, if the device is up and
configured (with an IP address).
Such profiles are commonly named like "eth0", and they are utterly
confusing to users, because they look as if NetworkManager actually
manages the device, when it really just shows that somebody else configures
the device.
We should express this better in the UI, hence add flags to indicate
that.
In practice, such profiles are UNSAVED, NM_GENERATED, and VOLATILE. But
add an explicit flag to represent that.
https://bugzilla.redhat.com/show_bug.cgi?id=1816202
nm_platform_ethtool_init_ring() only has one caller. It's simpler to
drop the function and implement it at the only place where it is needed.
Maybe there could be a place for a function to initialize NMEthtoolRingState,
one option after the other. However, at the moment there is only one
user, so don't implement it.
This fixes various minor issues:
- the function had a NMPlatform argument, although the argument
is not used. Thus function merely operates on a NMEthtoolRingState
instance and shouldn't have a nm_platform_*() name.
- nm_platform_ethtool_init_ring() returned a boolean, but all
code paths (except assertion failures) returned success.
- as the function returned an error status, the caller was compelled
to handle an error that could never happen.
- the option was specified by name, although we already have a more
efficient way to express the option: the NMEthtoolID. Also, the
caller already needed to resolve the name to the NMEthtoolID, so
there was no need to again lookup the ID by name.
Imagine you have a veth device. That device supports certain offload features
(like "ethtool.feature-rx-checksum") but doesn't support any ring
options. Even trying to read the current ring settings will fail.
If you try to activate that profile, NMDevice previously would always
try to fetch the ring options and log a warning and extra debugging
messages:
<trace> [1590511552.3943] ethtool[31]: ETHTOOL_GRINGPARAM, v: failed: Operation not supported
<trace> [1590511552.3944] ethtool[31]: get-ring: failure getting ring settings
<warn> [1590511552.3944] device (v): ethtool: failure getting ring settings (cannot read)
It does so, although you didn't specify any ring settings and there
was no need to fetch the ring settings to begin with.
Avoid this extra logging by only fetching the ring option when they
are actually required.
Setting the kernel token is not strictly necessary as the IPv6 address
is generated in userspace by NetworkManager. However it is convenient
for users to see that the value set in the profile is also set in the
kernel, to confirm that everything is working as expected.
The kernel allows setting a token only when 'accept_ra' is 1:
temporarily flip it if necessary. Unfortunately this will also
generate an additional Router Solicitation from kernel, but this is
not a big issue.
Usually stage1 is skipped for external or assumed devices. Add a
mechanism to call stage1 for all devices, similarly to what was
already done for stage2.
We are going to expose some of this API in libnm.
The name "gendata" (for "generic data") is not very suited. Instead,
call the public API nm_setting_option_*(). This also brings no naming
conflict, because currently no API exists with such naming.
Rename the internal API, so that it matches the API that we are going
to expose next.
We already have NMEthtoolID to handle coalesce options in a way that is
convenient programmatically. That is, we can iterate over all valid
coalesce options (it's just an integer) and use that in a more generic
way.
If NMEthtoolCoalesceState names all fields explicitly, we need explicit
code that names each coalesce option. Especially since NMEthtoolCoalesceState
is an internal and intermediate data structure, this is cumbersome
and unnecessary.
Thereby it also fixes the issue that nm_platform_ethtool_init_coalesce() has a
NMPlatform argument without actually needing it.
nm_platform_ethtool_init_coalesce() does not operate on a NMPlatform
instance, and should not have the appearance of being a method of
NMPlatform.
nm-device now applies ethtool ring settings during stage 2 "device
config" of the connection activation.
ring settings will be then restored (according to what the state
was before the connection got activated on the device) when the
connection is deactivated during the device cleanup.
One thing to be noted is that unset ring settings (in the profile)
will not be touched at all by NetworkManager so that if the NIC driver
sets some default values these will be preserved unless specifically
overridden by the connection profile.
https://bugzilla.redhat.com/show_bug.cgi?id=1614700
Only in one moment we need the old and requested settings together:
during _ethtool_coalesce_set(). But for that we shouldn't track both
states in "NMEthtoolCoalesceState".
Simplify "NMEthtoolCoalesceState" to only contain one set of options.
By tracking less state, the code becomes simpler, because you don't
need to wonder where the old and requested state is used.
When the interface is in IPv4 or IPv6 shared mode and the user didn't
specify an explicit zone, use the nm-shared one.
Note that masquerade is still done through iptables direct calls
because at the moment it is not possible for a firewalld zone to do
masquerade based on the input interface.
The firewalld zone is needed on systems where firewalld is using the
nftables backend and the 'iptables' binary uses the iptables API
(instead of the nftables one). On such systems, even if the traffic is
allowed in iptables by our direct rules, it can still be dropped in
nftables by firewalld.
nm-device now applies ethtool coalesce settings during stage 2 "device
config" of the connection activation.
Coalesce settings will be then restored (according to what the state
was before the connection got activated on the device) when the
connection is deactivated during the device cleanup.
One thing to be noted is that unset coalesce settings (in the profile)
will not be touched at all by NetworkManager so that if the NIC driver
sets some default values these will be preserved unless specifically
overridden by the connection profile.
https://bugzilla.redhat.com/show_bug.cgi?id=1614700
If the driver is unknown, that doesn't necessarily mean that the match
passes. Instead, the match passes if there is no positive match that
asks for the driver name.
Add a new "driver" match option to nm-settings. It allows to disable a
network connection configuration if a pattern is found or is not found
in the device driver name.
Add a new "kernel-command-line" match option to nm-settings. It allows
to disable a network connection configuration if a pattern is found or
is not found in /proc/cmdline.
GObject signals only complicate the code and are less efficient.
Also, NM_DEVICE_AUTH_REQUEST signal really invoked an asynchronous
request. Of course, fundamentally emitting a signal *is* the same as
calling a method. However, implementing this as signal is really not
nice nor best practice. For one, there is a (negligible) overhead emitting
a GObject signal. But what is worse, GObject signals are not as strongly
typed and make it harder to understand what happens.
The signal had the appearance of providing some special decoupling of
NMDevice and NMManager. Of course, in practice, they were not more
decoupled (both forms are the same in nature), but it was harder to
understand how they work together.
Add and call a method nm_manager_device_auth_request() instead. This
has the notion of invoking an asynchronous method. Also, never invoke
the callback synchronously and provide a cancellable. Like every asynchronous
operation, it *must* be cancellable, and callers should make sure to
provide a mechanism to abort.
NMDevice already has access to the NMSettings singleton. It is permissible that
NMDevice *knows* about NMManager. The current alternative is emitting GObject signals
like NM_DEVICE_AUTH_REQUEST, pretending that NMDevice and NMManager would be completely
independent, or that there could be anybody else handling the request aside NMManager.
No, NMManager and NMDevice may know each other and refer to each other. Just like
NMDevice also knows and refers to NMSettings.
Conceptionally, the MUD URL really depends on the device, and not so
much the connection profile. That is, when you have a specific IoT
device, then this device probably should use the same MUD URL for all
profiles (at least by default).
We already have a mechanism for that: global connection defaults. Use
that. This allows a vendor drop pre-install a file
"/usr/lib/NetworkManager/conf.d/10-mud-url.conf" with
[connection-10-mud-url]
connection.mud-url=https://example.com
Note that we introduce the special "connection.mud-url" value "none", to
indicate not to use a MUD URL (but also not to consult the global connection
default).
If a device only has an IPv6 link-local address, we don't generate an
assumed connection. Therefore, when a new slave connection (without IP
configuration) is activated on the device, we don't deactivate any
existing connection and the link-local address remains configured.
The IP configuration of an activated slave should be predictable and
not depend on the previous state; let's flush addresses and routes on
activation.
https://bugzilla.redhat.com/show_bug.cgi?id=1816517https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/480
- avoid g_assert(). Either we want to gracefully assert (g_return_*()) or we
want to use assertions that are disabled in production builds (nm_assert());
- rename variable s_connection to s_con. This is how variables for this
purpose are commonly called.
If we change the the MTU of an ovs interface only through netlink, the
change could be overridden by ovs-vswitchd at any time when other
interfaces change. Set the MTU also in the ovsdb to prevent such
changes.
Note that if the MTU comes from the connection, we already set the
ovsdb MTU at creation time and so this other update becomes
useless. But it is needed when changing the MTU at runtime (reapply)
or when the MTU comes from a different source (e.g. DHCP).
g_clear_pointer() would always cast the destroy notify function
pointer to GDestroyNotify. That means, it lost some type safety, like
GPtrArray *ptr_arr = ...
g_clear_pointer (&ptr_arr, g_array_unref);
Since glib 2.58 ([1]), g_clear_pointer() is also more type safe. But
this is not used by NetworkManager, because we don't set
GLIB_VERSION_MIN_REQUIRED to 2.58.
[1] f9a9902aac
We have nm_clear_pointer() to avoid this issue for a long time (pre
1.12.0). Possibly we should redefine in our source tree g_clear_pointer()
as nm_clear_pointer(). However, I don't like to patch glib functions
with our own variant. Arguably, we do patch g_clear_error() in
such a manner. But there the point is to make the function inlinable.
Also, nm_clear_pointer() returns a boolean that indicates whether
anything was cleared. That is sometimes useful. I think we should
just consistently use nm_clear_pointer() instead, which does always
the preferable thing.
Replace:
sed 's/\<g_clear_pointer *(\([^;]*\), *\([a-z_A-Z0-9]\+\) *)/nm_clear_pointer (\1, \2)/g' $(git grep -l g_clear_pointer) -i
I think it's preferable to use nm_clear_g_free() instead of
g_clear_pointer(, g_free). The reasons are not very strong,
but I think it is overall preferable to have a shorthand for this
frequently used functionality.
sed 's/\<g_clear_pointer *(\([^;]*\), *\(g_free\) *)/nm_clear_g_free (\1)/g' $(git grep -l g_clear_pointer) -i
We usually want to schedule stage2 when we just completed with the previous
stage (or, if we are currently in stage2, and want to re-enter it).
In those cases, the conditions are often right to just proceed right away.
No need to schedule the stage on an idle handler. Allow to invoke stage2
right away.
There was only API to schedule the stage on an idle handler.
Sometimes, we are just in the right situation to schedule the stage
right away. It should be possibly to avoid going through the extra hop.
For now, none of the caller makes use of this. So, there isn't any
actual change in behavior. But by adding this possibility, we may do
use in the future.
