[PATCH 0/3] net/bond: fix secondary process crash and related cleanup

[PATCH 0/3] net/bond: fix secondary process crash and related cleanup

[Stephen Hemminger]

Patch 1 fixes the 8023ad dedicated-queue APIs which ignored the return
of bond_ethdev_mode_set() and left the enabled flag inconsistent on
failure. Ordered first so the secondary guard added next does not
leave shared state corrupted.

Patch 2 fixes the secondary process crash: a bonding port attached in
a secondary crashes on the first Rx or Tx burst because the probe
path never installs burst functions. Blackhole stubs are installed
and mode changes from secondary are rejected. Fully sharing bonding
state across processes is out of scope.

Patch 3 drops redundant %s/func from log call sites now that
RTE_BOND_LOG supplies the prefix.

Stephen Hemminger (3):
  net/bonding: restore dedicated queue state on mode set error
  net/bonding: prevent crash on Rx/Tx from secondary process
  net/bonding: remove redundant function names from log

 drivers/net/bonding/rte_eth_bond_8023ad.c | 19 ++++---
 drivers/net/bonding/rte_eth_bond_api.c    |  4 +-
 drivers/net/bonding/rte_eth_bond_pmd.c    | 66 ++++++++++++++++++-----
 3 files changed, 66 insertions(+), 23 deletions(-)

-- 
2.53.0

[PATCH 1/3] net/bonding: restore dedicated queue state on mode set error

[Stephen Hemminger]

The calls to enable and disable dedicated queues are missing
proper error handling. If setting bonding mode fails,
restore original state and propagate the error return value.

Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index ba88f6d261..a74b0059ac 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1727,7 +1727,7 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_8023ad_dedicated_queues_enable)
 int
 rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port)
 {
-	int retval = 0;
+	int ret;
 	struct rte_eth_dev *dev;
 	struct bond_dev_private *internals;
 
@@ -1744,17 +1744,20 @@ rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port)
 	if (dev->data->dev_started)
 		return -1;
 
+	uint8_t old = internals->mode4.dedicated_queues.enabled;
 	internals->mode4.dedicated_queues.enabled = 1;
+	ret = bond_ethdev_mode_set(dev, internals->mode);
+	if (ret != 0)
+		internals->mode4.dedicated_queues.enabled = old;
 
-	bond_ethdev_mode_set(dev, internals->mode);
-	return retval;
+	return ret;
 }
 
 RTE_EXPORT_SYMBOL(rte_eth_bond_8023ad_dedicated_queues_disable)
 int
 rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port)
 {
-	int retval = 0;
+	int ret;
 	struct rte_eth_dev *dev;
 	struct bond_dev_private *internals;
 
@@ -1768,9 +1771,11 @@ rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port)
 	if (dev->data->dev_started)
 		return -1;
 
+	uint8_t old = internals->mode4.dedicated_queues.enabled;
 	internals->mode4.dedicated_queues.enabled = 0;
+	ret = bond_ethdev_mode_set(dev, internals->mode);
+	if (ret != 0)
+		internals->mode4.dedicated_queues.enabled = old;
 
-	bond_ethdev_mode_set(dev, internals->mode);
-
-	return retval;
+	return ret;
 }
-- 
2.53.0

Re: [PATCH 1/3] net/bonding: restore dedicated queue state on mode set error

[Bruce Richardson]

On Fri, Apr 17, 2026 at 09:51:35AM -0700, Stephen Hemminger wrote:
> The calls to enable and disable dedicated queues are missing
> proper error handling. If setting bonding mode fails,
> restore original state and propagate the error return value.
> 
> Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/bonding/rte_eth_bond_8023ad.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
> index ba88f6d261..a74b0059ac 100644
> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> @@ -1727,7 +1727,7 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_8023ad_dedicated_queues_enable)
>  int
>  rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port)
>  {
> -	int retval = 0;
> +	int ret;

Any particular reason for the variable rename? Not that it's a problem, but
it does expand the diff.

>  	struct rte_eth_dev *dev;
>  	struct bond_dev_private *internals;
>  
> @@ -1744,17 +1744,20 @@ rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port)
>  	if (dev->data->dev_started)
>  		return -1;
>  
> +	uint8_t old = internals->mode4.dedicated_queues.enabled;

Should you add a check that old != 1, and just return ok if so?

