DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] net/bond: fix secondary process crash and related cleanup
@ 2026-04-17 16:51 Stephen Hemminger
  2026-04-17 16:51 ` [PATCH 1/3] net/bonding: restore dedicated queue state on mode set error Stephen Hemminger
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-04-17 16:51 UTC (permalink / raw)
  To: dev; +Cc: 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] net/bonding: restore dedicated queue state on mode set error
  2026-04-17 16:51 [PATCH 0/3] net/bond: fix secondary process crash and related cleanup Stephen Hemminger
@ 2026-04-17 16:51 ` Stephen Hemminger
  2026-04-17 16:51 ` [PATCH 2/3] net/bonding: prevent crash on Rx/Tx from secondary process Stephen Hemminger
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-04-17 16:51 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, stable, Chas Williams, Min Hu (Connor),
	Tomasz Kulasek, Declan Doherty

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] net/bonding: prevent crash on Rx/Tx from secondary process
  2026-04-17 16:51 [PATCH 0/3] net/bond: fix secondary process crash and related cleanup Stephen Hemminger
  2026-04-17 16:51 ` [PATCH 1/3] net/bonding: restore dedicated queue state on mode set error Stephen Hemminger
@ 2026-04-17 16:51 ` Stephen Hemminger
  2026-04-17 16:51 ` [PATCH 3/3] net/bonding: remove redundant function names from log Stephen Hemminger
  2026-05-08 17:59 ` [PATCH 0/3] net/bond: fix secondary process crash and related cleanup Stephen Hemminger
  3 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-04-17 16:51 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, stable, Chas Williams, Min Hu (Connor),
	Anatoly Burakov, Qi Zhang

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] net/bonding: remove redundant function names from log
  2026-04-17 16:51 [PATCH 0/3] net/bond: fix secondary process crash and related cleanup Stephen Hemminger
  2026-04-17 16:51 ` [PATCH 1/3] net/bonding: restore dedicated queue state on mode set error Stephen Hemminger
  2026-04-17 16:51 ` [PATCH 2/3] net/bonding: prevent crash on Rx/Tx from secondary process Stephen Hemminger
@ 2026-04-17 16:51 ` Stephen Hemminger
  2026-05-08 17:59 ` [PATCH 0/3] net/bond: fix secondary process crash and related cleanup Stephen Hemminger
  3 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-04-17 16:51 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Chas Williams, Min Hu (Connor)

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] net/bond: fix secondary process crash and related cleanup
  2026-04-17 16:51 [PATCH 0/3] net/bond: fix secondary process crash and related cleanup Stephen Hemminger
                   ` (2 preceding siblings ...)
  2026-04-17 16:51 ` [PATCH 3/3] net/bonding: remove redundant function names from log Stephen Hemminger
@ 2026-05-08 17:59 ` Stephen Hemminger
  3 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-05-08 17:59 UTC (permalink / raw)
  To: dev; +Cc: Chas Williams, Min Hu (Connor)

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-08 17:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 16:51 [PATCH 0/3] net/bond: fix secondary process crash and related cleanup Stephen Hemminger
2026-04-17 16:51 ` [PATCH 1/3] net/bonding: restore dedicated queue state on mode set error Stephen Hemminger
2026-04-17 16:51 ` [PATCH 2/3] net/bonding: prevent crash on Rx/Tx from secondary process Stephen Hemminger
2026-04-17 16:51 ` [PATCH 3/3] net/bonding: remove redundant function names from log Stephen Hemminger
2026-05-08 17:59 ` [PATCH 0/3] net/bond: fix secondary process crash and related cleanup Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox