These are unused now so remove them and revert most of
e0394689b3 which attempted to fix the same
issue of the platform wifi API not mapping well the nl80211 commands
resulting in redundant netlink commands being used.
In the wext driver there are still three individual getters for the
three values and nm_wifi_utils_get_station() uses either these or the
collective get_station method depending on the driver.
Merge nm_platform_wifi_get_bssid, nm_platform_wifi_get_quality,
nm_platform_wifi_get_rate into one utility, nm_platform_wifi_get_station
that uses the single NL80211_CMD_GET_STATION command dump when the
nl80211 driver is used. With wext each function mapped to one ioctl
while with nl80211 all three can be obtained with one netlink command.
The new function should use the minimum number of calls with either
driver.
Add a parameter to the 'link_add()' virtual function so that
the MTU will be configured (via netlink) by the kernel when
creating the link.
https://bugzilla.redhat.com/show_bug.cgi?id=1778590
Signed-off-by: Antonio Cardace <acardace@redhat.com>
- we commonly use "int addr_family" as parameters to functions.
But then inside the function, we often need to do something for
IPv4 or IPv6 specifically. Instead of having lots of redundant
"if (addr_family == AF_INET)" checks, prefer to have a variable
IS_IPv4 and/or use NM_IS_IPv4() macro.
- don't make the "IS_IPv4" variable a gboolean but an int. gboolean
is a typedef for int, so it's in practice exactly the same. However,
we use "IS_IPv4" as index to arrays of length 2, where at position
"1" we have the value related to IPv4. Using a gboolean to index
an array is a bit odd. Maybe a "int" is preferable here.
This is more about doing consistently one or the other. There are
no strong reasons to prefer gboolean or int.
The DHCP client likes to order multiple default routes by adding
them with different, increasing metric.
To support that, let "metric_any" not completely disable the "metric"
field, but instead interpret it as an offset that should be added to
the default metric.
It is not yet used, but it will be used to mark instances that
are not supposed to be configured in platform, because ACD is
either still pending of failed.
When we (for example) receive a DHCP lease, we track the routes that
should be configured via NMPlatformIP[46]Route instances. Thus, this
structure does not only track the routes that are configured (and
cached in NMPlatform), but it is also used to track the routes that
we want to configure.
This is also the case with the "rt_source" field, which represents the
NMIPConfigSource enum for routes that we want to configure, but
for routes in the cache it corresponds to rtm_protocol.
Note that NMDhcpClient creates NMIP4Config instances, which tracks the
routes as NMPlatformIP4Route instances. Previously, NMDhcpClient didn't
have any way to leave the table/metric undecided, but this information
isn't part of the DHCP lease tself. Instead, NMDevice knows the table/metric
to use. This has various problems:
- NMDhcpClient needs to know the table/metric, for no other purpose
than to set the value when creating the NMIP4Config instance for the
lease. We first pass the information down, only so that it can be
returned with the lease information.
- during reapply or when connectivity check changes, the effectively
used table/metric can change. Previously, we would have to
re-generate the NMIP4Config instances.
Improve that by allowing to leave the table/metric undecided. Higher
layers can decide the effective metric to use.
Run:
./contrib/scripts/nm-code-format.sh -i
./contrib/scripts/nm-code-format.sh -i
Yes, it needs to run twice because the first run doesn't yet produce the
final result.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
nm_utils_inet4_ntop() is public API of libnm. Also, it accepts a
%NULL buffer to use a static buffer. That is error prone and we
should not use such convenience behavior for our own code.
The kernel of Ubuntu 16.04 doesn't support IFLA_BR_VLAN_STATS_ENABLED.
If we want to run on such old kernels (which we probably do), we need to
detect that, and act accordingly.
This makes the macro more function like. Also, taking a pointer
makes it a bit clearer that this possibly changes the value.
Of course, it's not a big difference to before, but this
form seems slightly preferable to me.
This parameter really affects whether a candidate in @addresses_prune will be
considered or not. Since we already construct the prune list separately, this
parameter is at the wrong place.
This requires us to re-implement nm_platform_lookup_clone(). While the
function has a predicate callback that we could use for this purpose,
I will later add a separate predicate argument to
nm_platform_ip_address_get_prune_list(). When that happens, it would
be cumbersome to chain the two function pointers. Instead, reimplement
nm_platform_lookup_clone().
Follow the pattern of nm_platform_ip_route_sync(), which also accepts
the list of addresses that are potential candidates for removal.
This allows the caller to carefully construct the list of addresses
which are possibly removed, so that sync (possibly) only adds new
addresses.
It is beneficial to have both address families side by side.
A lot of operations are exactly the same, so it's preferable to see
that. Especially in the cases where they differ, it's preferable to see
how they differ (and why).
They ensure to consistently return -1, 0, 1. Also, I think they are
easier to understand.
What is in general hard to understand, whether a comparison sorts
ascending or descending. The macros maybe make that easier too, but it's
still confusing. That's why we have a test.
Pre-generate routes in the local table that are configured
by kernel when an ip-address is assigned to an interface.
This helps NM taking into account routes that are not to be deleted
when a connection is reapplied (or deactivated) on an interface instead of only
ignoring (when pruning) IPv6 routes having metric 0 and routes belonging
to the local table having 'kernel' as proto.
https://bugzilla.redhat.com/show_bug.cgi?id=1821787
@routes are the list of routes we want to configure. This contains
routes from DHCP and manual routes in the profile. It also contains
externally present routes, including the metric=0 routes in the local
table.
Trying to add an IPv6 route with metric zero adds instead a route with
metric 1024.
Usually, we wouldn't do that, because that route was present externally,
so it possibly is still present (in the platform cache) during sync and
we skip the addition. However, there is a race where the external route
might just disappear and we'd add a route with metric 1024.
Avoid that.
Userspace cannot add IPv6 routes with metric 0. Trying to do that, will
be coerced by kernel to route metric 1024. For IPv4 this is different,
and metric zero is commonly allowed.
However, kernel itself can add IPv6 routes with metric zero:
# ip -6 route show table local
local fe80::2029:c7ff:fec9:698a dev v proto kernel metric 0 pref medium
That means, we must not treat route metric zero special for most cases.
Only, when we want to add routes (based on user configuration), we must
coerce a route metric of zero to 1024.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/563
nm_platform_ethtool_init_ring() only has one caller. It's simpler to
drop the function and implement it at the only place where it is needed.
Maybe there could be a place for a function to initialize NMEthtoolRingState,
one option after the other. However, at the moment there is only one
user, so don't implement it.
This fixes various minor issues:
- the function had a NMPlatform argument, although the argument
is not used. Thus function merely operates on a NMEthtoolRingState
instance and shouldn't have a nm_platform_*() name.
- nm_platform_ethtool_init_ring() returned a boolean, but all
code paths (except assertion failures) returned success.
- as the function returned an error status, the caller was compelled
to handle an error that could never happen.
- the option was specified by name, although we already have a more
efficient way to express the option: the NMEthtoolID. Also, the
caller already needed to resolve the name to the NMEthtoolID, so
there was no need to again lookup the ID by name.
Rework qdisc synchronization. The previous implementation added all
known qdiscs and removed unneeded ones from platform; this had some
problems:
- kernel doesn't allow to add (with exclusive flag) a qdisc if one
with the same parent already exists;
- if we use the replace flag instead of add, then it becomes possible
to add a new qdisc with the same parent of an existing one. However
if the existing qdisc is of the same kind, kernel will try to to
change() it, which fails for some qdiscs (e.g. sfq).
- kernel doesn't allow to delete a qdisc with handle of zero because
that is the default qdisc and can only be replaced;
Fix that.
