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.
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.
(cherry picked from commit fcd907e062)
To embrace inclusive language, deprecate the NMSettingConnection
autoconnect-slaves property and introduce autoconnect-ports property.
(cherry picked from commit 194455660d)
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.
(cherry picked from commit 2f1b599fe3)
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
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.
To embrace inclusive language, deprecate the NMSettingConnection
slave-type property and introduce port-type property.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
"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().
Now that we require glib 2.42, we can use G_PARAM_EXPLICIT_NOTIFY flag.
The benefit is that this flag saves a notification, when the property
value does not change.
The downside is, that implementations of set_property() must remember to
emit _notify() when required. This is somewhat alleviated by using
_nm_setting_property_set_property_direct(), which does this
automatically.
Se the flag for G_PARAM_EXPLICIT_NOTIFY for direct boolean properties.
For now, only do it for boolean properties, because of the danger of
getting this wrong. We must review all callers to make sure that they
don't implement set_properties() and don't forget to notify.
We will deprecate "connection.master" for "connection.controller". The
old property will become an alias for the new one. That means, when
setting the property we must emit notifications for both the old and the
new property.
Add "NMSettInfoProperty.direct_also_notify" for that. This is not fully
flexible, as it only works for direct properties (duh) and only allows
to specify one addtitional GParamSpec (of the same NMSetting). It is
however sufficient for our use.
direct_hook is a union, which currently only has one member (the set function
for a direct string). Extending this might make sense, for other set functions
(e.g. overwriting setting a strv array).
However, then the name was bad. The union is already for the set-function of
direct properties, it's not a place for various kinds of hooks (as it is
a union).
Rename.
For most strv or string properties, we cannot distinguish between
NULL/unset/default and empty.
It would be difficult to enter in nmcli or grasp how it differs. There
are probably many bugs, where we accept empty strings, and fail to
handle them correctly.
Anyway. For most strv arrays, and empty array and NULL/unset/default are
treated the same. That means, g_object_get() tends to always return NULL
(never an empty strv array) and g_object_set() of an empty strv array
will internally leave the GArray at NULL.
For a few properties, there is a difference. See "ipv[46].dns-options".
See also "clear_emptyunset_fcn" hook in libnm-setting.
Add a way to handle such strv properties with the "direct" mechanism.
Unfortunately, there are several possibilities how to handle NULL and
empty arrays. Therefore we have different variants.
Clean this up, and add a way to preserve whether the array is empty
(previous variants could not distinguish that).
Functions are also renamed, so that if you backport a user of the new
API, you'll get a compiler error if this patch is missing.
Also, nm_strvarray_get_strv_notnull() no longer takes a pointer to a
"GArray*". Previously, it used that to fake an empty strv array. Now
this returns NM_STRV_EMPTY_CC().
_nm_utils_dns_option_validate() allows specifying the address family,
and filters based on that. Note that all options are valid for IPv6,
but some are not valid for IPv4.
It's not obvious, that such filtering is only performed if
"option_descs" argument is provied. Otherwise, the "ipv6" argument is
ignored.
Regardless, it's also confusing to have a boolean "ipv6". When most
callers don't want a filtering based on the address family. They
actually don't want any filtering at all, as they don't pass an
"option_descs". At the same time passing a TRUE/FALSE "ipv6" is
redundant and ignored. It should be possible, to explicitly not select
an address family (as it's ignored anyway).
Instead, make the "gboolean ipv6" argument an "int addr_family".
Selecting AF_UNSPEC means clearly to accept any address family.
c_siphash_init() requires a 16 bytes array. That is cumbersome to use.
We have NM_HASH_SEED_16() macro for helping with that. It's still
cumbersome.
Most of the time, the caller just wants to pick an arbitrarily chosen,
fixed number. Add NM_HASH_SEED_16_U64() which takes a number and gives
a 16 seed array. The argument is in host endianness, but the resulting
seed array has it encoded in big endianness, to be architecture
independent.
Some Applications require to explicitly enable or disable EEE.
Therefore introduce EEE (Energy Efficient Ethernet) support with:
* ethtool.eee on/off
Unit test case included.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
Adds a new WiFi 6GHz capability flag, NM_WIFI_DEVICE_CAP_FREQ_6GHZ,
along side the existing NM_WIFI_DEVICE_CAP_FREQ_2GHZ &
NM_WIFI_DEVICE_CAP_FREQ_5GHZ flags.
Gnome settings utilizes the 2 existing flags to present supported
bands in gnome-settings. I will be using this additional flag in
modifications there.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1739
In nm_setting_infiniband_get_virtual_interface_name(), no longer try to
detect whether the cached value is still up to date. Instead, as we now
have a fix sized buffer for the name, just always generate the name on
every call. It's simpler.
This is the same what kernel does, when the parent name is so long
that it would result in a too long overall name.
We need that the result is still a valid interface name.
NetworkManager does not support changing the interface name for
infiniband interfaces. Consequently, we verify that
"connection.interface-name" is either unset or set to the expected
"$parent.$p_key". Anything else wouldn't work anyway and is rejected as
invalid configuration. That brings problems however.
Rejecting invalid configuration seems fine at first:
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name xxx
Error: Error writing connection: connection.interface-name: interface name of software infiniband device must be 'ib0.8010' or unset (instead it is 'xxx')
However, when we modify the p-key, we also get an error message:
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name ib0.8010 |
nmcli --offline connection modify infiniband.p-key 5
Error: Error writing connection: connection.interface-name: interface name of software infiniband device must be 'ib0.0005' or unset (instead it is 'ib0.8010')
It's worse, because ifcfg-rh reader will mangle the PKEY_ID with |=0x8000 to set
the full membership flag. That means, if you add a profile like
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x0010 connection.interface-name ib0.0010
it gets written to ifcfg-rh file. Then upon reload it's invalid (as the
interface name mismatches).
There are multiple solutions for this. For example, ifcfg-rh reader could also
mangle the connection.interface-name, so that the overall result is valid. Or
we could just not validate at all, and accept any bogus interface-name.
With this patch instead we will just normalize the invalid configuration to
make it right.
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name ib0.8010 |
nmcli --offline connection modify infiniband.p-key 5
...
The downside is that this happens silently, so a user doesn't
notice that configuration is ignored:
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name foo
...
interface-name=ib0.8010
This approach still seems preferable, because setting
"connection.interface-name" for infiniband profiles makes little sense,
so what we care here is to avoid problems.
src/libnm-core-impl/tests/test-keyfile.c: In function '_invalid_option_write_handler':
src/libnm-core-impl/tests/test-keyfile.c:917:9: error: 'message' may be used uninitialized [-Werror=maybe-uninitialized]
917 | g_assert(message && strstr(message, "ethtool.bogus"));
| ^
src/libnm-core-impl/tests/test-keyfile.c:905:29: note: 'message' was declared here
905 | const char *message;
| ^
lto1: all warnings being treated as errors
Having a list with only one element is often interesting to know. For
example, if you are about to unlink an element, you may want to check
whether afterwards the list is empty.
Add c_list_is_empty_or_single() for that. It is probably more efficient than
plain c_list_length_is(list, 1) and also a better name.
The new arp_missed_max option valid range is 0-255 where value 0 means
not set. Please notice that this option is not compatible with 802.3AD,
balance-tlb and balance-alb modes.
This setting allows the user to remove the local route rule that is
autogenerated for both IPv4 and IPv6. By default, NetworkManager won't
touch the local route rule.
In kernel, the valid range for the weight is 1-256 (on netlink this is
expressed as u8 in rtnh_hops, ranging 0-255).
We need an additional value, to represent
- unset weight, for non-ECMP routes in kernel.
- in libnm API, to express routes that should not be merged as ECMP
routes (the default).
Extend the type in NMPlatformIP4Route.weight to u16, and fix the code
for the special handling of the numeric range.
Also the libnm API needs to change. Modify the type of the attribute on
D-Bus from "b" to "u", to use a 32 bit integer. We use 32 bit, because
we already have common code to handle 32 bit unsigned integers, despite
only requiring 257 values. It seems better to stick to a few data types
(u32) instead of introducing more, only because the range is limited.
Co-Authored-By: Fernando Fernandez Mancera <ffmancera@riseup.net>
Fixes: 1bbdecf5e1 ('platform: manage ECMP routes')
