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.
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.
"nm-glib" h contains compat wrappers for older glib versions. This file
used to be copied over to VPN plugins, to use the same compat code. It
was thus interesting to also have compat code for glib versions, that
were no longer supported by NetworkManager itself.
This was fine. But glib 2.42 is more than 8 years old. At this point,
there really is no need to support that, even if you copy the file out
of NetworkManager source tree.
Drop those compat wrappers.
This patch add support to HSR/PRP interface. Please notice that PRP
driver is represented as HSR too. They are different drivers but on
kernel they are integrated together.
HSR/PRP is a network protocol standard for Ethernet that provides
seamless failover against failure of any network component. It intends
to be transparent to the application. These protocols are useful for
applications that request high availability and short switchover time
e.g electrical substation or high power inverters.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1791
It also doesn't make much sense to have this. We may use a
GPtrArray to construct and keep track of a (dynamic) strv list.
Then we add the strings to the GPtrArray one by one.
We almost never will want to create a GPtrArray based on a strv array.