>  	internals->mode4.dedicated_queues.enabled = 1;
> +	ret = bond_ethdev_mode_set(dev, internals->mode);
> +	if (ret != 0)
> +		internals->mode4.dedicated_queues.enabled = old;
>  

Looking through the code, for 8023ad mode, I don't see any way of failure,
so this failure handling seems pointless. The function
bond_mode_8023ad_enable() loops through all the devices in the bond but
never checks for error for any of them, so always returns zero.

What is missing here is a check that the internals->mode is actually set to
8023ad.

Similar feedback applies to the disable function below.

/Bruce

[PATCH 2/3] net/bonding: prevent crash on Rx/Tx from secondary process

[Stephen Hemminger]

The bonding PMD's secondary process attach path registered the
ethdev but never installed rx_pkt_burst or tx_pkt_burst, leaving
both as NULL. Any rx_burst or tx_burst call from a secondary
process therefore crashed with a NULL pointer dereference.

Fully sharing bonding state across processes would be
overly complex. Instead, install blackhole burst functions
in the secondary so the data path is safe by default. Rx returns 0
and Tx frees the mbufs and reports them as transmitted,
matching /dev/null semantics so applications do not spin
retrying. Each stub logs once at NOTICE level on first use.

Also reject bond mode changes from a secondary process.
rte_eth_bond_mode_set() is callable from secondary, and
without this guard it would overwrite the secondary's safe
burst stubs with real per-mode functions whose internal
state is not valid outside the primary, reintroducing the
crash by another path.

This keeps secondary support available for the more
common use cases of procinfo and packet capture.

Bugzilla ID: 1698
Fixes: 4852aa8f6e21 ("drivers/net: enable hotplug on secondary process")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 43 +++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 96725071da..6a42257d2b 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -56,6 +56,33 @@ get_vlan_offset(struct rte_ether_hdr *eth_hdr, uint16_t *proto)
 	return vlan_offset;
 }
 
+static uint16_t
+bond_ethdev_rx_secondary(void *queue __rte_unused,
+			 struct rte_mbuf **bufs __rte_unused, uint16_t nb_pkts __rte_unused)
+{
+	static bool once = true;
+
+	if (once) {
+		/* once per process is enough of a notice */
+		RTE_BOND_LOG(NOTICE, "receive not supported in secondary");
+		once = false;
+	}
+	return 0;
+}
+
+static uint16_t
+bond_ethdev_tx_secondary(void *queue __rte_unused, struct rte_mbuf **bufs, uint16_t nb_pkts)
+{
+	static bool once = true;
+
+	if (once) {
+		RTE_BOND_LOG(NOTICE, "transmit not supported in secondary");
+		once = false;
+	}
+	rte_pktmbuf_free_bulk(bufs, nb_pkts);
+	return nb_pkts;
+}
+
 static uint16_t
 bond_ethdev_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 {
@@ -1599,6 +1626,11 @@ bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, uint8_t mode)
 {
 	struct bond_dev_private *internals;
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		RTE_BOND_LOG(ERR, "Setting mode in secondary not allowed");
+		return -1;
+	}
+
 	internals = eth_dev->data->dev_private;
 
 	switch (mode) {
@@ -3799,9 +3831,18 @@ bond_probe(struct rte_vdev_device *dev)
 			RTE_BOND_LOG(ERR, "Failed to probe %s", name);
 			return -1;
 		}
-		/* TODO: request info from primary to set up Rx and Tx */
+
 		eth_dev->dev_ops = &default_dev_ops;
 		eth_dev->device = &dev->device;
+
+		/*
+		 * Propagation of bond mode would require adding
+		 * MP client/server support and lots of error handling.
+		 *
+		 * For now just install a black hole.
+		 */
+		eth_dev->tx_pkt_burst = bond_ethdev_tx_secondary;
+		eth_dev->rx_pkt_burst = bond_ethdev_rx_secondary;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
-- 
2.53.0

Re: [PATCH 2/3] net/bonding: prevent crash on Rx/Tx from secondary process

[Bruce Richardson]

