colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libnm,core: deprecate "active_slave" and alias it for "primary"

Browse Source 
Setting "active_slave" fails unless the slave is currently present and
IFF_UP. That complicates the code, because we cannot set the property
at any time, but only under the right circumstances.

But really, "active_slave" option is something for debugging. It's not
an option which should be set by NetworkManager. The right option
instead is "primary", which will tell kernel to make the slave active,
when it is ready.

Deprecate the "active_slave" option and make it an alias for "primary".

https://bugzilla.redhat.com/show_bug.cgi?id=1856640
This commit is contained in:
Thomas Haller
2020-09-09 12:40:58 +02:00
parent 22ff4bfd18
commit 58da09439a
2 changed files with 24 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
libnm-core/nm-setting-bond.c
View File

@@ -111,9 +111,9 @@ _nm_assert_bond_meta (const OptionMeta *option_meta)
}));
return TRUE;
case NM_BOND_OPTION_TYPE_IP:
case NM_BOND_OPTION_TYPE_IFNAME:
nm_assert (option_meta->val);
/* fall-through */
case NM_BOND_OPTION_TYPE_IFNAME:
case NM_BOND_OPTION_TYPE_MAC:
nm_assert (!option_meta->list);
nm_assert (option_meta->min == 0);
@@ -151,7 +151,7 @@ NM_UTILS_STRING_TABLE_LOOKUP_STRUCT_DEFINE (
}
},
{ return NULL; },
{ NM_SETTING_BOND_OPTION_ACTIVE_SLAVE, { "", NM_BOND_OPTION_TYPE_IFNAME } },
{ NM_SETTING_BOND_OPTION_ACTIVE_SLAVE, { NULL, NM_BOND_OPTION_TYPE_IFNAME } },
{ NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO, { "65535", NM_BOND_OPTION_TYPE_INT, 1, 65535 } },
{ NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM, { NULL, NM_BOND_OPTION_TYPE_MAC } },
{ NM_SETTING_BOND_OPTION_AD_SELECT, { "stable", NM_BOND_OPTION_TYPE_BOTH, 0, 2, _option_default_strv_ad_select } },
@@ -295,6 +295,17 @@ _bond_get_option_normalized (NMSettingBond* self,
value = _bond_get_option (self, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP);
if (!value)
value = _bond_get_option (self, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA);
} else if (NM_IN_STRSET (option,
NM_SETTING_BOND_OPTION_ACTIVE_SLAVE)) {
/* "active_slave" is deprecated, and an alias for "primary". The property
* itself always normalizes to %NULL. */
value = NULL;
} else if (NM_IN_STRSET (option,
NM_SETTING_BOND_OPTION_PRIMARY)) {
/* "active_slave" is deprecated, and an alias for "primary". */
value = _bond_get_option (self, NM_SETTING_BOND_OPTION_PRIMARY);
if (!value)
value = _bond_get_option (self, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
} else
value = _bond_get_option (self, option);

74
src/devices/nm-device-bond.c
View File

@@ -181,12 +181,15 @@ update_connection (NMDevice *device, NMConnection *connection)
}
/* Read bond options from sysfs and update the Bond setting to match */
options = nm_setting_bond_get_valid_options (s_bond);
options = nm_setting_bond_get_valid_options (NULL);
for (; options[0]; options++) {
const char *option = options[0];
gs_free char *value = NULL;
char *p;
if (NM_IN_STRSET (option, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE))
continue;
value = nm_platform_sysctl_master_get_option (nm_device_get_platform (device),
ifindex,
option);
@@ -317,40 +320,6 @@ 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 void
set_bond_attrs_or_default (NMDevice *device, NMSettingBond *s_bond, const char *const *attr_v)
{
@@ -398,7 +367,6 @@ apply_bonding_config (NMDeviceBond *self)
set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_MODE);
set_bond_arp_ip_targets (device, s_bond);
set_bond_attr_active_slave (device, s_bond);
set_bond_attrs_or_default (device,
s_bond,
@@ -434,45 +402,26 @@ enslave_slave (NMDevice *device,
gboolean configure)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
gboolean success = TRUE;
const char *slave_iface = nm_device_get_ip_iface (slave);
NMConnection *master_con;
nm_device_master_check_slave_physical_port (device, slave, LOGD_BOND);
if (configure) {
gboolean success;
nm_device_take_down (slave, TRUE);
success = nm_platform_link_enslave (nm_device_get_platform (device),
nm_device_get_ip_ifindex (device),
nm_device_get_ip_ifindex (slave));
nm_device_bring_up (slave, TRUE, NULL);
if (!success)
if (!success) {
_LOGI (LOGD_BOND, "enslaved bond slave %s: failed", nm_device_get_ip_iface (slave));
return FALSE;
_LOGI (LOGD_BOND, "enslaved bond slave %s", slave_iface);
/* The active_slave option can be set only after the interface is enslaved */
master_con = nm_device_get_applied_connection (device);
if (master_con) {
NMSettingBond *s_bond = nm_connection_get_setting_bond (master_con);
const char *active;
if (s_bond) {
active = nm_setting_bond_get_option_or_default (s_bond,
NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
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),
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));
}
}
}
_LOGI (LOGD_BOND, "enslaved bond slave %s", nm_device_get_ip_iface (slave));
} else
_LOGI (LOGD_BOND, "bond slave %s was enslaved", slave_iface);
_LOGI (LOGD_BOND, "bond slave %s was enslaved", nm_device_get_ip_iface (slave));
return TRUE;
}
@@ -642,7 +591,6 @@ reapply_connection (NMDevice *device, NMConnection *con_old, NMConnection *con_n
/* Below we set only the bond options that kernel allows to modify
* while keeping the bond interface up */
set_bond_attr_active_slave (device, s_bond);
set_bond_arp_ip_targets (device, s_bond);
set_bond_attrs_or_default (device,