libnm-core: remove unsupported bond options during normalization

In an ideal world, we should not validate connections containing
options not valid for the current bond mode. However adding such
restriction now means that during an upgrade to the new NM version
some connections that were valid before become invalid, possibly
disrupting connectivity.

Instead, consider invalid options as a normalizable error and remove
them during normalization.

Converting the setting to a "canonical" form without invalid options
is important for the connection matching logic, where such invalid
options can cause false mismatches.
This commit is contained in:
Beniamino Galvani 2017-06-05 13:51:18 +02:00
parent 14529f5b46
commit f25e008e2f
3 changed files with 98 additions and 0 deletions

View file

@ -921,6 +921,36 @@ _normalize_bond_mode (NMConnection *self, GHashTable *parameters)
return FALSE;
}
static gboolean
_normalize_bond_options (NMConnection *self, GHashTable *parameters)
{
NMSettingBond *s_bond = nm_connection_get_setting_bond (self);
gboolean changed = FALSE;
const char *name, *mode_str;
NMBondMode mode;
guint32 num, i;
/* Strip away unsupported options for current mode */
if (s_bond) {
mode_str = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MODE);
mode = _nm_setting_bond_mode_from_string (mode_str);
if (mode == NM_BOND_MODE_UNKNOWN)
return FALSE;
again:
num = nm_setting_bond_get_num_options (s_bond);
for (i = 0; i < num; i++) {
if ( nm_setting_bond_get_option (s_bond, i, &name, NULL)
&& !_nm_setting_bond_option_supported (name, mode)) {
nm_setting_bond_remove_option (s_bond, name);
changed = TRUE;
goto again;
}
}
}
return changed;
}
static gboolean
_normalize_wireless_mac_address_randomization (NMConnection *self, GHashTable *parameters)
{
@ -1296,6 +1326,7 @@ nm_connection_normalize (NMConnection *connection,
was_modified |= _normalize_ethernet_link_neg (connection);
was_modified |= _normalize_infiniband_mtu (connection, parameters);
was_modified |= _normalize_bond_mode (connection, parameters);
was_modified |= _normalize_bond_options (connection, parameters);
was_modified |= _normalize_wireless_mac_address_randomization (connection, parameters);
was_modified |= _normalize_team_config (connection, parameters);
was_modified |= _normalize_team_port_config (connection, parameters);

View file

@ -557,6 +557,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
const char *arp_ip_target = NULL;
const char *lacp_rate;
const char *primary;
NMBondMode bond_mode;
g_hash_table_iter_init (&iter, priv->options);
while (g_hash_table_iter_next (&iter, (gpointer) &key, (gpointer) &value)) {
@ -791,6 +792,23 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
return NM_SETTING_VERIFY_NORMALIZABLE;
}
/* normalize unsupported options for the current mode */
bond_mode = _nm_setting_bond_mode_from_string (mode_new);
g_hash_table_iter_init (&iter, priv->options);
while (g_hash_table_iter_next (&iter, (gpointer) &key, NULL)) {
if (nm_streq (key, "mode"))
continue;
if (!_nm_setting_bond_option_supported (key, bond_mode)) {
g_set_error (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("'%s' option is not valid with mode '%s'"),
key, mode_new);
g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
return NM_SETTING_VERIFY_NORMALIZABLE;
}
}
return TRUE;
}

View file

@ -182,6 +182,54 @@ test_compare (void)
((const char *[]){ "num_unsol_na", "4", "num_grat_arp", "4", NULL }));
}
static void
test_normalize_options (const char **opts1, const char **opts2)
{
gs_unref_object NMConnection *con = NULL;
NMSettingBond *s_bond;
GError *error = NULL;
gboolean success;
const char **p;
int num = 0;
create_bond_connection (&con, &s_bond);
for (p = opts1; p[0] && p[1]; p += 2)
g_assert (nm_setting_bond_add_option (s_bond, p[0], p[1]));
nmtst_assert_connection_verifies_and_normalizable (con);
nmtst_connection_normalize (con);
success = nm_setting_verify ((NMSetting *) s_bond, con, &error);
nmtst_assert_success (success, error);
for (p = opts2; p[0] && p[1]; p += 2) {
g_assert_cmpstr (nm_setting_bond_get_option_by_name (s_bond, p[0]), ==, p[1]);
num++;
}
g_assert_cmpint (num, ==, nm_setting_bond_get_num_options (s_bond));
}
static void
test_normalize (void)
{
test_normalize_options (
((const char *[]){ "mode", "802.3ad", "ad_actor_system", "00:02:03:04:05:06", NULL }),
((const char *[]){ "mode", "802.3ad", "ad_actor_system", "00:02:03:04:05:06", NULL }));
test_normalize_options (
((const char *[]){ "mode", "1", "miimon", "1", NULL }),
((const char *[]){ "mode", "active-backup", "miimon", "1", NULL }));
test_normalize_options (
((const char *[]){ "mode", "balance-alb", "tlb_dynamic_lb", "1", NULL }),
((const char *[]){ "mode", "balance-alb", NULL }));
test_normalize_options (
((const char *[]){ "mode", "balance-tlb", "tlb_dynamic_lb", "1", NULL }),
((const char *[]){ "mode", "balance-tlb", "tlb_dynamic_lb", "1", NULL }));
test_normalize_options (
((const char *[]){ "mode", "balance-rr", "ad_actor_sys_prio", "4", "packets_per_slave", "3", NULL }),
((const char *[]){ "mode", "balance-rr", "packets_per_slave", "3", NULL }));
}
#define TPATH "/libnm/settings/bond/"
NMTST_DEFINE ();
@ -193,6 +241,7 @@ main (int argc, char **argv)
g_test_add_func (TPATH "verify", test_verify);
g_test_add_func (TPATH "compare", test_compare);
g_test_add_func (TPATH "normalize", test_normalize);
return g_test_run ();
}