Re: [PATCH] net/bonding: fix descriptor limit reporting

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

Chas, Cornor, could you review the patch, please.

Thanks,
Andrew.

On 9/11/22 15:19, Ivan Malov wrote:
> Commit 5be3b40fea60 ("net/bonding: fix values of descriptor limits")
> breaks reporting of "nb_min" and "nb_align" values obtained from
> back-end devices' descriptor limits. This means that work done
> by eth_bond_slave_inherit_desc_lim_first() as well as
> eth_bond_slave_inherit_desc_lim_next() gets dismissed.
> 
> Revert the offending commit and use proper workaround
> for the test case mentioned in the said commit.
> 
> Meanwhile, the test case itself might be poorly constructed.
> It tries to run a bond with no back-end devices attached,
> but, according to [1] ("Requirements / Limitations"),
> at least one back-end device must be attached.
> 
> [1] doc/guides/prog_guide/link_bonding_poll_mode_drv_lib.rst
> 
> Fixes: 5be3b40fea60 ("net/bonding: fix values of descriptor limits")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 3191158ca7..a5429f5e97 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -2200,8 +2200,6 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   
>   	uint16_t max_nb_rx_queues = UINT16_MAX;
>   	uint16_t max_nb_tx_queues = UINT16_MAX;
> -	uint16_t max_rx_desc_lim = UINT16_MAX;
> -	uint16_t max_tx_desc_lim = UINT16_MAX;
>   
>   	dev_info->max_mac_addrs = BOND_MAX_MAC_ADDRS;
>   
> @@ -2235,12 +2233,6 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   
>   			if (slave_info.max_tx_queues < max_nb_tx_queues)
>   				max_nb_tx_queues = slave_info.max_tx_queues;
> -
> -			if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
> -				max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;
> -
> -			if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
> -				max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;
>   		}
>   	}
>   
> @@ -2252,8 +2244,10 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   	memcpy(&dev_info->default_txconf, &internals->default_txconf,
>   	       sizeof(dev_info->default_txconf));
>   
> -	dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
> -	dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;
> +	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
> +	       sizeof(dev_info->rx_desc_lim));
> +	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
> +	       sizeof(dev_info->tx_desc_lim));
>   
>   	/**
>   	 * If dedicated hw queues enabled for link bonding device in LACP mode
> @@ -3387,6 +3381,13 @@ bond_alloc(struct rte_vdev_device *dev, uint8_t mode)
>   	memset(&internals->rx_desc_lim, 0, sizeof(internals->rx_desc_lim));
>   	memset(&internals->tx_desc_lim, 0, sizeof(internals->tx_desc_lim));
>   
> +	/*
> +	 * Do not restrict descriptor counts until
> +	 * the first back-end device gets attached.
> +	 */
> +	internals->rx_desc_lim.nb_max = UINT16_MAX;
> +	internals->tx_desc_lim.nb_max = UINT16_MAX;
> +
>   	memset(internals->active_slaves, 0, sizeof(internals->active_slaves));
>   	memset(internals->slaves, 0, sizeof(internals->slaves));
>