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

bond: improve compatibility check of options and modes

Browse Source 
We print an error when the write of a bond options fails as this is
considered an effect of a wrong configuration (or a bug in the checks
done by NM) that the user should notice. But not all options are
supported in all bonding modes and so we ignore some unsupported
options for the current mode to avoid populating logs with useless
errors.

Improve the code there by using a more generic approach and
synchronize the mode/option compatibility table with kernel (file
drivers/net/bonding/bond_options.c).

https://bugzilla.gnome.org/show_bug.cgi?id=767776
https://bugzilla.redhat.com/show_bug.cgi?id=1352131
This commit is contained in:
Beniamino Galvani
2016-07-04 16:25:39 +02:00
parent de2ce68a9f
commit dd1c453ff7
3 changed files with 133 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
libnm-core/nm-core-internal.h
View File

@@ -307,6 +307,22 @@ _nm_setting_bond_get_option_type (NMSettingBond *setting, const char *name);
/***********************************************************/
typedef enum {
NM_BOND_MODE_UNKNOWN = 0,
NM_BOND_MODE_ROUNDROBIN,
NM_BOND_MODE_ACTIVEBACKUP,
NM_BOND_MODE_XOR,
NM_BOND_MODE_BROADCAST,
NM_BOND_MODE_8023AD,
NM_BOND_MODE_TLB,
NM_BOND_MODE_ALB,
} NMBondMode;
NMBondMode _nm_setting_bond_mode_from_string (const char *str);
gboolean _nm_setting_bond_option_supported (const char *option, NMBondMode mode);
/***********************************************************/
gboolean _nm_utils_inet6_is_token (const struct in6_addr *in6addr);
#endif

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

