Kernel does not allow setting the MTU of a VLAN larger
then the MTU of the underlying device. Hence, we might
initially fail to set a large MTU of the VLAN, but we
have to retry when the MTU of the parent changes.
https://bugzilla.redhat.com/show_bug.cgi?id=1414901
By using a macro, we don't cast all the types to guint. Instead,
we use their native types directly. Hence, we don't need
nm_hash_update_uint64() nor nm_hash_update_ptr().
Also, for types smaller then guint like char, we save hashing
the all zero bytes.
Replace the usage of g_str_hash() with our own nm_str_hash().
GLib's g_str_hash() uses djb2 hashing function, just like we
do at the moment. The only difference is, that we use a diffrent
seed value.
Note, that we initialize the hash seed with random data (by calling
getrandom() or reading /dev/urandom). That is a change compared to
before.
This change of the hashing function and accessing the random pool
might be undesired for libnm/libnm-core. Hence, the change is not
done there as it possibly changes behavior for public API. Maybe
we should do that later though.
At this point, there isn't much of a change. This patch becomes
interesting, if we decide to use a different hashing algorithm.
The privious NM_HASH_* macros directly operated on a guint value
and were thus close to the actual implementation.
Replace them by adding a NMHashState struct and accessors to
update the hash state. This hides the implementation better
and would allow us to carry more state. For example, we could
switch to siphash24() transparently.
For now, we still do a form basically djb2 hashing, albeit with
differing start seed.
Also add nm_hash_str() and nm_str_hash():
- nm_hash_str() is our own string hashing implementation
- nm_str_hash() is our own string implementation, but with a
GHashFunc signature, suitable to pass it to g_hash_table_new().
Also, it has this name in order to remind you of g_str_hash(),
which it is replacing.
Introduce a NM_HASH_INIT() function. It makes the places
where we initialize a hash with a certain seed visually clear.
Also, move them from "shared/nm-utils/nm-shared-utils.h" to
"shared/nm-utils/nm-macros-internal.h". We might want to
have NM_HASH_INIT() non-inline (hence, define it in the
source file).
For routes and the default-route from NDisc, set the router preference
RTA_PREF.
Also, previously, we would only configure one IPv6 default-route. That by itself
was not really a problem, as long as NetworkManager would always make sure that
it configured the route to the ~best~ router.
Actually, NM should have done that already. It keeps the list of gateways
sorted, and prefers them according to their preference. But maybe
it didn't, so we have bug rh#1445417 (??).
Change that by configuring a default-route for all gateways, with
appropriate router prefrence. In case, kernel doesn't support RTA_PREF
yet, only configure all routes that share the same maxiumum preference.
https://bugzilla.redhat.com/show_bug.cgi?id=1445417
Instead of having 3 properties @gateway, @never_default and @has_gateway
on NMIP4Config/NMIP6Config that determine the default-route, track the
default-route as a regular route.
The gateway setting is the configuration knob for the default-route.
Since an NMIP4Config/NMIP6Config instance only has one gateway property,
it cannot track more then one default-routes (see related bug rh#1445417).
Especially with policy routing, it might be interesting to configure a
default-route in multiple tables.
Also, later it might be interesting to allow adding default-routes as
regular static routes in a connection, so that the user can configure additional
route parameters for the default-route or add default-routes in multiple tables.
With this patch, default-routes now have a rt_source property according to their
origin.
Also, the previous commits of this branch broke handling of the
default-route :) . That should be working now again.
The MSS is only set for VPN connections (by merging it in the respective
NMIP4Config/NMIP6Config).
It is also only used when setting the MSS of the default route.
Don't track that property in NMIP4Config/NMIP6Config, instead, set the
mss of the route directly in nm_vpn_connection_ip4_config_get() and
nm_vpn_connection_ip6_config_get().
There is a potential change in behavior here: NMDevice also consisdered
the MSS for the default route, but that would only be set if the MSS
gets merged from an vpn-ip-config. Which at most is the case for
iterface-less VPN types (libreswan). But even in that case, it doesn't
seem right to me to use the VPN's MSS for the device's default-route.
We added "ipv4.route-table-sync" and "ipv6.route-table-sync" to not change
behavior for users that configured policy routing outside of NetworkManager,
for example, via a dispatcher script. Users had to explicitly opt-in
for NetworkManager to fully manage all routing tables.
These settings were awkward. Replace them with new settings "ipv4.route-table"
and "ipv6.route-table". Note that this commit breaks API/ABI on the unstable
development branch by removing recently added API.
As before, a connection will have no route-table set by default. This
has the meaning that policy-routing is not enabled and only the main table
will be fully synced. Once the user sets a table, we recognize that and
NetworkManager manages all routing tables.
The new route-table setting has other important uses: analog to
"ipv4.route-metric", it is the default that applies to all routes.
Currently it only works for static routes, not DHCP, SLAAC,
default-route, etc. That will be implemented later.
For static routes, each route still can explicitly set a table, and
overwrite the per-connection setting in "ipv4.route-table" and
"ipv6.route-table".
The line
device (wlan0): state change: ip-config -> ip-check (reason none, internal state managed)
prints the sys-iface-state, which is at other places logged with
device[0x55914506ed70] (wlan0): sys-iface-state: external -> managed
For consistency, name the same parameter the same.
The name nm_device_get_priority() is misleading. Nowadays it's only used
for the default route metric, and nothing else.
Rename it, and make it static.
Found by clang warning:
src/devices/nm-device.c:11370:14: error: use of logical '||' with constant operand [-Werror,-Wconstant-logical-operand]
|| NM_UNMANAGED_USER_UDEV
^ ~~~~~~~~~~~~~~~~~~~~~~
Fixes: 5778bc6a34
If unrealize() failed we returned without thawing notify signals. Fix
this by moving g_object_freeze_notify() after the
unrealization/deletion but before the properties are reset in
unrealize_notify().
Fixes: a93807c288
- use nm_utils_addr_family_to_char(). It asserts that the input argument
is either AF_INET or AF_INET6.
- rename variable @family to @addr_family for consistency.
- when logging addr_family for activation-stage, use v4 or v6 instead
of numeric AF_INET/AF_INET6.
Move creating the logging output inside the logging macro, so it is
evaluated lazyly. Also, use a stack-allocated buffer.
Drop the redundant @inet4 variable.
Since commit 6845b9b80a ("device: delay
startup complete until device is initialized in platform", we also wait
for devices that are still initializing platform/UDEV.
Obviously, that only applies to realized devices.
Otherwise, an unrealized device is going to block startup complete.
Fixes: 6845b9b80a
Sometimes, when we have a platform object, we need to keep it
alive, because any subsequent platform operation might invalidate
the object.
Previously, we achieved that by copying the NMPlatformLink data.
For a while now, all platform object are immuable and recounted.
We should not copy the instance to a NMPlatformLink object, because
then the instance is no longer a full NMPObject. Instead, just take an
additional reference. Since the object must be immutable, that is
just as safe. But now callees down the stack get a proper NMPObject
instance, and might reference it too.
We already have various ways to mark a device as unmanaged.
1) via udev-rule ENV{NM_UNMANAGED}. This can be overwritten via D-Bus
at runtime.
2) via settings plugin. That is NM_CONTROLLED=no for ifcfg-rh and
keyfile.unmanaged-devices in NetworkManager.conf.
3) at runtime, via D-Bus. This is persisted in the run state file
and persists restarts (but not reboot).
This adds another way via NetworkManager.conf file. Note that the
existing keyfile.unmanaged-devices (above 2) is also a configuration
optin in NetworkManager.conf. However it has various downsides:
- it cannot be overwritten at runtime (see commit
c210134bd5).
- you can only explicitly mark a device as unmanaged. That means,
you cannot use it to manage a device which is unmanaged due to
a udev rule.
- the name "keyfile.*" sounds like it's only relevant for the keyfile settings
plugin. Nowadays the keyfile plugin is always loaded, so the option applies
to NetworkManager in general.
https://github.com/NetworkManager/NetworkManager/pull/29
nm_device_match_parent() is called to check whether a device is
compatible with a given parent (UUID or interface). Accept any UUID If
there is no connection active on the device.
Without this, when there is a VLAN/MACVLAN connection with a parent
UUID the manager would create the device in
system_create_virtual_device(), realize it and then at the next call
of system_create_virtual_device() it would notice that the connection
is not compatible with the device because of the parent UUID;
therefore the manager would try to create again the same device,
failing.
https://mail.gnome.org/archives/networkmanager-list/2017-September/msg00034.html
Since commit a21b8882cc ("device: update
external configuration before commit"), we correctly re-sync the
external IP configuration before a merge, in case we notice that
there were some changes in platform.
Go a step further, and do the full update_ext_ip_config(). We should
have one way how to capture the external config, including intersect
and subtract. Otherwise, we end up with an @ext_ip4_config, which is
different from how it looks usually.
Refactor the code. There should be no changes in behavior at all.
The point is, to be able to reuse update_ext_ip_config() in the
next commit.
And also, I think it's an antipattern to have mirroring functions like
ip4_xyz() and ip6_xyz(). Instead, there should be one function, with
extra addr_family argument. That way, it'much clearer where two
implementations differ and where they are identical.
Basically, it moves the differentiation per the address family down
the call stack, closer to the place where the behavior is actually
different.
If the commit of static connection parameters fails before starting
RA, we should reset @con_ip6_config; otherwise any external update
arriving before the commit of RA parameters will remove from
@con_ip6_config all parameters not present externally, because in
update_ip6_config() we do:
/* This function was called upon external changes. Remove the configuration
* (addresses,routes) that is no longer present externally from the internal
* config. This way, we don't re-add addresses that were manually removed
* by the user. */
if (priv->con_ip6_config)
nm_ip6_config_intersect (priv->con_ip6_config, priv->ext_ip6_config);
Instead if @con_ip6_config is cleared it will be rebuilt from the
connection setting at the next commit.
Fixes-test: @ipv6_preserve_cached_routes
When a device managed by NetworkManager is configured manually (adding
ip addresses), NetworkManager will track the device configuration with
an in-memory only config, marking the device as "external".
Devices marked external should be just tracked but left untouched.
This does not happens on current code base: if an ipv4 address is added,
NM generates the in-memory connection, marking the ipv6.method as "ignore".
While activating the connection, NM will process the IPv6 "ignore" method:
this implies leaving the IPv6LL address generation to the kernel. To
trigger this NM will disable/enable IPv6 on the interface.
This not only may change the device configuration but may cause also
a potential race with an external IPv6 assignment on the device.
NetworkManager should do nothing to IPv6 when method is "ignore" and
connection is marked as "external": this commit fixes this behavior.
Note that if/once an IPv6 address is externally added, IPv6 method in the
tracked connection is changed to "manual" and a link local address will be
generated if needed.
https://bugzilla.redhat.com/show_bug.cgi?id=1462260
Before commit 6698bf58bb, we would rely on
kernel to add the device-route for manual IPv6 routes. We broke that and now
kernel would still add the device-route, however nm_platform_ip_route_sync()
would delete it immediately after.
That is because previously nm_platform_ip_route_sync() would ignore routes
with rtm_protocol RTPRO_KERNEL. Now, it will sync and delete those too.
Fix that by adding the device-route like we do it for IPv4. This also
fixes an actual issue where the automatically added route always had
route-metric 256. Instead, we now use the metric from ipv6.route-metric
setting.
Fixes: 6698bf58bb
If we don't commit the IP config, we must merge the currently tracked
default route. Otherwise, on every non-commit call of
ip4_config_merge_and_apply(), the default-route gets lost.
Fixes: 77ec302714
Kernel does not allow to add IPv6 routes with "src", as long as the
corresponding address is still tentative (related bug rh#1457196).
The workaround for this is cumbersome. First, when we fail to add such a
route with "pref_src", we guess that it happend due to this issue. In
that case, nm_ip6_config_commit() returns the list of routes that could
not be added for the moment (but hopefully can be added later).
We track this list in NMDevice, and keep trying to merge the routes
back into ip6_config. In order to not try indefinitely, keep track of a
timestamp when we tried to add this route for the first time.
Another uglyness is that pending tentative routes don't explicitly block
activation. In practice they may do, because for these routes we also have
an IPv6 address that is still doing DAD, so the IP configuration is
still pending due to that.
https://bugzilla.redhat.com/show_bug.cgi?id=1452684
For IPv6, we create device routes when processing the RA and add it to
NMIP6Config like any other route. For IPv4 we didn't do that. Instead
we created the list of device routes during nm_ip4_config_commit() and
passed it to nm_platform_ip_route_sync().
In many cases we want to treat IPv4 and IPv6 generically. That looks nicer
if we distingish by an @addr_family integer, instead of a boolean.
Replace the @is_ipv6 boolean with an @addr_family paramter. The @is_ipv6
boolean is inconsistent with other places where we use @is_ipv4 to
indicate the opposite. Eventually, we should use @addr_family
everywhere.
Also, at the call site it's not immediately clear what TRUE/FALSE means,
here AF_INET/AF_INET6 is better.
- cleanup data type and use guint32 consistently. We might want to
introduce a new "infinity" value. But since libnm's
NM_SETTING_IP_CONFIG_DHCP_TIMEOUT asserts against the range
0 - G_MAXINT32, we cannot express it as -1 anyway. So, infinity
will have the numerical value G_MAXINT32, hence guint32 is just
fine.
- make use of existing ipv6.dhcp-timeout setting and add global
default configuration in NetworkManager.conf
- instead of having subclasses call nm_device_set_dhcp_timeout(),
add a virtual function get_dhcp_timeout().
Previously when the interface created by pppd was already the one we
expected, we would rename it to itself and remove the device from the
manager. Don't do it.
Fixes: 6c3195931e
Remove NMDefaultRouteManager. Instead, add the default-route to the
NMIP4Config/NMIP6Config instance.
This basically reverts commit e8824f6a52.
We added NMDefaultRouteManager because we used the corresponding to `ip
route replace` when configuring routes. That would replace default-routes
on other interfaces so we needed a central manager to coordinate routes.
Now, we use the corresponding of `ip route append` to configure routes,
and each interface can configure routes indepdentently.
In NMDevice, when creating the default-route, ignore @auto_method for
external devices. We shall not touch these devices.
Especially the code in NMPolicy regarding selection of the best-device
seems wrong. It probably needs further adjustments in the future.
Especially get_best_ip_config() should be replaced, because this
distinction VPN vs. devices seems wrong to me.
Thereby, remove the @ignore_never_default argument. It was added by
commit bb75026004, I don't think it's
needed anymore.
This brings another change. Now that we track default-routes in
NMIP4Config/NMIP6Config, they are also exposed on D-Bus like regular
routes. I think that makes sense, but it is a change in behavior, as
previously such routes were not exposed there.
When creating the NMIP4Config/NMIP6Config instance, we must always use the right
ifindex. That is the ifindex, on which we want to apply the config. It also means,
that for device-based VPNs (those with priv->ip_ifindex set, like OpenVPN), the
parent's config must have the ip-ifindex of the parent device. Not of the
VPN's device.
One effect of this bug is that in add_ip4_vpn_gateway_route() we resolve
the route to the external gateway and only accept it if it's on the
parent device. But since the ifindex of the config was wrong, we would accept
route on the wrong interface.
https://bugzilla.gnome.org/show_bug.cgi?id=787370