On Fri, Apr 17, 2026 at 09:51:36AM -0700, Stephen Hemminger wrote:
> The bonding PMD's secondary process attach path registered the
> ethdev but never installed rx_pkt_burst or tx_pkt_burst, leaving
> both as NULL. Any rx_burst or tx_burst call from a secondary
> process therefore crashed with a NULL pointer dereference.
> 
> Fully sharing bonding state across processes would be
> overly complex. Instead, install blackhole burst functions
> in the secondary so the data path is safe by default. Rx returns 0
> and Tx frees the mbufs and reports them as transmitted,
> matching /dev/null semantics so applications do not spin
> retrying. Each stub logs once at NOTICE level on first use.
> 
> Also reject bond mode changes from a secondary process.
> rte_eth_bond_mode_set() is callable from secondary, and
> without this guard it would overwrite the secondary's safe
> burst stubs with real per-mode functions whose internal
> state is not valid outside the primary, reintroducing the
> crash by another path.
> 
> This keeps secondary support available for the more
> common use cases of procinfo and packet capture.
> 
> Bugzilla ID: 1698
> Fixes: 4852aa8f6e21 ("drivers/net: enable hotplug on secondary process")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

However, see comment below.


>  drivers/net/bonding/rte_eth_bond_pmd.c | 43 +++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 96725071da..6a42257d2b 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -56,6 +56,33 @@ get_vlan_offset(struct rte_ether_hdr *eth_hdr, uint16_t *proto)
>  	return vlan_offset;
>  }
>  
> +static uint16_t
> +bond_ethdev_rx_secondary(void *queue __rte_unused,
> +			 struct rte_mbuf **bufs __rte_unused, uint16_t nb_pkts __rte_unused)
> +{
> +	static bool once = true;
> +
> +	if (once) {
> +		/* once per process is enough of a notice */
> +		RTE_BOND_LOG(NOTICE, "receive not supported in secondary");

Since this does nothing, I think that an ERROR level warning is more
appropriate than notice. If not for Rx, certainly for Tx which just
silently drops packets!

> +		once = false;
> +	}
> +	return 0;
> +}
> +
> +static uint16_t
> +bond_ethdev_tx_secondary(void *queue __rte_unused, struct rte_mbuf **bufs, uint16_t nb_pkts)
> +{
> +	static bool once = true;
> +
> +	if (once) {
> +		RTE_BOND_LOG(NOTICE, "transmit not supported in secondary");
> +		once = false;
> +	}
> +	rte_pktmbuf_free_bulk(bufs, nb_pkts);
> +	return nb_pkts;
> +}
> +
>  static uint16_t
>  bond_ethdev_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  {
> @@ -1599,6 +1626,11 @@ bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, uint8_t mode)
>  {
>  	struct bond_dev_private *internals;
>  
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> +		RTE_BOND_LOG(ERR, "Setting mode in secondary not allowed");
> +		return -1;
> +	}
> +
>  	internals = eth_dev->data->dev_private;
>  
>  	switch (mode) {
> @@ -3799,9 +3831,18 @@ bond_probe(struct rte_vdev_device *dev)
>  			RTE_BOND_LOG(ERR, "Failed to probe %s", name);
>  			return -1;
>  		}
> -		/* TODO: request info from primary to set up Rx and Tx */
> +
>  		eth_dev->dev_ops = &default_dev_ops;
>  		eth_dev->device = &dev->device;
> +
> +		/*
> +		 * Propagation of bond mode would require adding
> +		 * MP client/server support and lots of error handling.
> +		 *
> +		 * For now just install a black hole.
> +		 */
> +		eth_dev->tx_pkt_burst = bond_ethdev_tx_secondary;
> +		eth_dev->rx_pkt_burst = bond_ethdev_rx_secondary;
>  		rte_eth_dev_probing_finish(eth_dev);
>  		return 0;
>  	}
> -- 
> 2.53.0
> 

[PATCH 3/3] net/bonding: remove redundant function names from log

[Stephen Hemminger]

