bond: merge branch 'bg/bond-option-normalize-rh1457909'

Normalize bond options to ensure a correct connection matching upon
daemon restart.

https://bugzilla.redhat.com/show_bug.cgi?id=1457909
(cherry picked from commit 4be0cdd358a9253e874eaaf9e244339d8fc6aa33)

4 files changed, 106 insertions, 2 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 ();
 }
diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c
index 3325c9485e..c8748fe8f2 100644
--- a/src/devices/nm-device-bond.c
+++ b/src/devices/nm-device-bond.c
@@ -155,6 +155,7 @@ update_connection (NMDevice *device, NMConnection *connection)
 {
 	NMSettingBond *s_bond = nm_connection_get_setting_bond (connection);
 	int ifindex = nm_device_get_ifindex (device);
+	NMBondMode mode = NM_BOND_MODE_UNKNOWN;
 	const char **options;
 
 	if (!s_bond) {
@@ -164,7 +165,7 @@ update_connection (NMDevice *device, NMConnection *connection)
 
 	/* Read bond options from sysfs and update the Bond setting to match */
 	options = nm_setting_bond_get_valid_options (s_bond);
-	while (options && *options) {
+	for (; *options; options++) {
 		gs_free char *value = nm_platform_sysctl_master_get_option (nm_device_get_platform (device), ifindex, *options);
 		const char *defvalue = nm_setting_bond_get_option_default (s_bond, *options);
 		char *p;
@@ -176,6 +177,12 @@ update_connection (NMDevice *device, NMConnection *connection)
 				*p = '\0';
 		}
 
+		if (nm_streq (*options, NM_SETTING_BOND_OPTION_MODE))
+			mode = _nm_setting_bond_mode_from_string (value);
+
+		if (!_nm_setting_bond_option_supported (*options, mode))
+			continue;
+
 		if (   value
 		    && value[0]
 		    && !ignore_if_zero (*options, value)
@@ -190,7 +197,6 @@ update_connection (NMDevice *device, NMConnection *connection)
 
 			nm_setting_bond_add_option (s_bond, *options, value);
 		}
-		options++;
 	}
 }