The purpose of this is to allow overriding to_dbus_fcn and from_dbus_fcn when
necessary (such as for special behavior regarding a deprecated/aliased properties).
Introduce a new option to NMSettingIpConfig. The new option is ternary
type being the default value set to disabled. When enabled,
NetworkManager will instruct the DHCP client to send RELEASE message
when IP addresses are being removed.
The new option at NMSettingConnection allow the user to specify if the
connection needs to be down when powering off the system. This is useful
for IP address removal prior powering off. In order to accomplish that,
we listen on "Shutdown" systemd DBus signal.
The option is set to FALSE by default, it can be specified globally on
configuration file or per profile.
If the device-handler of the generic connection is set, the connection
is virtual and the device is created by invoking the device-handler
via NetworkManager-dispatcher service.
With this change, a generic device now represents two different device
classes:
- existing interfaces that are not natively supported or recognized
by NetworkManager. Those devices have the `has_device_handler`
property set to FALSE;
- interfaces that are created by NM by invoking the device-handler;
they have `has_device_handler` set to TRUE.
Add a new "generic.device-handler" property that specifies the name of
a dispatcher script to be invoked to add and delete the interface for
this connection.
Add property to allow changing the eswitch mode between legacy SRIOV and
switchdev. Allow also to set "preserve" to prevent NM from modifying the
eswitch mode.
With a static array, we indicate that the argument must not be NULL.
Gcc-14.0.1-0.2.fc40 now warns against that:
CC src/libnm-base/libnm_base_la-nm-base.lo
In file included from ../src/libnm-std-aux/nm-default-std.h:102,
from ../src/libnm-glib-aux/nm-default-glib.h:11,
from ../src/libnm-glib-aux/nm-default-glib-i18n-lib.h:13,
from ../src/libnm-base/nm-base.c:3:
../src/libnm-base/nm-base.c: In function 'nm_net_devname_infiniband':
../src/libnm-std-aux/nm-std-aux.h:191:12: error: 'nonnull' argument 'name' compared to NULL [-Werror=nonnull-compare]
191 | if (expr) \
| ^
../src/libnm-std-aux/nm-std-aux.h:202:27: note: in expansion of macro '_NM_BOOLEAN_EXPR_IMPL'
202 | _NM_BOOLEAN_EXPR_IMPL(NM_UNIQ, expr))
| ^~~~~~~~~~~~~~~~~~~~~
../src/libnm-glib-aux/nm-macros-internal.h:1693:31: note: in expansion of macro 'NM_BOOLEAN_EXPR'
1693 | #define _G_BOOLEAN_EXPR(expr) NM_BOOLEAN_EXPR(expr)
| ^~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gmacros.h:1244:43: note: in expansion of macro '_G_BOOLEAN_EXPR'
1244 | #define G_LIKELY(expr) (__builtin_expect (_G_BOOLEAN_EXPR(expr), 1))
| ^~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gmessages.h:656:9: note: in expansion of macro 'G_LIKELY'
656 | if (G_LIKELY (expr)) \
| ^~~~~~~~
../src/libnm-base/nm-base.c:57:5: note: in expansion of macro 'g_return_val_if_fail'
57 | g_return_val_if_fail(name, NULL);
| ^~~~~~~~~~~~~~~~~~~~
../src/libnm-core-impl/nm-setting-wireguard.c: In function '_nm_wireguard_peer_set_public_key_bin':
../src/libnm-core-impl/nm-setting-wireguard.c:316:8: error: 'nonnull' argument 'public_key' compared to NULL [-Werror=nonnull-compare]
316 | if (!public_key)
| ^
Convert these checks to an nm_assert() to suppress the warning.
Otherwise, gcc-14.0.1-0.2.fc40 warns:
../src/libnm-core-impl/nm-utils.c: In function _nm_utils_strstrdictkey_create:
../src/libnm-core-impl/nm-utils.c:5076:16: error: allocation of insufficient size '1' for type 'NMUtilsStrStrDictKey' {aka 'struct _NMUtilsStrStrDictKey'} with size '2' [-Werror=alloc-size]
5076 | return g_malloc0(1);
| ^~~~~~~~~~~~
gcc-14.0.1-0.2.fc40 warns:
CC src/libnm-core-impl/libnm_core_impl_la-nm-setting-team.lo
../src/libnm-core-impl/nm-setting-team.c: In function nm_team_link_watcher_new_ethtool:
../src/libnm-core-impl/nm-setting-team.c:127:13: error: allocation of insufficient size 16 for type NMTeamLinkWatcher with size 48 [-Werror=alloc-size]
127 | watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
| ^
Currently the internal DHCP client sets traffic class "CS6" in the DS
field of the IP header for outgoing packets.
dhclient sets the field according to the definition of TOS (RFC 1349),
which was was deprecated in 1998 by RFC 2474 in favor of DSCP.
Introduce a new property IPvX.dhcp-dscp (currently valid only for
IPv4) to specify a custom DSCP value for DHCP backends that support it
(currently, only the internal one).
Define the default value to CS0, because:
- section 4.9 of RFC 4594 specifies that DHCP should use the standard
(CS0 = 0) service class;
- section 3.2 says that class CS6 is for "transmitting packets
between network devices (routers) that require control (routing)
information to be exchanged between nodes", listing "OSPF, BGP,
ISIS, RIP" as examples of such traffic. Furthermore, it says that:
User traffic is not allowed to use this service class. By user
traffic, we mean packet flows that originate from user-controlled
end points that are connected to the network.
- we got reports of some Cisco switches dropping DHCP packets because
of the CS6 marking.
In order to make _nm_setting_property_define_direct_enum more flexible,
this patch is introducing property_type argument to it. When set to NULL
it will set property_type to nm_sett_info_propert_type_direct_enum.
Defining the wrong from_dbus/to_dbus functions is something not
probable. The unit test is just getting in the way of those who knows
what they do and force contributors to change the same thing in multiple
places.
This property indicates that a non-null strv array is expected, and
an empty strv array should be returned instead of NULL if it hadn't
been created yet.
The comparison checking for MAC address equality had previously been flipped around.
Fixes: b084ad7f2b ('libnm-core: canonicalize hardware addresses in settings')
The default in systemd-resolved is nowadays "yes". In any case, since
the setting is configurable systemd-resolved, don't describe it in the
manual page.
Instead, clarify the behavior and try to improve the documentation.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1467
OpenShift MetalLB team requests to configure additional routes
whenever the nodes does not have a configured IP address or route for
the subnet in which MetalLB issues addresses.
Note in linux network stack, it does not matter what interface you add
the address on a node (for example, loopback), the kernel is always
processing arp-requests and sending arp-replies to any of them, this
behavior is considered correct and, moreover, it is widely used in the a
dynamic environment as Kubernetes.
https://issues.redhat.com/browse/RHEL-5098https://gitlab.freedesktop.org/NetworkManager/NetworkManager-ci/-/merge_requests/1587
"nm_setting_hsr_get_port1" is new API and verify() already enforces that
the strings are not empty. The flag is redundant.
Also drop it from a few other places, where it's redundant.
Most properties don't accept empty strings and reject them during
verify().
All _nm_setting_property_define_direct_mac_address() call
nm_utils_hwaddr_valid() on the string, which rejects empty strings.
Clear the .direct_string_allow_empty flag for those. The usage of the
flag is misleading.
Most string properties should not accept empty strings. Add a generic
way to reject them during verify.
Add a new flag NMSettInfoProperty.direct_string_allow_empty.
Note that properties must opt-in to allow empty values. Since all
existing properties didn't have this check (but hopefully re-implemented
it in verify()), all existing properties get this flag set to TRUE.
The main point here it that new properties get the strict check by
default.
We should also review existing uses of direct_string_allow_empty,
whether the flag can be cleared. This can be done if verify() already
enforces a non-empty string, or if we accept to break behavior by
tightening up the check.
Current verifications happens by implementing NMSetting's verify().
Add code for a second step of validation, that can operate based on the
known type.
The use case will be to reject empty strings.
To embrace inclusive language, deprecate the NMSettingConnection
slave-type property and introduce port-type property.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
A duplicate address is a serious issue which leads to non-working
setups or problems hard to debug. Enable IPv4 duplicate address
detection (aka ACD, RFC 5227) by default to detect such problems.
While the RFC recommends a timeout of 9 seconds, a comment in n-acd
sources says:
A 9s timeout for successful link setups is not acceptable today.
Hence, we will just go forward and ignore the proposed values. On
both wired and wireless local links round-trip latencies of below
3ms are common. We require the caller to set a timeout multiplier,
where 1 corresponds to a total probe time between 0.5 ms and 1.0
ms. On modern networks a multiplier of about 100 should be a
reasonable default. To comply with the RFC select a multiplier of
9000.
Set a default timeout of 200ms, which is the double of the value
suggested in n-acd sources. 200ms sounds quick enough, and gives at
least ~100ms to other hosts to reply.
See also the Fedora change proposal:
https://fedoraproject.org/wiki/Changes/Enable_IPv4_Address_Conflict_Detection
"nm-property-compare.c" only contains nm_property_compare(), which is
broken.
It tries to compare string dictionaries as equal regardless of the
order of elements. It gets it wrong, for dictionaries with duplicate
keys. Which means, it can only be used with trusted variants that are
known to not contain duplicates. Which is quite a non-starter.
Also, the idea of a compare function for GVariant that ignores the order
of dictionary elements seems wrong. Even if for a certain application
the order does not matter, it still depends what the upper layer makes
of duplicate keys (will they bail out, or take the first/last occurrence
of a duplicate key?). nm_property_compare() doesn't have the knowledge
how upper layer handles it, and it's not obvious what's the right
choice. For example, if you use g_variant_lookup(), the first occurrence
is preferred. If you iterate over the children, possibly later
occurrences overwrite earlier ones.
It's ill defined, and maybe shouldn't be done. What should instead
happen, is that upper layers normalize (sort, uniquify) the keys, so
that we can do a full comparison. For that we have nm_g_variant_cmp().
Drop the now unused code. The core of the function still exists as
nm_g_variant_cmp().
nm_property_compare() makes a misguided attempt to compare dictionaries
regardless of their order.
However, if variants contain duplicate keys, then the implementation
is wrong and cannot handle it correctly.
Regardless of that. While in some sense the order of dictionary keys is
irrelevant, this is not the right place to perform such normalization.
If the order of things doesn't matter, then NMSetting must normalize the
property (e.g. by sorting the keys). At that point, the GVariant shall
be compared fully.
The supervision address is read-only. It is constructed by kernel and
only the last byte can be modified by setting the multicast-spec as
documented indeed.
As 1.46 was not released yet, we still can drop the whole API for this
setting property. We are keeping the NMDeviceHsr property as it is a
nice to have for reading it.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1823
Fixes: 5426bdf4a1 ('HSR: add support to HSR/PRP interface')
The previous code is not entirely obvious, because as always,
verify() and normalize() must agree in what they are about to
do.
Make that clearer by adding _nm_setting_wireless_normalize_mac_address_randomization(),
which evaluates the desired settings. This is the used both by verify()
and normalize().
nm_utils_escape_ssid() uses a static buffer, which makes it non
thread-safe. We shouldn't have such API in libnm. We could improve that
by using a thread-local storage, but that brings overhead, for a
function that really isn't useful.
It's not useful, because the escaping is very naive. You are better
served with:
- nm_utils_ssid_to_utf8(): gives UTF-8, but looses information.
- nm_utils_bin2hexstr(): does not loose information, but makes the
name unreadable.
Maybe the best way to escape the SSID (which can be binary, but usually
is UTF-8), are the utf8safe functions. That is because it makes the
blob UFT-8, while not loosing/hiding any bytes (the escaping can be
reversed). This API is currently not exposed to the users, if there were
a need, then this could be done as a 3rd way for printing SSIDs.
However, nm_utils_escape_ssid() is bad either way. Deprecate.
Check for invalid DNS, addresses and routes errors in the `_from_dbus`
functions. With NM_SETTING_PARSE_FLAGS_STRICT, stop parsing and return
error at first error. With NM_SETTING_PARSE_FLAGS_BEST_EFFORT don't
return any error and return the values of the list which are valid.
This is the same that was done in a previous commit for ipv4 properties.
Report an error if the data type of the address-labels received via DBus
is not the expected.
Also, fix the assignment of the labels to their corresponding addresses.
As they are matched by array position, if any invalid address was
received, the array of addresses that we generate is shorter than the
array of address-labels. We were not considering this so we were
assigning the address-labels to incorrect addresses. Fix it by moving the
assignment of the labels to _nm_utils_ip4_addresses_from_variant, where
we still have the information of what the original position in the array
the address had.
Invalid addresses received via DBUS were just ignored and filtered out,
only emitting a warning to the logs. If there were still some valid
addresses, those were configured and the client was unaware of the
errors. Only if there was not any valid address at all and method was
manual, an error was returned from `verify`, but not reflecting the
real cause:
ipv4.addresses: this property cannot be empty for 'method=manual'
Check for invalid addresses errors in the `_from_dbus` functions. With
NM_SETTING_PARSE_FLAG_STRICT, parsing is aborted on first error and
error is returned. With NM_SETTING_PARSE_BEST_EFFORT, we keep parsing
and set only the valid values.
Actually, the invalid addresses were dropped in a helper function that
converts from GVariant to NMIPAddress. As it is part of the public API,
we can't change now its signature to add the GError argument. Instead,
create a new internal function and call it from the public one. The
public function will ignore the error, as it was doing previously, but
it won't emit any warning to avoid spamming the logs (we don't even
know if ignoring the invalid values was intentional when calling the
function). The new internal function might be made public in
the future, deprecating the other, but probably it is not necessary
because clients are never going to receive invalid addresses from the
daemon.
Do the same as explained above for DNS entries and routes.
Also, fix the documentation of nm_utils_ip_routes_to/from_dbus, which
said that it accepts new style routes but described the old style ones.
