nm-setting-bond: if unset consider bond options with default values for validation

Doing 'verify()' with options such as 'miimon' and 'num_grat_arp' set to
arbitrary values it's not consistent with what NetworkManager does to
bond options when activating the bond through 'apply_bonding_config()'
(at a later stage) because the said values do not
correspond to what the default values for those options are.

This leads to an inconsistency with the 'miimon' parameter for example,
where 'verify()' is done while assuming it's 0 if not set but its
default value is actually 100.

Fixes: 8775c25c33 ('libnm: verify bond option in defined order')
This commit is contained in:
Antonio Cardace
2020-02-20 15:38:01 +01:00
parent d482eec6b2
commit c07f3b518c
2 changed files with 30 additions and 20 deletions

View File

@@ -497,6 +497,10 @@ typedef enum {
NMBondOptionType NMBondOptionType
_nm_setting_bond_get_option_type (NMSettingBond *setting, const char *name); _nm_setting_bond_get_option_type (NMSettingBond *setting, const char *name);
const char*
bond_get_option_or_default (NMSettingBond *self, const char *option);
/*****************************************************************************/ /*****************************************************************************/
/* nm_connection_get_uuid() asserts against NULL, which is the right thing to /* nm_connection_get_uuid() asserts against NULL, which is the right thing to

View File

@@ -605,15 +605,29 @@ _nm_setting_bond_option_supported (const char *option, NMBondMode mode)
return !NM_FLAGS_ANY (_bond_option_unsupp_mode (option), BIT (mode)); return !NM_FLAGS_ANY (_bond_option_unsupp_mode (option), BIT (mode));
} }
const char*
bond_get_option_or_default (NMSettingBond *self,
const char *option)
{
const char *value;
value = g_hash_table_lookup (NM_SETTING_BOND_GET_PRIVATE (self)->options, option);
if (!value)
return nm_setting_bond_get_option_default (self, option);
return value;
}
static gboolean static gboolean
verify (NMSetting *setting, NMConnection *connection, GError **error) verify (NMSetting *setting, NMConnection *connection, GError **error)
{ {
NMSettingBond *self = NM_SETTING_BOND (setting);
NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting); NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting);
int mode; int mode;
int miimon = 0; int miimon;
int arp_interval = 0; int arp_interval;
int num_grat_arp = -1; int num_grat_arp;
int num_unsol_na = -1; int num_unsol_na;
const char *mode_orig; const char *mode_orig;
const char *mode_new; const char *mode_new;
const char *arp_ip_target = NULL; const char *arp_ip_target = NULL;
@@ -622,7 +636,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
NMBondMode bond_mode; NMBondMode bond_mode;
guint i; guint i;
const NMUtilsNamedValue *n; const NMUtilsNamedValue *n;
const char *value;
_ensure_options_idx_cache (priv); _ensure_options_idx_cache (priv);
@@ -643,18 +656,10 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
} }
} }
value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_MIIMON); miimon = _atoi (bond_get_option_or_default (self, NM_SETTING_BOND_OPTION_MIIMON));
if (value) arp_interval = _atoi (bond_get_option_or_default (self, NM_SETTING_BOND_OPTION_ARP_INTERVAL));
miimon = atoi (value); num_grat_arp = _atoi (bond_get_option_or_default (self, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP));
value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_ARP_INTERVAL); num_unsol_na = _atoi (bond_get_option_or_default (self, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA));
if (value)
arp_interval = atoi (value);
value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP);
if (value)
num_grat_arp = atoi (value);
value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA);
if (value)
num_unsol_na = atoi (value);
/* Can only set one of miimon and arp_interval */ /* Can only set one of miimon and arp_interval */
if (miimon > 0 && arp_interval > 0) { if (miimon > 0 && arp_interval > 0) {
@@ -685,7 +690,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY, NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("'%s' is not a valid value for '%s'"), _("'%s' is not a valid value for '%s'"),
value, NM_SETTING_BOND_OPTION_MODE); mode_orig, NM_SETTING_BOND_OPTION_MODE);
g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS); g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
return FALSE; return FALSE;
} }
@@ -840,8 +845,9 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
return FALSE; return FALSE;
} }
if ( (num_grat_arp != -1 && num_unsol_na != -1) if ( g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP)
&& (num_grat_arp != num_unsol_na)) { && g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA)
&& num_grat_arp != num_unsol_na) {
g_set_error (error, g_set_error (error,
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY, NM_CONNECTION_ERROR_INVALID_PROPERTY,