On netlink layer, this field is uint8_t/uchar.
A larger (signed) plen makes no sense. Adjust the signatures
to have only guint8.
(cherry picked from commit 14ee5dd2f8)
- 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
This enum was unused and meaningless because the platform signals
are emitted as a consequence of netlink messages. It is not clear
whether a netlink message was received due to an external event
or an internal action.
Previsously, _LOGT() could be disabled at compile time. Thus it
was different then the other macros _LOGD(), _LOGI(), etc.
OTOH, _LOGt() was the macro that always was compiled in.
Swap the name of the macros. Now the upper-case macros are always
enabled, while the lower-case macro _LOGt() is enabled depending
on compile configuration.
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
Allows to enable more-asserts more granularly.
Unfortunately, the old check was "${enable_more_asserts} == "yes", thus
we cannot extend "--enable-more-assert=level" because that would mean
that the same build script cannot set the option on both old and new
NetworkManager.
Thus, add a new option --with-more-asserts=level. If you put the
following in your build script, it will work as expected whether
you build a new or an old version of NetworkManager.
./configure --enable-more-asserts --with-more-asserts=5
The logging macros _LOGD(), etc. are specific to each
file as they format the message according to their context.
Still, they were cumbersome to define and their implementation
was repeated over and over (slightly different at times).
Move the declaration of these macros to "nm-logging.h".
The source file now only needs to define _NMLOG(), and either
_NMLOG_ENABLED() or _NMLOG_DOMAIN.
This reduces code duplication and encourages a common implementation
and usage of these macros.
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.
Kernel does not allow to add the same route (as determined by network/plen,metric)
on two different interfaces (ifindex). In case of conflict, NMRouteManager used to
ignore any but the firstly added route.
On the other hand, we cannot add a gateway-route, if there is no direct
route to the gateway. Hence, skipping duplicate routes can mean that we
skip a direct route what was necessary to add another gateway-route,
which then leads to a failure to add that route.
This also applies to IPv4 device routes that since recently are managed
by NMRouteManager.
For example, say you connect two interfaces to the same IP subnet.
The route-metric can conflict if the interfaces are of the same type
or if the user explicitly configured a conflict.
In case of conflicts, NMRouteManager would only configure the first
appearing route and skip the shadowed route on the second interface.
Now we cannot configure gateway-routes on the second interface because
the gateway is unreachable.
There are many scenarios where this issue can happen, especially with
default-routes and user-configured-routes.
For example with default-routes, ip4_config_merge_and_apply() would check
if the default-gateway requires an explict route and possibly add it.
But then NMRouteManager might not add the route because it is shadowed
by a route on an other interface.
This patch solves the issue by having NMRouteManager configure shadowed
routes too, similar to what NMDefaultRouteManager does.
It does that by searching for an unused, non-conflicting, higher metric
for the route, i.e. bump the metric by 1 until we can add it without
conflict.
Also note that NMRouteManager still ensures that for conflicting routes
the best route sticks to the interface that configured it first. That
means if you later add the conflicting route on another interface, it
will be added with higher metric and the data is still routed along the
first interface.
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
Soon we will subscribe to the platform instance for change signals.
If a singleton instance uses another singleton instance, it should
keep a reference to it, especially if it subscribes to a signal
(that will be disconnected on dispose()).
With NM_MORE_LOGGING disabled, we still want the compiler to evaluate
the argument list. By wrapping it in "if(FALSE)", we get compile time
checks, but the logging statement will be optimized out.
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".
Refactor the implementation of nm_route_manager_ip4_route_sync()
and nm_route_manager_ip6_route_sync().
- merge the implementations for IPv4 and IPv6.
- pre-sort the routes and iterate them in a way that we don't
need to lookup a route in other lists. Do this by iterating
two sorted lists at a time in a merge-sort way.
The runtime complexity of sync is now O(n*ln(n)).
- previously, the algorithm would merge routes it found in platform
to priv->ipx_routes. That was wrong, because then we loose the
information which routes we wanted to configure internally and which
are present externally.
Instead, priv->ipx_routes now contains all the routes that were
explicitly configured via sync(). Hence, it knows what should be
configured (@ipx_routes) and can compare to what is configured
(@plat_routes).
https://bugzilla.gnome.org/show_bug.cgi?id=740064
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.
