Often, the code paths for IPv4 and IPv6 are very similar. We should try
to unify those code paths. The main advantage of doing that, is that
we don't unintentionally end up doing different things. And of course,
it removes duplicate code.
In a first step, unify handling of the grace timeout for DHCPv4 and
DHCPv6.
We will add a property NM_NDISC_RA_TIMEOUT for which this name is better
suited. The problem is really that our convention for object properties
and signals defines have no prefix to indicate whether it's a property
or a signal.
Rename.
This is C, we have almost no IDE support. And ctags/cscope is much more
helpful if we use unique names.
Don't use the get_dhcp_timeout() name, because that is already used in
"src/devices/nm-device.c" already. Rename.
The _GET_PRIVATE() macros are all implemented based on
_NM_GET_PRIVATE(). That macro tries to be more type safe and uses
_Generic() to do the right thing. Explicitly casting is not only
unnecessary, it defeats these (static) type checks.
Don't do that.
Several macros are used to define function. They had a "_STATIC" variant,
to define the function as static.
I think those macros should not try to abstract entirely what they do.
They should not accept the function scope as argument (or have two
variants per scope). This also because it might make sense to add
additional __attribute__(()) to the function. That only works, if
the macro does not pretend to *not* define a plain function.
Instead, embrace what the function does and let the users place the
function scope as they see fit.
This also follows what is already done with
static NM_CACHED_QUARK_FCN ("autoconnect-root", autoconnect_root_quark)
In all the cases, we don't want to perform locale dependent comparison.
$ sed -i 's/\<strcasecmp\>/g_ascii_\0/g' $(git grep -w -l strcasecmp -- ':(exclude)shared/systemd/' )
We keep adding capabilities. Tracking them individually via boolean (or
ternary) properties is cumbersome.
Instead, use an enum NMSupplCapType and a corresponding bitmask
NMSupplCapMask. The latter can track whether a capability is detected,
detected to be absent or not detected (unknown).
Most callers would pass FALSE to nm_utils_error_is_cancelled(). That's
not very useful. Split the two functions and have nm_utils_error_is_cancelled()
and nm_utils_error_is_cancelled_is_disposing().
When we deactivate a virtual device, we usually schedule the deletion
of the link in an idle handler. That action will be executed at a
later time when the device is already in the disconnected state.
Similarly, for ovs interfaces we send the deletion command to the
ovsdb and then proceed to the disconnected state.
However, in the first case there is the guarantee that the link will
be deleted at some point, while for ovs interfaces it may happen that
ovs decides to reuse the same link if there is an addition
queued. Since reusing the same link confuses NM, let's implement
deactivate_async() for ovs-interfaces and wait that the link actually
goes away before proceeding.
https://bugzilla.redhat.com/show_bug.cgi?id=1782701https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/402
If the current lease expires, we start the grace period in which the
clients starts again from the INIT DHCP state (i.e. sending DISCOVER
messages). If it is able to obtain a new lease, it must be accepted or
otherwise the client will not renew it.
Currently the DHCP client reports the BOUND state not only when the
lease is obtained initially but also when it is renewed. Having a
different state for the renewal will be used by NMDevice in the next
patch to determine whether the lease needs to be accept()ed or not.
Currently the duration of the DHCP grace period (in which we try to
acquire a new lease after expiration) is hardcoded to 480
seconds. That value seems arbitrary and too long for the default
configuration. Since we already have a property that allows the user
to configure how long NM should try to get the lease initially, it
makes sense to use it also for retries after lease expirations.
In particular, setting the ipvx.dhcp-timeout to a high value extends
also the grace period to a very long time, potentially forever.
and _nm_utils_inet6_ntop() instead of nm_utils_inet6_ntop().
nm_utils_inet4_ntop()/nm_utils_inet6_ntop() are public API of libnm.
For one, that means they are only available in code that links with
libnm/libnm-core. But such basic helpers should be available everywhere.
Also, they accept NULL as destination buffers. We keep that behavior
for potential libnm users, but internally we never want to use the
static buffers. This patch needs to take care that there are no callers
of _nm_utils_inet[46]_ntop() that pass NULL buffers.
Also, _nm_utils_inet[46]_ntop() are inline functions and the compiler
can get rid of them.
We should consistently use the same variant of the helper. The only
downside is that the "good" name is already taken. The leading
underscore is rather ugly and inconsistent.
Also, with our internal variants we can use "static array indices in
function parameter declarations" next. Thereby the compiler helps
to ensure that the provided buffers are of the right size.
I think it's technically not correct to rely on the "sentinal" field
being immediately after the previous field, due to alignment. Implement
the macro differently.
Add a 'in-state-change' pending action to be sure the device always has a
pending when transitioning between states (this prevents callbacks to mark
startup as complete while running _set_state_full()).
This is needed as during the 'failed'->'disconnected' the pending action 'activation-*'
for the device is removed resulting in an empty pending_actions list which then
triggers 'check_if_startup_complete()' that will find no pending action and mark
startup as complete even if the device could have been activated with another connection.
https://bugzilla.redhat.com/show_bug.cgi?id=1759956
It seems to complicate things more than helping. Drop it. What we still have
is a wrapper around plain g_dbus_connection_signal_subscribe(). That one is
trivial and helpful. The previous wrapper seems to add more complexity.
nm_dbus_connection_signal_subscribe_object_manager() wraps the subscription. The problem
is that this requires to pass a destroy notify function for cleaning up. Such a destroy
notify function will result in an idle source when unsubscribing, which keeps the associated
GMainContext alive (until it gets iterated some more). That seems error prone and outright
unsuitable for NMClient.
While the helper may be useful, it cannot be used by NMClient. So, there is only one
user of this function and we don't expect a second one. It seems better to get rid of
this wrapper and implement it directly.
This property is currently most likely not used. Also, because libnm doesn't
expose it and the only known user of this API (gnome-network-displays) doesn't
use it.
In the future we may want to expand on the Groups API. E.g. exposing groups as
separate D-Bus objects, in which case a better property type would be "ao" and
not "as". For now, that is unclear nor requested.
Remove the property for now.
Add VRF support to the daemon. When the device we are activating is a
VRF or a VRF's slave, put routes in the table specified by the VRF
connection.
Also, introduce a VRF device type in libnm.
There is however a serious issue currently: when NetworkManager creates
virtual devices, it starts from an unrealized NMDevice, creates the
netdev device, realizes the device, and transitions through states
UNMANAGED and DISCONNECTED. Thereby, the state of NMDevice gets cleared
again. That means, if the profile has "connection.stable-id=${RANDOM}"
and "ethernet.cloned-mac-address=stable", then we will first set a
random MAC address when creating the device. Then, the NMDevice
transitions through UNMANAGED state, forgets the MAC address it
generated and creates a new MAC address in stage 1. This should be
fixed by better handling unrealized devices. It also affects all
software devices that set the MAC address upon creation of the
interfaces (as they all should).
IP tunnels honor ethernet.cloned-mac-address. That is a MAC address of 6 bytes (ETH_ALEN).
Note that for example for gre tunnels, kernel exposes an address 00:00:00:00. Hence, trying
to set ethernet.cloned-mac-address with an gre tunnel leads to an assertion failure.
Instead, report and log a regular error.
Don't build the same sources multiple times. The test code should
statically link against the tested code, just like the device plugin
that uses the code in production.
Oddly enough, valgrind was not complaining about this leak...
Fixes: 87b2d783b6 ('core: accept 'ssids':aay option in RequestScan() dictionary parameter')
