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

[Chas Williams <3chas3@gmail.com>]

Acked-by: Chas Williams <3chas3@gmail.com>

On 10/17/22 04:40, Andrew Rybchenko wrote:
> 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));
>