Re: [PATCH V2] net/bonding: add link speeds configuration

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

On 9/15/22 22:09, lihuisong (C) wrote:
> 
> 在 2022/9/15 21:43, Chas Williams 写道:
>> On 9/15/22 09:14, Huisong Li wrote:
>>> This patch adds link speeds configuration.
>>>
>>> ---
>>>   -v2: resend due to CI compiling failure.
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> ---
>>>   drivers/net/bonding/rte_eth_bond_pmd.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
>>> b/drivers/net/bonding/rte_eth_bond_pmd.c
>>> index 3191158ca7..0c1dbc3ff6 100644
>>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>>> @@ -1717,6 +1717,8 @@ slave_configure(struct rte_eth_dev 
>>> *bonded_eth_dev,
>>>         slave_eth_dev->data->dev_conf.rxmode.mtu =
>>>               bonded_eth_dev->data->dev_conf.rxmode.mtu;
>>> +    slave_eth_dev->data->dev_conf.link_speeds =
>>> +            bonded_eth_dev->data->dev_conf.link_speeds;
>>>         slave_eth_dev->data->dev_conf.txmode.offloads |=
>>>           bonded_eth_dev->data->dev_conf.txmode.offloads;
>>
>>
>> Where did bonded_eth_dev->data->dev_conf.link_speeds come from? This 
>> is rather messy since you might need to worry about intersection of 
>> speed_capa from the set of members in the bond group.
> Hi chas,
> 
> Thanks for your review.
> 
> bonded_eth_dev->data->dev_conf.link_speeds come from user. I don't think 
> we need to complicate it.
> Not all PMDs report 'speed_capa'. But the slave device knows if the 
> specified speed is supported.
> So I think that it is better to leave to the slave device to determine 
> whether the speed is configured successfully.
>> .

I still think it would be a good idea to create an intersection of the
supported speeds in bonding's dev_conf.speed_capa if we are going to start
trying to set the speeds via the bonding interface. The PMDs that don't
report a valid speed_capa are technically broken.