From 8c6f8c65955d18ca9b43ad2bcd1bccf2cd85e7ed Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Mon, 5 Jun 2017 13:51:18 +0200
Subject: [PATCH 1/2] 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)
(cherry picked from commit ac7a5c074c72310d8328fb448824d29bbec932f3)
---
libnm-core/nm-connection.c | 31 +++++++++++++++++++++++
libnm-core/nm-setting-bond.c | 18 +++++++++++++
libnm-core/tests/test-setting-bond.c | 49 ++++++++++++++++++++++++++++++++++++
3 files changed, 98 insertions(+)
diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index ecfb978..c1b7506 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 9a8bdc3..b62964c 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 91a8199..e6a65bb 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 ();
}
--
2.9.3
From 0d96d249ffe95e0232b8247e6bb9c1385a2b4940 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Mon, 5 Jun 2017 14:48:08 +0200
Subject: [PATCH 2/2] bond: add only supported options to the generated
connection
Upstream commit [1] changed in the kernel the default value of
tlb_dynamic_lb bond from 1 to 0 when the mode is not tlb. This is not
wrong, as the option value doesn't really matter for other modes, but
it breaks the connection matching because we read back a 0 value when
we expect a default of 1.
Fix this in a generic way by ignoring altogether options that are not
relevant for the current bond mode, because they are removed from the
connection during normalization.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8b426dc54cf4056984bab7dfa48c92ee79a46434
https://bugzilla.redhat.com/show_bug.cgi?id=1457909
(cherry picked from commit 056a973a4fdb68abe8bc7bfc5f31250345d71f21)
(cherry picked from commit 61817661c899844ddb364ebd529716f574146588)
---
src/devices/nm-device-bond.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c
index 3325c94..c8748fe 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++;
}
}
--
2.9.3