[PATCH net-next 0/6] bonding: refactor bond_enslave() to fix XDP ordering and error rollback

[PATCH net-next 0/6] bonding: refactor bond_enslave() to fix XDP ordering and error rollback

[Hangbin Liu]

`bond_enslave()` is over 500 lines long with complex logic and numerous
error rollback paths. It has XDP ordering isssue and missing some error
rollbacks for historical reasons (found by Sashiko when review bond
patches). The fixes are trivial, I plan to refactor the function to make
the logic clearer and resolved the issues. This patch set is the first
phase.

This series hardens bond_enslave() in two stages: first fix real
ordering and rollback bugs, then factor the function so the
infallible commit point and fail-fast validation stay obvious.

Problems fixed (patches 1-3):

1. XDP capability checks ran after the slave was opened, linked and
   could already be visible to the datapath . A slave without
   ndo_xdp_xmit could therefore be reached from bond_xdp_xmit().
   Checks move to the top of bond_enslave(), before any mutation.

2. Fallible XDP setup (dev_xdp_propagate()) ran after publication.
   On failure the undo ladder jumped to err_sysfs_del and leaked:
   - promiscuity / allmulti / synced addresses
   - ns maddrs added just before the failure

3. Error paths were incomplete versus release:
   - bond_alb_init_slave() success + later failure skipped
     bond_alb_deinit_slave() (TLB/RLB state, borrowed MAC)
   - bond_3ad_bind_slave() + later failure skipped
     bond_3ad_unbind_slave() (port left on its aggregator)
   - automatic fail_over_mac = "active" was not restored on failure

Cleanup (patches 4-6), no intended behaviour change beyond patch 5's
extack string alignment:

4. bond_publish_slave() — explicit infallible commit point
5. bond_slave_xdp_check() — shared by bond_enslave() and bond_xdp_set()
6. bond_enslave_validate() — all read-only checks

Testing: build debug kernel and run all kernel selftests

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
Hangbin Liu (6):
      bonding: check XDP support before enslaving changes state
      bonding: finish XDP setup before publishing the slave
      bonding: complete rollback of bond_enslave() error paths
      bonding: factor out slave publication
      bonding: factor out XDP slave check
      bonding: factor out bond_enslave_validate

 drivers/net/bonding/bond_main.c | 174 ++++++++++++++++++++++++++--------------
 1 file changed, 116 insertions(+), 58 deletions(-)
---
base-commit: 573af801552999954caa8347e4490b04cf4763f0
change-id: 20261020-bond_enslave_restruct-30760e62def9

Best regards,
--  
Hangbin Liu <liuhangbin@kylinos.cn>

[PATCH net-next 1/6] bonding: check XDP support before enslaving changes state

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

The XDP capability checks (ndo_bpf/ndo_xdp_xmit) and the slave XDP
program count check in bond_enslave() run after the slave has already
been opened, attached and made visible to the datapath. In particular
the slave can become bond->curr_active_slave, or be published via
bond->slave_cnt, before we verify that slave_dev->netdev_ops->ndo_xdp_xmit
exists, so bond_xdp_xmit() may call a NULL ndo_xdp_xmit.

Move both checks to the validation section at the top of bond_enslave(),
before the first state change. On failure nothing has been mutated yet,
so the function can return directly. The dev_xdp_propagate() setup is
left in place for now and still uses the same error path.

Error messages and errno values are unchanged (-EOPNOTSUPP with the
same netlink extack strings), and the success path is unaffected
beyond running the read-only checks earlier.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/bonding/bond_main.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9249a79a168e..340af4c3fa20 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1948,6 +1948,23 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		return -EPERM;
 	}
 
