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 f25e008e2fe655bbddbb8a66612a9d141e982049)

3 files changed, 98 insertions, 0 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index ecfb9780be..c1b75068cf 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -913,6 +913,36 @@ _normalize_bond_mode (NMConnection *self, GHashTable *parameters)
 }
 
 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)
 {
 	NMSettingWireless *s_wifi = nm_connection_get_setting_wireless (self);
@@ -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);
diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c
index 9a8bdc3702..b62964c721 100644
--- a/libnm-core/nm-setting-bond.c
+++ b/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;
 }
 
diff --git a/libnm-core/tests/test-setting-bond.c b/libnm-core/tests/test-setting-bond.c
index 91a8199719..e6a65bba64 100644
--- a/libnm-core/tests/test-setting-bond.c
+++ b/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 ();
 }