All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] net/bonding: reject control operations in secondary
@ 2026-07-08 17:42 Weijun Pan
  2026-07-22 23:10 ` Stephen Hemminger
  2026-07-26 17:32 ` Stephen Hemminger
  0 siblings, 2 replies; 3+ messages in thread
From: Weijun Pan @ 2026-07-08 17:42 UTC (permalink / raw)
  To: Chas Williams, Min Hu (Connor), Anatoly Burakov; +Cc: dev, Weijun Pan

The bonding PMD installs safe secondary burst functions when real
secondary datapath support is not available. This avoids crashes, but
secondary processes must not be able to change bonding control-plane
state if the primary process is the owner of that state.

Reject bonding control-plane operations from secondary processes. This
keeps bonding configuration and LACP state owned by the primary process
and is a prerequisite for future limited secondary datapath support.

Bugzilla ID: 1900

Signed-off-by: Weijun Pan <wpan3636@gmail.com>
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 31 ++++++++++++
 drivers/net/bonding/rte_eth_bond_api.c    | 47 +++++++++++++++++
 drivers/net/bonding/rte_eth_bond_pmd.c    | 61 +++++++++++++++++++++++
 3 files changed, 139 insertions(+)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index d1f30229d0..685255ffc3 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1428,11 +1428,24 @@ rte_eth_bond_8023ad_conf_get(uint16_t port_id,
 	return 0;
 }
 
+static int
+bond_8023ad_primary_only(const char *op)
+{
+	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
+		return 0;
+
+	RTE_BOND_LOG(ERR, "%s not supported in secondary process", op);
+	return -ENOTSUP;
+}
+
 RTE_EXPORT_SYMBOL(rte_eth_bond_8023ad_agg_selection_set)
 int
 rte_eth_bond_8023ad_agg_selection_set(uint16_t port_id,
 		enum rte_bond_8023ad_agg_selection agg_selection)
 {
+	if (bond_8023ad_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct rte_eth_dev *bond_dev;
 	struct bond_dev_private *internals;
 	struct mode8023ad_private *mode4;
@@ -1506,6 +1519,9 @@ int
 rte_eth_bond_8023ad_setup(uint16_t port_id,
 		struct rte_eth_bond_8023ad_conf *conf)
 {
+	if (bond_8023ad_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct rte_eth_dev *bond_dev;
 	int err;
 
@@ -1590,6 +1606,9 @@ int
 rte_eth_bond_8023ad_ext_collect(uint16_t port_id, uint16_t member_id,
 				int enabled)
 {
+	if (bond_8023ad_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct port *port;
 	int res;
 
@@ -1612,6 +1631,9 @@ int
 rte_eth_bond_8023ad_ext_distrib(uint16_t port_id, uint16_t member_id,
 				int enabled)
 {
+	if (bond_8023ad_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct port *port;
 	int res;
 
@@ -1664,6 +1686,9 @@ int
 rte_eth_bond_8023ad_ext_slowtx(uint16_t port_id, uint16_t member_id,
 		struct rte_mbuf *lacp_pkt)
 {
+	if (bond_8023ad_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct port *port;
 	int res;
 
@@ -1725,6 +1750,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_8023ad_dedicated_queues_enable)
 int
 rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port)
 {
+	if (bond_8023ad_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct rte_eth_dev *dev;
 	struct bond_dev_private *internals;
 
@@ -1754,6 +1782,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_8023ad_dedicated_queues_disable)
 int
 rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port)
 {
+	if (bond_8023ad_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct rte_eth_dev *dev;
 	struct bond_dev_private *internals;
 
diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index d9b6f1c417..3b754bf8e0 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -150,10 +150,24 @@ deactivate_member(struct rte_eth_dev *eth_dev, uint16_t port_id)
 	}
 }
 
+static int
+bond_api_primary_only(const char *op)
+{
+	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
+		return 0;
+
+	RTE_BOND_LOG(ERR, "%s not supported in secondary process", op);
+	return -1;
+}
+
+
 RTE_EXPORT_SYMBOL(rte_eth_bond_create)
 int
 rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct bond_dev_private *internals;
 	struct rte_eth_dev *bond_dev;
 	char devargs[52];
@@ -193,6 +207,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_free)
 int
 rte_eth_bond_free(const char *name)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	return rte_vdev_uninit(name);
 }
 
@@ -638,6 +655,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_member_add)
 int
 rte_eth_bond_member_add(uint16_t bonding_port_id, uint16_t member_port_id)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct rte_eth_dev *bonding_eth_dev;
 	struct bond_dev_private *internals;
 
@@ -777,6 +797,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_member_remove)
 int
 rte_eth_bond_member_remove(uint16_t bonding_port_id, uint16_t member_port_id)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct rte_eth_dev *bonding_eth_dev;
 	struct bond_dev_private *internals;
 	int retval;
@@ -800,6 +823,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_mode_set)
 int
 rte_eth_bond_mode_set(uint16_t bonding_port_id, uint8_t mode)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct rte_eth_dev *bonding_eth_dev;
 
 	if (valid_bonding_port_id(bonding_port_id) != 0)
@@ -832,6 +858,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_primary_set)
 int
 rte_eth_bond_primary_set(uint16_t bonding_port_id, uint16_t member_port_id)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct bond_dev_private *internals;
 
 	if (valid_bonding_port_id(bonding_port_id) != 0)
@@ -921,6 +950,9 @@ int
 rte_eth_bond_mac_address_set(uint16_t bonding_port_id,
 		struct rte_ether_addr *mac_addr)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct rte_eth_dev *bonding_eth_dev;
 	struct bond_dev_private *internals;
 
@@ -947,6 +979,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_mac_address_reset)
 int
 rte_eth_bond_mac_address_reset(uint16_t bonding_port_id)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct rte_eth_dev *bonding_eth_dev;
 	struct bond_dev_private *internals;
 
@@ -989,6 +1024,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_xmit_policy_set)
 int
 rte_eth_bond_xmit_policy_set(uint16_t bonding_port_id, uint8_t policy)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct bond_dev_private *internals;
 
 	if (valid_bonding_port_id(bonding_port_id) != 0)
