str_if_set() was added to replace the non-standard gcc extension "?:".
However, "?:" is supported by clang as well and we already use it at
several places.
Also, str_if_set() did not follow our naming scheme and renaming to
nm_str_if_set() would be ugly. So just drop it.
Most functions defined in "nm-platform-utils.h" perform a lookup
of link properties, for example via ethtool or sysfs. Those functions
depend on the system configuration, such as the current network namespace.
Move the simple helper functions away to "nm-core-internal.h", so that
all remaining functions from "nm-platform-utils.h" are really related to
somthing that interacts with the system/kernel.
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
Like we already do for IPv6 addresses, we should expose addresses
in a defined, stable sort order.
Clients usually show the addresses in the same order as obtained
via D-Bus.
- "gsystem-local-alloc.h" and <gio/gio.h> are already included via
"nm-default.h". No need to include them separately.
- include "nm-macros-internal.h" via "nm-default.h" and drop all
explict includes.
- in the modified files, ensure that we always include "config.h"
and "nm-default.h" first. As second, include the header file
for the current source file (if applicable). Then follow external
includes and finally internal nm includes.
- include nm headers inside source code files with quotes
- internal header files don't need to include default headers.
They can savely assume that "nm-default.h" is already included
and with it glib, nm-glib.h, nm-macros-internal.h, etc.
This is the same as 04c70c76bc for the
NMIP4Config and NMIP6Config structures. The new field makes debugging
of issues related to IP configuration much easier.
dhclient adds a trailing dot to domain search list entries received
from the server, while the same domains received by other means
(dhcpcd on RA) don't have the final dot. The result is that
resolv.conf can be populated with duplicated entries.
Fix this by stripping the trailing dot when a new search domain is
added to a IP configuration.
https://bugzilla.gnome.org/show_bug.cgi?id=758777
When @src didn't have a gateway and @dst did, the function left @dst's
gateway set to 0.0.0.0; fix this and unset the gateway in such case.
Fixes: 063677101a
The peer-address (IFA_ADDRESS) can also be all-zero (0.0.0.0).
That is distinct from an usual address without explicit peer-address,
which implicitly has the same peer and local address.
Previously, we treated an all-zero peer_address as having peer and
local address equal. This is especially grave, because the peer is part
of the primary key for an IPv4 address. So we not only get a property of
the address wrong, but we wrongly consider two different addresses as
one and the same.
To properly handle these addresses, we always must explicitly set the peer.
Usually, the peer-address is the same as the local address.
In case where it is not, it is the peer-address that determines
the IPv4 device-route. So we must use the peer-address.
Also, don't consider device-routes with the first octet of zero,
just like kernel does.
Also, nm_ip4_config_get_subnet_for_host() is effectively the same
as nm_ip4_config_destination_is_direct(). So drop it.
Arguably, it is more convenient to use the static buffer as
it saves typing.
But having such a low-level function use a static buffer also
limits the way how to use it. As it was, you could not avoid
using the static buffer.
E.g. you cannot do:
char buf[100];
_LOGD ("nmp-object: %s; platform-link: %s",
nmp_object_to_string (nmpobj, buf, sizeof(buf)),
nm_platform_link_to_string (link));
This will fail for non-obvious reasons because both
to-string functions end up using the same static buffer.
Also change the to-string implementations to accept NULL
as valid and return it as "(null)".
https://bugzilla.gnome.org/show_bug.cgi?id=756427
Kernel allows to add the same IPv4 address that only differs by
peer-address (IFL_ADDRESS):
$ ip link add dummy type dummy
$ ip address add 1.1.1.1 peer 1.1.1.3/24 dev dummy
$ ip address add 1.1.1.1 peer 1.1.1.4/24 dev dummy
RTNETLINK answers: File exists
$ ip address add 1.1.1.1 peer 1.1.2.3/24 dev dummy
$ ip address show dev dummy
2: dummy@NONE: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default
link/ether 52:58:a7:1e:e8:93 brd ff:ff:ff:ff:ff:ff
inet 1.1.1.1 peer 1.1.1.3/24 scope global dummy
valid_lft forever preferred_lft forever
inet 1.1.1.1 peer 1.1.2.3/24 scope global dummy
valid_lft forever preferred_lft forever
We must also consider peer-address, otherwise platform will treat
two different addresses as one and the same.
https://bugzilla.gnome.org/show_bug.cgi?id=756356
Also change the semantic of nm_ip6_config_address_exists()
to ignore the prefix length. It seems more correct this way,
but as there are no users of the function it doesn't actually
matter.
Kernel treats IPv4 addresses with different netmask/prefix-length as
different addresses.
It is wrong to merge them together in nm_ip4_config_add_address().
For IPv6 addresses that is not the case and you cannot configure
two IPv6 addresses that only differ by plen (on the same interface).
The new flags are not yet used, so there is no change in functionality.
The flags NM_IP_CONFIG_MERGE_NO_ROUTES and NM_IP_CONFIG_MERGE_NO_DNS go
together with the 'ignore-auto-routes' and 'ignore-auto-dns' setting.
Note that for IPv4, NM_IP_CONFIG_MERGE_NO_DNS also ignores NIS, WINS, and dns-options.
This is different from current other places that handle 'ignore-auto-dns'
and only care about nameservers, domains, and searches.
Move D-Bus export/unexport handling into NMExportedObject and remove
type-specific export/get_path methods (export paths are now specified
at the class level, and NMExportedObject handles the counters for all
exported types automatically).
Since all exportable objects now use the same get_path() method, we
can also add some helper methods to simplify get_property()
implementations for object-path and object-path-array properties.
Add NMExportedObject, make it the base class of all D-Bus-exported
types, and move the nm-properties-changed-signal logic into it. (Also,
make NMSettings use the same properties-changed code as everything
else, which it was not previously doing, presumably for historical
reasons).
(This is mostly just shuffling code around at this point, but
NMExportedObject will be more important in the gdbus port, since
gdbus-codegen doesn't do a very good job of supporting objects that
export multiple interfaces [as each NMDevice subclass does, for
example], so we will need more glue/helper code in NMExportedObject
then.)
Add an argument @full_sync to the sync method of NMRouteManager.
@full_sync was what we did up to now, meaning, we removed every
route on the interface that was no on our internal list of known
routes.
Now with !@full_sync, only remove routes that were tracked previously.
This means, we will only remove routes that were added by us previously.
Don't make use of the new option yet. So there is no change of behavior
yet.
When adding an IPv4 address, kernel will also add a device-route.
We don't want that route because it has the wrong metric. Instead,
we add our own route (with a different metric) and remove the
kernel-added one.
This could be avoided if kernel would support an IPv4 address flag
IFA_F_NOPREFIXROUTE like it does for IPv6 (see related bug rh#1221311).
One important thing is, that we want don't want to manage the
device-route on assumed devices. Note that this is correct behavior
if "assumed" means "do-not-touch".
If "assumed" means "seamlessly-takeover", then this is wrong.
Imagine we get a new DHCP address. In this case, we would not manage
the device-route on the assumed device. This cannot be fixed without
splitting unmanaged/assumed with related bug bgo 746440.
This is no regression as we would also not manage device-routes
for assumed devices previously.
We also don't want to remove the device-route if the user added
it externally. Note that here we behave wrongly too, because we
don't record externally added kernel routes in update_ip_config().
This still needs fixing.
Let IPv4 device-routes also be managed by NMRouteManager. NMRouteManager
has a list of all routes and can properly add, remove, and restore
the device route as needed.
One problem is, that the device-route does not get added immediately
with the address. It only appears some time later. This is solved
by NMRouteManager watching platform and if a matchin device-route shows up
within a short time after configuring addresses, remove it.
If the route appears after the short timeout, assume they were added for
other reasons (e.g. by the user) and don't remove them.
https://bugzilla.gnome.org/show_bug.cgi?id=751264https://bugzilla.redhat.com/show_bug.cgi?id=1211287
The previous version causes an unsigned integer underflow. That
is not wrong, but still change it.
Also use g_array_remove_index_fast() because the list of routes
is unsorted anyway.
ip[46]_config_merge_and_apply() do assume the settings that are merged later in
override the previously set ones and not the other way around. Otherwise e.g. a
gateway address from DHCP could override what's set in the connection.
Some DHCP servers send specific options to give a hint that clients
should avoid unneeded data usage.
Add a metered flag to NMIP4Config to keep track of this information.
When generating a connection to assume it, also record the route-metric.
Do that by looking at the metric of the (best) default-route.
This is especially important since d51975ed92.
Now NM would also manage the default-route for assumed connections.
So the generated assumed connection would have a route metric based on
the device type, which might differ from the external configuration.
This caused NM to replace the externally configured default-route.
https://bugzilla.gnome.org/show_bug.cgi?id=750405
"nm-utils-private.h" should not be used outside of libnm-core/.
core/ should only use public API or "nm-core-internal.h".
Also, "nm-setting-ip-config.h" is a public header and should
not contain internal defines. Move them to "nm-core-internal.h"
too.
Fixes: 019943bb5d
Otherwise it stays zero and hits an assertion when the route is applied:
NetworkManager:ERROR:nm-route-manager.c:179:nm_route_manager_ip4_route_sync:
assertion failed: (known_route->ifindex)
https://bugzilla.gnome.org/show_bug.cgi?id=745844
Most nm_platform_*() functions operate on the platform
singleton nm_platform_get(). That made sense because the
NMPlatform instance was mainly to hook fake platform for
testing.
While the implicit argument saved some typing, I think explicit is
better. Especially, because NMPlatform could become a more usable
object then just a hook for testing.
With this change, NMPlatform instances can be used individually, not
only as a singleton instance.
Before this change, the constructor of NMLinuxPlatform could not
call any nm_platform_*() functions because the singleton was not
yet initialized. We could only instantiate an incomplete instance,
register it via nm_platform_setup(), and then complete initialization
via singleton->setup().
With this change, we can create and fully initialize NMPlatform instances
before/without setting them up them as singleton.
Also, currently there is no clear distinction between functions
that operate on the NMPlatform instance, and functions that can
be used stand-alone (e.g. nm_platform_ip4_address_to_string()).
The latter can not be mocked for testing. With this change, the
distinction becomes obvious. That is also useful because it becomes
clearer which functions make use of the platform cache and which not.
Inside nm-linux-platform.c, continue the pattern that the
self instance is named @platform. That makes sense because
its type is NMPlatform, and not NMLinuxPlatform what we
would expect from a paramter named @self.
This is a major diff that causes some pain when rebasing. Try
to rebase to the parent commit of this commit as a first step.
Then rebase on top of this commit using merge-strategy "ours".
No functional change, a cosmetic thing for now.
We want it set before any routes are added and ensure routes have a valid
ifindex before we pass it to the platform.
In a future NMRouteManager will need to look up the route for a device in
its cache thus we'll need to make sure routes passed to the it have an
appropriate ifindex set.
Create a NMRouteManager singleton.
Refactor, no functional changes apart from change of log domain from
LOGD_PLATFORM to LOGD_CORE.
Subsequent commit will keep track of the conflicting routes, avoid overwriting
older ones with newer ones and apply the new ones when the old ones go away.
