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.

(cherry picked from commit f25e008e2f)

libnm-core/nm-connection.c

@@ -912,6 +912,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)
 {
@@ -1275,6 +1305,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);

libnm-core/nm-setting-bond.c

@@ -542,6 +542,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)) {
@@ -776,6 +777,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;
 }
 

libnm-core/tests/test-setting-bond.c

@@ -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 ();
 }