The function name is already printed as part of RTE_BOND_LOG().

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/bonding/rte_eth_bond_api.c |  4 ++--
 drivers/net/bonding/rte_eth_bond_pmd.c | 23 ++++++++++-------------
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 9e5df67c18..78361e73d4 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -485,8 +485,8 @@ __eth_bond_member_add_lock_free(uint16_t bonding_port_id, uint16_t member_port_i
 	ret = rte_eth_dev_info_get(member_port_id, &dev_info);
 	if (ret != 0) {
 		RTE_BOND_LOG(ERR,
-			"%s: Error during getting device (port %u) info: %s",
-			__func__, member_port_id, strerror(-ret));
+			"Error during getting device (port %u) info: %s",
+			member_port_id, strerror(-ret));
 
 		return ret;
 	}
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 6a42257d2b..e2819c931b 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -210,8 +210,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
 	int ret = rte_flow_validate(member_port, &flow_attr_8023ad,
 			flow_item_8023ad, actions, &error);
 	if (ret < 0) {
-		RTE_BOND_LOG(ERR, "%s: %s (member_port=%d queue_id=%d)",
-				__func__, error.message, member_port,
+		RTE_BOND_LOG(ERR, "%s (member_port=%u queue_id=%u)",
+				error.message, member_port,
 				internals->mode4.dedicated_queues.rx_qid);
 		return -1;
 	}
@@ -219,8 +219,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
 	ret = rte_eth_dev_info_get(member_port, &member_info);
 	if (ret != 0) {
 		RTE_BOND_LOG(ERR,
-			"%s: Error during getting device (port %u) info: %s",
-			__func__, member_port, strerror(-ret));
+			"Error during getting device (port %u) info: %s",
+			member_port, strerror(-ret));
 
 		return ret;
 	}
@@ -228,8 +228,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
 	if (member_info.max_rx_queues < bond_dev->data->nb_rx_queues ||
 			member_info.max_tx_queues < bond_dev->data->nb_tx_queues) {
 		RTE_BOND_LOG(ERR,
-			"%s: Member %d capabilities doesn't allow allocating additional queues",
-			__func__, member_port);
+			"Member %u capabilities doesn't allow allocating additional queues",
+			member_port);
 		return -1;
 	}
 
@@ -249,9 +249,8 @@ bond_8023ad_slow_pkt_hw_filter_supported(uint16_t port_id) {
 		ret = rte_eth_dev_info_get(bond_dev->data->port_id, &bond_info);
 		if (ret != 0) {
 			RTE_BOND_LOG(ERR,
-				"%s: Error during getting device (port %u) info: %s",
-				__func__, bond_dev->data->port_id,
-				strerror(-ret));
+				"Error during getting device (port %u) info: %s",
+				bond_dev->data->port_id, strerror(-ret));
 
 			return ret;
 		}
@@ -2347,10 +2346,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 			ret = rte_eth_dev_info_get(member.port_id, &member_info);
 			if (ret != 0) {
 				RTE_BOND_LOG(ERR,
-					"%s: Error during getting device (port %u) info: %s",
-					__func__,
-					member.port_id,
-					strerror(-ret));
+					"Error during getting device (port %u) info: %s",
+					member.port_id, strerror(-ret));
 
 				return ret;
 			}
-- 
2.53.0

Re: [PATCH 3/3] net/bonding: remove redundant function names from log

[Bruce Richardson]

On Fri, Apr 17, 2026 at 09:51:37AM -0700, Stephen Hemminger wrote:
> The function name is already printed as part of RTE_BOND_LOG().
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

