From cb125986922b6e0bd33d68306f1602b19ea1a3de Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Jul 2020 09:16:51 +0200 Subject: [PATCH 1/1] bond: avoid setting "active_slave" option without interface enslaved Kernel will reject setting "active_slave", if the interface is not enslaved or not up. We already handle that by setting the option whenever we enslave an interface. However, we also must not set it initially, otherwise we get an ugly error log message: NetworkManager[939]: [1594709143.7459] platform-linux: sysctl: setting net:/sys/class/net/bond99/bonding/active_slave to eth1 (current value is ) NetworkManager[939]: [1594709143.7459] platform-linux: sysctl: failed to set bonding/active_slave to eth1: (22) Invalid argument NetworkManager[939]: [1594709143.7460] device (bond99): failed to set bonding attribute active_slave to eth1 ... kernel: bond99: (slave eth1): Device is not bonding slave kernel: bond99: option active_slave: invalid value (eth1) See-also: https://bugzilla.redhat.com/show_bug.cgi?id=1856640 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/577 (cherry picked from commit f0a39b517e06470c4b176ef0bbd49c2eef1d7ad7) (cherry picked from commit e48c908e8cb5e0b426a4bf8d99608f73cd7890e4) --- src/devices/nm-device-bond.c | 43 +++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 2fedc753f350..e36eba61b0b2 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -8,6 +8,7 @@ #include "nm-device-bond.h" #include +#include #include "NetworkManagerUtils.h" #include "nm-device-private.h" @@ -261,6 +262,40 @@ set_bond_attr_or_default (NMDevice *device, _set_bond_attr (device, opt, value); } +static void +set_bond_attr_active_slave (NMDevice *device, NMSettingBond *s_bond) +{ + NMDeviceBond *self = NM_DEVICE_BOND (device); + const NMPlatformLink *plink; + const char *value; + const char *error_reason; + int ifindex; + + value = nm_setting_bond_get_option_or_default (s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE); + if (!value) + return; + + if (!nm_str_is_empty (value)) { + ifindex = nm_device_get_ifindex (device); + plink = nm_platform_link_get_by_ifname (nm_device_get_platform (device), value); + if (!plink) + error_reason = "does not exist"; + else if (plink->master != ifindex) + error_reason = "is not yet enslaved"; + else if (!NM_FLAGS_HAS (plink->n_ifi_flags, IFF_UP)) + error_reason = "is not up"; + else + error_reason = NULL; + + if (error_reason) { + _LOGT (LOGD_BOND, "bond option 'active_slave' not set as device \"%s\" %s", value, error_reason); + return; + } + } + + _set_bond_attr (device, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE, value); +} + static gboolean apply_bonding_config (NMDeviceBond *self) { @@ -300,7 +335,7 @@ apply_bonding_config (NMDeviceBond *self) nm_setting_bond_get_option_or_default (s_bond, NM_SETTING_BOND_OPTION_ARP_IP_TARGET)); set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM); - set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE); + set_bond_attr_active_slave (device, s_bond); set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO); set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_SELECT); set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY); @@ -378,7 +413,7 @@ enslave_slave (NMDevice *device, if (nm_streq0 (active, nm_device_get_iface (slave))) { nm_platform_sysctl_master_set_option (nm_device_get_platform (device), nm_device_get_ifindex (device), - "active_slave", + NM_SETTING_BOND_OPTION_ACTIVE_SLAVE, active); _LOGD (LOGD_BOND, "setting slave %s as active one for master %s", active, nm_device_get_iface (device)); @@ -577,10 +612,8 @@ reapply_connection (NMDevice *device, NMConnection *con_old, NMConnection *con_n mode = _nm_setting_bond_mode_from_string (value); g_return_if_fail (mode != NM_BOND_MODE_UNKNOWN); - /* Primary */ set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_PRIMARY); - /* Active slave */ - set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE); + set_bond_attr_active_slave (device, s_bond); } /*****************************************************************************/ -- 2.26.2