+	/* Check XDP support before any state change, so a slave without
+	 * XDP can never become visible to bond_xdp_xmit().
+	 */
+	if (bond->xdp_prog) {
+		if (!slave_ops->ndo_bpf || !slave_ops->ndo_xdp_xmit) {
+			SLAVE_NL_ERR(bond_dev, slave_dev, extack,
+				     "Slave does not support XDP");
+			return -EOPNOTSUPP;
+		}
+
+		if (dev_xdp_prog_count(slave_dev) > 0) {
+			SLAVE_NL_ERR(bond_dev, slave_dev, extack,
+				     "Slave has XDP program loaded, please unload before enslaving");
+			return -EOPNOTSUPP;
+		}
+	}
+
 	/* set bonding device ether type by slave - bonding netdevices are
 	 * created with ether_setup, so when the slave type is not ARPHRD_ETHER
 	 * there is a need to override some of the type dependent attribs/funcs.
@@ -2311,15 +2328,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		unblock_netpoll_tx();
 	}
 
-	if (!slave_dev->netdev_ops->ndo_bpf ||
-	    !slave_dev->netdev_ops->ndo_xdp_xmit) {
-		if (bond->xdp_prog) {
-			SLAVE_NL_ERR(bond_dev, slave_dev, extack,
-				     "Slave does not support XDP");
-			res = -EOPNOTSUPP;
-			goto err_dec_slave_cnt;
-		}
-	} else if (bond->xdp_prog) {
+	/* XDP capability and prog-count were already validated above */
+	if (bond->xdp_prog) {
 		struct netdev_bpf xdp = {
 			.command = XDP_SETUP_PROG,
 			.flags   = 0,
@@ -2327,13 +2337,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 			.extack  = extack,
 		};
 
-		if (dev_xdp_prog_count(slave_dev) > 0) {
-			SLAVE_NL_ERR(bond_dev, slave_dev, extack,
-				     "Slave has XDP program loaded, please unload before enslaving");
-			res = -EOPNOTSUPP;
-			goto err_dec_slave_cnt;
-		}
-
 		res = dev_xdp_propagate(slave_dev, &xdp);
 		if (res < 0) {
 			/* ndo_bpf() sets extack error message */

-- 
2.56.0

[PATCH net-next 2/6] bonding: finish XDP setup before publishing the slave

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

bond_enslave() currently runs the fallible XDP setup
(dev_xdp_propagate()) after the slave has already been published:
slave_cnt is incremented, ns maddrs are added and the active slave
may have been selected. If XDP setup then fails, the error path
jumps to err_sysfs_del and leaks:

  - the promiscuity/allmulti increments and synced addresses applied
    to the slave earlier in the function, which are only undone on
    an immediate allmulti failure,
  - the ns maddrs added just before the failure, which no enslave
    error path ever removes.

Move dev_xdp_propagate() to the last fallible step, right after the
promiscuity/allmulti/address-sync block and before
WRITE_ONCE(bond->slave_cnt), so that no handled fallible call
follows the publication of the slave and a failure can no longer
leak slave_cnt or ns maddrs.

bond_xdp_set_features() is moved along with this reordering so XDP
features are updated before the slave is published. Route the XDP
failure through a new err_set_xdp label that undoes the promiscuity,
allmulti and address-sync state, mirroring what __bond_release_one()
does.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/bonding/bond_main.c | 52 +++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 340af4c3fa20..80b6c18ee9c2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2313,22 +2313,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		}
 	}
 
-	WRITE_ONCE(bond->slave_cnt, bond->slave_cnt + 1);
-	netdev_compute_master_upper_features(bond->dev, true);
-	bond_set_carrier(bond);
-
-	/* Needs to be called before bond_select_active_slave(), which will
-	 * remove the maddrs if the slave is selected as active slave.
+	/* XDP capability and prog-count were already validated above.
+	 * Setup runs as the last fallible step before the slave is
+	 * published, so no undo of this block can ever be needed and
+	 * a failure here cannot leak slave_cnt or ns maddrs.
 	 */