>  drivers/net/bonding/rte_eth_bond_api.c |  4 ++--
>  drivers/net/bonding/rte_eth_bond_pmd.c | 23 ++++++++++-------------
>  2 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
> index 9e5df67c18..78361e73d4 100644
> --- a/drivers/net/bonding/rte_eth_bond_api.c
> +++ b/drivers/net/bonding/rte_eth_bond_api.c
> @@ -485,8 +485,8 @@ __eth_bond_member_add_lock_free(uint16_t bonding_port_id, uint16_t member_port_i
>  	ret = rte_eth_dev_info_get(member_port_id, &dev_info);
>  	if (ret != 0) {
>  		RTE_BOND_LOG(ERR,
> -			"%s: Error during getting device (port %u) info: %s",
> -			__func__, member_port_id, strerror(-ret));
> +			"Error during getting device (port %u) info: %s",

While updating, I think it would be good to update the log messages to be
more natural-sounding English. s/during getting/getting/

> +			member_port_id, strerror(-ret));
>  
>  		return ret;
>  	}
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 6a42257d2b..e2819c931b 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -210,8 +210,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
>  	int ret = rte_flow_validate(member_port, &flow_attr_8023ad,
>  			flow_item_8023ad, actions, &error);
>  	if (ret < 0) {
> -		RTE_BOND_LOG(ERR, "%s: %s (member_port=%d queue_id=%d)",
> -				__func__, error.message, member_port,
> +		RTE_BOND_LOG(ERR, "%s (member_port=%u queue_id=%u)",
> +				error.message, member_port,
>  				internals->mode4.dedicated_queues.rx_qid);
>  		return -1;
>  	}
> @@ -219,8 +219,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
>  	ret = rte_eth_dev_info_get(member_port, &member_info);
>  	if (ret != 0) {
>  		RTE_BOND_LOG(ERR,
> -			"%s: Error during getting device (port %u) info: %s",
> -			__func__, member_port, strerror(-ret));
> +			"Error during getting device (port %u) info: %s",
> +			member_port, strerror(-ret));
>  
>  		return ret;
>  	}
> @@ -228,8 +228,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
>  	if (member_info.max_rx_queues < bond_dev->data->nb_rx_queues ||
>  			member_info.max_tx_queues < bond_dev->data->nb_tx_queues) {
>  		RTE_BOND_LOG(ERR,
> -			"%s: Member %d capabilities doesn't allow allocating additional queues",
> -			__func__, member_port);
> +			"Member %u capabilities doesn't allow allocating additional queues",
> +			member_port);
>  		return -1;
>  	}
>  
> @@ -249,9 +249,8 @@ bond_8023ad_slow_pkt_hw_filter_supported(uint16_t port_id) {
>  		ret = rte_eth_dev_info_get(bond_dev->data->port_id, &bond_info);
>  		if (ret != 0) {
>  			RTE_BOND_LOG(ERR,
> -				"%s: Error during getting device (port %u) info: %s",
> -				__func__, bond_dev->data->port_id,
> -				strerror(-ret));
> +				"Error during getting device (port %u) info: %s",
> +				bond_dev->data->port_id, strerror(-ret));
>  
>  			return ret;
>  		}
> @@ -2347,10 +2346,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  			ret = rte_eth_dev_info_get(member.port_id, &member_info);
>  			if (ret != 0) {
>  				RTE_BOND_LOG(ERR,
> -					"%s: Error during getting device (port %u) info: %s",
> -					__func__,
> -					member.port_id,
> -					strerror(-ret));
> +					"Error during getting device (port %u) info: %s",
> +					member.port_id, strerror(-ret));
>  
>  				return ret;
>  			}

Re: [PATCH 0/3] net/bond: fix secondary process crash and related cleanup

[Stephen Hemminger]

On Fri, 17 Apr 2026 09:51:34 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Patch 1 fixes the 8023ad dedicated-queue APIs which ignored the return
> of bond_ethdev_mode_set() and left the enabled flag inconsistent on
> failure. Ordered first so the secondary guard added next does not
> leave shared state corrupted.
> 
> Patch 2 fixes the secondary process crash: a bonding port attached in
> a secondary crashes on the first Rx or Tx burst because the probe
> path never installs burst functions. Blackhole stubs are installed
> and mode changes from secondary are rejected. Fully sharing bonding
> state across processes is out of scope.
> 
> Patch 3 drops redundant %s/func from log call sites now that
> RTE_BOND_LOG supplies the prefix.
> 
> Stephen Hemminger (3):
>   net/bonding: restore dedicated queue state on mode set error
>   net/bonding: prevent crash on Rx/Tx from secondary process
>   net/bonding: remove redundant function names from log
> 
>  drivers/net/bonding/rte_eth_bond_8023ad.c | 19 ++++---
>  drivers/net/bonding/rte_eth_bond_api.c    |  4 +-
>  drivers/net/bonding/rte_eth_bond_pmd.c    | 66 ++++++++++++++++++-----
>  3 files changed, 66 insertions(+), 23 deletions(-)
> 

Would like review of this before adding to next-net

[PATCH v2 0/4] net/bond: fixes and cleanup

[Stephen Hemminger]

Automated analysis of the bonding found a few minor things.
The bug fix is in patch 3 for secondary process crash does rx/tx. 

The cleanups are in handling of 8023ad mode setting
and the logging macros.

v2 - feedback about the mode setting and log messages

Stephen Hemminger (4):
  net/bonding: make 8023ad enable function void
  net/bonding: check mode before setting dedicated queues
  net/bonding: prevent crash on Rx/Tx from secondary process
  net/bonding: remove redundant function names from log

 drivers/net/bonding/eth_bond_8023ad_private.h | 17 +----
 drivers/net/bonding/rte_eth_bond_8023ad.c     | 16 ++--
 drivers/net/bonding/rte_eth_bond_api.c        |  4 +-
 drivers/net/bonding/rte_eth_bond_pmd.c        | 73 ++++++++++++++-----
 4 files changed, 67 insertions(+), 43 deletions(-)

