Add a "a{sv}" output argument to "AddAndActivate2" D-Bus API.
"AddAndActivate2" replaces "AddAndActivate" with more options.
It also has a dictionary argument to be forward compatible so that we
hopefully won't need an "AddAndActivate3". However, it lacked a similar
output dictionary. Add it for future extensibility. I think this is
really to workaround a shortcoming of D-Bus, which does provide strong
typing and type information about its API, but does not allow to extend
an existing API in a backward compatible manner. So we either resort to
Method(), Method2(), Method3() variants, or a catch-all variant with a
generic "a{sv}" input/output argument.
In libnm, rename "nm_client_add_and_activate_connection_options()" to
"nm_client_add_and_activate_connection2()". I think libnm API should have
an obvious correspondence with D-Bus API. Or stated differently, if
"AddAndActivateOptions" would be a better name, then the D-Bus API should
be renamed. We should prefer one name over the other, but regardless
of which is preferred, the naming for D-Bus and libnm API should
correspond.
In this case, I do think that AddAndActivate2() is a better name than
AddAndActivateOptions(). Hence I rename the libnm API.
Also, unless necessary, let libnm still call "AddAndActivate" instead of
"AddAndActivate2". Our backward compatibility works the way that libnm
requires a server version at least as new as itself. As such, libnm
theoretically could assume that server version is new enough to support
"AddAndActivate2" and could always use the more powerful variant.
However, we don't need to break compatibility intentionally and for
little gain. Here, it's easy to let libnm also handle old server API, by
continuing to use "AddAndActivate" for nm_client_add_and_activate_connection().
Note that during package update, we don't restart the currently running
NetworkManager instance. In such a scenario, it can easily happen that
nmcli/libnm is newer than the server version. Let's try a bit harder
to not break that.
Changes as discussed in [1].
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/37#note_79876
Don't return success from a nested code path. Handle all errors
first, and return early. Well, we cannot really return right away
because we need to handle the failure. So, at least, check for errors
and "goto fail".
If the child is respawning too fast, consider the plugin failed so
that upstream servers are written to resolv.conf until the plugin gets
restarted after the delay.
When the dnsmasq process dies, two events are generated:
(1) a NM_DNS_PLUGIN_FAILED signal in nm-dns-dnsmasq.c:name_owner_changed()
(2) a NM_DNS_PLUGIN_CHILD_QUIT signal in nm-dns-plugin.c:from watch_cb()
Event (1) is handled by updating resolv.conf with upstream servers,
(2) by restarting the child process.
The order in which the two signals are received is not deterministic,
so when (1) comes after (2) the manager leaves upstream servers in
resolv.conf even if a dnsmasq instance is running.
When dnsmasq disappears from D-Bus and we know that the process is not
running, we should not emit a FAILED signal because the disappearing
is caused by the process termination, and that event is already
handled by the manager.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/105
nmcli connection modify t ipv4.dns-options ndots:2
nmcli connection modify t +ipv4.dns-options ndots:4
should set dns-options to 'ndots:4', so we must remove other
occurences of the same option before adding it, otherwise the setting
refuses to set the same option again.
Appending to the ipvx.dns-options property:
nmcli connection modify con +ipv4.dns-options rotate
currently is buggy because it resets the list to contain only
'rotate'. The setter function should not clear the list.
https://bugzilla.redhat.com/show_bug.cgi?id=1665649
wpa_supplicant is going to change the global default for PMF from 0
(disabled) to 1 (optional) [1], so NM code needs to be adjusted to
work with all wpa_supplicant versions. Furthermore, it is better to
set optional PMF using the 'Pmf' property instead of the 'ieee80211w'
configuration option because the former better handles missing support
in driver [2].
Note that each interface in wpa_supplicant has its own copy of global
configuration and so 'global' options must still be set on each
interface. So, let's set Pmf=1 when each interface gets created and
override it with ieee80211w={0,2} if needed during association.
[1] http://lists.infradead.org/pipermail/hostap/2018-November/039009.html
[2] http://lists.infradead.org/pipermail/hostap/2019-January/039215.htmlhttps://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/104
_log_connection_get_property() is a hack, as it cannot meaningfully print complex
properties. Also, it uses _nm_setting_get_property() which can only work with GObject
base properties.
Don't assert against _nm_setting_get_property() returning success. Eventually
we should replace _nm_setting_get_property() by something better. But for the moment,
it's fine to being unable to print a property value.
Curreently all aggregate types only care about secrets.
The check for secets is done by checking for NM_SETTING_PARAM_SECRET
flag. Assert that this check is suitable to identify a secret.
NMSetting's compare_property() has and had two callers:
nm_setting_compare() and nm_setting_diff().
compare_property() accepts a NMSettingCompareFlags argument, but
at the same time, both callers have another complex (and
inconsistent!) set of pre-checks for shortcuting the call of
compare_property(): should_compare_prop().
Merge should_compare_prop() into compare_property(). This way,
nm_setting_compare() and nm_setting_diff() has less additional
code, and are simpler to follow. Especially nm_setting_compare()
is now trivial. And nm_setting_diff() is still complicated, but
not related to the question how the property compares or whether
it should be compared at all.
If you want to know whether it should be compared, all you need to do
now is follow NMSettingClass.compare_property().
This changes function pointer NMSettingClass.compare_property(),
which is public API. However, no user can actually use this (and shall
not!), because _nm_setting_class_commit_full() etc. is private API. A
user outside of libnm-core cannot create his/her own subclasses of
NMSetting, and never could in the past. So, this API/ABI change doesn't
matter.
nm_setting_compare() and nm_setting_diff() both call the virtual
function compare_property(). But their check for determining whether
to call the virtual function differs.
In a first step, merge the implementations so that the check is clearly
similar in both cases.
The flags NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS and
NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS act on the secret flags
to decide whether to ignore a secret.
But there was not test how this behaved, if the two settings had
differing flags.
ethernet.cloned-mac-address is also marked as inferrable. I think the concept
of NM_SETTING_PARAM_INFERRABLE is fundamentally wrong just like the entire
assume approach. Anyway, if ethernet's property is inferrable, so should
be Wi-Fi's.
This bug had no effect, because NM_SETTING_COMPARE_FLAG_IGNORE_REAPPLY_IMMEDIATELY
has only one user, and it's used there in combination with
nm_setting_compare(). No caller passed this flag to nm_setting_diff().
Fixes: c9b3617c35
When STP is disabled, the bridge parameters 'priority', 'forward-delay',
'hello-time' and 'max-age' are irrelevant.
We already skip them when loading a connection profile from a ifcfg file.
Do the same when generating a connection from a configured device, in
order to possibly assume the connection.
...also when the connection is created at NetworkManager
startup to map an already configured bridge.
Ensure the device has configuration values that fall inside
NetworkManager boundaries, otherwise map the value with a default.
We have bridge min/max/default values in core-internal. Do the same
for bridge port ones.
We will soon use those values to enforce limits when assuming a
bridge port configuration.
In NetworkManager we have a default port path-cost equal to 100.
In the linux kernel the default port cost depends upon the interface
speed: 2 for 10Gb, 4 for 1Gb, 19 for 100Mb and 100 for 10Mb (or when the
interface speed is not available, like current virtio_net driver).
Allow NetworkManager to assume bridge port connections also when the
path-cost differs: this will allow us to assume bridge ports created
outside NetworkManager (e.g. in initrd) that will likely have a different
"cost" value.
Since we already cached the result of getpagesize() in a static variable (at
two places), move the code to nm-shared-utils, so it is reusable.
Also, use sysconf() instead of getpagesize(), like suggested by `man
getpagesize`.
Using strncpy() in the macro directly can result in a compiler warning.
We don't want to replace this with memcpy(), because strncpy() aborts
on the first NUL and fills the rest with NUL. Since nm_strndup_a() is a
replacement for g_strndup(), we want to do that here as well.
In file included from ../shared/nm-default.h:294,
from ../libnm-core/nm-utils.c:22:
../libnm-core/nm-utils.c: In function nm_sock_addr_endpoint_new:
../shared/nm-utils/nm-shared-utils.h:281:4: error: strncpy output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]
strncpy (_s, _str, _len); \
^~~~~~~~~~~~~~~~~~~~~~~~
../libnm-core/nm-utils.c:154:26: note: in expansion of macro nm_strndup_a
host = _parse_endpoint (nm_strndup_a (200, endpoint, l_endpoint - 1, &host_clone),
^~~~~~~~~~~~
../libnm-core/nm-utils.c:152:15: note: length computed here
l_endpoint = strlen (endpoint) + 1;
^~~~~~~~~~~~~~~~~
Now that the default for the internal client is "mac", we don't need
this snippet anymore. Drop it.
Don't renumber the source files but leave the gap at Source3. Everytime
we add config snippets the numbers need to be shuffled, so don't fill
the gap and maybe use it in the future.
https://bugzilla.redhat.com/show_bug.cgi?id=1661165
The "ipv4.dhcp-client-id" is configurable per-profile and the default
value can be overwritten via connection defaults in NetworkManager.conf.
For "dhclient" DHCP plugin, the ultimate default for "ipv4.dhcp-client-id"
is determined by dhclient itself, or possibly by user configuration from
"/etc/dhcp".
For the "internal" DHCP plugin, the default must be decided by
NetworkManager. Also, the default here is important, as we preferably
won't change it anymore. That is because a changing the client-id
will result in different IP addresses after upgrade of NetworkManager
version. That should be avoided.
Still, change it now. If a downstream does not want this, it either needs
to patch the sources or add a configuration snippet like:
[connection-internal-dhcp-client-id-duid]
match-device=dhcp-plugin:internal
ipv4.dhcp-client-id=duid
The reason to change from the previous default "duid" to "mac" are the
following:
- "duid" is an RFC 4361 compatible client-id ([1]) and "mac" is defined
in RFC 2132.
- "duid" cannot (easily) be predicated a-priori as it is a hash of the
interface-name and "/etc/machine-id". In particular in cloud and server
environments, admins often prefer "mac" as they do know the MAC address
and pre-configure the DHCP server accordingly.
- with "dhclient" plugin, the default is decided by dhclient package or
user configuration in "/etc/dhcp". However, in fact the default is often
"client-identifier hardware" (for example on RHEL/CentOS).
- for RHEL/CentOS we require a way to select "mac" as default. That was
done by installing a configuration snippet via the NetworkManager-config-server
package. It's confusing to have the default depend on a package. Avoid
that. Also, users required "mac" in certain scenarios, but no users
explicitly asked for "duid" as default.
- our "duid" implementation generates a 32 bit IAID based on a hash of the
interface-name, and only 8 bytes entropy that contains a hash
of "/etc/machine-id". The point is, that is not a lot of entropy to
avoid conflicting client-ids. Another point is, that the choosen algorithm
for "duid" is suitable for RFC 4361, but it's only one of many possibly
implementations. Granted, each possibility has up and downsides but selecting
one of them as default seems wrong (given that it has obvious downsides
already). For "mac" there is only one straight-forward way to implement
it.
- RFC 7844 (Anonymity Profiles for DHCP Clients) is not yet supported by
NetworkManager. But we should not select a default client-id which
counteracts anonymit. Choosing "mac" does not reveal information which
is not already exposed.
[1] https://tools.ietf.org/html/rfc4361#section-4https://bugzilla.redhat.com/show_bug.cgi?id=1661165
