Commit graph

12539 commits

Author SHA1 Message Date
Thomas Haller
01fda25250 all: unify spelling of "fall-through" comment for switch statements
We used "/* fall through */" and "/* fall-through */" inconsistently.
Rename to use only one variant.

(cherry picked from commit ffa098edae)
2020-02-22 10:50:07 +01:00
Thomas Haller
02ba767496 device: allow setting "arp_validate" with supported bond modes
arp_validate is allowed for several bonding modes, at least since commit [1].

The validation was too strict. Just use set_bond_attr() directly, that
already correctly encodes whether to set the value or not.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=13ac34a8866e31b31db6237c73aa558aff84d765

(cherry picked from commit d9d51dd42d)
2020-02-19 17:30:15 +01:00
Beniamino Galvani
987c1d282a Revert "core: create virtual device on settings changes in idle handler"
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)
2020-02-19 10:06:21 +01:00
Thomas Haller
0c8046574e ifcfg: fix writer when changing OVS slave to clear previous settings
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
2020-02-18 18:37:45 +01:00
Beniamino Galvani
c4eb0c6852 ovs: discard link updates when deactivating
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)
2020-02-17 19:08:21 +01:00
Beniamino Galvani
2e5e409bf2 ovs: rework asynchronous deactivation of ovs interfaces
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)
2020-02-17 19:08:20 +01:00
Antonio Cardace
c3b0f1d4f5 all: use nm_utils_ifname_valid_kernel() instead of nm_utils_is_valid_iface_name()
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)
2020-02-17 17:28:14 +01:00
Antonio Cardace
816aacc4c2 nm-device-factory: remove ifname check as it prevents activating OVS connections
(cherry picked from commit 0cac094c93)
2020-02-17 17:25:52 +01:00
Antonio Cardace
e52fccb0d0 nm-dhcp-client: use nm_assert() to check ifname
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)
2020-02-17 17:25:52 +01:00
Thomas Haller
f259b4b6c0 ndisc: implement "ipv6.ra-timeout" property
(cherry picked from commit 9dde86d02c)
2020-02-17 15:06:10 +01:00
Thomas Haller
1f4c8a0f54 ndisc: rename NM_NDISC_RA_TIMEOUT signal to NM_NDISC_RA_TIMEOUT_SIGNAL
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)
2020-02-17 15:06:10 +01:00
Thomas Haller
7c86bc9c97 ndisc: improve logging message with nm_ndisc_start() and minor cleanup
(cherry picked from commit b2e30b3c9a)
2020-02-17 15:06:10 +01:00
Thomas Haller
c1b640da2d libnm,cli,ifcfg-rh: add ipv6.ra-timeout configuration option
(cherry picked from commit de4aeb34e3)
2020-02-17 15:06:10 +01:00
Thomas Haller
60ef3a3e59 ifcfg-rh: belatedly add support for "ipv6.dhcp-timeout" setting
(cherry picked from commit 843c546b84)
2020-02-17 15:06:10 +01:00
Thomas Haller
fe6c3f0867 ifcfg-rh: fix potential crash with variadic argument make_ip6_setting()
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)
2020-02-17 15:06:10 +01:00
Thomas Haller
7751f853c3 ifcfg-rh: inline unnecessary function write_ip6_setting_dhcp_hostname()
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)
2020-02-17 15:06:10 +01:00
Thomas Haller
2ed1a6803e device: don't schedule grace timeout if dhcp-timeout is infinity
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)
2020-02-17 15:06:10 +01:00
Thomas Haller
bd02523b6b device/trivial: rename get_dhcp_timeout() virtual function to have a unique name
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)
2020-02-17 15:06:10 +01:00
Beniamino Galvani
9a1bbef1b4 dhcp: nettools: move to failed state if event dispatch fails
(cherry picked from commit 45521b1b38)
2020-02-11 09:28:27 +01:00
Beniamino Galvani
a0c209b653 team: ignore bus name appearance when killing teamd
If we are currently killing teamd, we are not interested in knowing
when it becomes ready.

(cherry picked from commit 554e9be5b0)
2020-02-11 09:21:35 +01:00
Beniamino Galvani
77635adb50 team: ignore bus name appearance if we can't determine process id
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)
2020-02-11 09:21:33 +01:00
Thomas Haller
c1417087c8 platform: fix GCC warning about zero-length array in nmp_utils_ethtool_get_permanent_address()
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)
2020-02-10 13:34:33 +01:00
Thomas Haller
286bb2f029 platform: fix GCC warning about zero-length array in ethtool_get_stringset()
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)
2020-02-10 13:34:33 +01:00
Thomas Haller
273fcf51bd Revert "platform: fix GCC warning about zero-lenght array (2)"
This reverts commit 5076fc0ca0.