-- 
2.53.0

[PATCH v2 1/4] net/bonding: make 8023ad enable function void

[Stephen Hemminger]

The function never returns an error. Cleanup the call sites.

The 8023ad disable function was never implemented,
remove prototype.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/bonding/eth_bond_8023ad_private.h | 17 +----------------
 drivers/net/bonding/rte_eth_bond_8023ad.c     |  4 +---
 drivers/net/bonding/rte_eth_bond_pmd.c        |  7 +++----
 3 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/drivers/net/bonding/eth_bond_8023ad_private.h b/drivers/net/bonding/eth_bond_8023ad_private.h
index ab7d15f81a..bd7a5848de 100644
--- a/drivers/net/bonding/eth_bond_8023ad_private.h
+++ b/drivers/net/bonding/eth_bond_8023ad_private.h
@@ -209,25 +209,10 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
  * @internal
  *
  * Enables 802.1AX mode and all active members on bonding interface.
- *
- * @param dev Bonding interface
- * @return
- *  0 on success, negative value otherwise.
  */
-int
+void
 bond_mode_8023ad_enable(struct rte_eth_dev *dev);
 
-/**
- * @internal
- *
- * Disables 802.1AX mode of the bonding interface and members.
- *
- * @param dev Bonding interface
- * @return
- *   0 on success, negative value otherwise.
- */
-int bond_mode_8023ad_disable(struct rte_eth_dev *dev);
-
 /**
  * @internal
  *
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index ba88f6d261..eba713e381 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1287,7 +1287,7 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
 		bond_mode_8023ad_start(dev);
 }
 
-int
+void
 bond_mode_8023ad_enable(struct rte_eth_dev *bond_dev)
 {
 	struct bond_dev_private *internals = bond_dev->data->dev_private;
@@ -1296,8 +1296,6 @@ bond_mode_8023ad_enable(struct rte_eth_dev *bond_dev)
 	for (i = 0; i < internals->active_member_count; i++)
 		bond_mode_8023ad_activate_member(bond_dev,
 				internals->active_members[i]);
-
-	return 0;
 }
 
 int
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 96725071da..7fcb3ec7d7 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1619,8 +1619,7 @@ bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, uint8_t mode)
 		eth_dev->rx_pkt_burst = bond_ethdev_rx_burst;
 		break;
 	case BONDING_MODE_8023AD:
-		if (bond_mode_8023ad_enable(eth_dev) != 0)
-			return -1;
+		bond_mode_8023ad_enable(eth_dev);
 
 		if (internals->mode4.dedicated_queues.enabled == 0) {
 			eth_dev->rx_pkt_burst = bond_ethdev_rx_burst_8023ad;
@@ -1641,13 +1640,13 @@ bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, uint8_t mode)
 		eth_dev->rx_pkt_burst = bond_ethdev_rx_burst_active_backup;
 		break;
 	case BONDING_MODE_ALB:
-		if (bond_mode_alb_enable(eth_dev) != 0)
-			return -1;
+		bond_mode_alb_enable(eth_dev);
 
 		eth_dev->tx_pkt_burst = bond_ethdev_tx_burst_alb;
 		eth_dev->rx_pkt_burst = bond_ethdev_rx_burst_alb;
 		break;
 	default:
+		RTE_BOND_LOG(ERR, "Invalid mode %#x", mode);
 		return -1;
 	}
 
-- 
2.53.0

[PATCH v2 2/4] net/bonding: check mode before setting dedicated queues

[Stephen Hemminger]

The calls to enable and disable dedicated queues are missing
check for mode.

Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index eba713e381..d1f30229d0 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1725,7 +1725,6 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_8023ad_dedicated_queues_enable)
 int
 rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port)
 {
-	int retval = 0;
 	struct rte_eth_dev *dev;
 	struct bond_dev_private *internals;
 
@@ -1742,17 +1741,19 @@ rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port)
 	if (dev->data->dev_started)
 		return -1;
 
+	if (internals->mode != BONDING_MODE_8023AD)
+		return -1;
+
 	internals->mode4.dedicated_queues.enabled = 1;
 
 	bond_ethdev_mode_set(dev, internals->mode);
-	return retval;
+	return 0;
 }
 
 RTE_EXPORT_SYMBOL(rte_eth_bond_8023ad_dedicated_queues_disable)
 int
 rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port)
 {
-	int retval = 0;
 	struct rte_eth_dev *dev;
 	struct bond_dev_private *internals;
 
@@ -1766,9 +1767,12 @@ rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port)
 	if (dev->data->dev_started)
 		return -1;
 
+	if (internals->mode != BONDING_MODE_8023AD)
+		return -1;
+
 	internals->mode4.dedicated_queues.enabled = 0;
 
 	bond_ethdev_mode_set(dev, internals->mode);
 
-	return retval;
+	return 0;
 }
-- 
2.53.0

[PATCH v2 3/4] net/bonding: prevent crash on Rx/Tx from secondary process

[Stephen Hemminger]

The bonding PMD's secondary process attach path registered the
ethdev but never installed rx_pkt_burst or tx_pkt_burst, leaving
both as NULL. Any rx_burst or tx_burst call from a secondary
process therefore crashed with a NULL pointer dereference.

Fully sharing bonding state across processes would be
overly complex. Instead, install blackhole burst functions
in the secondary so the data path is safe by default. Rx returns 0
and Tx frees the mbufs and reports them as transmitted,
matching /dev/null semantics so applications do not spin
retrying. Each stub logs once at NOTICE level on first use.

Also reject bond mode changes from a secondary process.
rte_eth_bond_mode_set() is callable from secondary, and
without this guard it would overwrite the secondary's safe
burst stubs with real per-mode functions whose internal
state is not valid outside the primary, reintroducing the
crash by another path.

This keeps secondary support available for the more
common use cases of procinfo and packet capture.

Bugzilla ID: 1698
Fixes: 4852aa8f6e21 ("drivers/net: enable hotplug on secondary process")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 43 +++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 7fcb3ec7d7..8a9c329fb8 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -56,6 +56,33 @@ get_vlan_offset(struct rte_ether_hdr *eth_hdr, uint16_t *proto)
 	return vlan_offset;
 }
 
+static uint16_t
+bond_ethdev_rx_secondary(void *queue __rte_unused,
+			 struct rte_mbuf **bufs __rte_unused, uint16_t nb_pkts __rte_unused)
+{
+	static bool once = true;
+
+	if (once) {
+		/* once per process is enough of a notice */
+		RTE_BOND_LOG(ERR, "receive not supported in secondary");
+		once = false;
+	}
+	return 0;
+}
+
+static uint16_t
+bond_ethdev_tx_secondary(void *queue __rte_unused, struct rte_mbuf **bufs, uint16_t nb_pkts)
+{
+	static bool once = true;
+
+	if (once) {
+		RTE_BOND_LOG(ERR, "transmit not supported in secondary");
+		once = false;
+	}
+	rte_pktmbuf_free_bulk(bufs, nb_pkts);
+	return nb_pkts;
+}
+
 static uint16_t
 bond_ethdev_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 {
@@ -1599,6 +1626,11 @@ bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, uint8_t mode)
 {
 	struct bond_dev_private *internals;
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		RTE_BOND_LOG(ERR, "Setting mode in secondary not allowed");
+		return -1;
+	}
+
 	internals = eth_dev->data->dev_private;
 
 	switch (mode) {
@@ -3798,9 +3830,18 @@ bond_probe(struct rte_vdev_device *dev)
 			RTE_BOND_LOG(ERR, "Failed to probe %s", name);
 			return -1;
 		}