@@ -463,6 +463,60 @@ _nm_setting_bond_get_option_type (NMSettingBond *setting, const char *name)
g_assert_not_reached ();
}
NMBondMode
_nm_setting_bond_mode_from_string (const char *str)
{
g_return_val_if_fail (str, NM_BOND_MODE_UNKNOWN);
if (nm_streq (str, "balance-rr"))
return NM_BOND_MODE_ROUNDROBIN;
if (nm_streq (str, "active-backup"))
return NM_BOND_MODE_ACTIVEBACKUP;
if (nm_streq (str, "balance-xor"))
return NM_BOND_MODE_XOR;
if (nm_streq (str, "broadcast"))
return NM_BOND_MODE_BROADCAST;
if (nm_streq (str, "802.3ad"))
return NM_BOND_MODE_8023AD;
if (nm_streq (str, "balance-tlb"))
return NM_BOND_MODE_TLB;
if (nm_streq (str, "balance-alb"))
return NM_BOND_MODE_ALB;
return NM_BOND_MODE_UNKNOWN;
}
#define BIT(x) (1 << (x))
const static struct {
const char *option;
NMBondMode unsupp_modes;
} bond_unsupp_modes[] = {
{ NM_SETTING_BOND_OPTION_PACKETS_PER_SLAVE, ~(BIT (NM_BOND_MODE_ROUNDROBIN)) },
{ NM_SETTING_BOND_OPTION_ARP_VALIDATE, BIT (NM_BOND_MODE_8023AD) | BIT (NM_BOND_MODE_TLB) | BIT (NM_BOND_MODE_ALB) },
{ NM_SETTING_BOND_OPTION_ARP_INTERVAL, BIT (NM_BOND_MODE_8023AD) | BIT (NM_BOND_MODE_TLB) | BIT (NM_BOND_MODE_ALB) },
{ NM_SETTING_BOND_OPTION_LACP_RATE, ~(BIT (NM_BOND_MODE_8023AD)) },
{ NM_SETTING_BOND_OPTION_PRIMARY, ~(BIT (NM_BOND_MODE_ACTIVEBACKUP) | BIT (NM_BOND_MODE_TLB) | BIT (NM_BOND_MODE_ALB)) },
{ NM_SETTING_BOND_OPTION_ACTIVE_SLAVE, ~(BIT (NM_BOND_MODE_ACTIVEBACKUP) | BIT (NM_BOND_MODE_TLB) | BIT (NM_BOND_MODE_ALB)) },
{ NM_SETTING_BOND_OPTION_TLB_DYNAMIC_LB, ~(BIT (NM_BOND_MODE_TLB)) },
{ NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO, ~(BIT (NM_BOND_MODE_8023AD)) },
{ NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM, ~(BIT (NM_BOND_MODE_8023AD)) },
{ NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY, ~(BIT (NM_BOND_MODE_8023AD)) },
};
gboolean
_nm_setting_bond_option_supported (const char *option, NMBondMode mode)
{
guint i;
for (i = 0; i < G_N_ELEMENTS (bond_unsupp_modes); i++) {
if (nm_streq (option, bond_unsupp_modes[i].option))
return !NM_FLAGS_HAS (bond_unsupp_modes[i].unsupp_modes, BIT (mode));
}
return TRUE;
}
static gboolean
verify (NMSetting *setting, NMConnection *connection, GError **error)
{

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

@@ -118,12 +118,15 @@ complete_connection (NMDevice *device,
/******************************************************************/
static gboolean
set_bond_attr (NMDevice *device, const char *attr, const char *value)
set_bond_attr (NMDevice *device, NMBondMode mode, const char *attr, const char *value)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
gboolean ret;
int ifindex = nm_device_get_ifindex (device);
if (!_nm_setting_bond_option_supported (attr, mode))
return FALSE;
ret = nm_platform_sysctl_master_set_option (NM_PLATFORM_GET, ifindex, attr, value);
if (!ret)
_LOGW (LOGD_HW, "failed to set bonding attribute '%s' to '%s'", attr, value);
@@ -201,6 +204,7 @@ master_update_slave_connection (NMDevice *self,
static void
set_arp_targets (NMDevice *device,
NMBondMode mode,
const char *value,
const char *delim,
const char *prefix)
@@ -214,7 +218,7 @@ set_arp_targets (NMDevice *device,
for (iter = items; iter && *iter; iter++) {
if (*iter[0]) {
tmp = g_strdup_printf ("%s%s", prefix, *iter);
set_bond_attr (device, "arp_ip_target", tmp);
set_bond_attr (device, mode, "arp_ip_target", tmp);
g_free (tmp);
}
}
@@ -223,6 +227,7 @@ set_arp_targets (NMDevice *device,
static void
set_simple_option (NMDevice *device,
NMBondMode mode,
const char *attr,
NMSettingBond *s_bond,
const char *opt)
@@ -232,18 +237,20 @@ set_simple_option (NMDevice *device,
value = nm_setting_bond_get_option_by_name (s_bond, opt);
if (!value)
value = nm_setting_bond_get_option_default (s_bond, opt);
set_bond_attr (device, attr, value);
set_bond_attr (device, mode, attr, value);
}
static NMActStageReturn
apply_bonding_config (NMDevice *device)
{
NMDeviceBond *self = NM_DEVICE_BOND (device);
NMConnection *connection;
NMSettingBond *s_bond;
int ifindex = nm_device_get_ifindex (device);
const char *mode, *value;
const char *mode_str, *value;
char *contents;
gboolean set_arp_interval = TRUE;
NMBondMode mode;
/* Option restrictions:
*
@@ -263,98 +270,84 @@ apply_bonding_config (NMDevice *device)
s_bond = nm_connection_get_setting_bond (connection);
g_assert (s_bond);
mode = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MODE);
if (mode == NULL)
mode = "balance-rr";
mode_str = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MODE);
if (!mode_str)
mode_str = "balance-rr";
mode = _nm_setting_bond_mode_from_string (mode_str);
if (mode == NM_BOND_MODE_UNKNOWN) {
_LOGW (LOGD_BOND, "unknown bond mode '%s'", mode_str);
return NM_ACT_STAGE_RETURN_FAILURE;
}
/* Set mode first, as some other options (e.g. arp_interval) are valid
* only for certain modes.
*/
set_bond_attr (device, mode, "mode", mode_str);
value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MIIMON);
if (value && atoi (value)) {
/* clear arp interval */
set_bond_attr (device, "arp_interval", "0");
set_bond_attr (device, mode, "arp_interval", "0");
set_arp_interval = FALSE;
set_bond_attr (device, "miimon", value);
set_simple_option (device, "updelay", s_bond, NM_SETTING_BOND_OPTION_UPDELAY);
set_simple_option (device, "downdelay", s_bond, NM_SETTING_BOND_OPTION_DOWNDELAY);
set_bond_attr (device, mode, "miimon", value);
set_simple_option (device, mode, "updelay", s_bond, NM_SETTING_BOND_OPTION_UPDELAY);
set_simple_option (device, mode, "downdelay", s_bond, NM_SETTING_BOND_OPTION_DOWNDELAY);
} else if (!value) {
/* If not given, and arp_interval is not given, default to 100 */
long int val_int;
char *end;
/* If not given, and arp_interval is not given or disabled, default to 100 */
value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_INTERVAL);
errno = 0;
val_int = strtol (value ? value : "0", &end, 10);
if (!value || (val_int == 0 && errno == 0 && *end == '\0'))
set_bond_attr (device, "miimon", "100");
if (_nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXUINT32, 0) == 0)
set_bond_attr (device, mode, "miimon", "100");
}
/* The stuff after 'mode' requires the given mode or doesn't care */
set_bond_attr (device, "mode", mode);
/* arp_interval not compatible with ALB, TLB */
if (NM_IN_STRSET (mode, "balance-alb", "balance-tlb"))
set_arp_interval = FALSE;
if (set_arp_interval) {
set_simple_option (device, "arp_interval", s_bond, NM_SETTING_BOND_OPTION_ARP_INTERVAL);
set_simple_option (device, mode, "arp_interval", s_bond, NM_SETTING_BOND_OPTION_ARP_INTERVAL);
/* Just let miimon get cleared automatically; even setting miimon to
* 0 (disabled) clears arp_interval.
*/
}
/* ARP validate: value > 0 only valid in active-backup mode */
value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_VALIDATE);
/* arp_validate > 0 only valid in active-backup mode */
if ( value
&& !nm_streq (value, "0")
&& !nm_streq (value, "none")
&& nm_streq (mode, "active-backup"))
set_bond_attr (device, "arp_validate", value);
&& mode == NM_BOND_MODE_ACTIVEBACKUP)
set_bond_attr (device, mode, "arp_validate", value);
else
set_bond_attr (device, "arp_validate", "0");
set_bond_attr (device, mode, "arp_validate", "0");
if (NM_IN_STRSET (mode, "active-backup", "balance-alb", "balance-tlb")) {
value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_PRIMARY);
set_bond_attr (device, "primary", value ? value : "");
set_simple_option (device, "lp_interval", s_bond, NM_SETTING_BOND_OPTION_LP_INTERVAL);
}
/* Primary */
value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_PRIMARY);
set_bond_attr (device, mode, "primary", value ? value : "");
/* Clear ARP targets */
/* ARP targets: clear and initialize the list */
contents = nm_platform_sysctl_master_get_option (NM_PLATFORM_GET, ifindex, "arp_ip_target");
set_arp_targets (device, contents, " \n", "-");
set_arp_targets (device, mode, contents, " \n", "-");
value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_IP_TARGET);
set_arp_targets (device, mode, value, ",", "+");
g_free (contents);
/* Add new ARP targets */
value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_IP_TARGET);
set_arp_targets (device, value, ",", "+");
set_simple_option (device, "primary_reselect", s_bond, NM_SETTING_BOND_OPTION_PRIMARY_RESELECT);
set_simple_option (device, "fail_over_mac", s_bond, NM_SETTING_BOND_OPTION_FAIL_OVER_MAC);
set_simple_option (device, "use_carrier", s_bond, NM_SETTING_BOND_OPTION_USE_CARRIER);
set_simple_option (device, "ad_select", s_bond, NM_SETTING_BOND_OPTION_AD_SELECT);
set_simple_option (device, "xmit_hash_policy", s_bond, NM_SETTING_BOND_OPTION_XMIT_HASH_POLICY);
set_simple_option (device, "resend_igmp", s_bond, NM_SETTING_BOND_OPTION_RESEND_IGMP);
set_simple_option (device, "active_slave", s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
set_simple_option (device, "all_slaves_active", s_bond, NM_SETTING_BOND_OPTION_ALL_SLAVES_ACTIVE);
set_simple_option (device, "num_grat_arp", s_bond, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP);
set_simple_option (device, "num_unsol_na", s_bond, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA);
if (nm_streq (mode, "802.3ad")) {
set_simple_option (device, "lacp_rate", s_bond, NM_SETTING_BOND_OPTION_LACP_RATE);
set_simple_option (device, "ad_actor_sys_prio", s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO);
set_simple_option (device, "ad_actor_system", s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM);
set_simple_option (device, "ad_user_port_key", s_bond, NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY);
set_simple_option (device, "min_links", s_bond, NM_SETTING_BOND_OPTION_MIN_LINKS);
}
if (nm_streq (mode, "active-backup"))
set_simple_option (device, "arp_all_targets", s_bond, NM_SETTING_BOND_OPTION_ARP_ALL_TARGETS);
if (nm_streq (mode, "balance-rr"))
set_simple_option (device, "packets_per_slave", s_bond, NM_SETTING_BOND_OPTION_PACKETS_PER_SLAVE);
if (nm_streq (mode, "balance-tlb"))
set_simple_option (device, "tlb_dynamic_lb", s_bond, NM_SETTING_BOND_OPTION_TLB_DYNAMIC_LB);
set_simple_option (device, mode, "primary_reselect", s_bond, NM_SETTING_BOND_OPTION_PRIMARY_RESELECT);
set_simple_option (device, mode, "fail_over_mac", s_bond, NM_SETTING_BOND_OPTION_FAIL_OVER_MAC);
set_simple_option (device, mode, "use_carrier", s_bond, NM_SETTING_BOND_OPTION_USE_CARRIER);
set_simple_option (device, mode, "ad_select", s_bond, NM_SETTING_BOND_OPTION_AD_SELECT);
set_simple_option (device, mode, "xmit_hash_policy", s_bond, NM_SETTING_BOND_OPTION_XMIT_HASH_POLICY);
set_simple_option (device, mode, "resend_igmp", s_bond, NM_SETTING_BOND_OPTION_RESEND_IGMP);
set_simple_option (device, mode, "active_slave", s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
set_simple_option (device, mode, "all_slaves_active", s_bond, NM_SETTING_BOND_OPTION_ALL_SLAVES_ACTIVE);
set_simple_option (device, mode, "num_grat_arp", s_bond, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP);
set_simple_option (device, mode, "num_unsol_na", s_bond, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA);
set_simple_option (device, mode, "lacp_rate", s_bond, NM_SETTING_BOND_OPTION_LACP_RATE);
set_simple_option (device, mode, "ad_actor_sys_prio", s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO);
set_simple_option (device, mode, "ad_actor_system", s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM);
set_simple_option (device, mode, "ad_user_port_key", s_bond, NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY);
set_simple_option (device, mode, "min_links", s_bond, NM_SETTING_BOND_OPTION_MIN_LINKS);
set_simple_option (device, mode, "arp_all_targets", s_bond, NM_SETTING_BOND_OPTION_ARP_ALL_TARGETS);
set_simple_option (device, mode, "packets_per_slave", s_bond, NM_SETTING_BOND_OPTION_PACKETS_PER_SLAVE);
set_simple_option (device, mode, "tlb_dynamic_lb", s_bond, NM_SETTING_BOND_OPTION_TLB_DYNAMIC_LB);
set_simple_option (device, mode, "lp_interval", s_bond, NM_SETTING_BOND_OPTION_LP_INTERVAL);
return NM_ACT_STAGE_RETURN_SUCCESS;
}