Sometimes these function may set errno to unexpected values like EAGAIN.
This causes confusion. Avoid that by using our own wrappers that retry
in that case. For example, in rhbz#1797915 we have failures like:
errno = 0;
v = g_ascii_strtoll ("10", 0, &end);
if (errno != 0)
g_assert_not_reached ();
as g_ascii_strtoll() would return 10, but also set errno to EAGAIN.
Work around that by using wrapper functions that retry. This certainly
should be fixed in glib (or glibc), but the issues are severe enough to
warrant a workaround.
Note that our workarounds are very defensive. We only retry 2 times, if
we get an unexpected errno value. This is in the hope to recover from
a spurious EAGAIN. It won't recover from other errors.
https://bugzilla.redhat.com/show_bug.cgi?id=1797915
Be more graceful and allow whitespaces around the floating point number
for DEVTIMEOUT. Note that _nm_utils_ascii_str_to_int64() is already graceful
against whitespace, so also be it with the g_ascii_strtod() code path.
g_ascii_strtoull() returns a guint64, which is very wrong to directly pass
to the variadic argument list of g_object_set(). We expect a guint there
and need to cast.
While at it, use _nm_utils_ascii_str_to_int64() to parse and validate the input.
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.
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.
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
Move local variables to inner scope.
Also, drop code comment that doesn't give additional information
beyond what is already plainly visible in source code.
This pull requests sets the OWE flag for an open network advertising an
OWE enabled transition BSSID. This way, hostapd will automatically
connect to the OWE secured BSSID advertised in the transition mode
information element.
Signed-off-by: David Bauer <mail@david-bauer.net>
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/442
When we receive a "InterfaceRemoved" signal, we will end up calling
set_state_down(). That emits a "state" change signal, which causes
NMDeviceWifi to unref the supplicant interface. This may already
give up the last reference, and we cleanup the supplicant state
(by again calling set_state_down()). When we return, set_state_down()
will crash because it operates on an already destroyed instance.
Avoid that by keeping a reference to the interface during set_state_down().
Fixes: b83f07916a ('supplicant: large rework of wpa_supplicant handling')
https://bugzilla.redhat.com/show_bug.cgi?id=1815058
Since ifcfg-rh doesn't write out to file the 'connection.timestamp' property
let's add it before comparing an updated connection with the plugin's reread
one otherwise the comparison operation would always fail.
The fix is not necessary for the keyfile plugin, because the reader/writer
correctly reads/writes the connection timestamp.
For many purposes, the supplicant features are not very interesting (as
they are also mostly static for a certain release/distribution). Combine
the multiple logging lines into one.
Also, sort the NMSupplCapType enum values consistently with the order
in which we log them.
Also, rename the logging output for features to match the enum name.
E.g. "FAST" instead of "EAP-FAST".
Now:
> supplicant: supported features: AP+ PMF+ FILS- P2P+ FT+ SHA384+ MESH+ FAST+ WFD+
When a device gets a prefix delegation, we call
nm_device_use_ip6_subnet() for all other devices that have IPv6
sharing enabled, which changes the current IPv6 configuration and
notifies NMPolicy. When updating the DNS configuration in NMPolicy, we
should notify all devices except the one that triggered the change.
https://bugzilla.redhat.com/show_bug.cgi?id=1488030
Configuration stages like act_stage2_config() can postpone progressing
to the next stage. Currently, when the condition that we wait for gets
satisfied, the code schedules the next stage from there.
I think that is wrong, because when we postpone from act_stage2_config(),
follow up steps of stage2 get skipped. Thus, when we are ready to progress,
the class should enter stage 2 again.
This requires that stage2 becomes reentrant and that the code reenters the
same stage.
We usually want to schedule stage2 when we just completed with the previous
stage (or, if we are currently in stage2, and want to re-enter it).
In those cases, the conditions are often right to just proceed right away.
No need to schedule the stage on an idle handler. Allow to invoke stage2
right away.
There was only API to schedule the stage on an idle handler.
Sometimes, we are just in the right situation to schedule the stage
right away. It should be possibly to avoid going through the extra hop.
For now, none of the caller makes use of this. So, there isn't any
actual change in behavior. But by adding this possibility, we may do
use in the future.
