We try to set only one time the MTU from the connection to not
interfere with manual user changes.
If at some point the parent interface changes temporarily MTU to a
lower value (for example, because the connection was reactivated), the
kernel will also lower the MTU on child interface and we will not
update it ever again.
Add a workaround to this. If we detect that the MTU we want to set
from connection is higher that the allowed one, go into a state where
we follow the parent MTU until it is possible to set again the desired
MTU. This is a bit ugly, but I can't think of any nicer way to do it.
https://bugzilla.redhat.com/show_bug.cgi?id=1751079
Introduce a generic function to set a MTU based on parent's one. Also
define a device-specific @mtu_parent_delta value that specifies the
difference from parent MTU that should be set by default. For VLAN it
is zero but other interface types (for example MACsec) require a
positive value due to encapsulation overhead.
found by cppcheck
[src/devices/nm-device.c:3032] -> [src/devices/nm-device.c:3025]: (warning) Either the condition '!handle' is redundant or there is possible null pointer dereference: handle.
https://github.com/NetworkManager/NetworkManager/pull/352
Only reapply the IP configuration on link up if the IP state is CONF
or DONE. Previously we also reapplied it when the device was
disconnected (IP state NONE) and this could lead to a situation where
an incomplete config was applied; then we intersected the desired
configuration with the external - incomplete - one, causing the
removal of part of desired configuration (for example the default
route).
Fixes: d0b16b9283 ('device: unconditionally reapply IP configuration on link up')
https://bugzilla.redhat.com/show_bug.cgi?id=1754511https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/291
This is a complete refactoring of the bluetooth code.
Now that BlueZ 4 support was dropped, the separation of NMBluezManager
and NMBluez5Manager makes no sense. They should be merged.
At that point, notice that BlueZ 5's D-Bus API is fully centered around
D-Bus's ObjectManager interface. Using that interface, we basically only
call GetManagedObjects() once and register to InterfacesAdded,
InterfacesRemoved and PropertiesChanged signals. There is no need to
fetch individual properties ever.
Note how NMBluezDevice used to query the D-Bus properties itself by
creating a GDBusProxy. This is redundant, because when using the ObjectManager
interfaces, we have all information already.
Instead, let NMBluezManager basically become the client-side cache of
all of BlueZ's ObjectManager interface. NMBluezDevice was mostly concerned
about caching the D-Bus interface's state, tracking suitable profiles
(pan_connection), and moderate between bluez and NMDeviceBt.
These tasks don't get simpler by moving them to a seprate file. Let them
also be handled by NMBluezManager.
I mean, just look how it was previously: NMBluez5Manager registers to
ObjectManager interface and sees a device appearing. It creates a
NMBluezDevice object and registers to its "initialized" and
"notify:usable" signal. In the meantime, NMBluezDevice fetches the
relevant information from D-Bus (although it was already present in the
data provided by the ObjectManager) and eventually emits these usable
and initialized signals.
Then, NMBlue5Manager emits a "bdaddr-added" signal, for which NMBluezManager
creates the NMDeviceBt instance. NMBluezManager, NMBluez5Manager and
NMBluezDevice are strongly cooperating to the point that it is simpler
to merge them.
This is not mere refactoring. This patch aims to make everything
asynchronously and always cancellable. Also, it aims to fix races
and inconsistencies of the state.
- Registering to a NAP server now waits for the response and delays
activation of the NMDeviceBridge accordingly.
- For NAP connections we now watch the bnep0 interface in platform, and tear
down the device when it goes away. Bluez doesn't send us a notification
on D-Bus in that case.
- Rework establishing a DUN connection. It no longer uses blocking
connect() and does not block until rfcomm device appears. It's
all async now. It also watches the rfcomm file descriptor for
POLLERR/POLLHUP to notice disconnect.
- drop nm_device_factory_emit_component_added() and instead let
NMDeviceBt directly register to the WWan factory's "added" signal.
If DHCPv4 fails but IPv6 succeeds it makes sense to continue trying
DHCP so that we will eventually be able to get an address if the DHCP
server comes back. Always keep the client running; it will be only
terminated when the connection is brought down.
https://bugzilla.redhat.com/show_bug.cgi?id=1688329
In the accept() callback, the nettools client creates a UDP socket
with the received address as source, so the address must be already
configured on the interface.
Also, handle errors returned by nm_dhcp_client_accept().
Fixes: 401fee7c20 ('dhcp: support notifying the client of the result of DAD')
The nm-owned flag indicates whether the device was created by NM. If
the realization step fails, the device was not created and so nm-owned
should not be updated.
When the master AC becomes ready, activate_stage1_device_prepare() is
called in a idle handler. If the master AC fails in the meantime, it
will change state to deactivating or deactivated. We must check for
that condition before proceeding with slave activation. Note the the
'master_ready' flag of an AC is never cleared after it is set.
Fixes: 5b677d5a3b ('device: move check for master from nm_device_activate_schedule_stage2_device_config() to end of stage1')
https://bugzilla.redhat.com/show_bug.cgi?id=1747998
With accept_ra set to 1, kernel sends its own router solicitation
messages and parses the advertisements. This duplicates what NM
already does in userspace and has unwanted consequences like [1] and
[2].
The only reason why accept_ra was re-enabled in the past was to apply
RA parameters like ReachableTime and RetransTimer [3]; but now NM
supports them and so accept_ra can be turned off again.
Also, note that previously the option was set in
addrconf6_start_with_link_ready(), and so this was done only when the
method was 'auto'. Instead, now we clear it for all methods except
'ignore'.
[1] https://mail.gnome.org/archives/networkmanager-list/2019-June/msg00027.html
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1734470
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1068673
IPv6 router advertisement messages contain the following parameters
(RFC 4861):
- Reachable time: 32-bit unsigned integer. The time, in
milliseconds, that a node assumes a neighbor is reachable after
having received a reachability confirmation. Used by the Neighbor
Unreachability Detection algorithm. A value of zero means
unspecified (by this router).
- Retrans Timer: 32-bit unsigned integer. The time, in milliseconds,
between retransmitted Neighbor Solicitation messages. Used by
address resolution and the Neighbor Unreachability Detection
algorithm. A value of zero means unspecified (by this router).
Currently NM ignores them; however, since it leaves accept_ra=1, the
kernel parses RAs and applies those parameters for us [1].
In the next commit kernel handling of RAs will be disabled, so let NM
set those neighbor-related parameters.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ipv6/ndisc.c?h=v5.2#n1353
Note that by now no callers of nm_device_activate_schedule_stage2_device_config()
are left. All previous callers now re-schedule stage1 instead of directly
scheduling stage2.
Note that if stage2 later also gets re-factored to re-enter itself
instead of scheduling stage3 right away, the function will be used
again.
That means, we can move the check for the master where it belongs: as
part (and at the end of) stage1.
Also, slightly simplify the code. The handler master_ready_cb()
no longer directly calls master_ready(). It's enough to always
enter stage1 again.
Also drop master_ready_handled. We don't need to remember that this
condition was satsified. We can just check it always when we reach
the place in activate_stage1_device_prepare().
NMDevice's act_stage1_prepare() now does nothing. Calling it is not
useful and has no effect.
In general, when a subclass overwrites a virtual function, it must be
defined whether the subclass must, may or must-not call the parents
implementation. Likewise, it must be clear when the parents
implementation should be chained: first, as last, does it matter?
In any case, that very much depends on how the parent is implemented
and this can only be solved by documentation and common conventions.
It's a forgiving approach to have a parents implementation do nothing,
then the subclass may call it at any time (or not call it at all).
This is especially useful if classes don't know their parent class well.
But in NetworkManager code the relationship between classes are known
at compile time, so every of these classes knows it derives directly
from NMDevice.
This forgingin approach was what NMDevice's act_stage1_prepare() was doing.
However, it also adds lines of code resulting in a different kind of complexity.
So, it's not clear that this forgiving approach is really better. Note
that it also has a (tiny) runtime and code-size overhead.
Change the expectation of how NMDevice's act_stage1_prepare() should be
called: it is no longer implemented, and subclasses *MUST* not chain up.
Note that all subclasses of NMDevice that implement act_stage1_prepare(), call
the parents implementation as very first thing.
Previously, NMDevice's act_stage1_prepare() was setting up SR-IOV. But that is
problemantic. Note that it may have returned NM_ACT_STAGE_RETURN_POSTPONE, in which
case subclasses would just skip their action and return to the caller. Later,
sriov_params_cb() would directly call nm_device_activate_schedule_stage2_device_config(),
and thus act_stage1_prepare() was never executed for the subclass. That
is wrong.
First, I don't think it is good to let subclasses decide whether to call a
parents implementation (and when). It just causes ambiguity. In
the best case we do it always in the same order, in the worst case we
call the parent at the wrong time or never. Instead, we want to initialize
SR-IOV always and early in stage1, so we should just do it directly from
activate_stage1_device_prepare(). Now NMDevice's act_stage1_prepare() does
nothing.
The bigger problem is that when a device wants to resume a stage that
it previously postponed, that it would schedule the next stage!
Instead, it should schedule the same stage again instead. That allows
to postpone the completion of a stage for multiple reasons, and each
call to a certain stage merely notifies that something changed and
we need to check whether we can now complete the stage.
For that to work, stages must become re-entrant. That means we need to
remember whether an action that we care about needs to be started, is pending
or already completed.
Compare this to nm_device_activate_schedule_stage3_ip_config_start(),
which checks whether firewall is configured. That is likewise the wrong
approach. Callers that were in stage2 and postponed stage2, and later would
schedule stage3 when they are ready.
Then nm_device_activate_schedule_stage3_ip_config_start() would check whether
firewall is also ready, and do nothing if that's not the case (relying
that when the firewall code completes to call
nm_device_activate_schedule_stage3_ip_config_start().
- use a [2] array for IPv4/IPv6 variants and a IS_IPv4 variable,
like we do for other places that have similar implementations for
both address families.
- drop ActivationHandleData and use the fields directly. Also drop
activation_source_get_by_family().
- rename "act_handle*" field to "activation_source_*", to follow the
naming of the related accessor functions.
- downgrade the severity of some logging messages.
- Don't use GDBusProxy but plain GDBusConnection. NMFirewallManager
is very simple, it doesn't use any of the features that GDBusProxy
provides.
- make NMFirewallManagerCallId typedef a pointer to the opaque call-id
struct, instead of the struct itself. It's confusing to have a
variable that does not look like a pointer and assigning %NULL to
it.
- internally drop the CBInfo typename and name the call-id variable
constsistantly as "call_id".
- no need to keep the call-id struct alive after cancelling it. That
simplifies the lifetime managment of the pending call because the
completion callback is always invoked shortly before destroying
the call-id.
- note that the caller is no longer allowed to cancel a call-id from
inside the completion callback. That just complicates the
implementation and is not necessary. Assert against that.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/230
Not all masters type have a platform link and so it's wrong to check
for it to decide whether the slave should be really released. Move the
check to master devices that need it (bond, bridge and team).
OVS ports don't need the check because they don't call to platform to
remove a slave.
https://bugzilla.redhat.com/show_bug.cgi?id=1733709
(cherry picked from commit 57e3734b6c)
We set nm-owned to indicate whether a software device was created by
NM or it was pre-existing. When checking the existence, we must verify
also whether the link type is compatible with the device, otherwise it
is possible to match unrelated interfaces. For example, when checking
for the existence of an ovs-bridge (which is not compatible with any
platform link) we could match a unrelated platform link with the same
name.
https://bugzilla.redhat.com/show_bug.cgi?id=1733709
(cherry picked from commit 3cb4b36261)
For WireGuard (like for all IP-tunnels and IP-based VPNs), the IP addresses of
the peers must be reached outside the tunnel/VPN itself.
For VPN connections, NetworkManager usually adds a direct /32 route to
the external VPN gateway to the underlying device. For WireGuard that is
not done, because injecting a route to another device is ugly and error
prone. Worse: WireGuard with automatic roaming and multiple peers makes this
more complicated.
This is commonly a problem when setting the default-route via the VPN,
but there are also other subtle setups where special care must be taken
to prevent such routing loops.
WireGuard's wg-quick provides a simple, automatic solution by adding two policy
routing rules and relying on the WireGuard packets having a fwmark set (see [1]).
Let's also do that. Add new properties "wireguard.ip4-auto-default-route"
and "wireguard.ip6-auto-default-route" to enable/disable this. Note that
the default value lets NetworkManager automatically choose whether to
enable it (depending on whether there are any peers that have a default
route). This means, common scenarios should now work well without additional
configuration.
Note that this is also a change in behavior and upon package upgrade
NetworkManager may start adding policy routes (if there are peers that
have a default-route). This is a change in behavior, as the user already
clearly had this setup working and configured some working solution
already.
The new automatism picks the rule priority automatically and adds the
default-route to the routing table that has the same number as the fwmark.
If any of this is unsuitable, then the user is free to disable this
automatism. Note that since 1.18.0 NetworkManager supports policy routing (*).
That means, what this automatism does can be also achieved via explicit
configuration of the profile, which gives the user more flexibility to
adjust all parameters explicitly).
(*) but only since 1.20.0 NetworkManager supports the "suppress_prefixlength"
rule attribute, which makes it impossible to configure exactly this rule-based
solution with 1.18.0 NetworkManager.
[1] https://www.wireguard.com/netns/#improved-rule-based-routing
Via Update2() D-Bus API there are three ways how a profile can be stored
(or migrated) to in-memory:
- NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY
- NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED
- NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_ONLY
With the recent rework of settings I dropped NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY
and it had the same meaning as NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED.
However, the way NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED was implemented is
problematic. The problem is that it leaves the profile on disk but creates an
in-memory representation which shadows the persistent storage. Later,
when storing the profile to disk again, a new filename is chosen.
This allows via D-Bus API to toggle between NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED
and NM_SETTINGS_UPDATE2_FLAG_TO_DISK, and thereby pilling up profiles on disk.
Also, there is no D-Bus API to do anything sensible with these leaked, shadowed
profiles on disk.
Note that if we have a read-only profile in /usr/lib or in ifupdown
plugin, then the problem is not made any worse. That is, because via D-Bus
API such profiles can be made in-memory, and afterwards stored to /etc.
Thereby too the profile gets duplicate on disk, but this game only
works once. Afterwards, you cannot repeat it to create additional
profiles on disk. It means, you can only leak profiles once, and only
if they already exist in read-only storage to begin with.
This problem with NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED already existed
before the settings-delegate-storage rework, and is unrelated to whether in-memory
profiles now happen to be persisted to /run.
Note that NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_ONLY is simple and does not suffer
from this problem. When you move a profile to in-memory-only, it gets deleted from
persistent storage and no duplication happens.
The problem is that NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED used to
forget about the profile that it shadows, and that is wrong.
So, first re-add proper support for NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY. This
works by remembering the "shadowed-storage" path for in-memory profiles.
When later saving such a profile to disk again, the shadowed-storage
will be re-used. Likewise, when deleting such a profile, the shadowed
storage will be deleted.
Note that we keep NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED and it
also remembers the shadowed storage (but without "owning" it). That means,
when such a profile gets saved to disk again, the orginal storage is
reused too. As such, during future updates it behaves just like
NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY. The difference is when deleting
such a profile. In this case, the profile is left on storage and a
tombstone gets written. So, how is this better than before and why even
keep this complicated flag?
First, we keep this flag because we really want the ansible role to be
able to do in-memory changes only. That implies being able to delete a
profile from NetworkManager's view, but not from persistent storage. Without
this flag there is no way to do that. You can only modify an on-disk profile
by shadowing it, but you could not delete it form NetworkManager's view
while keeping it on disk.
The new form of NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED is safe and avoids
the duplication problem because also for tombstones it remembers the original
"shadowed-storage". That is, when the profile gets recreated later via
D-Bus API AddConnection, then the re-created profile will still reference
and reuse the shadowed storage that it had before deletion.
nm_config_set_no_auto_default_for_device() is called by NMSettings,
so it makes sense that also NMSettings checks whether the device is
blocked.
Of course, there is little difference in practice.
The only downside is that most device types don't implement
new_default_connection(). So the previous form performed the
cheaper check first. On the other hand, we do expect to have
profiles for the devices anyway.
Only NMDeviceEthernet implements new_default_connection(). Anyway, it
makes only sense to do this precheck by the caller first, and not by
each implementation.
Completely rework how settings plugin handle connections and how
NMSettings tracks the list of connections.
Previously, settings plugins would return objects of (a subtype of) type
NMSettingsConnection. The NMSettingsConnection was tightly coupled with
the settings plugin. That has a lot of downsides.
Change that. When changing this basic relation how settings connections
are tracked, everything falls appart. That's why this is a huge change.
Also, since I have to largely rewrite the settings plugins, I also
added support for multiple keyfile directories, handle in-memory
connections only by keyfile plugin and (partly) use copy-on-write NMConnection
instances. I don't want to spend effort rewriting large parts while
preserving the old way, that anyway should change. E.g. while rewriting ifcfg-rh,
I don't want to let it handle in-memory connections because that's not right
long-term.
--
If the settings plugins themself create subtypes of NMSettingsConnection
instances, then a lot of knowledge about tracking connections moves
to the plugins.
Just try to follow the code what happend during nm_settings_add_connection().
Note how the logic is spread out:
- nm_settings_add_connection() calls plugin's add_connection()
- add_connection() creates a NMSettingsConnection subtype
- the plugin has to know that it's called during add-connection and
not emit NM_SETTINGS_PLUGIN_CONNECTION_ADDED signal
- NMSettings calls claim_connection() which hocks up the new
NMSettingsConnection instance and configures the instance
(like calling nm_settings_connection_added()).
This summary does not sound like a lot, but try to follow that code. The logic
is all over the place.
Instead, settings plugins should have a very simple API for adding, modifying,
deleting, loading and reloading connections. All the plugin does is to return a
NMSettingsStorage handle. The storage instance is a handle to identify a profile
in storage (e.g. a particular file). The settings plugin is free to subtype
NMSettingsStorage, but it's not necessary.
There are no more events raised, and the settings plugin implements the small
API in a straightforward manner.
NMSettings now drives all of this. Even NMSettingsConnection has now
very little concern about how it's tracked and delegates only to NMSettings.
This should make settings plugins simpler. Currently settings plugins
are so cumbersome to implement, that we avoid having them. It should not be
like that and it should be easy, beneficial and lightweight to create a new
settings plugin.
Note also how the settings plugins no longer care about duplicate UUIDs.
Duplicated UUIDs are a fact of life and NMSettings must handle them. No
need to overly concern settings plugins with that.
--
NMSettingsConnection is exposed directly on D-Bus (being a subtype of
NMDBusObject) but it was also a GObject type provided by the settings
plugin. Hence, it was not possible to migrate a profile from one plugin to
another.
However that would be useful when one profile does not support a
connection type (like ifcfg-rh not supporting VPN). Currently such
migration is not implemented except for migrating them to/from keyfile's
run directory. The problem is that migrating profiles in general is
complicated but in some cases it is important to do.
For example checkpoint rollback should recreate the profile in the right
settings plugin, not just add it to persistent storage. This is not yet
properly implemented.
--
Previously, both keyfile and ifcfg-rh plugin implemented in-memory (unsaved)
profiles, while ifupdown plugin cannot handle them. That meant duplication of code
and a ifupdown profile could not be modified or made unsaved.
This is now unified and only keyfile plugin handles in-memory profiles (bgo #744711).
Also, NMSettings is aware of such profiles and treats them specially.
In particular, NMSettings drives the migration between persistent and non-persistent
storage.
Note that a settings plugins may create truly generated, in-memory profiles.
The settings plugin is free to generate and persist the profiles in any way it
wishes. But the concept of "unsaved" profiles is now something explicitly handled
by keyfile plugin. Also, these "unsaved" keyfile profiles are persisted to file system
too, to the /run directory. This is great for two reasons: first of all, all
profiles from keyfile storage in fact have a backing file -- even the
unsaved ones. It also means you can create "unsaved" profiles in /run
and load them with `nmcli connection load`, meaning there is a file
based API for creating unsaved profiles.
The other advantage is that these profiles now survive restarting
NetworkManager. It's paramount that restarting the daemon is as
non-disruptive as possible. Persisting unsaved files to /run improves
here significantly.
--
In the past, NMSettingsConnection also implemented NMConnection interface.
That was already changed a while ago and instead users call now
nm_settings_connection_get_connection() to delegate to a
NMSimpleConnection. What however still happened was that the NMConnection
instance gets never swapped but instead the instance was modified with
nm_connection_replace_settings_from_connection(), clear-secrets, etc.
Change that and treat the NMConnection instance immutable. Instead of modifying
it, reference/clone a new instance. This changes that previously when somebody
wanted to keep a reference to an NMConnection, then the profile would be cloned.
Now, it is supposed to be safe to reference the instance directly and everybody
must ensure not to modify the instance. nmtst_connection_assert_unchanging()
should help with that.
The point is that the settings plugins may keep references to the
NMConnection instance, and so does the NMSettingsConnection. We want
to avoid cloning the instances as long as they are the same.
Likewise, the device's applied connection can now also be referenced
instead of cloning it. This is not yet done, and possibly there are
further improvements possible.
--
Also implement multiple keyfile directores /usr/lib, /etc, /run (rh #1674545,
bgo #772414).
It was always the case that multiple files could provide the same UUID
(both in case of keyfile and ifcfg-rh). For keyfile plugin, if a profile in
read-only storage in /usr/lib gets modified, then it gets actually stored in
/etc (or /run, if the profile is unsaved).
--
While at it, make /etc/network/interfaces profiles for ifupdown plugin reloadable.
--
https://bugzilla.gnome.org/show_bug.cgi?id=772414https://bugzilla.gnome.org/show_bug.cgi?id=744711https://bugzilla.redhat.com/show_bug.cgi?id=1674545
4 properties are not really relevant for an already activated connection
or it makes not sense to change them. These are connection.id, connection.uuid,
connection.autoconnect and connection.stable-id.
For convenience, we allow to reapply these. This way, one can take
a different setting (e.g. with a different connection.id or
connection.uuid) and reapply them, but such changes are silently
ignored.
However this was done wrongly. Instead of reverting the change to the new
applied connection, we would change the input connection.
This is bad, for example with
nmcli connection up uuid cb922f18-e99a-49c6-b200-1678b5070a82
nmcli connection modify cb922f18-e99a-49c6-b200-1678b5070a82 con-name "bogus"
nmcli device reapply eth0
the last re-apply would reset the settings-connection's connection ID to
what was before, while accepting the new name on the applied-connection
(while it should have been rejected).
Fixes: bf3b3d444c ('device: avoid changing immutable properties during reapply')
IP addresses, routes, TC and QDiscs are all tied to a certain interface.
So when NetworkManager manages an interface, it can be confident that
all related entires should be managed, deleted and modified by NetworkManager.
Routing policy rules are global. For that we have NMPRulesManager which
keeps track of whether NetworkManager owns a rule. This allows multiple
connection profiles to specify the same rule, and NMPRulesManager can
consolidate this information to know whether to add or remove the rule.
NMPRulesManager would also support to explicitly block a rule by
tracking it with negative priority. However that is still unused at
the moment. All that devices do is to add rules (track with positive
priority) and remove them (untrack) once the profile gets deactivated.
As rules are not exclusively owned by NetworkManager, NetworkManager
tries not to interfere with rules that it knows nothing about. That
means in particular, when NetworkManager starts it will "weakly track"
all rules that are present. "weakly track" is mostly interesting for two
cases:
- when NMPRulesManager had the same rule explicitly tracked (added) by a
device, then deactivating the device will leave the rule in place.
- when NMPRulesManager had the same rule explicitly blocked (tracked
with negative priority), then it would restore the rule when that
block gets removed (as said, currently nobody actually does this).
Note that when restarting NetworkManager, then the device may stay and
the rules kept. However after restart, NetworkManager no longer knows
that it previously added this route, so it would weakly track it and
never remove them again.
That is a problem. Avoid that, by whenever explicitly tracking a rule we
also make sure to no longer weakly track it. Most likely this rule was
indeed previously managed by NetworkManager. If this was really a rule
added by externally, then the user really should choose distinct
rule priorities to avoid such conflicts altogether.
The previous code returned that the device was available when it had
only unmanaged-flags that can be overridden by user, without actually
considering the @flags argument.
Fixes: 920346a5b9 ('device: add and use overrule-unmanaged flag for nm_device_check_connection_available()')
Drop nm_platform_link_get_address_as_bytes() and introduce
nmp_link_address_get_as_bytes() so that it becomes possible to obtain
also the broadcast address without an additional lookup of the link.
The DHCP client is not meant to use the assigned address before DAD
has completed successfully, if enabled. And if DAD fails, the server
should be notified with a DECLINE, in order to potentially blacklist
the address.
Currently, none of the clients support this, but add the required
callbacks, and allow clients to opt in if they want.
nm_connection_verify() returns success for fully valid (normalized)
connections and also connections that are NM_SETTING_VERIFY_NORMALIZABLE.
We really want to fully normalize the profiles during add-and-activate.
NMConnectivity can now distinguish between LIMITED and NONE connectivity
and it does so based on whether IP addresses and routes are configured.
Previously, NMConnectivity would not differenciate between limited and
no connectivity, which is why NMDevice added some additional logic on top
to coerce LIMITED to NONE (if the device is not logically connected).
But note that the connectivity state (whether a network is reachable on
an interface) depends on what is configured in kernel and whether the
internet is reachable on that interface. It does not depend on the
logical device state.
On the other hand, whether the device is configured in a manner to have
connectivity depends on the logical state of the device (as NetworkManager
is configuring the device).
So, in many cases, the logical state and the connectivity state agree now,
but for the right reasons.
This reverts commit 4c4dbcb78d.
If the interface has no carrier, no addresses or no routes there is no
point in starting a connectivity check on it because it will fail.
Moreover, doing the check on a device without routes causes the
addition of a negative entry in the ARP table for each of the
addresses associated with the connectivity check host; this can lead
to poor network performances.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/181
Changing "ipv4.route-table" and "ipv6.route-table" was not allowed
during reapply.
The main difficulty for supporting that is changing the sync-mode.
With route-table 0, we don't sync all tables but only the main table.
So, when reapply changes from full-sync to no-full-sync, it's slightly
more complicated.
But it's probably not too complicated either. The change from
no-full-sync to full-sync is simple: we just start doing a full-sync.
The reverse change is slightly more complicated, because we need to
do one last full-sync, to get rid of routes that we configured on those
other tables.