@@ -1034,6 +1072,9 @@ RTE_EXPORT_SYMBOL(rte_eth_bond_link_monitoring_set)
 int
 rte_eth_bond_link_monitoring_set(uint16_t bonding_port_id, uint32_t internal_ms)
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct bond_dev_private *internals;
 
 	if (valid_bonding_port_id(bonding_port_id) != 0)
@@ -1063,6 +1104,9 @@ rte_eth_bond_link_down_prop_delay_set(uint16_t bonding_port_id,
 				       uint32_t delay_ms)
 
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct bond_dev_private *internals;
 
 	if (valid_bonding_port_id(bonding_port_id) != 0)
@@ -1091,6 +1135,9 @@ int
 rte_eth_bond_link_up_prop_delay_set(uint16_t bonding_port_id, uint32_t delay_ms)
 
 {
+	if (bond_api_primary_only(__func__) != 0)
+		return -1;
+
 	struct bond_dev_private *internals;
 
 	if (valid_bonding_port_id(bonding_port_id) != 0)
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 6a4f997b5a..b6e38d4a60 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2052,9 +2052,22 @@ bond_ethdev_primary_set(struct bond_dev_private *internals,
 static int
 bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev);
 
+static int
+bond_ethdev_primary_only(const char *op)
+{
+	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
+		return 0;
+
+	RTE_BOND_LOG(ERR, "%s not supported in secondary process", op);
+	return -ENOTSUP;
+}
+
 static int
 bond_ethdev_start(struct rte_eth_dev *eth_dev)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_dev_private *internals;
 	int i;
 
@@ -2186,6 +2199,9 @@ bond_ethdev_free_queues(struct rte_eth_dev *dev)
 int
 bond_ethdev_stop(struct rte_eth_dev *eth_dev)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_dev_private *internals = eth_dev->data->dev_private;
 	uint16_t i;
 	int ret;
@@ -2400,6 +2416,9 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 static int
 bond_ethdev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	int res;
 	uint16_t i;
 	struct bond_dev_private *internals = dev->data->dev_private;
@@ -2431,6 +2450,9 @@ bond_ethdev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 		uint16_t nb_rx_desc, unsigned int socket_id __rte_unused,
 		const struct rte_eth_rxconf *rx_conf, struct rte_mempool *mb_pool)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_rx_queue *bd_rx_q = (struct bond_rx_queue *)
 			rte_zmalloc_socket(NULL, sizeof(struct bond_rx_queue),
 					0, dev->data->numa_node);
