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

Bruce Richardson bruce.richardson at intel.com
Wed May 27 18:19:27 CEST 2026


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 at dpdk.org
> 
> Signed-off-by: Stephen Hemminger <stephen at 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



More information about the dev mailing list