g_string_new_len() allocates the buffer with length
bytes. Maybe it should be obvious (wasn't to me), but
if a init argument is given, that is taken as containing
length bytes.
So,
str = g_string_new_len (init, len);
is more like
str = g_string_new_len (NULL, len);
g_string_append_len (str, init, len);
and not (how I wrongly thought)
str = g_string_new_len (NULL, len);
g_string_append (str, init);
Fixes: 95b006c244
Previously, if "main.rc-manager" was set to "unmanaged"
and "/etc/resolv.conf" was symlink to our internal file
"/var/run/NetworkManager/resolv.conf", NM would not rewrite
the file, in an attempt to honor the requirement of NetworkManager
not changing resolv.conf.
No longer special case this. I think it was wrong and inconsistent.
If the user specifies rc-manager unmanaged, he also should manage
/etc/resolv.conf accordingly. And if the user decided to symlink
it to our internal file, that is fine. It should not stop NM from
updating that file.
Also, this was the only cases, where NM would not write our internal
resolv.conf (errors aside). It was inconsitent, and also not documented
behavior. Instead, it is documented that `man NetworkManager.conf`:
Regardless of this setting, NetworkManager will always write
resolv.conf to its runtime state directory
/var/run/NetworkManager/resolv.conf.
When a DNS plugin is enabled (like "main.dns=dnsmasq" or "main.dns=systemd-resolved"),
the name servers announced to the rc-manager are coerced to be 127.0.0.1
or 127.0.0.53.
Depending on the "main.rc-manager" setting, also "/etc/resolv.conf"
contains only this coerced name server to the local caching service.
The same is true for "/var/run/NetworkManager/resolv.conf" file, which
contains what we would write to "/etc/resolv.conf" (depending on
the "main.rc-manager" configuration).
Write a new file "/var/run/NetworkManager/no-stub-resolv.conf", which contains
the original name servers, uncoerced. Like "/var/run/NetworkManager/resolv.conf",
this file is always written.
The effect is, when one enables "main.dns=systemd-resolved", then there
is still a file "no-stub-resolv.conf" with the same content as with
"main.dns=default".
The no-stub-resolv.conf may be a possible solution, when a user wants
NetworkManager to update systemd-resolved, but still have a regular
/etc/resolv.conf [1]. For that, the user could configure
[main]
dns=systemd-resolved
rc-manager=unmanaged
and symlink "/etc/resolv.conf" to "/var/run/NetworkManager/no-stub-resolv.conf".
This is not necessarily the only solution for the problem and does not preclude
options for updating systemd-resolved in combination with other DNS plugins.
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/20
Before:
"manager: check_if_startup_complete returns FALSE because of eth0"
Now:
"manager: startup complete is waiting for device 'eth0' (autoactivate)"
Also, the logging line is now more a human readable sentence, but still
follows the same pattern as later
"manager: startup complete"
Meaning: grepping for "startup complete" becomes more helpful because
one first finds the reasons why startup-complete is not yet reached,
followed by the moment when it is reached.
The script didn't include all the symbols needed by plugins because
libNetworkManager.a, as built by meson, doesn't include symbols from
other static libraries that are linked in. Since we used
libNetworkManager.a to know which symbols are potentiall available
from NM, the result was an incomplete list.
Unfortunately, the only way to include the whole static library is to
create a dependency object and use 'link_whole', but this is only
available in meson >= 0.46. Since 'link_whole' is available for
executables in meson >= 0.40, create a fake executable and use that to
enumerate symbols.
Also add tests to check that plugins can be loaded correctly.
Fixes: dfa2a2b40c
char[] is not a char *; it can not go NULL.
test-ifupdown.c:147:27: error: address of array 'n->name' will always evaluate
to 'true' [-Werror,-Wpointer-bool-conversion]
g_assert (b->name && n->name);
test-ifupdown.c:156:27: error: address of array 'm->key' will always evaluate
to 'true' [-Werror,-Wpointer-bool-conversion]
g_assert (k->key && m->key);
src/devices/nm-acd-manager.c:419:31: error: variable 'info' is uninitialized
when used here [-Werror,-Wuninitialized]
nm_utils_inet4_ntop (info->address, NULL),
Also make sure the secrets request callback only send a reply to IWD and
the Connect method return callback executes the device state change to
"disconnected".
state_changed (called when IWD signalled device state change) was
supposed to not change NM device state on connect success or failure and
instead wait for the DBus Connect() method callback but it would
actually still call nm_device_emit_recheck_auto_activate on failure so
refactor state_changed and network_connect_cb to make sure
the state change and nm_device_emit_recheck_auto_activate are only
called from network_connect_cb.
This fixes a race where during a switch from one network to another NM
would immediately start a new activation after state_changed and
network_connect_cb would then handle the Connect failure and mark the
new activation as failed.
When creating the mirror 802.1x connections for IWD 802.1x profiles
set the NM_SETTING_SECRET_FLAG_NOT_SAVED flag on the secrets that
may at some point be requested from our agent. The saved secrets could
not be used anyway because of our use of
NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW in
nm_device_iwd_agent_query. But also try to respect whatever secret
caching policy has been configured in the IWD profile for those secrets,
IWD would be responsible for storing them if it was allowed in the
profile.
In two places stop using g_dbus_proxy_new_* to create whole new
interface proxy objects for net.connman.iwd.Network as this will
normally have a huge overhead compared to asking the ObjectManager
client that we already have in NMIwdManager for those proxies.
dbus-monitor shows that for each network path returned by
GetOrderedNetworks () -- and we call it every 10 or 20 seconds and may
get many dozens of networks back -- gdbus would do the following each
time:
org.freedesktop.DBus.AddMatch("member=PropertiesChanged")
org.freedesktop.DBus.StartServiceByName("net.connman.iwd")
org.freedesktop.DBus.GetNameOwner("net.connman.iwd")
org.freedesktop.DBus.Properties.GetAll("net.connman.iwd.Network")
org.freedesktop.DBus.RemoveMatch("member=PropertiesChanged")
nm-initrd-generator scans the command line for options relevant to network
configuration and creates configuration files for an early instance of
NetworkManager run from the initial ramdisk during early boot.
This is loosely based on nms-ibft-reader, but with some significant
changes. Notably, it parses /sys/firmware/ibft directly instead of
iscsiadm output.
iscsiadm is not available on early boot (perhaps it's too large) and
turns out that parsing sysfs directly is easier and more
straightforwared anyways. A win-win situation.
It is not useful alone, it's in a separate commit just for the sake of
easier review.
We need a mode that:
* doesn't leave processes behind
* doesn't force an internal dhclient
* doesn't auto-generate default connections
* doesn't write out files into libdir, only /run
The original configure-and-quit mode doesn't really fit the initrd use. But
it's proobably not a good idea to just change its behavior.
This is useful for in-memory connections to persist NetworkManager
restarts (as opposed to machine restarts).
Perhaps most improtantly, this allows generating in-memory connections outside
NetworkManager, e.g. passing configuration from early boot firmware in initrd.
Note that this does *not* aspire to do more than it says on the tin:
Notably, it doesn't touch the problem of provisioning connections in multiple
persistent connection directories and thus doesn't have to deal with the
problem of deleting or overlaying the connections tha (rh #772414) deals
with.
Especially with configure-and-quit, it's easy to encounter a condition,
where the device reached a failed state, policy decides to quit, but the
active connection is not yet torn down from the device.
Upon the next start NetworkManager would think the connection succeeded
activating.
Make them just ask for connections from GDBus, as other D-Bus clients
do. GDBus anyway reuses the connection if it has one, but allows us to
deal with errors in a more civilized manner.
Using these unormalized was wrong all along, but by chance didn't hit
paths that needed normalized connections. This may change if we
actually write in memory connections to /run with the keyfile plugin,
because that one wants them normalized.
This also saves some work, because normalization does boring things for
us, such as adding default ipv4/ipv6/proxy settings everywhere.
On networked boot we need to somehow communicate this to the early boot
machinery. Sadly, no DBus there and we're running in configure-and-quit
mode.
Abusing the state file for this sounds almost reasonable and is
reasonably straightforward thing to do.
libcurl does not allow removing easy-handles from within a curl
callback.
That was already partly avoided for one handle alone. That is, when
a handle completed inside a libcurl callback, it would only invoke the
callback, but not yet delete it. However, that is not enough, because
from within a callback another handle can be cancelled, leading to
the removal of (the other) handle and a crash:
==24572== at 0x40319AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==24572== by 0x52DDAE5: Curl_close (url.c:392)
==24572== by 0x52EC02C: curl_easy_cleanup (easy.c:825)
==24572== by 0x5FDCD2: cb_data_free (nm-connectivity.c:215)
==24572== by 0x5FF6DE: nm_connectivity_check_cancel (nm-connectivity.c:585)
==24572== by 0x55F7F9: concheck_handle_complete (nm-device.c:2601)
==24572== by 0x574C12: concheck_cb (nm-device.c:2725)
==24572== by 0x5FD887: cb_data_invoke_callback (nm-connectivity.c:167)
==24572== by 0x5FD959: easy_header_cb (nm-connectivity.c:435)
==24572== by 0x52D73CB: chop_write (sendf.c:612)
==24572== by 0x52D73CB: Curl_client_write (sendf.c:668)
==24572== by 0x52D54ED: Curl_http_readwrite_headers (http.c:3904)
==24572== by 0x52E9EA7: readwrite_data (transfer.c:548)
==24572== by 0x52E9EA7: Curl_readwrite (transfer.c:1161)
==24572== by 0x52F4193: multi_runsingle (multi.c:1915)
==24572== by 0x52F5531: multi_socket (multi.c:2607)
==24572== by 0x52F5804: curl_multi_socket_action (multi.c:2771)
Fix that, by never invoking any callbacks when we are inside a libcurl
callback. Instead, the handle is marked for completion and queued. Later,
we complete all queue handles separately.
While at it, drop the @error argument from NMConnectivityCheckCallback.
It was only used to signal cancellation. Let's instead signal that via
status NM_CONNECTIVITY_CANCELLED.
https://bugzilla.gnome.org/show_bug.cgi?id=797136https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1792745https://bugzilla.opensuse.org/show_bug.cgi?id=1107197https://github.com/NetworkManager/NetworkManager/pull/207
Fixes: d8a31794c8
As we accept addr_family %AF_UNSPEC to detect the address family,
we also need to return it. Just returning the binary address without
the address family makes no sense.
Note that NMSettingEthtool and NMSettingMatch don't have such
functions either.
We have API
nm_connection_get_setting (NMConnection *, GType)
nm_connection_get_setting_by_name (NMConnection *, const char *)
which can be used generically, meaning: the requested setting type
is an argument to the function. That is generally more useful and
flexible.
Don't add API which duplicates existing functionality and is (arguably)
inferiour. Drop it now. This is an ABI/API break for the current development
cycle where the 1.14.0 API is still unstable. Indeed it's already after
1.14-rc1, which is ugly. But it's also unlikely that somebody already uses
this API/ABI and is badly impacted by this change.
Note that nm_connection_get_setting() and nm_connection_get_setting_by_name()
are slightly inconvenient in C still, because they usually require a cast.
We should fix that by changing the return type to "void *". Such
a change may be possibly any time without breaking API/ABI (almost, it'd
be an API change when taking a function pointer without casting).
(cherry picked from commit a10156f516)
We cannot be sure who holds a reference to the proxy, and
who is gonna call us back after the VPN connection instance
is destroyed.
(cherry picked from commit 6ebb9091d2)
Got this assertion:
NetworkManager[12939]: <debug> [1536917977.4868] active-connection[0x563d8fd34540]: set state deactivated (was deactivating)
...
NetworkManager[12939]: nm-openvpn[1106] <info> openvpn[1132]: send SIGTERM
NetworkManager[12939]: nm-openvpn[1106] <info> wait for 1 openvpn processes to terminate...
NetworkManager[12939]: nm-openvpn[1106] <warn> openvpn[1132] exited with error code 1
NetworkManager[12939]: <info> [1536917977.5035] vpn-connection[0x563d8fd34540,2fdeaea3-975f-4325-8305-83ebca5eaa26,"my-openvpn-Red-Hat",0]: VPN plugin: requested secrets; state disconnected (9)
NetworkManager[12939]: plugin_interactive_secrets_required: assertion 'priv->vpn_state == STATE_CONNECT || priv->vpn_state == STATE_NEED_AUTH' failed
Meaning. We should either ensure that secrets_required_cb() signal callback
is disconnected from proxy's signal, or we gracefully handle callbacks at
unexpected moments. Do the latter.
(cherry picked from commit 92344dd084)
dhcpcd version 6, the first supporting IPv6, was released more than 5
years ago. Remove all checks on version number and IPv6 support.
(cherry picked from commit e0c49d7341)
Rename variables for the error number. Commonly the naming
is:
- errno: the error number from <errno.h> itself
- errsv: a copy of errno
- nlerr: a netlink error number
- err: an error code, but not a errno/errsv and not
a netlink error number.
(cherry picked from commit f4de941d98)
Internal DHCPv4 client requires a valid MAC address for functioning.
Just always require a MAC address to start DHCP, both v4 and v6.
We have no MAC address for example on Layer3 devices like tun or wireguard.
Also, before "0a797bdc2a systemd/dhcp: fix assertion starting DHCP
client without MAC address", if we tired to start sd_dhcp_client without
setting a MAC address, an assertion was triggered.
(cherry picked from commit e8fa75ce06)
An assertion in dhcp_network_bind_raw_socket() is triggered when
starting an sd_dhcp_client without setting setting a MAC address
first.
- sd_dhcp_client_start()
- client_start()
- client_start_delayed()
- dhcp_network_bind_raw_socket()
In that case, the arp-type and MAC address is still unset. Note that
dhcp_network_bind_raw_socket() already checks for a valid arp-type
and MAC address below, so we should just gracefully return -EINVAL.
Maybe sd_dhcp_client_start() should fail earlier when starting without
MAC address. But the failure here will be correctly propagated and
the start aborted.
See-also: https://github.com/systemd/systemd/pull/10054
(cherry picked from commit 34af574d58)
It's enough that all code paths in impl_ppp_manager_set_ifindex() log exactly
one message. Also, give all messages the same prefix, so that it's clear where
they come from.
(cherry picked from commit 2a45c32e8c)
