The automatic scanning every 20 seconds while connected has been
annoying users because of the extra connection latency, drop it. The
UIs are supposed to be requesting scans whenever an AP list update is
needed (?).
Fix a crash on device unplugging caused by keeping our signal handlers
for GDBusProxies connected after a call to dispose(). Do this by
replacing most cleanup steps by a nm_device_iwd_set_dbus_object(self, NULL)
call which is more meticulous.
As one of the arguments in unsigned, the calculation is performed as
unsigned integers. That can actually lead to the wrong result. Fix it by
casting to the right (signed) types.
Emitting signals is relatively expensive, because the arguments have to be packed
into a GValue. Avoid some overhad by only passing one signal argument: the notify-data
which also contains the type. Also with this we can use g_cclosure_marshal_VOID__POINTER.
Also, it's nice to have the type field part of the notify-data. Because clearly
the notify-data union is unusable without knowing the type. That means, if a user
passes the notify-data to a function, they anyway would also need to pass along
the type.
NML3Cfg tends to perform actions on an idle handler. That means, when
it configures something on platform, it tends to ignore the changes and
process them later.
That means the currently tracked NMPObject with the platform link may
not be the same as NMPlatform currently has cached.
Instead, track them both, and extend the API so that it's clear that
there is a difference. You now need to say whether you want the instance
from the platform cache (the "next") or the currently used instance. Of
course, after the idle handler runs, "next" and the current one
converge.
This is useful because we want to reason about the link state (also) by
looking a our NML3Cfg instance. Since it already is connected to
platform, it can expose the same NMPObject.
- add nm_l3cfg_platform_commit_on_idle_schedule() so that internal (and
external) code can schedule a commit on an idle handler. This already
existed, but is exposed now.
- rename nm_l3cfg_platform_commit() to simply nm_l3cfg_commit(). There
is no other form than "platform" commit, so the name was
unnecessarily long.
- also don't let nm_l3cfg_commit() return a boolean success. It's not
useful, because commits can be triggered internally (by NML3Cfg
itself) or by other users. Instead, there is the "post-commit" event,
and anybody who cares about such a failure would need to handle it
there.
Our NML3Cfg instance is the IP configuration manage of one ifindex.
Often users have an NML3Cfg instance at hand, but they still need to
react to platform signals. Instead of requiring those users to register
their own signal (which also gets notifications about uninteresting
interfaces), re-emit the signal from NML3Cfg.
We already had NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE which
does something similar, but collects multiple changes and emits them
on an idle handler.
This flag indicates that the NML3ConfigData should be ignored for most
purposes, except for doing ACD.
Note that as users can call nm_l3cfg_add_config() multiple times for
the same NML3ConfigData, a higher layer that enables ACD/IPv4LL can
then decide to actually use the configuration, while some layers
only have it hooked up to do ACD.
Now that NMPlatformIP[46]Route can contain a wildcard table/metric, we
can set the effectivey table/metric per NML3ConfigData that we merge.
Pass it to nm_l3cfg_add_config().
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.
Previously, both nm_setting_connection_add_permission() and the GObject
property setter would merely assert that the provided values are valid
(and otherwise don't do anything). That is bad for handling errors.
For example, we use the property setter to initialize the setting from
keyfile and GVariant (D-Bus). That means, if a user provides an invalid
permissions value, we would emit a g_critical() assertion failure, but
otherwise ignore the configuration. What we instead need to do is to
accept the value, and afterwards fail verification. That way, a proper error
message can be generated.
$ mcli connection add type ethernet autoconnect no ifname bogus con-name x connection.permissions 'bogus:'
(process:429514): libnm-CRITICAL **: 12:12:00.359: permission_new: assertion 'strchr (uname, ':') == NULL' failed
(process:429514): libnm-CRITICAL **: 12:12:00.359: nm_setting_connection_add_permission: assertion 'p != NULL' failed
Connection 'x' (2802d117-f84e-44d9-925b-bfe26fd85da1) successfully added.
$ $ nmcli -f connection.permissions connection show x
connection.permissions: --
While at it, also don't track the permissions in a GSList. Tracking one
permission in a GSList requires 3 allocations (one for the user string,
one for the Permission struct, and one for the GSList struct). Instead,
use a GArray. That is still not great, because GArray cannot be embedded
inside NMSettingConnectionPrivate, so tracking one permission also
requires 3 allocations (which is really a fault of GArray). So, GArray
is not better in the common case where there is only one permissions. But even
in the worst case (only one entry), GArray is no worse than GSList.
Also change the API of nm_setting_connection_add_permission().
Previously, the function would assert that the arguments are in
a certain form (strcmp (ptype, "user") == 0), but still document
the such behaviors like regular operation ("[returns] %FALSE if @ptype
or @pitem was invalid"). Don't assert against the function arguments.
Also, if you first set the user to "fo:o", then
nm_setting_connection_add_permission() would accept it -- only at
a later phase, the property setter would assert against such values.
Also, the function would return %FALSE both if the input value was
invalid (an error) and if the value already existed. I think the
function should not treat a duplicate entry like a badly formatted
input.
Now the function does much less asserting of the arguments, but will
return %FALSE only if the values are invalid. And it will silently ignore
duplicate entries.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/636
Compare to the connection's GetSettings() call, which is not protected
by policykit permissions. It only checks that the requesting user is
allowed according to "connection.permission".
Previously, device's GetAppliedConnection() requires "network-control"
permissions. This although it only reads a profile, without modifying
anything. That seems unnecessary, also because in the common case the
applied connection is identical to the current settings connection, and
the latter can be read without special permissions.
Don't require a special policykit permission to read the applied
connection.
https://bugzilla.redhat.com/show_bug.cgi?id=1882380
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>
Adding NML3Cfg support is a large effort that is done in parallel.
While already parts of the code is merged, it is not actually used
yet. Remove the parts from NMDevice that interact with NML3Cfg
before we actually start using it.
The point is that we might want to do a release before switching
over to the new way. For that release, we should not have the unused
code in NMDevice.
This patch will later be restored and extended.
But also take a reference whenever we have any configurations
registered. Registering a configuration means to automatically
keep the instance alive.
Any user must take care to unregister again when it no longer requires
the configuration.
Currently all NMDevice instance register to the platform change signals,
then if a signal for their IP ifindex appears, they schedule a task on
an idle handler. That is wasteful.
NML3Cfg already gets a notification on an idle handler and can just re-emit
it to the respective listeners.
With this, there is only one subscriber to the platform signals (NMNetns)
which then multiplexes the signals to the right NML3Cfg instances, and
further.
The caller may want to know the merge NML3ConfigData as it would be used
for the next commit, without already committing it.
Track both the NML3ConfigData instance that is merged from the
registered items, and the one that was used the last time during
commit.
It's often convenient not to require the caller to check for
%NULL. On the other hand, there are cases where having a %NULL instance
is a bug, so gracefully accepting %NULL might hide bugs.
Still, change it.