(cherry picked from commit 0931c4f2ea)
2020-02-10 13:34:33 +01:00
Thomas Haller
f140e21689 Revert "platform: fix GCC warning about zero-lenght array (1)"
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)
2020-02-10 13:34:33 +01:00
Beniamino Galvani
a1b0edd24b ovs: wait that link disappears before continuing with deactivation
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=1782701
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/402
(cherry picked from commit 623a1e1f99)
2020-02-05 22:32:52 +01:00
Beniamino Galvani
c02b0181cf platform: fix GCC warning about zero-lenght array (2)
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)
2020-02-03 15:31:30 +01:00
Beniamino Galvani
a7c1b324fd platform: fix GCC warning about zero-lenght array (1)
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)
2020-02-03 15:31:28 +01:00
Beniamino Galvani
7ba2040caa all: remove wrong CURL option initialization
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)
2020-02-03 15:31:18 +01:00
Beniamino Galvani
d2354387d9 ndisc: remove upper bound for IPv6 autoconfiguration timeout
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)
2020-01-30 15:43:18 +01:00
Beniamino Galvani
6c1daa4fa9 device: accept new leases in NM_DEVICE_IP_STATE_DONE
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)
2020-01-30 15:43:16 +01:00
Beniamino Galvani
5182ca8782 dhcp: introduce EXTENDED state for renewals
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)
2020-01-30 15:42:58 +01:00
Beniamino Galvani
4505c25270 dhcp: derive the grace period duration from the timeout property
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)
2020-01-30 15:42:56 +01:00
Beniamino Galvani
4b32506f5e device: change return type of get_dhcp_timeout()
(cherry picked from commit d0caad7a8d)
2020-01-30 15:42:55 +01:00
Beniamino Galvani
66d90dc382 device: move code around
(cherry picked from commit 70f1ee0ffb)
2020-01-30 15:42:54 +01:00
Thomas Haller
26d6ac5385 supplicant: fix memory corruption with wrong argument to NM_SUPPLICANT_INTERFACE_GROUP_FORMATION_FAILURE signal
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)
2020-01-30 11:44:10 +01:00
Beniamino Galvani
82f1129005 manager: check for master existence before realizing the device
If we find out that no compatible master connection exists, we
shouldn't realize the slave in the first place.

(cherry picked from commit ab57b575a0)
2020-01-29 16:47:36 +01:00
Beniamino Galvani
51d7941b0e manager: skip activation of a virtual device if master is missing
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)
2020-01-29 16:47:34 +01:00
Antonio Cardace
11c989bb04 nm-device: add new pending action to keep the device busy when in between states
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)
2020-01-29 12:02:52 +01:00
Beniamino Galvani
6f66ff845f dhcp: don't add server-id option to the parameter request list
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)
2020-01-25 11:43:53 +01:00
Thomas Haller
86b2858019 initrd/cmdline: minor style cleanups
(cherry picked from commit 9f95b797f1)
2020-01-14 16:45:41 +01:00
Lubomir Rintel
9fddb395d5 initrd/cmdline: obey rd.iscsi.ibft
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)
2020-01-14 16:45:41 +01:00
Lubomir Rintel
7ffac26a49 initrd/ibft-reader: don't set con.interface-name in iBFT connections
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)
2020-01-14 16:45:41 +01:00
Beniamino Galvani
92b382e2f8 dhcp: nettools: handle 'retracted' event as 'expired'
The 'retracted' event is emitted when the client receives a NAK in the
rebooting, requesting, renewing or rebinding state, while 'expired'
means that the client wasn't able to renew the lease before expiry.

In both cases the old lease is no longer valid and n-dhcp4 keep trying
to get a lease, so the two events should be handlded in the same way.

Note that the systemd client doesn't have a 'retracted' event and
considers all NAKs as 'expired' events.

(cherry picked from commit 2523000b36)
2020-01-09 09:47:57 +01:00
Thomas Haller
568c19f07d device/wifi: fix memleak parsing SSID arguments for "RequestScan"
Oddly enough, valgrind was not complaining about this leak...

Fixes: 87b2d783b6 ('core: accept 'ssids':aay option in RequestScan() dictionary parameter')
(cherry picked from commit 5ed1edc02a)
2020-01-08 12:16:11 +01:00
Antonio Cardace
6d58932875 platform/linux: add support for /31 prefixes on IPv4 point-to-point links
Previously NetworkManager would wrongly add a broadcast address for the
network prefix that would collide with the IP address of the host on
the other end of the point-to-point link thus exhausting the IP address
space of the /31 network and preventing communication between the two
nodes.

Configuring a /31 address before this commit:
	IP addr -> 10.0.0.0/31, broadcast addr -> 10.0.0.1

If 10.0.0.1 is configured as a broadcast address the communication
with host 10.0.0.1 will not be able to take place.

Configuring a /31 address after this commit:
	IP addr -> 10.0.0.0/31, no broadcast address

Thus 10.0.0.0/31 and 10.0.0.1/31 are able to correctly communicate.

See RFC-3021. https://tools.ietf.org/html/rfc3021

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/295

https://bugzilla.redhat.com/show_bug.cgi?id=1764986
(cherry picked from commit fa144b5ae9)
2020-01-07 17:15:25 +01:00
Thomas Haller
4b1053da91 core: add and indicate NM_CAPABILITY_OVS capability on D-Bus
https://bugzilla.redhat.com/show_bug.cgi?id=1785147
(cherry picked from commit 785da51d83)
2019-12-24 14:37:46 +01:00
Beniamino Galvani
46ad3aa4f3 dhcp: nettools: start from init-reboot phase when reusing address
If we know the address used previously, also tell the client to start
from the init-reboot phase, so that it will start with a DHCP request
instead of a discover.

(cherry picked from commit 6af6f70d81)
2019-12-23 16:45:44 +01:00
Beniamino Galvani
bee319bf94 dhcp: nettools: fix parsing of classless routes option
Fixes: 6adade6f21 ('dhcp: add nettools dhcp4 client')
(cherry picked from commit dd3114deb0)
2019-12-23 16:45:43 +01:00
Beniamino Galvani
09597c105e dhcp: test parsing of domain-search option
Add a test for the parsing of the the domain-search option.

(cherry picked from commit c9fbdf3cb0)
2019-12-23 16:45:41 +01:00