@@ -2455,6 +2477,9 @@ bond_ethdev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 		uint16_t nb_tx_desc, unsigned int socket_id __rte_unused,
 		const struct rte_eth_txconf *tx_conf)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_tx_queue *bd_tx_q  = (struct bond_tx_queue *)
 			rte_zmalloc_socket(NULL, sizeof(struct bond_tx_queue),
 					0, dev->data->numa_node);
@@ -2697,6 +2722,9 @@ bond_ethdev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats,
 static int
 bond_ethdev_stats_reset(struct rte_eth_dev *dev)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_dev_private *internals = dev->data->dev_private;
 	int i;
 	int err;
@@ -2714,6 +2742,9 @@ bond_ethdev_stats_reset(struct rte_eth_dev *dev)
 static int
 bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_dev_private *internals = eth_dev->data->dev_private;
 	int i;
 	int ret = 0;
@@ -2768,6 +2799,9 @@ bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev)
 static int
 bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_dev_private *internals = dev->data->dev_private;
 	int i;
 	int ret = 0;
@@ -2871,6 +2905,9 @@ bond_ethdev_promiscuous_update(struct rte_eth_dev *dev)
 static int
 bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_dev_private *internals = eth_dev->data->dev_private;
 	int i;
 	int ret = 0;
@@ -2925,6 +2962,9 @@ bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev)
 static int
 bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct bond_dev_private *internals = eth_dev->data->dev_private;
 	int i;
 	int ret = 0;
@@ -3186,6 +3226,9 @@ static int
 bond_ethdev_rss_reta_update(struct rte_eth_dev *dev,
 		struct rte_eth_rss_reta_entry64 *reta_conf, uint16_t reta_size)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	unsigned i, j;
 	int result = 0;
 	int member_reta_size;
@@ -3246,6 +3289,9 @@ static int
 bond_ethdev_rss_hash_update(struct rte_eth_dev *dev,
 		struct rte_eth_rss_conf *rss_conf)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	int i, result = 0;
 	struct bond_dev_private *internals = dev->data->dev_private;
 	struct rte_eth_rss_conf bond_rss_conf;