-		/* TODO: request info from primary to set up Rx and Tx */
+
 		eth_dev->dev_ops = &default_dev_ops;
 		eth_dev->device = &dev->device;
+
+		/*
+		 * Propagation of bond mode would require adding
+		 * MP client/server support and lots of error handling.
+		 *
+		 * For now just install a black hole.
+		 */
+		eth_dev->tx_pkt_burst = bond_ethdev_tx_secondary;
+		eth_dev->rx_pkt_burst = bond_ethdev_rx_secondary;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
-- 
2.53.0

[PATCH v2 4/4] net/bonding: remove redundant function names from log

[Stephen Hemminger]

The function name is already printed as part of RTE_BOND_LOG().
Fix grammar in log messages.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/bonding/rte_eth_bond_api.c |  4 ++--
 drivers/net/bonding/rte_eth_bond_pmd.c | 23 ++++++++++-------------
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 9e5df67c18..d9b6f1c417 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -485,8 +485,8 @@ __eth_bond_member_add_lock_free(uint16_t bonding_port_id, uint16_t member_port_i
 	ret = rte_eth_dev_info_get(member_port_id, &dev_info);
 	if (ret != 0) {
 		RTE_BOND_LOG(ERR,
-			"%s: Error during getting device (port %u) info: %s",
-			__func__, member_port_id, strerror(-ret));
+			"Error getting device (port %u) info: %s",
+			member_port_id, strerror(-ret));
 
 		return ret;
 	}
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 8a9c329fb8..6a4f997b5a 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -210,8 +210,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
 	int ret = rte_flow_validate(member_port, &flow_attr_8023ad,
 			flow_item_8023ad, actions, &error);
 	if (ret < 0) {
-		RTE_BOND_LOG(ERR, "%s: %s (member_port=%d queue_id=%d)",
-				__func__, error.message, member_port,
+		RTE_BOND_LOG(ERR, "%s (member_port=%u queue_id=%u)",
+				error.message, member_port,
 				internals->mode4.dedicated_queues.rx_qid);
 		return -1;
 	}
@@ -219,8 +219,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
 	ret = rte_eth_dev_info_get(member_port, &member_info);
 	if (ret != 0) {
 		RTE_BOND_LOG(ERR,
-			"%s: Error during getting device (port %u) info: %s",
-			__func__, member_port, strerror(-ret));
+			"Error getting device (port %u) info: %s",
+			member_port, strerror(-ret));
 
 		return ret;
 	}
