NMNDisc has two implementations: lndp and fake. Fake only exists as a
stub for unit tests, otherwise there is no purpose to it. Also, we won't
ever add another implementation beside lndp. If lndp is not suitable, it
would be replaced, but not accompanied by a second implementation.
As such, nm_lndp_ndisc_get_sysctl() has no purpose to be in
"nm-lndp-ndisc.c". This split does not exist to abstract "nm-ndisc.c"
from NMPlatform. It exists to make it easier to test.
Stage2 can be called multiple times. Ensure that tc_commit() is only
called the first time. This is important now that tc synchronization
requires to clear all qdiscs and recreate them.
Update nm_platform_qdisc_sync() and nm_platform_tfilter_sync() to
avoid looking into the platform cache, so that we no longer require to
keep tc and qdiscs in the cache.
There is no API in kernel to retrieve tc objects only for a specific
interface, so NM had to receive all tc events, even for unmanaged
interfaces. This could cause high CPU usage in some scenarios with
many objects.
Instead, try to delete root qdiscs and filters and then add the known
ones.
Also, combine the two functions together since they are related. In
particular, removing all qdiscs also removes all attached filters.
"assuming" means to gracefully take over after restart. The result
should be a working configuration with a device fully managed by
NetworkManager.
If we are assuming, and the interface is down we still want to set it
up.
Starting with OVS plugin installed but OVS service stopped, would lead to
<trace> [1631531732.8896] ovsdb: connect: opening /run/openvswitch/db.sock failed ("error connecting socket (No such file or directory)"). Retry with nm-sudo
...
<trace> [1631531732.9751] ovsdb: connect: failure to get FD from nm-sudo: GDBus.Error:org.gtk.GDBus.UnmappedGError.Quark._g_2dio_2derror_2dquark.Code1: error connecting socket (No such file or directory)
If we already know that the socket file does not exist, we don't need to ask nm-sudo.
That would only make sense, if nm-sudo somehow saw a different file systemd than
NetworkManager, but that is (currently) not the case.
The check whether the current setting are already as expected are wrong.
The reason is that nm_platform_ethtool_set_link_settings() also sets
the announced ethernet modes, but nm_platform_ethtool_get_link_settings()
does not give them.
That means, we cannot check whether the current link configuration is
the same, because the getter doesn't give that information.
Consequently, we must not skip the setting on the assumption that
there is nothing to change.
This bug has bad effects. If the device is currently activated with ethtool
option set, then re-activating the profile will result in wrongly
skipping the update.
There is no point in logging the current speed/duplex. OK, with
the "*", we could at least see whether the printed values are
to be set, or are currently configured on the interface.
But mixing these two outputs is confusing and meaningless.
Either log what we are about to do, or what the current configuration
is. Not a mix of both.
It's great to have functions that cannot fail, because it allows to
skip any error handling.
_set_stable_privacy() as it was could not fail, so the only reason why
nm_utils_ipv6_addr_set_stable_privacy() could fail is because the DAD
counter exhausted.
Also, it will be useful to have a function that does not do the counter
check, where the caller wants to handle that differently.
Rename some functions, and make the core nm_utils_ipv6_addr_set_stable_privacy()
not failable.
While NMUtilsIPv6IfaceId is only 8 bytes large, it seems unidiomatic to
pass the plain struct around.
With a "const NMUtilsIPv6IfaceId *" argument it is more clear what the
meaning of this is.
Change to use pointers.
The preference for IPv6 routes was added in kernel v4.1,
22 June 2015. It is even in latest RHEL7 kernels.
Drop trying to be compatible with such old kernels.
We use extended IFA_FLAGS for IFA_F_MANAGETEMPADDR (IPv6) and
IFA_F_NOPREFIXROUTE (IPv4 and IPv6).
These flags for IPv4 were added to kernel 3.14, 30 March, 2014.
The flag for IPv4 was added to kernel 4.4, 11 January 2016.
Even latest RHEL-7 kernels have backport for IFA_F_NOPREFIXROUTE
for IPv4 (rh#1221311).
Drop this. The backward compatibility code paths are likely broken
anyway, and add considerable complexity.
This is supported since kernel 3.17, dated 5 October, 2014. Drop the backward
compatibility for that.
It's very hard to sensibly support a mode where we set the interface up,
but prevent kernel from enabling IPv6. We would hack around that by disabling
IPv6 altogether.
But these code paths are not tested and likely make no sense. And it's hard
to implement a sensible behavior in this case anyway.
The term "user_ipv6ll" is confusing and not something somebody familiar
with kernel or `ip -d link` would understand.
Also, it maps a boolean to addr-gen-mode "none" or "eui64", although
there are 2 more address generation modes in kernel.
Don't abstract the underlying API, and name things as they are in
kernel.
NMDevice's act_stage3_ip_config_start() has an out parameter,
so that an NMIPConfig object can be returned. That is (luckily)
not used much, and it's fundamentally flawed. We want that
the start method becomes simpler and idempotent. That argument
is problematic there.
Instead, of the result is already ready, postpone the activation
and process the return on an idle handler.
Why not use nm_device_set_dev2_ip_config() to pass the configuration?
Good question, who knows? For now, just mimic the previous behavior.
Usually the IP configuration would be announced late, so we can just
do that artificially by scheduling an idle action.
The idea, that callers to nm_modem_stage3_ip4_config_start() pass
a NMDeviceClass, which then might invoke a virtual function on
the class is really bad.
nm_modem_stage3_ip4_config_start() should indicate what the caller
should do.
The main problem is, that act_stage3_ip_config_start() is not supposed
to be called by anybody except NMDevice. It's an internal hook, not for
NMModem's concern.
These type-specific getters are not very useful. _nm_connection_get_setting() is
better because the setting type is a parameter so they can be used more generically.
Have less code and use generic helpers.
Use sizeof(queue_id_str), so we don't rely on _MAX_QUEUE_ID_STR_LEN
being the correct size for the string.
Also, let's create an excessively large buffer. True, the previous size
should have always be enough, so in practice there is no difference.
But what if it were not? Should we try to handle an error? How? Just asserting
or report a failure? But we don't because the error cannot happen, can't
it?
Don't answer any of these questions, but by making the string buffer
larger, it's even less likely that these questions become relevant.
If for some reason nm_device_get_iface() gives a long string, then we
don't care and let kernel reject the invalid interface name.
While both functions are basically the same, the majority of the time
we use g_snprintf(). There is no strong reason to prefer one or the
other, but let's keep using one variant.
Add and call the new `nm_platform_link_get_permanent_address()` to
obtain `l_perm_address` via netlink or lookup via ethtool if kernel
does not expose the `IFLA_PERM_ADDRESS`.
And call the new `nm_platform_link_get_permanent_address()` in the unit
tests.
https://bugzilla.redhat.com/show_bug.cgi?id=1987286
Signed-off-by: Wen Liang <liangwen12year@gmail.com>
Replace the arguments "buf+length" of
`nm_platform_link_get_permanent_address()` with "NMPLinkAddress *out_addr"
Signed-off-by: Wen Liang <liangwen12year@gmail.com>
Use `_nm_connection_ensure_setting()` to eliminate the
duplicated codes. This function will retrieve the specific setting from
connection, if not found, create new one and attach to the connection.
Signed-off-by: Gris Ge <fge@redhat.com>
NMDhcpClient communicates events via GObject signals. GObject signals in
principle could have several subscribers. In practice, a NMDhcpClient
instance has only one subscriber, because it was constructed with
certain parameters, so it's unlikely to be shared.
That one subscriber, always needs to subscribe to all signals
("state-changed" and "prefix-delegated"), Unless the subscriber only
creates a IPv4 client. In which case they won't subscribe to
"prefix-delegated", but that signal is also not invoked for IPv4
clients.
Combine the signals in one, and pass all parameters via a new
NMDhcpClientNotfiyData payload. I feel this is nicer, to pack all
parameters together. I find this more type-aware, where we can
switch (in the callback) based on a notify-type enum, instead
of subscribing multiple signal handlers.
With l3cfg work, DHCP handling will be refactored, where this model of
having one "generic" notify signal makes more sense than here. For the
moment, it is arguably pretty much the same. Also, because NMDhcpClient
subscribes two different handlers for IPv4 and IPv6. In the future,
there will be only one notify handler, and that can cover IPv4 and IPv6
and both "state-changed" and "prefix-delegated" (and other notification
types).
add a NM_DISPATCHER_ACTION_DHCP_CHANGE_X() macro that can select the
right action based on a parameter.
Also rename the IPv4/IPv6 enum values, so that their naming scheme works
better with the NM_DISPATCHER_ACTION_DHCP_CHANGE_X() macro.
We call add/remove pending actions for every state change.
I think GSList is never the best choice of a data structure. Use a plain
array instead. Keep it sorted, so we can use binary search.
Each NML3ConfigData should have a source set, and in fact most callers
would call nm_l3_config_data_set_source() right after creating the
instance.
Move the source parameter to the new() constructor function. Also remove
the setter, making the source of an instance immutable.
As every l3cfg instance generally has a clear purpose, the source should
always be known from the start and doesn't need to change.
The "utils" part does not seem useful in the name.
Note that we also have NMStrBuf, which is named nm_str_buf_*().
There is an unfortunate similarity between the two, but it's still
distinct enough (in particular, because one takes an NMStrBuf and
the other not).
Naming is important, because the name of a thing should give you a good
idea what it does. Also, to find a thing, it needs a good name in the
first place. But naming is also hard.
Historically, some strv helper API was named as nm_utils_strv_*(),
and some API had a leading underscore (as it is internal API).
This was all inconsistent. Do some renaming and try to unify things.
We get rid of the leading underscore if this is just a regular
(internal) helper. But not for example from _nm_strv_find_first(),
because that is the implementation of nm_strv_find_first().
- _nm_utils_strv_cleanup() -> nm_strv_cleanup()
- _nm_utils_strv_cleanup_const() -> nm_strv_cleanup_const()
- _nm_utils_strv_cmp_n() -> _nm_strv_cmp_n()
- _nm_utils_strv_dup() -> _nm_strv_dup()
- _nm_utils_strv_dup_packed() -> _nm_strv_dup_packed()
- _nm_utils_strv_find_first() -> _nm_strv_find_first()
- _nm_utils_strv_sort() -> _nm_strv_sort()
- _nm_utils_strv_to_ptrarray() -> nm_strv_to_ptrarray()
- _nm_utils_strv_to_slist() -> nm_strv_to_gslist()
- nm_utils_strv_cmp_n() -> nm_strv_cmp_n()
- nm_utils_strv_dup() -> nm_strv_dup()
- nm_utils_strv_dup_packed() -> nm_strv_dup_packed()
- nm_utils_strv_dup_shallow_maybe_a() -> nm_strv_dup_shallow_maybe_a()
- nm_utils_strv_equal() -> nm_strv_equal()
- nm_utils_strv_find_binary_search() -> nm_strv_find_binary_search()
- nm_utils_strv_find_first() -> nm_strv_find_first()
- nm_utils_strv_make_deep_copied() -> nm_strv_make_deep_copied()
- nm_utils_strv_make_deep_copied_n() -> nm_strv_make_deep_copied_n()
- nm_utils_strv_make_deep_copied_nonnull() -> nm_strv_make_deep_copied_nonnull()
- nm_utils_strv_sort() -> nm_strv_sort()
Note that no names are swapped and none of the new names existed
previously. That means, all the new names are really new, which
simplifies to find errors due to this larger refactoring. E.g. if
you backport a patch from after this change to an old branch, you'll
get a compiler error and notice that something is missing.
Configuration can have [device*] and [connection*] settings and both
can include a 'match-device=' key, which is a list of device-specs.
Introduce a new 'allowed-connections' key for [device*] sections,
which specifies a list of connection-specs to indicate which
connections can be activated on the device.
With this, it becomes possible to have a device configuration like:
[device-enp1s0]
match-device=interface-name:enp1s0
allowed-connections=except:origin:nm-initrd-generator
so that NM in the real root ignores connections created by the
nm-initrd-generator, and starts activating a persistent
connection. This requires also setting 'keep-configuration=no' to not
generate an assumed connection.
To talk to ovsdb, we use the unix socket at
/var/run/openvswitch/db.sock. But that socket is owned by another user
and NetworkManager would need dac_override capability to open it.
We want to drop dac_override, but we still need to talk to ovsdb. Add a
GetFD() method to nm-sudo.
We still first try to open the socket directly. Maybe it just works.
Note that SELinux may block passing file descriptors from nm-sudo. If it
doesn't work for you, test with SELinux permissive mode and wait for an
SELinux update.
