Add an assert (g_return_val_if_reached()) that the interface name is
valid and shorter then 16 bytes. If it happened to be longer, strncpy()
would not have zero terminated the interface name.
The compatibily wrapper for rtnl_addr_flags2str() did not
behave identical because libnl adds a trailing ',' if it
encounters unknown attributes.
Also add test cases.
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
When checking whether an address from platform should be deleted, we
compare the address with our list of @known_addresses.
For that we must also check for expired lifetimes, because
@known_addresses might contain expired addresses.
build_rtnl_addr() has two parameters "lifetime" and "preferred". Both
count from *now*.
Fix nmp_object_to_nl() to properly set these timestamps. This bug had
not real consequences, because the only place where we use
nmp_object_to_nl() the arguments are 0.
Change nm_platform_link_get() to return the cached NMPlatformLink
instance. Now what all our implementations (fake and linux) have such a
cache internal object, let's just expose it directly.
Note that the lifetime of the exposed link object is possibly quite
short. A caller must copy the returned value if he intends to preserve
it for later.
Also add nm_platform_link_get_by_ifname() and modify nm_platform_link_get_by_address()
to return the instance.
Certain functions, such as nm_platform_link_get_name(),
nm_platform_link_get_ifindex(), etc. are solely implemented based
on looking at the returned NMPlatformLink object. No longer implement
them as virtual functions but instead implement them in the base class
(nm-platform.c).
This removes code and eliminates the redundancy of the exposed
NMPlatformLink instance and the nm_platform_link_get_*() accessors.
Thereby also fix a bug in NMFakePlatform that tracked the link address
in a separate "address" field, instead of using "link.addr". That was
a case where the redundancy actually led to a bug in fake platform.
Also remove some stub implementations in NMFakePlatform that just
bail out. Instead allow for a missing virtual functions and perform
the "default" action in the accessor.
An example for that is nm_platform_link_get_permanent_address().
We must check if a conflicting route/address is configured on
*any* interface, not only on the target ifindex.
This at least restores the previous behavior. Note that this whole
check_reinstall_device_route() still has problems and it might
be better to move it all to NMRouteManager.
Fixes: 470bcefa5f
After doing all the refactoring, rename the ObjectType enum to NMPObjectType.
I didn't do this earlier to avoid problems with rebasing. But since I will
backport the platform changes to nm-1-0, this is the right time to get
the name right.
warning: function declaration isn’t a prototype [-Wstrict-prototypes]
In C function() and function(void) are two different prototypes (as opposed to
C++).
function() accepts an arbitrary number of arguments
function(void) accepts zero arguments
After refactoring platform, nm_platform_ipx_route_get_all() and
nm_platform_ipx_address_get_all() was broken for calling with a
non-posititive ifindex (which has the meaning: ignore ifindex).
While fixing that, also refactor the NMPCacheIdType so that it matches
better the supported id-types.
Fixes: 470bcefa5f
For NMPlatform instances we had an error reporting mechanism
which stores the last error reason in a private field. Later we
would check it via nm_platform_get_error().
Remove this. It was not used much, and it is not a great way
to report errors.
One problem is that at the point where the error happens, you don't
know whether anybody cares about an error code. So, you add code to set
the error reason because somebody *might* need it (but in realitiy, almost
no caller cares).
Also, we tested this functionality which is hardly used in non-testing code.
While this was a burden to maintain in the tests, it was likely still buggy
because there were no real use-cases, beside the tests.
Then, sometimes platform functions call each other which might overwrite the
error reason. So, every function must be cautious to preserve/set
the error reason according to it's own meaning. This can involve storing
the error code, calling another function, and restoring it afterwards.
This is harder to get right compared to a "return-error-code" pattern, where
every function manages its error code independently.
It is better to return the error reason whenever due. For that we already
have our common glib patterns
(1) gboolean fcn (...);
(2) gboolean fcn (..., GError **error);
In few cases, we need more details then a #gboolean, but don't want
to bother constructing a #GError. Then we should do instead:
(3) NMPlatformError fcn (...);
Later remove nm_platform_get_error() and signal errors via return
error codes.
Also, fix nm_platform_infiniband_partition_add() and
nm_platform_vlan_add() to check the type of the existing link
and fail with WRONG_TYPE otherwise.
- rename "NONE" to "SUCCESS", what it really is.
- change the to-string result not to contain spaces
and being closer the name of the enum value.
- add new error reasons "UNSPECIFIED" and "BUG".
- remove the code comments around the enum definition.
They add no further description about why this error
happens and only paraphrase the name of the enum.
- reserve negative integers for 'errno'. This is neat
because if we get a system error we can pass on the
underlying errno as cause.