-	bond_slave_ns_maddrs_add(bond, new_slave);
-
-	if (bond_uses_primary(bond)) {
-		block_netpoll_tx();
-		bond_select_active_slave(bond);
-		unblock_netpoll_tx();
-	}
-
-	/* XDP capability and prog-count were already validated above */
 	if (bond->xdp_prog) {
 		struct netdev_bpf xdp = {
 			.command = XDP_SETUP_PROG,
@@ -2341,19 +2330,35 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		if (res < 0) {
 			/* ndo_bpf() sets extack error message */
 			slave_dbg(bond_dev, slave_dev, "Error %d calling ndo_bpf\n", res);
-			goto err_dec_slave_cnt;
+			goto err_set_xdp;
 		}
 		if (bond->xdp_prog)
 			bpf_prog_inc(bond->xdp_prog);
 	}
 
+	bond_xdp_set_features(bond_dev);
+
+	/* Must called before bond_update_slave_arr() or bond_select_active_slave() */
+	WRITE_ONCE(bond->slave_cnt, bond->slave_cnt + 1);
+	netdev_compute_master_upper_features(bond->dev, true);
+	bond_set_carrier(bond);
+
+	/* Needs to be called before bond_select_active_slave(), which will
+	 * remove the maddrs if the slave is selected as active slave.
+	 */
+	bond_slave_ns_maddrs_add(bond, new_slave);
+
+	if (bond_uses_primary(bond)) {
+		block_netpoll_tx();
+		bond_select_active_slave(bond);
+		unblock_netpoll_tx();
+	}
+
 	/* broadcast mode uses the all_slaves to loop through slaves. */
 	if (bond_mode_can_use_xmit_hash(bond) ||
 	    BOND_MODE(bond) == BOND_MODE_BROADCAST)
 		bond_update_slave_arr(bond, NULL);
 
-	bond_xdp_set_features(bond_dev);
-
 	slave_info(bond_dev, slave_dev, "Enslaving as %s interface with %s link\n",
 		   bond_is_active_slave(new_slave) ? "an active" : "a backup",
 		   new_slave->link != BOND_LINK_DOWN ? "an up" : "a down");
@@ -2363,8 +2368,15 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	return 0;
 
 /* Undo stages on error */
-err_dec_slave_cnt:
-	WRITE_ONCE(bond->slave_cnt, bond->slave_cnt - 1);
+err_set_xdp:
+	if (!bond_uses_primary(bond)) {
+		if (bond_dev->flags & IFF_PROMISC)
+			dev_set_promiscuity(slave_dev, -1);
+		if (bond_dev->flags & IFF_ALLMULTI)
+			dev_set_allmulti(slave_dev, -1);
+		if (bond_dev->flags & IFF_UP)
+			bond_hw_addr_flush(bond_dev, slave_dev);
+	}
 
 err_sysfs_del:
 	bond_sysfs_slave_del(new_slave);

-- 
2.56.0

[PATCH net-next 3/6] bonding: complete rollback of bond_enslave() error paths

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

The bond_enslave() undo labels are missing three cleanups that the
release path (__bond_release_one) already performs:

- If bond_alb_init_slave() succeeds but a later step fails, the
  slave is never passed to bond_alb_deinit_slave(), leaving TLB/RLB
  state and a possibly borrowed MAC address behind.

- If the slave is bound into 802.3ad (bond_3ad_bind_slave()) and a
  later step fails, bond_3ad_unbind_slave() is never called, so the
  port stays attached to its aggregator.

- When fail_over_mac is automatically set to "active" because the
  first slave cannot change its MAC address, a subsequent failure
  leaves the modified parameter on the bond.

Add undo steps for these 3 cleanups when error out.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/bonding/bond_main.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 80b6c18ee9c2..f696a0d15034 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1890,7 +1890,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	const struct net_device_ops *slave_ops = slave_dev->netdev_ops;
+	int orig_fail_over_mac = bond->params.fail_over_mac;
 	struct slave *new_slave = NULL, *prev_slave;
+	bool alb_initialized = false;
 	struct sockaddr_storage ss;
 	int res = 0, i;
 
@@ -2120,6 +2122,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		res = bond_alb_init_slave(bond, new_slave);
 		if (res)
 			goto err_close;
+		alb_initialized = true;
 	}
 
 	res = vlan_vids_add_by_dev(slave_dev, bond_dev);
@@ -2388,6 +2391,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	netdev_rx_handler_unregister(slave_dev);
 
 err_detach:
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		bond_3ad_unbind_slave(new_slave);
 	vlan_vids_del_by_dev(slave_dev, bond_dev);
 	if (rcu_access_pointer(bond->primary_slave) == new_slave)
 		RCU_INIT_POINTER(bond->primary_slave, NULL);
@@ -2402,6 +2407,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	slave_disable_netpoll(new_slave);
 
 err_close:
+	if (alb_initialized)
+		bond_alb_deinit_slave(bond, new_slave);
 	if (!netif_is_bond_master(slave_dev))
 		slave_dev->priv_flags &= ~IFF_BONDING;
 	dev_close(slave_dev);
@@ -2427,6 +2434,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	kobject_put(&new_slave->kobj);
 
 err_undo_flags:
