It's possible that the modem is enabled outside of NM. If not
re-check, device could stay disabled throughout until something else
about the modem changes again.
If modem is not at least "registered", a connection is not happening, which
means IP settings change is probably not interesting. Avoid trying to
parse it, so that we don't trigger connection failure when there isn't
one.
Downstream patches for this does it through NMSettings plugin, however
settings plugin are hard to maintain and complicated architecture wise
as well.
So directly create a connection profiles in-memory from the
nm-modem-ofono side. Those profiles are created in /run, and are not
added as a persistent connection, because connection state quite depends
on the state of ofono
This also allows us to drop the hack where we are keeping track of
active context/APN through the connection name, i.e if connection name
was in /imsi/context1 format, it was used. Instead now, Connection name
is actual context name which is user friendly ("Vodafone Connect" e.g.
in my case), and details like IMSI and context are stored internally.
[ratchanan@ubports.com:
- forward-ported to main branch.
- fold "wwan/ofono: handle context removal" into this commit.
- track the "preferred"-ness of the context and react accordingly.
Creates proxies for all retrived contexts to listen to changes.
While at it, also track name and type.
- use, instead of ignore, internet APN. Also support internet+mms APN.
- correct priv->contexts' value destroy function.
- factor out UUID generation as a helper function.
- handle the case where context dictionary is missing required keys.
- simplify nm_ofono_connection_new's arguments and rename to
add_or_update_connection. Makes it handle the case where the
connection already exists.
- also simplify other functions' arguments.
- clean up code and comments. Fix memory problems. Get rid of warnings.
]
Co-authored-by: Ratchanan Srirattanamet <ratchanan@ubports.com>
The DNS name can now also contain the DoT server name. It's not longer a
binary IP address only.
Extend NML3ConfigData to account for that. To track the additional
data, use the string representation. The alternative to have a separate
type that contains the parsed information would be cumbersome too.
It is almost always wrong, to split IPv4 and IPv6 behaviors at a high level.
Most of the code does something very similar. Combine the two functions.
and let them handle the difference closer to where it is.
Handle IP Configuration requests from IWD so that, when IWD's main.conf
setting [General].NetworkConfigurationEnabled is true, we don't try to
run DHCP or static addressing in parallel with IWD's internal DHCP or
static addressing.
Since part of the IWD secret agent and the new NetConfig agent
registration code is common, the agent object's path is changed.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1337
The "device ... not available because device is strictly unmanaged" is
almost certainly the least interesting of the reasons why connection
can't be activated on a device.
Invent a new error level for it and demote it.
Before:
Error: Connection activation failed: No suitable device found
for this connection (device lo not available because
device is strictly unmanaged).
After
Error: Connection activation failed: No suitable device found
for this connection (device eth0 not available because
profile is not compatible with device (...)).
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1433
It is not possible to configure a VLAN interface on unmanaged NIC.
This forces users who only want to create a VLAN interface to take
ownership over possibly shared underlying NIC.
In OpenShift, the SR-IOV operator is currently not using
NetworkManager to configure VFs. When it starts working with a NIC,
it explicitly makes it unmanaged. Then, users cannot create a VLAN
interface on PFs managed by the operator.
This commit eliminates this issue by allowing configuring VLAN on
an interface without requesting it to be managed by NetworkManager.
This commit is part of a broader change that eliminates inheriting
the unmanaged condition from the parent of a device, for all device
types:
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1418https://bugzilla.redhat.com/show_bug.cgi?id=2110307
ensure_teamd_connection() is called from multiple spots. Sometimes
we call opportunistically without having started teamd (e.g. when on
update_connection() when generating a connection for teaming device that
was created) and handle the failure to connect gracefully.
Let's not pollute the logs with things on ERROR level that are not
actually serious. Replace the logging statements with DEBUG or WARN
depending on whether we expect ensure_teamd_connection() to actually
succeed.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1422
Call teamdctl_port_config_update_raw() when we're attaching a port even
if all of team-slave setting properties are default.
This is done to ensure teamd "knows" about the port (that is,
"teamdctl ... port present" returns success) when we're done activating
the slave connection. It will pick it up anyway from netlink, but that
can happen after the activation is done, resulting in a possible race.
Fixes-test: @remove_active_team_profile
https://bugzilla.redhat.com/show_bug.cgi?id=2102375https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1421
Clang 15 ([1], [2]) added
Added the -Wunreachable-code-generic-assoc diagnostic flag (grouped
under the -Wunreachable-code flag) which is enabled by default and warns
the user about _Generic selection associations which are unreachable
because the type specified is an array type or a qualified type.
This causes compiler warnings with various uses of _Generic():
../src/libnm-glib-aux/nm-shared-utils.h:2489:12: error: due to lvalue conversion of the controlling expression, association of type 'const char *const *const' will never be selected becaus
e it is qualified [-Werror,-Wunreachable-code-generic-assoc]
return nm_strv_find_first((const char *const *) strv->pdata, strv->len, str);
^
../src/libnm-glib-aux/nm-shared-utils.h:475:25: note: expanded from macro 'nm_strv_find_first'
_nm_strv_find_first(NM_CAST_STRV_CC(list), (len), (needle))
^
../src/libnm-glib-aux/nm-macros-internal.h:397:22: note: expanded from macro 'NM_CAST_STRV_CC'
const char *const*const: (const char *const*) (value), \
^
Clang is correct.
[1] https://releases.llvm.org/15.0.0/tools/clang/docs/ReleaseNotes.html#improvements-to-clang-s-diagnostics
[2] https://reviews.llvm.org/D125259
For a long time we have a function like nm_uuid_generate_from_strings().
This was recently reworked and renamed, but it preserved behavior. Preserving
behavior is important for this function, because for the existing users,
we need to keep generating the same UUIDs.
Originally, this function was a variadic function with NULL sentinel.
That means, when you write
nm_uuid_generate_from_strings(uuid_type, type_arg, v1, v2, v3, NULL);
and v2 happens to be NULL, then v3 is ignored. That is most likely not
what the user intended. Maybe they had a bug and v2 should not be NULL.
But nm_uuid_generate_from_strings() should not require that all
arguments are non-NULL and it should not ignore arguments after the
first NULL.
For example, one user works around this via
uuid = nm_uuid_generate_from_strings_old("ibft",
s_hwaddr,
s_vlanid ? "V" : "v",
s_vlanid ? s_vlanid : "",
s_ipaddr ? "A" : "DHCP",
s_ipaddr ? s_ipaddr : "");
which is cumbersome and ugly.
That will be fixed next, by adding a function that doesn't suffer
from this problem. But "this problem" is part of the API of the
function, we cannot just change it. Instead, rename it and all
users, so they can keep doing the same.
New users of course should no longer use the "old" function.
At startup, we remove from ovsdb any existing interface created by NM
and later an interface with the same name might be readded. This can
cause race conditions. Consider this series of events:
1. at startup NM removes the entry from ovsdb;
2. ovsdb reports success;
3. NM inserts an interface with the same name again;
4. ovs-vswitch monitors ovsdb changes, and gets events for removal and
insertion. Depending on how those events are split in different
batches, it might decide:
4a. to delete the link and add it back, or
4b. to keep the existing link because the delete and insertion
cancel out each other.
When NM sees the link staying in platform, it doesn't know if it's
because of 4b or because 4a will happen eventually.
To avoid this ambiguity, after ovsdb reports the successful deletion
NM should also wait that the link disappears from platform.
Unfortunately, this means that ovsdb gets a dependency to the platform
code.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1386
For connections with multi-connect property set to "multiple", the
autoconnect-retries should be tracked per device and not per connection.
That means, if autoconnect-retries is set to 2, each device using that
connection should retry to autoconnect 2 times.
The device autoconnect retries is -2 by default. This is a special
value, in NMPolicy context, if the connection used is multi-connect the
device value will be set to match the connection retries. Each time the
device picks a different connection, it will reset the device
autoconnect retries to -2 and if needed, sync. with the connection
retries.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1387https://bugzilla.redhat.com/show_bug.cgi?id=2039734
This partly reverts 1fe8166fc9 ('device: only deactivate when the master
we've enslaved to goes away').
If the controller fails while the port is not yet fully attached,
before this patch the following happened:
<info> [1664299566.1065] device (bond0): state change: ip-config -> failed (reason 'config-failed', sys-iface-state: 'managed')
...
<warn> [1664299566.1073] device (bond0): Activation: failed for connection 'bond0'
<trace> [1664299566.1073] device[6b76ac7314eb0b53] (bond0): master: release one slave a9f10ea824bb1725/eth1 (not enslaved) (configure)
<debug> [1664299566.1073] device[a9f10ea824bb1725] (eth1): unmanaged: flags set to [!sleeping,!by-type,!platform-init,!user-explicit,!user-settings,!user-conf=0x0/0x179/managed], forget [is-slave=0x800], reason removed)
...
<info> [1664299566.1080] device (eth1): state change: config -> ip-config (reason 'none', sys-iface-state: 'managed')
Note that now eth1 has no controller, but it lingers in "ip-config" state indefinitely.
If we look at a case where the port is already attached we see:
<info> [1664299540.9661] device (bond0): state change: secondaries -> failed (reason 'config-failed', sys-iface-state: 'managed')
...
<warn> [1664299540.9667] device (bond0): Activation: failed for connection 'bond0'
<trace> [1664299540.9667] device[6b76ac7314eb0b53] (bond0): master: release one slave a9f10ea824bb1725/eth1 (enslaved) (configure)
<debug> [1664299540.9667] platform: (eth1) link: releasing 10 from master 'bond0' (80)
...
<info> [1664299540.9740] device (bond0): detached bond port eth1
...
<debug> [1664299540.9749] device[a9f10ea824bb1725] (eth1): Activation: connection 'eth1' master failed
...
<warn> [1664299540.9749] device (eth1): queue-state[secondaries, reason:none, id:520]: replace previously queued state change
...
<debug> [1664299540.9750] device[a9f10ea824bb1725] (eth1): queue-state[deactivating, reason:dependency-failed, id:533]: queue state change
<debug> [1664299540.9751] device[a9f10ea824bb1725] (eth1): unmanaged: flags set to [!sleeping,!by-type,!platform-init,!user-explicit,!user-settings,!user-conf=0x0/0x179/managed], forget [is-slave=0x800], reason removed)
...
<debug> [1664299541.0201] device[a9f10ea824bb1725] (eth1): enslaved to unknown device 0 (??)
...
<debug> [1664299541.0227] device[a9f10ea824bb1725] (eth1): queue-state[deactivating, reason:dependency-failed, id:533]: change state
<info> [1664299541.0228] device (eth1): state change: ip-check -> deactivating (reason 'dependency-failed', sys-iface-state: 'managed')
Fix that by not ignoring the nm_device_slave_notify_release() call. Now we get:
<info> [1664391684.9757] device (bond0): state change: ip-config -> failed (reason 'config-failed', sys-iface-state: 'managed')
...
<debug> [1664391684.9759] active-connection[69c2b12d61f5b171]: set state deactivated (was activating)
<debug> [1664391684.9760] active-connection[142bb8240f6a696d]: check-master-ready: already signalled (state activating, master 0x56116f1480a0 is in state deactivated)
...
<debug> [1664391684.9762] manager: ActivatingConnection now (none)
...
<warn> [1664391684.9763] device (bond0): Activation: failed for connection 'bond0'
<trace> [1664391684.9763] device[142828814dec6e26] (bond0): master: release one slave 720791275fe8a68c/eth1 (not enslaved) (configure)
<debug> [1664391684.9763] device[720791275fe8a68c] (eth1): Activation: connection 'eth1' master failed
...
<debug> [1664391684.9764] device[720791275fe8a68c] (eth1): queue-state[deactivating, reason:dependency-failed, id:3047]: queue state change
<debug> [1664391684.9765] device[720791275fe8a68c] (eth1): unmanaged: flags set to [!sleeping,!by-type,!platform-init,!user-explicit,!user-settings,!user-conf=0x0/0x179/managed], forget [is-slave=0x800], reason removed)
...
<debug> [1664391684.9797] device[720791275fe8a68c] (eth1): queue-state[deactivating, reason:dependency-failed, id:3047]: change state
<info> [1664391684.9797] device (eth1): state change: config -> deactivating (reason 'dependency-failed', sys-iface-state: 'managed')
Commit 1fe8166fc9 ('device: only deactivate when the master we've
enslaved to goes away') added the "return", but it seems to also add it
in cases where we need to handle this. Restrict the return to cases if
we do "no-config".
https://bugzilla.redhat.com/show_bug.cgi?id=2130287
Fixes: 1fe8166fc9 ('device: only deactivate when the master we've enslaved to goes away')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1406
For new uses of nm_uuid_generate_from_strings() we should generate version5
UUIDs and we should use unique namespace UUID arguments.
The namespace UUID was so far replaced by always passing a special prefix
as first string. It seems nicer to use a namespace instead.
Version3 UUIDs should not be used for new applications.
Hence, nm_uuid_generate_from_strings_v3() is no longer a desirable way to
generate UUIDs, so drop the wrapper.
nm_uuid_generate_from_strings() uses variant3 UUIDs based on MD5.
We shouldn't use that in the future.
We will add a replacement, so rename this function so that the "good"
name is free again. Of course, code that uses this function currently
relies on that the behavior doesn't change. We cannot just drop it
entirely, but will replace it by something that gives the same result.
Rename.
If we're generating a connection for an externally configured slave,
refer the master by the UUID instead of the device name.
This doesn't matter most of the time. However, on a checkpoint restore
we need to make sure that a connection that is unambiguously the original
master is up.
Otherwise it could happen that a different connection was activated on the
same master device and the slaves being restored don't agree on which master
connection to bring up.
I can't think of any thing that would rely on this but I've been wrong
about more serious things before.
Fixes-test: @libnm_snapshot_reattach_unmanaged_ports_to_bridge
https://bugzilla.redhat.com/show_bug.cgi?id=2125615
If we're notified of a master appearing, make sure there's actually an
ifindex we're waiting for.
Triger an assertion failure if that is not the case, cause that's pretty
messed up.
Since commit a1de6810df ('device: don't ignore external slave removals')
we don't leave device_recheck_slave_status() on un-eslaving (that is
plink->master = 0) early enough.
This results in hooking of NM_MANAGER_DEVICE_IFINDEX_CHANGED even
when we're not actually waiting for any master device to come up,
accompanied by a messed up log entry:
device[3fa7cfc200be4e84] (portXc): enslaved to unknown device 0 (??)
We also log nonsense when we see any device's link being removed:
device[a9a4b65bde851bcf] (br0): ifindex: set ifindex 0 (old-l3cfg: 05c6a4409f84d9d2)
device[45d34e95fb71cce0] (portXa): master br0 with ifindex 0 appeared
We don't do further damage afterwards, so this is purely a cosmetic
annoyance.
This is something that does happen.
Is that a bug? If so, this should not be a warning message but an
assertion failure. If it's not a bug, then this does not warrant warning
level, because the user wouldn't know what to do about this and it's
something that occasionally happens.
Granted, the state handling in NMDevice is complex, that it's unclear
whether this indicates a problem or not. In any case, having a warning
does only confuse the user.
We already have src/linux-headers, where we have complete copies of linux
user space headers. Of course that exists, because we want to use certain
features and don't depend on the installed kernel headers. Which works
well, because kernel user space API is stable, and we anyway want to
support compiling against a newer kernel and run against an older (e.g.
in a container). So having our copy of newer kernel headers is merely
as if we compiled against as newer kernel.
Add "src/nm-compat-headers" which has a similar purpose, but a different
approach. Instead of replacing the included header entirely, include
the system header and patch it with #define.
Use this for "linux/if_addr.h". Of course, the approach here is that we
no longer include <linux/if_addr.h> directly, but instead include
"nm-compat-headers/linux/if_addr.h".
