The hardware address of a VLAN must be kept aligned with the one of
its parent device, and we already used a signal in NMDeviceVlan to
catch changes in parent address and update the VLAN device
accordingly.
But this didn't work in all cases because the change might happen
after the VLAN gets created but before we register the signal, so it
is necessary to add further checks to enforce the alignment during the
device activation.
https://bugzilla.redhat.com/show_bug.cgi?id=1325752
$ nmcli connection up my-connection
Connection successfully activated (D-Bus active path: /org/freedesktop/NetworkManager/ActiveConnection/42)
$ nmcli connection modify my-connection connection.id other-name
$ nmcli -f connection.id connection show other-name
connection.id: other-name
$ nmcli -f GENERAL.CONNECTION device show enp0s25
GENERAL.CONNECTION: my-connection
$ nmcli connection down other-name
Error: 'other-name' is not an active connection.
Error: no active connection provided.
$ nmcli connection down my-connection
Connection 'my-connection' successfully deactivated (D-Bus active path: /org/freedesktop/NetworkManager/ActiveConnection/13)
When modifying a connection, NMActiveConnection must update the D-Bus
properties that belong to the settings-connection.
This is mostly interesting of NMPolicy, which no longer needs to
subscribe to two almost identical signals (where the by-user signal
was always invoked together with the plain "updated" signal).
Instead of connecting to two similar signals, combine them into one
and pass "by_user" argument.
We still need to keep the original NM_SETTINGS_CONNECTION_UPDATED signal,
because it is exposed on D-Bus.
I want to combine NM_SETTINGS_CONNECTION_UPDATED and NM_SETTINGS_CONNECTION_UPDATED_BY_USER
into one signal. Thus, they must have same behavior with respect as to whether they are
scheduled on idle.
Emit NM_SETTINGS_CONNECTION_UPDATED right away.
The "Updated" signal is exposed via D-Bus on the settings object.
Removing the idle handling has no bad consequences there.
Apart from that, the signal has only the listener NMSettings::connection_updated().
connection_updated() first emits NM_SETTINGS_SIGNAL_CONNECTION_UPDATED, which only
has listener: NMPolicy::connection_updated(), which already delegates to
an idle handler.
Second, NMSettings::connection_updated() emits
NM_CP_SIGNAL_CONNECTION_UPDATED which has the following listeners:
- NMBluezDevice::cp_connection_updated(), which can cause the
connection to be tracked/untracked. Any further changes are
already delayed on an idle handler (check_emit_usable_schedule()).
- NMDevice::cp_connection_added_or_updated(), which causes the
connection to be tracked by the device. It also emits
"notify::available-connections", which however is only relevant
for the D-Bus bindings.
At last, it does available_connections_check_delete_unrealized()
which also is on an idle handler already.
Currently, NM_CP_SIGNAL_CONNECTION_UPDATED signal will be invoked on an
idle handler. I want to change that, so to be save, delay actions that
matter on an idle handler.
schedule_activate_all() needs to iterate over all devices and is thus
relatively costly (and scales O(n^2)).
By scheduling the action on an idle handler we delay and combine
multiple redundant requests.
Another reason is that NM_SETTINGS_CONNECTION_UPDATED is currently
executed on an idle handler which first leads to
NM_SETTINGS_SIGNAL_CONNECTION_UPDATED signal and eventually calls
schedule_activate_all().
I want to change that to emit the connection update signal immediately,
thus to preserve the delay, we delay handling in NMPolicy.
Due to a bug, NMManager would connect to "notify::connections"
and might miss an important notification when NMSettings declares
startup-complete.
Fixes: b067ca7034
Adding addresses with a prefix of zero is valid. Don't
reject them.
Note that this is an actual bug. If you configure an
address with prefix length zero, nmcli will report:
$nmcli connection
(process:1040): libnm-WARNING **: Ignoring invalid IP4 address: Invalid IPv4 address prefix '0'
Lots of cleanup and refactoring. Most notably, make the
prefix-length property guint8.
Also fix keeping the route cache in sync when "modifying"
an existing route via `ip route change` or `ip route replace`.
Consider:
unshare -n
ip link add d0 type dummy
ip link add d1 type dummy
ip link set d0 up
ip link set d1 up
ip addr add 192.168.100.5/24 dev d0
ip addr add 192.168.101.5/24 dev d1
ip route add 192.168.200.0/24 via 192.168.100.1
ip monitor &
ip route change 192.168.200.0/24 via 192.168.101.1
#prints 192.168.200.0/24 via 192.168.101.1 dev d1
ip route show
#192.168.100.0/24 dev d0 proto kernel scope link src 192.168.100.5
#192.168.101.0/24 dev d1 proto kernel scope link src 192.168.101.5
#192.168.200.0/24 via 192.168.101.1 dev d1
Note that `ip route change` replaced the exising route. "Replaced" in this
case means: the previous route on device "d0" got removed and a new route
on "d1" was added. However, kernel only sent one RTM_NEWROUTE event, no
RTM_DELROUTE that notifies about this change.
We need to workaround that by re-synching the routes when we receive a
RTM_NEWROUTE notification.
We aim to keep the platform cache up-to-date only via the netlink
events. However, due to kernel shortcomings we often have to resync
by re-requesting the data, which especially for routes and addresses
means a full dump (as you cannot request only specific route/address
information).
Thus it makes sense to avoid expensive dumps whenever we can.
We schedule dumps via "delayed-actions" and that is already smart
so that muliple schedulings are combined. However, before requesting
a new dump, we clear the flag that indicates that a dump is scheduled.
Thus, while processing the result of of a dump, we would re-schedule
anew which can be necessary in some cases.
In certain cases, we don't require a full resync, when we are in the
middle of processing a dump, because that one dump will provide us
with the full picture. Thus, we can avoid scheduling a new dump if
- we already scheduled a delayed action
- we are in the middle or processing a dump.
This can now be checked via delayed_action_refresh_all_in_progress().
Instead of returning only TRUE/FALSE, return the number of signals
that were received while waiting. This make the API cleared, because
previously I always had to check anew whether wait-for-signal returns
TRUE or FALSE on timeout.
Also, add nmtstp_assert_wait_for_signal() and nmtstp_assert_wait_for_signal_until()
macros.
This allows tests to use these functions on a different platform instance
then on the singleton. The change makes the argument list longer, which is
unfortunate. On the other hand, it makes those functions more useful
in general.
You can't have it all.
Also, they now follow the pattern of most functions in NM where the type
is a singleton: you always pass the singleton to the function, although
in the usual case there is only one singleton instance. This allows to
use the function also on the non-singleton instance.
