From 2d3094d4687c931274185d41a45a0750bfb0d7ed Mon Sep 17 00:00:00 2001
From: Antonio Cardace <acardace@redhat.com>
Date: Thu, 27 Feb 2020 17:20:09 +0100
Subject: [PATCH] nm-setting-bond: fix '[up|down]delay', 'miimon' validation

Just looking at the hashtable entry of 'updelay' and 'downdelay' options
is wrong, we have to inspect their values to check if they're
actually enabled or not.

Otherwise bond connections with valid settings will fail
when created:

$ nmcli c add type bond ifname bond99 bond.options miimon=0,updelay=0,mode=0
Error: Failed to add 'bond-bond99' connection: bond.options: 'updelay' option requires 'miimon' option to be set

Also add unit tests.

https://bugzilla.redhat.com/show_bug.cgi?id=1805184

Fixes: d595f7843e31 ('libnm: add libnm/libnm-core (part 1)')
(cherry picked from commit 50da785be14ac976f909b29766195baaa49e1934)
(cherry picked from commit 2644b0c753a04f26e99c723d89aa13a63e56ce34)
(cherry picked from commit a8846619aaace6811d21149e5ce48f6b99851ca1)
---
 libnm-core/nm-setting-bond.c    | 25 ++++++++++++++++++++-----
 libnm-core/tests/test-setting.c |  9 +++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c
index 33c01e3cdf..583e63be56 100644
--- a/libnm-core/nm-setting-bond.c
+++ b/libnm-core/nm-setting-bond.c
@@ -108,6 +108,16 @@ static const BondDefault defaults[] = {
 
 /*****************************************************************************/
 
+static int
+_atoi (const char *value)
+{
+	int v;
+
+	v = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT, -1);
+	nm_assert (v >= 0);
+	return v;
+};
+
 /**
  * nm_setting_bond_get_num_options:
  * @setting: the #NMSettingBond
@@ -663,21 +673,26 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 	}
 
 	if (miimon == 0) {
-		/* updelay and downdelay can only be used with miimon */
-		if (g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_UPDELAY)) {
+		gpointer delayopt;
+
+		/* updelay and downdelay need miimon to be enabled to be valid */
+		delayopt = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_UPDELAY);
+		if (delayopt && _atoi (delayopt) > 0) {
 			g_set_error (error,
 			             NM_CONNECTION_ERROR,
 			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
-			             _("'%s' option requires '%s' option to be set"),
+			             _("'%s' option requires '%s' option to be enabled"),
 			             NM_SETTING_BOND_OPTION_UPDELAY, NM_SETTING_BOND_OPTION_MIIMON);
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
 			return FALSE;
 		}
-		if (g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_DOWNDELAY)) {
+
+		delayopt = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_DOWNDELAY);
+		if (delayopt && _atoi (delayopt) > 0) {
 			g_set_error (error,
 			             NM_CONNECTION_ERROR,
 			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
-			             _("'%s' option requires '%s' option to be set"),
+			             _("'%s' option requires '%s' option to be enabled"),
 			             NM_SETTING_BOND_OPTION_DOWNDELAY, NM_SETTING_BOND_OPTION_MIIMON);
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
 			return FALSE;
diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c
index 5c9c614fb5..8bd5538ee6 100644
--- a/libnm-core/tests/test-setting.c
+++ b/libnm-core/tests/test-setting.c
@@ -590,6 +590,15 @@ test_bond_verify (void)
 	test_verify_options (TRUE,
 	                     "mode", "802.3ad",
 	                     "ad_actor_system", "ae:00:11:33:44:55");
+	test_verify_options (TRUE,
+	                     "mode", "0",
+	                     "miimon", "0",
+	                     "updelay", "0",
+	                     "downdelay", "0");
+	test_verify_options (TRUE,
+	                     "mode", "0",
+	                     "downdelay", "0",
+	                     "updelay", "0");
 }
 
 static void