@@ -3295,6 +3341,9 @@ bond_ethdev_rss_hash_conf_get(struct rte_eth_dev *dev,
 static int
 bond_ethdev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct rte_eth_dev *member_eth_dev;
 	struct bond_dev_private *internals = dev->data->dev_private;
 	int ret, i;
@@ -3324,6 +3373,9 @@ static int
 bond_ethdev_mac_address_set(struct rte_eth_dev *dev,
 			struct rte_ether_addr *addr)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	if (mac_address_set(dev, addr)) {
 		RTE_BOND_LOG(ERR, "Failed to update MAC address");
 		return -EINVAL;
@@ -3345,6 +3397,9 @@ bond_ethdev_mac_addr_add(struct rte_eth_dev *dev,
 			struct rte_ether_addr *mac_addr,
 			__rte_unused uint32_t index, uint32_t vmdq)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	struct rte_eth_dev *member_eth_dev;
 	struct bond_dev_private *internals = dev->data->dev_private;
 	int ret, i;
@@ -3381,6 +3436,9 @@ bond_ethdev_mac_addr_add(struct rte_eth_dev *dev,
 static void
 bond_ethdev_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return;
+
 	struct rte_eth_dev *member_eth_dev;
 	struct bond_dev_private *internals = dev->data->dev_private;
 	int i;
@@ -3965,6 +4023,9 @@ bond_remove(struct rte_vdev_device *dev)
 static int
 bond_ethdev_configure(struct rte_eth_dev *dev)
 {
+	if (bond_ethdev_primary_only(__func__) != 0)
+		return -ENOTSUP;
+
 	const char *name = dev->device->name;
 	struct bond_dev_private *internals = dev->data->dev_private;
 	struct rte_kvargs *kvlist = internals->kvlist;
-- 
2.34.1


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

* Re: [RFC PATCH] net/bonding: reject control operations in secondary
  2026-07-08 17:42 [RFC PATCH] net/bonding: reject control operations in secondary Weijun Pan
@ 2026-07-22 23:10 ` Stephen Hemminger
  2026-07-26 17:32 ` Stephen Hemminger
  1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2026-07-22 23:10 UTC (permalink / raw)
  To: Weijun Pan; +Cc: Chas Williams, Min Hu (Connor), Anatoly Burakov, dev

On Wed,  8 Jul 2026 12:42:04 -0500
Weijun Pan <wpan3636@gmail.com> wrote:

> +static int
> +bond_ethdev_primary_only(const char *op)
> +{
> +	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
> +		return 0;
> +
> +	RTE_BOND_LOG(ERR, "%s not supported in secondary process", op);
> +	return -ENOTSUP;
> +}
> +

Minor nit, the function returns 0 or -ENOTSUP but then caller
always ends up just checking for 0. It might look cleaner as boolean
function or wrap the whole thing as a macro like ethdev does.

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

* Re: [RFC PATCH] net/bonding: reject control operations in secondary
  2026-07-08 17:42 [RFC PATCH] net/bonding: reject control operations in secondary Weijun Pan
  2026-07-22 23:10 ` Stephen Hemminger
@ 2026-07-26 17:32 ` Stephen Hemminger
  1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2026-07-26 17:32 UTC (permalink / raw)
  To: Weijun Pan; +Cc: Chas Williams, Min Hu (Connor), Anatoly Burakov, dev

On Wed,  8 Jul 2026 12:42:04 -0500
Weijun Pan <wpan3636@gmail.com> wrote:

> The bonding PMD installs safe secondary burst functions when real
> secondary datapath support is not available. This avoids crashes, but
> secondary processes must not be able to change bonding control-plane
> state if the primary process is the owner of that state.
> 
> Reject bonding control-plane operations from secondary processes. This
> keeps bonding configuration and LACP state owned by the primary process
> and is a prerequisite for future limited secondary datapath support.
> 
> Bugzilla ID: 1900
> 
> Signed-off-by: Weijun Pan <wpan3636@gmail.com>
> ---

This is the right path, but AI review found things that need work.

Review: [RFC PATCH] net/bonding: reject control operations in secondary
Message-Id: <20260708174204.72574-1-wpan3636@gmail.com>

Verified against DPDK main (38f72e500b3b). Applies cleanly, builds clean
with -Dwerror=true, checkpatches.sh reports no issues.

The intent is right, but the checks are placed one layer too low, which
means the stated goal -- "secondary processes must not be able to change
bonding control-plane state" -- is not actually met for dev_configure,
and one legitimate secondary path is broken.

Errors
------

1. rte_eth_bond_pmd.c: bond_ethdev_configure()

   Returning -ENOTSUP from the dev_configure op does not prevent the
   secondary from mutating shared state -- it makes it worse.
   rte_eth_dev_configure() has already done this before it calls the PMD
   (lib/ethdev/rte_ethdev.c):

     1364  memcpy(&orig_conf, &dev->data->dev_conf, ...)
     1587  diag = eth_dev_rx_queue_config(dev, nb_rx_q);
     1596  diag = eth_dev_tx_queue_config(dev, nb_tx_q);
     1606  diag = dev->dev_ops->dev_configure(dev);
     1646  reset_queues:
     1647    eth_dev_rx_queue_config(dev, 0);
     1648    eth_dev_tx_queue_config(dev, 0);

   On the -ENOTSUP the code jumps to reset_queues, and
   eth_dev_rx_queue_config(dev, 0) calls dev_ops->rx_queue_release() on
   every queue and rte_free()s dev->data->rx_queues, then sets
   nb_rx_queues = 0 (lib/ethdev/ethdev_private.c:455). Those are the
   primary's queues in shared memory. bond_ethdev_rx_queue_release() /
   bond_ethdev_tx_queue_release() are not gated by this patch, so they
   run.

   Net effect: a stray rte_eth_dev_configure() from a secondary now
   destroys the primary's Rx/Tx queue arrays instead of being rejected.

2. rte_eth_bond_api.c: rte_eth_bond_free()

   This blocks the only supported way for a secondary to detach its
   local port. rte_vdev_uninit() is process-local and works in a
   secondary; bond_remove() has a deliberate secondary branch
   (rte_eth_bond_pmd.c:4003):

     if (rte_eal_process_type() != RTE_PROC_PRIMARY)
             return rte_eth_dev_release_port(eth_dev);

   That branch is now unreachable through the public API. Compare
   bond_ethdev_close() ten lines away (2310), which returns 0 rather
   than an error for non-primary, and the comment in
   rte_eth_dev_close() explaining that a secondary must be able to
   close to release its process-private resources. Teardown paths need
   to stay callable; only reconfiguration should be rejected.

Warnings
--------

3. Structural: use a separate ops table rather than 17 in-function
   checks.

   bond_probe() already installs the ops table for the secondary
   explicitly (rte_eth_bond_pmd.c:3889):

     eth_dev->dev_ops = &default_dev_ops;

   ethdev already returns -ENOTSUP for every one of these ops when the
   pointer is NULL -- dev_configure (1347), dev_start (1801),
   rx_queue_setup (2307), promiscuous_enable (3027), mtu_set (4400),
   reta_update (5007), mac_addr_add (5415), mac_addr_remove (5479),
   and so on -- and it does so *before* touching dev->data, which is
   exactly what fixes finding 1. Defining

     static const struct eth_dev_ops secondary_dev_ops = {
             .dev_close         = bond_ethdev_close,
             .dev_infos_get     = bond_ethdev_info,
             .link_update       = bond_ethdev_link_update,
             .stats_get         = bond_ethdev_stats_get,
             .reta_query        = bond_ethdev_rss_reta_query,
             .rss_hash_conf_get = bond_ethdev_rss_hash_conf_get,
             .eth_dev_priv_dump = bond_ethdev_priv_dump,
     };

   and assigning it in the secondary branch of bond_probe() gets you
   identical semantics with one hunk instead of eighteen, and no
   runtime check on the primary's path.

4. Wrong polarity. All three helpers test

     rte_eal_process_type() != RTE_PROC_SECONDARY

   The established idiom is != RTE_PROC_PRIMARY: 357 occurrences under
   drivers/ against 3 of the form used here, and both existing checks
   in this driver (bond_ethdev_close at 2310, bond_remove at 4003) use
   the primary form. As written, RTE_PROC_AUTO and RTE_PROC_INVALID
   fall through to the permissive path.

5. Three byte-identical static helpers in three files
   (bond_8023ad_primary_only, bond_api_primary_only,
   bond_ethdev_primary_only). One static inline in
   eth_bond_private.h.

6. The helper's return value is computed and discarded:

     if (bond_8023ad_primary_only(__func__) != 0)
             return -ENOTSUP;

   Either propagate it (ret = ...; if (ret != 0) return ret;) or make
   the helper return bool and name it accordingly.

7. Coverage gaps. flow_ops_get is not gated, so rte_flow_create() /
   rte_flow_destroy() from a secondary still mutate bonding state.
   rx_queue_release / tx_queue_release are ungated while the
   corresponding setup ops are gated -- see finding 1 for why that
   combination bites.

8. No documentation. This changes the observable behaviour of exported
   API, but doc/guides/prog_guide/link_bonding_poll_mode_drv_lib.rst
   says nothing about multi-process, and
   doc/guides/rel_notes/release_26_07.rst is not touched. The guide
   needs a short section stating which operations are primary-only.

9. Bugzilla ID 1900 is cited with no Fixes: tag and no
   Cc: stable@dpdk.org. If 1900 is a crash being fixed, both are
   needed. If this is groundwork for secondary datapath support, say
   so and drop the implication.

Info
----

10. Statements are inserted ahead of the declarations in every touched
    function. Legal under c11, but it inverts the DPDK function layout
    (declarations, blank line, statements) throughout. Folding the
    check into the existing declaration block would avoid it; the ops
    table in finding 3 avoids it entirely. Also, rte_eth_bond_api.c
    gains a double blank line after bond_api_primary_only().

11. rte_eth_dev_stop() resets that process's fast-path ops to the dummy
    functions before calling dev_stop (rte_ethdev.c:1819). With
    bond_ethdev_stop() now returning -ENOTSUP, a secondary that calls
    stop ends up with a dead local datapath, dev_started still 1, and
    no way to restart since dev_start is also gated.

Given findings 1 and 2, no Reviewed-by on this revision.

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

end of thread, other threads:[~2026-07-26 17:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 17:42 [RFC PATCH] net/bonding: reject control operations in secondary Weijun Pan
2026-07-22 23:10 ` Stephen Hemminger
2026-07-26 17:32 ` Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.