+	/* Restore fail_over_mac in case the automatic "active" setting
+	 * above was applied and a later step failed.
+	 */
+	bond->params.fail_over_mac = orig_fail_over_mac;
+
 	/* Enslave of first slave has failed and we need to fix master's mac */
 	if (!bond_has_slaves(bond)) {
 		if (ether_addr_equal_64bits(bond_dev->dev_addr,

-- 
2.56.0

[PATCH net-next 4/6] bonding: factor out slave publication

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

Factor the publication step of bond_enslave() into bond_publish_slave()
so the infallible "commit point" of the function is explicit and cannot
be interleaved with new fallible code by accident.

No functional change.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/bonding/bond_main.c | 49 ++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f696a0d15034..bb3752683456 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1884,6 +1884,34 @@ void bond_xdp_set_features(struct net_device *bond_dev)
 	xdp_set_features_flag(bond_dev, val);
 }
 
+/* Make the fully prepared slave visible to the rest of the bonding
+ * code.  All fallible steps of bond_enslave() must have completed
+ * before this runs: error paths never undo the publication.
+ */
+static void bond_publish_slave(struct bonding *bond, struct slave *slave)
+{
+	/* Must called before bond_update_slave_arr() or bond_select_active_slave() */
+	WRITE_ONCE(bond->slave_cnt, bond->slave_cnt + 1);
+	netdev_compute_master_upper_features(bond->dev, true);
+	bond_set_carrier(bond);
+
+	/* Needs to be called before bond_select_active_slave(), which will
+	 * remove the maddrs if the slave is selected as active slave.
+	 */
+	bond_slave_ns_maddrs_add(bond, slave);
+
+	if (bond_uses_primary(bond)) {
+		block_netpoll_tx();
+		bond_select_active_slave(bond);
+		unblock_netpoll_tx();
+	}
+
+	/* broadcast mode uses the all_slaves to loop through slaves. */
+	if (bond_mode_can_use_xmit_hash(bond) ||
+	    BOND_MODE(bond) == BOND_MODE_BROADCAST)
+		bond_update_slave_arr(bond, NULL);
+}
+
 /* enslave device <slave> to bond device <master> */
 int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		 struct netlink_ext_ack *extack)
@@ -2341,26 +2369,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 
 	bond_xdp_set_features(bond_dev);
 
-	/* Must called before bond_update_slave_arr() or bond_select_active_slave() */
-	WRITE_ONCE(bond->slave_cnt, bond->slave_cnt + 1);
-	netdev_compute_master_upper_features(bond->dev, true);
-	bond_set_carrier(bond);
-
-	/* Needs to be called before bond_select_active_slave(), which will
-	 * remove the maddrs if the slave is selected as active slave.
-	 */
-	bond_slave_ns_maddrs_add(bond, new_slave);
-
-	if (bond_uses_primary(bond)) {
-		block_netpoll_tx();
-		bond_select_active_slave(bond);
-		unblock_netpoll_tx();
-	}
-
-	/* broadcast mode uses the all_slaves to loop through slaves. */
-	if (bond_mode_can_use_xmit_hash(bond) ||
-	    BOND_MODE(bond) == BOND_MODE_BROADCAST)
-		bond_update_slave_arr(bond, NULL);
+	bond_publish_slave(bond, new_slave);
 
 	slave_info(bond_dev, slave_dev, "Enslaving as %s interface with %s link\n",
 		   bond_is_active_slave(new_slave) ? "an active" : "a backup",

-- 
2.56.0

[PATCH net-next 5/6] bonding: factor out XDP slave check

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

bond_enslave() and bond_xdp_set() carry near-identical XDP slave
capability checks. Factor them into bond_slave_xdp_check() so the
two paths cannot drift apart.

The only behaviour difference is the netlink extack string in
bond_xdp_set(): "Slave device does not support XDP" becomes
"Slave does not support XDP" to match bond_enslave(). Logic and
return values are unchanged.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/bonding/bond_main.c | 52 ++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index bb3752683456..5e36612cfaec 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1884,6 +1884,29 @@ void bond_xdp_set_features(struct net_device *bond_dev)
 	xdp_set_features_flag(bond_dev, val);
 }
 
+/* Verify that slave_dev can take part in the bond's XDP datapath.
+ * Returns 0 if the slave is usable, otherwise -EOPNOTSUPP.
+ */
+static int bond_slave_xdp_check(struct net_device *bond_dev,
+				struct net_device *slave_dev,
+				struct netlink_ext_ack *extack)
+{
+	if (!slave_dev->netdev_ops->ndo_bpf ||
+	    !slave_dev->netdev_ops->ndo_xdp_xmit) {
+		SLAVE_NL_ERR(bond_dev, slave_dev, extack,
+			     "Slave does not support XDP");
+		return -EOPNOTSUPP;
+	}
+
+	if (dev_xdp_prog_count(slave_dev) > 0) {
+		SLAVE_NL_ERR(bond_dev, slave_dev, extack,
+			     "Slave has XDP program loaded, please unload before enslaving");
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 /* Make the fully prepared slave visible to the rest of the bonding
  * code.  All fallible steps of bond_enslave() must have completed
  * before this runs: error paths never undo the publication.
@@ -1982,17 +2005,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	 * XDP can never become visible to bond_xdp_xmit().
 	 */
 	if (bond->xdp_prog) {
-		if (!slave_ops->ndo_bpf || !slave_ops->ndo_xdp_xmit) {
-			SLAVE_NL_ERR(bond_dev, slave_dev, extack,
-				     "Slave does not support XDP");
-			return -EOPNOTSUPP;
-		}
-
-		if (dev_xdp_prog_count(slave_dev) > 0) {
-			SLAVE_NL_ERR(bond_dev, slave_dev, extack,
-				     "Slave has XDP program loaded, please unload before enslaving");
-			return -EOPNOTSUPP;
-		}
+		res = bond_slave_xdp_check(bond_dev, slave_dev, extack);
+		if (res)
+			return res;
 	}
 
 	/* set bonding device ether type by slave - bonding netdevices are
@@ -5764,20 +5779,9 @@ static int bond_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	bond_for_each_slave(bond, slave, iter) {
 		struct net_device *slave_dev = slave->dev;
 
-		if (!slave_dev->netdev_ops->ndo_bpf ||
-		    !slave_dev->netdev_ops->ndo_xdp_xmit) {
-			SLAVE_NL_ERR(dev, slave_dev, extack,
-				     "Slave device does not support XDP");
-			err = -EOPNOTSUPP;
+		err = bond_slave_xdp_check(dev, slave_dev, extack);
+		if (err)
 			goto err;
-		}
-
-		if (dev_xdp_prog_count(slave_dev) > 0) {
-			SLAVE_NL_ERR(dev, slave_dev, extack,
-				     "Slave has XDP program loaded, please unload before enslaving");
-			err = -EOPNOTSUPP;
-			goto err;
-		}
 
 		err = dev_xdp_propagate(slave_dev, &xdp);
 		if (err < 0) {

-- 
2.56.0

[PATCH net-next 6/6] bonding: factor out bond_enslave_validate

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

bond_enslave() is a long linear function whose flow is hard to see
behind the inline blocks. Factor out bond_enslave_validate to do
all read-only checks.

Failures return directly, no undo needed at that point.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/bonding/bond_main.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5e36612cfaec..8f97b0e2c0b3 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1935,17 +1935,16 @@ static void bond_publish_slave(struct bonding *bond, struct slave *slave)
 		bond_update_slave_arr(bond, NULL);
 }
 
-/* enslave device <slave> to bond device <master> */
-int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
-		 struct netlink_ext_ack *extack)
+/* Fail-fast validation of the slave and the bond.  Everything here
+ * reads state only, so a failure needs no undo.  Must stay ahead of
+ * the first mutation in bond_enslave().
+ */
+static int bond_enslave_validate(struct net_device *bond_dev,
+				 struct net_device *slave_dev,
+				 struct netlink_ext_ack *extack)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	const struct net_device_ops *slave_ops = slave_dev->netdev_ops;
-	int orig_fail_over_mac = bond->params.fail_over_mac;
-	struct slave *new_slave = NULL, *prev_slave;
-	bool alb_initialized = false;
-	struct sockaddr_storage ss;
-	int res = 0, i;
+	int res;
 
 	if (slave_dev->type == ARPHRD_CAN) {
 		BOND_NL_ERR(bond_dev, extack,
@@ -2010,6 +2009,25 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 			return res;
 	}
 
+	return 0;
+}
+
+/* enslave device <slave> to bond device <master> */
+int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
+		 struct netlink_ext_ack *extack)
+{
+	struct bonding *bond = netdev_priv(bond_dev);
+	const struct net_device_ops *slave_ops = slave_dev->netdev_ops;
+	int orig_fail_over_mac = bond->params.fail_over_mac;
+	struct slave *new_slave = NULL, *prev_slave;
+	bool alb_initialized = false;
+	struct sockaddr_storage ss;
+	int res = 0, i;
+
+	res = bond_enslave_validate(bond_dev, slave_dev, extack);
+	if (res)
+		return res;
+
 	/* set bonding device ether type by slave - bonding netdevices are
 	 * created with ether_setup, so when the slave type is not ARPHRD_ETHER
 	 * there is a need to override some of the type dependent attribs/funcs.

-- 
2.56.0

Re: [PATCH net-next 0/6] bonding: refactor bond_enslave() to fix XDP ordering and error rollback

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.