@@ -228,8 +228,8 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
 	if (member_info.max_rx_queues < bond_dev->data->nb_rx_queues ||
 			member_info.max_tx_queues < bond_dev->data->nb_tx_queues) {
 		RTE_BOND_LOG(ERR,
-			"%s: Member %d capabilities doesn't allow allocating additional queues",
-			__func__, member_port);
+			"Member %u capabilities doesn't allow allocating additional queues",
+			member_port);
 		return -1;
 	}
 
@@ -249,9 +249,8 @@ bond_8023ad_slow_pkt_hw_filter_supported(uint16_t port_id) {
 		ret = rte_eth_dev_info_get(bond_dev->data->port_id, &bond_info);
 		if (ret != 0) {
 			RTE_BOND_LOG(ERR,
-				"%s: Error during getting device (port %u) info: %s",
-				__func__, bond_dev->data->port_id,
-				strerror(-ret));
+				"Error getting device (port %u) info: %s",
+				bond_dev->data->port_id, strerror(-ret));
 
 			return ret;
 		}
@@ -2346,10 +2345,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 			ret = rte_eth_dev_info_get(member.port_id, &member_info);
 			if (ret != 0) {
 				RTE_BOND_LOG(ERR,
-					"%s: Error during getting device (port %u) info: %s",
-					__func__,
-					member.port_id,
-					strerror(-ret));
+					"Error getting device (port %u) info: %s",
+					member.port_id, strerror(-ret));
 
 				return ret;
 			}
-- 
2.53.0

Re: [PATCH v2 0/4] net/bond: fixes and cleanup

[Stephen Hemminger]

On Thu, 28 May 2026 16:59:12 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Automated analysis of the bonding found a few minor things.
> The bug fix is in patch 3 for secondary process crash does rx/tx. 
> 
> The cleanups are in handling of 8023ad mode setting
> and the logging macros.
> 
> v2 - feedback about the mode setting and log messages
> 
> Stephen Hemminger (4):
>   net/bonding: make 8023ad enable function void
>   net/bonding: check mode before setting dedicated queues
>   net/bonding: prevent crash on Rx/Tx from secondary process
>   net/bonding: remove redundant function names from log
> 
>  drivers/net/bonding/eth_bond_8023ad_private.h | 17 +----
>  drivers/net/bonding/rte_eth_bond_8023ad.c     | 16 ++--
>  drivers/net/bonding/rte_eth_bond_api.c        |  4 +-
>  drivers/net/bonding/rte_eth_bond_pmd.c        | 73 ++++++++++++++-----
>  4 files changed, 67 insertions(+), 43 deletions(-)
> 

Ping. Need review and ack for these patches