A similar patch was done on master, but here the situation is different.
I feel we should not allow for the possibility where we invoke an event
that might mess with the source id. In practice there was no problem.
But it feels cleaner to clear it first.
Fixes: 843d696e46 ('dhcp: clean source on dispatch failure')
(cherry picked from commit 0549351111)
Fix the following warning:
NetworkManager[1524461]: Source ID 3844 was not found when attempting to remove it
g_logv (log_domain=0x7f2816fa676e "GLib", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7ffe697374d0) at gmessages.c:1391
g_log (log_domain=log_domain@entry=0x7f2816fa676e "GLib", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7f2816fae240 "Source ID %u was not found when attempting to remove it") at gmessages.c:1432
g_source_remove (tag=519) at gmain.c:2352
nm_clear_g_source (id=<optimized out>) at ./shared/nm-glib-aux/nm-macros-internal.h:1198
dispose (object=0x55f7289b1ca0) at src/dhcp/nm-dhcp-nettools.c:1433
g_object_unref (_object=<optimized out>) at gobject.c:3303
g_object_unref (_object=0x55f7289b1ca0) at gobject.c:3232
dhcp4_cleanup (self=self@entry=0x55f728af3b20, cleanup_type=cleanup_type@entry=CLEANUP_TYPE_DECONFIGURE, release=release@entry=0) at src/devices/nm-device.c:7565
...
Fixes: 45521b1b38 ('dhcp: nettools: move to failed state if event dispatch fails')
(cherry picked from commit 843d696e46)
Fail the enslavement of the ovs port if the bridge device is not
found, instead of generating assertions and potentially crash later.
https://bugzilla.redhat.com/show_bug.cgi?id=1797696
Fixes: 101e65d2bb ('ovs: allow changing mac address of bridges and interfaces')
(cherry picked from commit c5c49995b1)
The previous code tried to get the bridge active connection and it
used the port active connection instead in case of failure. This
doesn't seem right, as in nm-ovsdb.c the bridge AC is used to get the
bridge settings (including the uuid, interface name, and cloned mac).
In case of failure getting the bridge AC we should just fail.
Fixes: 830a5a14cb ('device: add support for OpenVSwitch devices')
(cherry picked from commit c8b5a3f91a)
Surisingly, the compiler may detect the remaining obj_type in
the default switch. Then, inlining nmp_class_from_type() it may detect
that this is only possible to hit with an out or range access to
_nmp_classes array.
Rework the code to avoid that compiler warning. It's either way not
supposed to happen.
Also, drop the default switch case and explicitly list the enum values.
Otherwise it is error prone to forget a switch case.
(cherry picked from commit 9848589fbf)
When AddConnection() or Update() terminate, the (unrealized) virtual
device should be already be available, otherwise an activation attempt
of that connection can fail.
https://bugzilla.redhat.com/show_bug.cgi?id=1804350
This reverts commit c163207b07.
(cherry picked from commit efc04b1285)
We need to reset the OVS_PORT and OVS_PORT_UUID variables.
Otherwise, clearing the slave type doesn't work.
On master this is solved differently, by automatically clearing all
variables that are not explicitly set.
Reproducer:
nmcli con del t-eth1
nmcli con add type ethernet autoconnect no ifname eth1 master port0 con-name t-eth1 slave-type ovs-port
echo "
remove ovs-interface
remove connection.master
remove connection.slave-type
print
save
quit
" | nmcli c edit t-eth1
nmcli con show t-eth1 | grep 'ovs\|slave-type'
Fixes: 1440fe6a88 ('ifcfg: don't forget master of ovs interfaces')
https://bugzilla.redhat.com/show_bug.cgi?id=1804167
When the ovs interface gets deactivated, it is released from the
master port and we call nm_device_update_from_platform_link (dev,
NULL) to ignore any later event for the interface. This is important
especially because it sets a zero ifindex on the interface and so,
later when the link disappears, we don't unmanage the device but
directly remove it.
However, since ovs commands are queued, the link could appear during
the deactivation and we need to ignore such events. Add a new device
method can_update_from_platform_link() for such purpose.
(cherry picked from commit e9fc1dea43)
Tracking the deletion of link by ifindex is difficult because the
ifindex of the device is updated through delayed (idle) calls in
NMDevice and so there is the possibility that at a certain time the
device ifindex is not in sync with platform state. It seems simpler to
watch instead the interface name. The ugly thing is that the interface
name can be changed externally, but if users do that on an activating
device they are looking for trouble.
Also change the deactivate code to deal with the scenario where we
already created the interface in the ovsdb but the link didn't show up
yet. To ensure a proper cleanup we must wait that the link appears and
then goes away; however the link may never appear if vswitchd sees
only the last state in ovsdb, and so we must use a ugly timeout to
avoid waiting forever.
https://bugzilla.redhat.com/show_bug.cgi?id=1787989
(cherry picked from commit 9c49f8a879)
nm_utils_is_valid_iface_name() is a public API of libnm-core, let's use
our internal API.
$ sed -i 's/\<nm_utils_is_valid_iface_name\>/nm_utils_ifname_valid_kernel/g' $(git grep -l nm_utils_is_valid_iface_name)
(cherry picked from commit 6e9a36ab9f)
so that it gets compiled out in production builds, this check is
carried out anyway when the connection is created.
(cherry picked from commit 9e27252c27)
We will add a property NM_NDISC_RA_TIMEOUT for which this name is better
suited. The problem is really that our convention for object properties
and signals defines have no prefix to indicate whether it's a property
or a signal.
Rename.
(cherry picked from commit 10f0253f2e)
It is undefined behavior and can lead to crashes or memory corruption.
In practice, this only had an issue on Big Endian systems.
Fixes: fdbf4ae5e6 ('ifcfg-rh: add IPV4_DHCP_TIMEOUT key for ipv4.dhcp-timeout property')
(cherry picked from commit 9b82d29f5f)
If a function is only called once, it may not help to simplify the code
but make it more complicated. It would only simplify the code, if it
had a clear, distinct purpose. That isn't the case here. Also, the
IPv4 writer doesn't have such a function either. Drop and inline it.
(cherry picked from commit d06092acbd)
It feels wrong to schedule a timeout with G_MAXUINT32, if we actually
disabled the timeout. Of course, in practice there should be little
difference.
(cherry picked from commit 4c101f36ec)
This is C, we have almost no IDE support. And ctags/cscope is much more
helpful if we use unique names.
Don't use the get_dhcp_timeout() name, because that is already used in
"src/devices/nm-device.c" already. Rename.
(cherry picked from commit be4129bb2d)
If the GetConnectionUnixProcessID() call fails, the process that
registered on the bus has died and we should ignore the name
appearance event.
(cherry picked from commit e94d76382c)
GCC 10 complains about accesses to elements of zero-length arrays that
overlap other members of the same object:
src/platform/nm-platform-utils.c: In function ‘nmp_utils_ethtool_get_permanent_address’:
src/platform/nm-platform-utils.c:854:29: error: array subscript 0 is outside the bounds of an interior zero-length array ‘__u8[0]’ {aka ‘unsigned char[0]’} [-Werror=zero-length-bounds]
854 | if (NM_IN_SET (edata.e.data[0], 0, 0xFF)) {
./shared/nm-glib-aux/nm-macros-internal.h:731:20: note: in definition of macro ‘_NM_IN_SET_EVAL_N’
Fix this warning.
(cherry picked from commit d892a35395)
GCC 10 complains about accesses to elements of zero-length arrays that
overlap other members of the same object:
src/platform/nm-platform-utils.c: In function ‘ethtool_get_stringset’:
src/platform/nm-platform-utils.c:355:27: error: array subscript 0 is outside the bounds of an interior zero-length array ‘__u32[0]’ {aka ‘unsigned int[0]’} [-Werror=zero-length-bounds]
355 | len = sset_info.info.data[0];
| ~~~~~~~~~~~~~~~~~~~^~~
In file included from src/platform/nm-platform-utils.c:12:
/usr/include/linux/ethtool.h:647:8: note: while referencing ‘data’
647 | __u32 data[0];
| ^~~~
Fix this warning.
(cherry picked from commit 16e1e44c5e)
I think this solution is not right, because "char buf" is not guaranteed
to have the correct alignment. Revert, and solve it differently.
This reverts commit 6345a66153.
(cherry picked from commit 1fd7e45139)
When we deactivate a virtual device, we usually schedule the deletion
of the link in an idle handler. That action will be executed at a
later time when the device is already in the disconnected state.
Similarly, for ovs interfaces we send the deletion command to the
ovsdb and then proceed to the disconnected state.
However, in the first case there is the guarantee that the link will
be deleted at some point, while for ovs interfaces it may happen that
ovs decides to reuse the same link if there is an addition
queued. Since reusing the same link confuses NM, let's implement
deactivate_async() for ovs-interfaces and wait that the link actually
goes away before proceeding.
https://bugzilla.redhat.com/show_bug.cgi?id=1782701https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/402
(cherry picked from commit 623a1e1f99)
GCC 10 complains about accesses to elements of zero-length arrays that
overlap other members of the same object:
src/platform/nm-platform-utils.c: In function ‘nmp_utils_ethtool_get_permanent_address’:
src/platform/nm-platform-utils.c:854:29: error: array subscript 0 is outside the bounds of an interior zero-length array ‘__u8[0]’ {aka ‘unsigned char[0]’} [-Werror=zero-length-bounds]
854 | if (NM_IN_SET (edata.e.data[0], 0, 0xFF)) {
./shared/nm-glib-aux/nm-macros-internal.h:731:20: note: in definition of macro ‘_NM_IN_SET_EVAL_N’
Fix this warning.
(cherry picked from commit 5076fc0ca0)
GCC 10 complains about accesses to elements of zero-length arrays that
overlap other members of the same object:
src/platform/nm-platform-utils.c: In function ‘ethtool_get_stringset’:
src/platform/nm-platform-utils.c:355:27: error: array subscript 0 is outside the bounds of an interior zero-length array ‘__u32[0]’ {aka ‘unsigned int[0]’} [-Werror=zero-length-bounds]
355 | len = sset_info.info.data[0];
| ~~~~~~~~~~~~~~~~~~~^~~
In file included from src/platform/nm-platform-utils.c:12:
/usr/include/linux/ethtool.h:647:8: note: while referencing ‘data’
647 | __u32 data[0];
| ^~~~
Fix this warning.
(cherry picked from commit 6345a66153)
curl_multi_setopt() accepts CURLMOPT_* options, not CURLOPT_*
ones. Found by GCC 10:
clients/cloud-setup/nm-http-client.c:700:38: error: implicit conversion from ‘enum <anonymous>’ to ‘CURLMoption’ [-Werror=enum-conversion]
700 | curl_multi_setopt (priv->mhandle, CURLOPT_VERBOSE, 1);
Fixes: 69f048bf0c ('cloud-setup: add tool for automatic IP configuration in cloud')
(cherry picked from commit c11ac34f4c)
As it is possible to configure an arbitrarily large DHCP timeout, it
should be possible to also set a large timeout for IPv6
autoconfiguration. Currently the timeout can only be changed via
sysctl. Leave the lower bound because the default kernel sysctl value
is 3 * 4 = 12 seconds and so without the lower limit the default
timeout would change from 30 to 12 seconds for every user, which seems
a big change and could possibly break users' setup.
https://bugzilla.redhat.com/show_bug.cgi?id=1795957
(cherry picked from commit d8e1f4c8ef)
If the current lease expires, we start the grace period in which the
clients starts again from the INIT DHCP state (i.e. sending DISCOVER
messages). If it is able to obtain a new lease, it must be accepted or
otherwise the client will not renew it.
(cherry picked from commit df75c21b4d)
Currently the DHCP client reports the BOUND state not only when the
lease is obtained initially but also when it is renewed. Having a
different state for the renewal will be used by NMDevice in the next
patch to determine whether the lease needs to be accept()ed or not.
(cherry picked from commit a4ddb56923)
Currently the duration of the DHCP grace period (in which we try to
acquire a new lease after expiration) is hardcoded to 480
seconds. That value seems arbitrary and too long for the default
configuration. Since we already have a property that allows the user
to configure how long NM should try to get the lease initially, it
makes sense to use it also for retries after lease expirations.
In particular, setting the ipvx.dhcp-timeout to a high value extends
also the grace period to a very long time, potentially forever.
(cherry picked from commit aee78ca788)
The signal is unused (and should be removed).
Still, the parameter passed to g_signal_emit() is a C string, not a
GVariant. I think as there are no subscribers, glib wouldn't actually
do anything with the arguments. Though, I am not sure whether glib still
tries to initialize a GValue with a GVariant type, leading to a crash.
Fixes: f05b7a78c9 ('supplicant: Track P2P Group information, creation and destruction')
(cherry picked from commit c106008091)
Don't realize a virtual device if the master is missing because in
such case the autoactivation can't start and a stale link will be
created.
(cherry picked from commit 336bfcabc4)
Add a 'in-state-change' pending action to be sure the device always has a
pending when transitioning between states (this prevents callbacks to mark
startup as complete while running _set_state_full()).
This is needed as during the 'failed'->'disconnected' the pending action 'activation-*'
for the device is removed resulting in an empty pending_actions list which then
triggers 'check_if_startup_complete()' that will find no pending action and mark
startup as complete even if the device could have been activated with another connection.
https://bugzilla.redhat.com/show_bug.cgi?id=1759956
(cherry picked from commit f583aec806)
The option is mandatory in the replies from server and so we don't
need to ask for it. dhclient doesn't do it either. But especially, it
seems that requesting the option causes some broken server
implementations to send duplicate instances of the option.
So, remove the option from the parameter request list of the internal
nettools and systemd DHCP implementation.
(cherry picked from commit 541db78259)
Do process the connections from the iBFT block if the rd.iscsi.ibft or
rd.iscsi.ibft=1 argument is present.
This is supposed to fix what was originally reported by Kairui Song
<kasong@redhat.com> here: https://github.com/dracutdevs/dracut/pull/697
(cherry picked from commit 39e1e723de)
If an argument in form ip=eth0:ibft is specified, we'd first create a
wired connection with con.interface-name and then proceed completing it
from the iBFT block. At that point we also add the MAC address, so the
interface-name is no longer necessary..
Worse even, for VLAN connections, it results in an attempt to create
a VLAN with the same name as the parent wired device. Ooops.
Let's just drop it. MAC address is guarranteed to be there and does the
right thing for both plain wired devices as well as VLANs.
(cherry picked from commit 59ead70952)
