We need to make sure StateChanged goes on the D-Bus only after the
policy is done dealing with the state change internally.
This is done so that we can be sure the DNS changes are committed at the
time "nmcli c up" returns.
https://bugzilla.redhat.com/show_bug.cgi?id=2006677
When NetworkManager is reloaded the config from active devices is not
being reloaded properly.
Related: https://bugzilla.redhat.com/1852445
Fixes: 121c58f0c4 ('core: set number of SR-IOV VFs asynchronously')
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
The MASTER property must be emitted on the port; while PORTS and
SLAVES on the controller.
Fixes: 9d2ed74e74 ('core: introduce device::ports property')
Drop a workaround added by commit a8ca7f537d ('ppp: work around PPP
bug that returns bogus nameservers'), in 2009.
Also drop the second workaround (`if (!num ...`), which was introduced
by commit 294a5e3153 ('modem: substitute known-good nameservers if PPP
doesn't return any (lp:434477)').
I hope this doesn't break something, but it really doesn't seem right in
2021.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/988
The property `PROP_PORTS` should be of type g_param_spec_variant() with
variant 'ao'. This way the variant can be cached.
The deprecated property 'device::slaves' in
'src/core/devices/nm-device.c' must have the same getter-implementation,
returning the same GVariant instance.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
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.
