From cb125986922b6e0bd33d68306f1602b19ea1a3de Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 14 Jul 2020 09:16:51 +0200
Subject: [PATCH 1/1] bond: avoid setting "active_slave" option without
 interface enslaved

Kernel will reject setting "active_slave", if the interface is not enslaved or not
up. We already handle that by setting the option whenever we enslave an interface.
However, we also must not set it initially, otherwise we get an ugly error log message:

    NetworkManager[939]: <debug> [1594709143.7459] platform-linux: sysctl: setting net:/sys/class/net/bond99/bonding/active_slave to eth1 (current value is )
    NetworkManager[939]: <error> [1594709143.7459] platform-linux: sysctl: failed to set bonding/active_slave to eth1: (22) Invalid argument
    NetworkManager[939]: <warn>  [1594709143.7460] device (bond99): failed to set bonding attribute active_slave to eth1
    ...
    kernel: bond99: (slave eth1): Device is not bonding slave
    kernel: bond99: option active_slave: invalid value (eth1)

See-also: https://bugzilla.redhat.com/show_bug.cgi?id=1856640

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/577
(cherry picked from commit f0a39b517e06470c4b176ef0bbd49c2eef1d7ad7)
(cherry picked from commit e48c908e8cb5e0b426a4bf8d99608f73cd7890e4)
---
 src/devices/nm-device-bond.c | 43 +++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c
index 2fedc753f350..e36eba61b0b2 100644
--- a/src/devices/nm-device-bond.c
+++ b/src/devices/nm-device-bond.c
@@ -8,6 +8,7 @@
 #include "nm-device-bond.h"
 
 #include <stdlib.h>
+#include <net/if.h>
 
 #include "NetworkManagerUtils.h"
 #include "nm-device-private.h"
@@ -261,6 +262,40 @@ set_bond_attr_or_default (NMDevice *device,
 	_set_bond_attr (device, opt, value);
 }
 
+static void
+set_bond_attr_active_slave (NMDevice *device, NMSettingBond *s_bond)
+{
+	NMDeviceBond *self = NM_DEVICE_BOND (device);
+	const NMPlatformLink *plink;
+	const char *value;
+	const char *error_reason;
+	int ifindex;
+
+	value = nm_setting_bond_get_option_or_default (s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
+	if (!value)
+		return;
+
+	if (!nm_str_is_empty (value)) {
+		ifindex = nm_device_get_ifindex (device);
+		plink = nm_platform_link_get_by_ifname (nm_device_get_platform (device), value);
+		if (!plink)
+			error_reason = "does not exist";
+		else if (plink->master != ifindex)
+			error_reason = "is not yet enslaved";
+		else if (!NM_FLAGS_HAS (plink->n_ifi_flags, IFF_UP))
+			error_reason = "is not up";
+		else
+			error_reason = NULL;
+
+		if (error_reason) {
+			_LOGT (LOGD_BOND, "bond option 'active_slave' not set as device \"%s\" %s", value, error_reason);
+			return;
+		}
+	}
+
+	_set_bond_attr (device, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE, value);
+}
+
 static gboolean
 apply_bonding_config (NMDeviceBond *self)
 {
@@ -300,7 +335,7 @@ apply_bonding_config (NMDeviceBond *self)
 	                 nm_setting_bond_get_option_or_default (s_bond, NM_SETTING_BOND_OPTION_ARP_IP_TARGET));
 
 	set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM);
-	set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
+	set_bond_attr_active_slave (device, s_bond);
 	set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO);
 	set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_SELECT);
 	set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY);
@@ -378,7 +413,7 @@ enslave_slave (NMDevice *device,
 				if (nm_streq0 (active, nm_device_get_iface (slave))) {
 					nm_platform_sysctl_master_set_option (nm_device_get_platform (device),
 					                                      nm_device_get_ifindex (device),
-					                                      "active_slave",
+					                                      NM_SETTING_BOND_OPTION_ACTIVE_SLAVE,
 					                                      active);
 					_LOGD (LOGD_BOND, "setting slave %s as active one for master %s",
 					       active, nm_device_get_iface (device));
@@ -577,10 +612,8 @@ reapply_connection (NMDevice *device, NMConnection *con_old, NMConnection *con_n
 	mode = _nm_setting_bond_mode_from_string (value);
 	g_return_if_fail (mode != NM_BOND_MODE_UNKNOWN);
 
-	/* Primary */
 	set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_PRIMARY);
-	/* Active slave */
-	set_bond_attr_or_default (device, s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
+	set_bond_attr_active_slave (device, s_bond);
 }
 
 /*****************************************************************************/
-- 
2.26.2