Scenario:
- have ethernet connection as unmanaged.
- create (or have) a suitable profile for the connection.
- make the device as managed. No default wired connection gets
created.
- delete the profile.
Note that NMManager does in manager_device_state_changed():
»···if (NM_IN_SET (new_state,
»··· NM_DEVICE_STATE_UNAVAILABLE,
»··· NM_DEVICE_STATE_DISCONNECTED))
»···»···nm_settings_device_added (priv->settings, device);
that means, when the device the next time goes through
UNAVAILABLE/DISCONNECTED states, we will suddenly create the
default "Wired connection 1" profile.
That doesn't seem right. When a device is suitable to have a
default-wired connection, we should only check once whether to
create it. We should not retry that later. The !no-auto-default
mechanism exists so we can start NetworkManager without a profile for
the device. It doesn't mean that we later one (after previously deciding
not to create a profile), we still create it.
https://bugzilla.redhat.com/show_bug.cgi?id=1687937https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/450
$ meson -Dmore_asserts=0 meson-build
$ ninja -C meson-build
[712/859] Compiling C object 'src/initrd/b383957@@nmi-core@sta/nmi-cmdline-reader.c.o'.
../src/initrd/nmi-cmdline-reader.c: In function ‘nmi_cmdline_reader_parse’:
../src/initrd/nmi-cmdline-reader.c:871:4: warning: ‘s_ip’ may be used uninitialized in this function [-Wmaybe-uninitialized]
871 | nm_setting_ip_config_add_dns (s_ip, ns);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/initrd/nmi-cmdline-reader.c:835:21: note: ‘s_ip’ was declared here
835 | NMSettingIPConfig *s_ip;
| ^~~~
Fixes: 25a2b6e14f ('initrd: rework command line parsing')
When starting with a locked modem, it may take some time for the user to
enter the PIN code, leading to the secrets request timing out. In that
case, we want the connection activation to be retried automatically once
the modem is unlocked, which can't be achieved if we propagate the error,
as the device will change state to 'failed'.
This patch ignores the 'no-secrets' error, as it means either the
request has timed out, or the user cancelled the request without
notifying NetworkManager. By doing this, we allow the connection to be
re-activated once the modem is unlocked.
Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
When the modem is unlocked externally to NetworkManager, it is kept in
the 'need-auth' state. The current connection activation continues as
if nothing had changed, and the secrets request for the PIN code (which
is no longer necessary) eventually times out. The device state is then
changed to 'failed', meaning there won't be a new try at activating the
default connection automatically.
In order to prevent this, and retry activating the default connection
when the modem gets unlocked, we change the device state to
'deactivating' when we identify the modem has been unlocked externally.
Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
The 'default_connection' created by the command line parser has
multiple purposes. It's the connection created for 'ip=' arguments
without command line, but is also created when there is a 'bootdev='
or for 'nameserver=' and no other connection exists at the moment the
argument is parsed. This is confusing and leads to a result that
depends on the order of parameters. For example:
$ /usr/libexec/nm-initrd-generator -c connections -- bootdev=eth1 ip=eth0:dhcp
$ ls connections/
default_connection.nmconnection eth0.nmconnection
$ /usr/libexec/nm-initrd-generator -c connections -- ip=eth0:dhcp bootdev=eth1
$ ls connections/
eth0.nmconnection eth1.nmconnection
Make this more explicit by tracking 'bootdev_connection' and
'default_connection' individually.
Also fix handling of 'nameserver', 'rd.peerdns' and 'rd.route'
arguments. First process all connections, and then set those
properties. In particular, now nameservers are applied to all
connections.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/391
Connections are kept in a hash table indexed by name. This causes non
deterministic output in get_conn() when we have to decide a default
connection and no bootdev was specified on the command line.
Also add an array that stores the original order in which interfaces
appear in the command line, and use it when we have to loop through
connections. The return value of nmi_cmdline_reader_parse() is still a
hash table because once we have generated connections, their order
doesn't matter.
Don't add an empty connection to the list if
nmi_ibft_update_connection_from_nic() fails when reading iBFT
information.
If the function fails in parse_ip(), continue with the existing
connection built from other command line options.
Also, fix a memory leak.
If we change the the MTU of an ovs interface only through netlink, the
change could be overridden by ovs-vswitchd at any time when other
interfaces change. Set the MTU also in the ovsdb to prevent such
changes.
Note that if the MTU comes from the connection, we already set the
ovsdb MTU at creation time and so this other update becomes
useless. But it is needed when changing the MTU at runtime (reapply)
or when the MTU comes from a different source (e.g. DHCP).
The ovs-vswitchd.conf.db(5) man page says about the the mtu_request
column in the Interface table:
"Requested MTU (Maximum Transmission Unit) for the interface. A
client can fill this column to change the MTU of an
interface [...] If this is not set and if the interface has
internal type, Open vSwitch will change the MTU to match the
minimum of the other interfaces in the bridge."
Therefore, if the connection specifies a MTU, set it early when adding
the interface to the ovsdb so that it will not be changed to the
minimum of other interfaces.
When we must synchronize IPv6 addresses, we compare the order of
addresses to set with what is currently set on platform. Starting from
addresses with lower priority, when a mismatch is found we remove it
from platform and also remove all following addresses, so that we can
re-add them in the right order.
Since kernel keeps addresses internally sorted by scope, we should
consider each scope separately in order to avoid unnecessary address
deletions. For example, if we want to configure addresses
fe80::1/64,2000::1/64 and we currently have on platform 2000::1/64,
it's not necessary to remove the existing address; we can just add the
link-local one.
Co-authored-by: Thomas Haller <thaller@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1814557
NMTST_SWAP() used memcpy() for copying the value, while NM_SWAP() uses
a temporary variable with typeof(). I think the latter is preferable.
Also, the macro is essentially doing the same thing.
nm_str_hash() uses siphash24. We want to use that everywhere.
The only concern would be that it is slower. But if that's the concern,
then we would have many more performance critical places to care
about first.
Our glib based code should also include our static utility library
libnm-glib-aux. This is our basic utility library that we want to
have around everywhere. Since we link statically, the linker will weed
out the unused stuff at compile time. So, there is no overhead, except
for the things that we actually use.
It's unnecessary and makes the function unnecessarily not thread safe.
Of course, also ndp_msg_opt_route_prefix() uses static variables, so
it's still not thread safe.
Fixes: c3a4656a68 ('rdisc: libndp implementation')
We should handle kernel command line like systemd does, with its
ConditionKernelCommandLine= setting.
For example, it tokenizes words between various white space characters,
not only space. Use nm_utils_strsplit_set_full() for that.
Note that we currently don't yet have a tokenizer that supports
quotation, like systemd does. We should extend
nm_utils_strsplit_set_full() for that.
NM_ASCII_SPACES contains the ASCII characters according to
g_ascii_isspace().
Add NM_ASCII_WHITESPACES which differs from NM_ASCII_SPACES by not
including "\f". In some cases, that character shall be excluded.
For example, this is what systemd uses as "WHITESPACE" define at
various places.
Also, reorder the spaces string so that plain space comes first. It is
expected that ' ' is much more frequently than newlines or tabs. While
the order here shouldn't matter, it seems preferably to order frequent
characters in front.
Otherwise, we just see "returncode: -11", which isn't very clear.
By default, our test nmcli invocations should never exit by a signal.
Usually, when we encounter a signal, it indicates to a bug and a crash
during the test.
Print more information about the exit code.
returncode: -11 (SIGNAL SIGSEGV)
g_clear_pointer() would always cast the destroy notify function
pointer to GDestroyNotify. That means, it lost some type safety, like
GPtrArray *ptr_arr = ...
g_clear_pointer (&ptr_arr, g_array_unref);
Since glib 2.58 ([1]), g_clear_pointer() is also more type safe. But
this is not used by NetworkManager, because we don't set
GLIB_VERSION_MIN_REQUIRED to 2.58.
[1] f9a9902aac
We have nm_clear_pointer() to avoid this issue for a long time (pre
1.12.0). Possibly we should redefine in our source tree g_clear_pointer()
as nm_clear_pointer(). However, I don't like to patch glib functions
with our own variant. Arguably, we do patch g_clear_error() in
such a manner. But there the point is to make the function inlinable.
Also, nm_clear_pointer() returns a boolean that indicates whether
anything was cleared. That is sometimes useful. I think we should
just consistently use nm_clear_pointer() instead, which does always
the preferable thing.
Replace:
sed 's/\<g_clear_pointer *(\([^;]*\), *\([a-z_A-Z0-9]\+\) *)/nm_clear_pointer (\1, \2)/g' $(git grep -l g_clear_pointer) -i
I think it's preferable to use nm_clear_g_free() instead of
g_clear_pointer(, g_free). The reasons are not very strong,
but I think it is overall preferable to have a shorthand for this
frequently used functionality.
sed 's/\<g_clear_pointer *(\([^;]*\), *\(g_free\) *)/nm_clear_g_free (\1)/g' $(git grep -l g_clear_pointer) -i
