The warning "-Wcast-align=strict" seems useful and will be enabled
next. Fix places that currently cause the warning by using the
new macro NM_CAST_ALIGN(). This macro also nm_assert()s that the alignment
is correct.
GArray.data is a char pointer. Most of the time we track other data in
a GArray. Casting that pointer can trigger "-Wcast-align=strict"
warnings.
Avoid them. Most of the time, instead use the nm_g_array*() helpers,
which also assert that the expected element size is correct.
openvswitch accepts "dot1q-tunnel" as vlan mode:
A dot1q-tunnel port is somewhat like an access port. Like an
access port, it carries packets on the single VLAN specified
in the tag column and this VLAN, called the service VLAN,
does not appear in an 802.1Q header for packets that ingress
or egress on the port. The main difference lies in the be‐
havior when packets that include a 802.1Q header ingress on
the port. Whereas an access port drops such packets, a
dot1q-tunnel port treats these as double-tagged with the
outer service VLAN tag and the inner customer VLAN taken
from the 802.1Q header. Correspondingly, to egress on the
port, a packet outer VLAN (or only VLAN) must be tag, which
is removed before egress, which exposes the inner (customer)
VLAN if one is present.
Support this mode.
Add a new "ovs-port.trunks" property that indicates which VLANs are
trunked by the port.
At ovsdb level the property is just an array of integers; on the
command line, ovs-vsctl accepts ranges and expands them.
In NetworkManager the ovs-port setting stores the trunks directly as a
list of ranges.
Support managing the loopback interface through NM as the users want to
set the proper mtu for loopback interface when forwarding the packets.
Additionally, the IP addresses, DNS, route and routing rules are also
allowed to configure for the loopback connection profiles.
https://bugzilla.redhat.com/show_bug.cgi?id=2060905
This is the version shipped in Fedora 37. As Fedora 37 is now out, the
core developers switch to it. Our gitlab-ci will also use that as base
image for the check-{patch.tree} tests and to generate the pages. There
is a need that everybody agrees on which clang-format version to use,
and that version should be the one of the currently used Fedora release.
Also update the used Fedora image in "contrib/scripts/nm-code-format-container.sh"
script.
The gitlab-ci still needs update in the following commit. The change
in isolation will break the "check-tree" test.
Previously we'd note if NM is stopped, but not if it's running.
I suppose it's nice for the user to know that the monitor started
running, but, it's also important for the monitor to be testable (so
that we know that we are ready to start adding mock objects, etc.)
This also gets rids of some duplication at expense of a little less
nuanced message.
This is the better name, becuse this is not in particular about "docs".
It's about generating an XML with the information from the settings
meta data for nmcli.
We will do something similar with the libnm-core meta data.
It just feels nicer to be explicit about the filenames and
not rely on a specific naming.
Also, in meson we can directly pass the target as argument, which
expands to the filename but also adds a dependency.
It seems really ugly, to pass a callback function of wrong
signature. Granted, it probably works due to the C calling
convention, but it seems odd.
Use callbacks of the proper type instead. Then we also don'
need g_signal_connect_swapped().
While at it, rename. "connected_state_cb()" seems a bad name.
Unfortunately, for this we require SetLinkDNSEx() API from v246.
That adds extra complexity.
If the configuration contains no server name, we continue using
SetLinkDNS(). Otherwise, at first we try using SetLinkDNSEx().
We will notice if that method is unsupported, reconfigure with
SetLinkDNS(), and set a flag to not try that again.
The pager_fallback() runs in the forked child process.
As such, it can only use functions from `man signal-safety`
or that are explicitly allowed.
We are mostly good, but g_printerr() is not allowed. It can deadlock.
Just avoid it. It's not very to print those error messages anyway.
setenv() cannot be called after fork, because it might allocate memory,
which can deadlock.
Instead, prepare the environment and use execvpe().
`man 2 fork` says:
After a fork() in a multithreaded program, the child can safely call
only async-signal-safe functions (see signal-safety(7)) until such time
as it calls execve(2).
This means, we are quite strongly limited what can be done in the child
process, before exec. setenv() is not listed as async-signal-safe, obviously
because it allocates memory, and malloc() isn't async-signal-safe either.
See also glib's documentation of GSpawnChildSetupFunc ([1]) about what
can be done in the child process.
[1] 08cb200aec/glib/gspawn.h (L124)
Fix the following crash:
$ nmcli device monitor a
Error: Device 'a' not found.
Segmentation fault (core dumped)
Found by coverity:
1. NetworkManager-1.41.3/src/nmcli/devices.c:0: scope_hint: In function 'do_devices_monitor'
2. NetworkManager-1.41.3/src/nmcli/devices.c:2932:28: warning[-Wanalyzer-null-dereference]: dereference of NULL 'devices'
2930| }
2931|
2932|-> for (i = 0; i < devices->len; i++)
2933| device_watch(nmc, g_ptr_array_index(devices, i));
2934|
Fixes: 2074b28976 ('nmcli/devices: return GPtrArray instead of GSList from get_device_list()')
Our docs can be long. It's important to be able to express paragraphs.
Honor a blank line to include a newline. For XML often whitespace is
ignored, but our tools can choose to honor the newline.
Also, don't strip the whitespace from the beginning and the end.
We keep whitespace for a certain indentation level, but additional
whitespace gets preserved. This is less important, because regular
spaces is indeed irrelevant. But when we write the annotations, we
should be in full control over spaces.
Before:
$ nmcli device connect veth0; echo $?
Error: Connection activation failed: (5) IP configuration could not be reserved (no available address, timeout, etc.).
0
After
$ nmcli device connect veth0; echo $?
Error: Connection activation failed: (5) IP configuration could not be reserved (no available address, timeout, etc.).
4
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/902
It seems uncommon that a command line tool warns about duplicate
paramters. Usually, the latter just overwrites the former. That is also
useful so that you can have for example an alias that sets a default
type
nmcli_import="nmcli connection import type keyfile"
but still call it like
nmcli_import file $FILE type openvpn
This is a change in behavior. Not only stop we printing a warning, we
will now prefer the latter argument. Previously, the first was honored.
This change in behavior is a problem, but such uses were warned against
in the past, and hopefully nobody did this or relied on this.
For convenience, allow also to match the UUID by prefix -- if the
"uuid" selector is used.
Note that still, there must be only one candidate found. The "uuid"
selector guarantees to find a unique connection.
$ nmcli -f connection.uuid,connection.id connection show uuid eb43d80c
The "connection.uuid" and the D-Bus path are supposed to be unique on
D-Bus. Anything else indicates to a bug somewhere.
Still, with `nmcli connection $operation [uuid|path] $arg ...` ensure
that the result is always unique.
In practice, this should make no difference. In the case of an
unexpected duplicate, it seems better to fail and uphold the
guarantee that these selectors give unique results.
Also, next we will accept matching prefixes of the UUID. While partial
match will then be supported, it should still be unique. That is, the
"uuid" specifier should always only yield one result. While this patch
should make not difference in practice today (albeit enforcing something
that should be valid), it will make a difference then.
These variants provide additional nm_assert() checks, and are thus
preferable.
Note that we cannot just blindly replace &g_array_index() with
&nm_g_array_index(), because the latter would not allow getting a
pointer at index [arr->len]. That might be a valid (though uncommon)
usecase. The correct replacement of &g_array_index() is thus
nm_g_array_index_p().
I checked the code manually and replaced uses of nm_g_array_index_p()
with &nm_g_array_index(), if that was a safe thing to do. The latter
seems preferable, because it is familar to &g_array_index().
Add deprecation tags to "subject-match" and "phase2-subject-match"
properties and adjust the documentation slightly.
They've been deprecated since commit 64b76ba906 ('libnm-core: add
domain-suffix-match properties to NMSetting8021x').
Previously, the deprecation data was included in <description*>, in form
of an integer. E.g.:
/**
* NMSettingLala:hello:
*
* Does this and that.
*
* Deprecated: 1.12: Be sad instead.
**/
Results in:
<property name="hello">
<description>Does this and that. Deprecated: 1</description>
</property>
Let's make it do this instead:
<property name="hello">
<description>Does this and that.</description>
<deprecated since="1.12">Be sad instead.</description>
</property>
Add option to set ofport_request when configuring ovs interface. When
connection with ofport_request configured is activated ovsdb will first
try to activated on the port set by ofport_request.
It is useful to modify the UUID in offline mode. Otherwise, it's
cumbersome to clone a profile, because the cloned profile will
have the same UUID (and NetworkManager cannot load them both
at the same time).
umask 077
nmcli --offline connection modify \
connection.id profile2 \
connection.uuid new \
< /etc/NetworkManager/system-connections/profile1.nmconnection \
> /etc/NetworkManager/system-connections/profile2.nmconnection \
The doctext doesn't actually work for `man nm-settings-nmcli`. The
generation of our docs is still an incomprehensible mess that needs
fixing.
1) The "enabled-on-global-iface" flag was odd. Instead, have only
and "enabled" flag and skip (by default) endpoints on interface
that have no default route. With the new flag "also-without-default-route",
this can be overruled. So previous "enabled-on-global-default" now is
the same as "enabled", and "enabled" from before behaves now like
"enabled,also-without-default-route".
2) What was also odd, as that the fallback default value for the flags
depends on "/proc/sys/net/mptcp/enabled". There was not one fixed
fallback default, instead the used fallback value was either
"enabled-on-global-iface,subflow" or "disabled".
Usually that is not a problem (e.g. the default value for
"ipv6.ip6-privacy" also depends on use_tempaddr sysctl). In this case
it is a problem, because the mptcp-flags (for better or worse) encode
different things at the same time.
Consider that the mptcp-flags can also have their default configured in
"NetworkManager.conf", a user who wants to switch the address flags
could previously do:
[connection.mptcp]
connection.mptcp-flags=0x32 # enabled-on-global-iface,signal,subflow
but then the global toggle "/proc/sys/net/mptcp/enabled" was no longer
honored. That means, MPTCP handling was always on, even if the sysctl was
disabled. Now, "enabled" means that it's only enabled if the sysctl
is enabled too. Now the user could write to "NetworkManager.conf"
[connection.mptcp]
connection.mptcp-flags=0x32 # enabled,signal,subflow
and MPTCP handling would still be disabled unless the sysctl
is enabled.
There is now also a new flag "also-without-sysctl", so if you want
to really enable MPTCP handling regardless of the sysctl, you can.
The point of that might be, that we still can configure endpoints,
even if kernel won't do anything with them. Then you could just flip
the sysctl, and it would start working (as NetworkManager configured
the endpoints already).
Fixes: eb083eece5 ('all: add NMMptcpFlags and connection.mptcp-flags property')
(cherry picked from commit c00873e08f)
The reproducer for another problem tripped an assertion failure:
$ nmcli con del act-conn
Connection 'act-conn' (...) successfully deleted.
$ nmcli con down another-conn
(process:94552): nm-CRITICAL **: 17:07:21.170: ((src/libnm-client-impl/nm-remote-connection.c:593)): assertion '<dropped>' failed
Connection 'another-conn' successfully deactivated (D-Bus active path: /org/freedesktop/NetworkManager/ActiveConnection/4)
$
What happens is that the second invocation, when resolving the
connection name into a NMRemoteConnection object, assumes an active
connection has a settings connection.
This assumption is likely to be wrong immediately after deleting a
connection was active, before giving the active connection enough time
to fully deactivate.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1317
