[dpdk-dev] [PATCH v3 27/27] net/ena: set link speed as none

[Michał Krawczyk]

2018-06-10 3:35 GMT+02:00 Chas Williams <3chas3 at gmail.com>:
>
>
> On Fri, Jun 8, 2018 at 3:37 PM Ferruh Yigit <ferruh.yigit at intel.com> wrote:
>>
>> On 6/7/2018 10:43 AM, Michal Krawczyk wrote:
>> > From: Rafal Kozik <rk at semihalf.com>
>> >
>> > Link speed should is not limited to 10Gb/s and it shouldn't be
>> > hardcoded.
>> >
>> > They link speed is set to none instead and the applications shouldn't
>> > rely on this value when using ENA PMD.
>>
>> Why not able to set link speed?
>>
>> And what is the link_autoneg, ETH_LINK_FIXED? What is the point of setting
>> link
>> speed FIXED and later speed value NONE?
>
>
> The link speed is part of the spanning tree path computation.  I don't think
> picking 0 (or undefined)
> is the appropriate choice.  For virtual interfaces link speed is a
> troublesome quantity but advertising
> undefined seems worse then just use some constant value.
>

Setting constant value is not making much sense in the virtualized
environment (ENA can be only used in the cloud). Link speed can be
very flexible in that case and that's why ENA isn't providing it. If
we will use constant value, what it should be? It may lead to
confusion or can result in the performance issues because some
applications are relying on this number to send max traffic.
Applications should not make assumption that the link speed is always
giving valid value and if it is not, they should try to configure rate
dynamically, instead of statically.

>>
>>
>> >
>> > Fixes: 1173fca ("ena: add polling-mode driver")
>>
>> For next time can you please use git alias to have consistent Fixes format
>>
>> >
>> > Signed-off-by: Rafal Kozik <rk at semihalf.com>
>> > Acked-by: Michal Krawczyk <mk at semihalf.com>
>> > ---
>> >  drivers/net/ena/ena_ethdev.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
>> > index 5c3b6494f..9ae73e331 100644
>> > --- a/drivers/net/ena/ena_ethdev.c
>> > +++ b/drivers/net/ena/ena_ethdev.c
>> > @@ -848,7 +848,7 @@ static int ena_link_update(struct rte_eth_dev *dev,
>> >       adapter = (struct ena_adapter *)(dev->data->dev_private);
>> >
>> >       link->link_status = adapter->link_status ? ETH_LINK_UP :
>> > ETH_LINK_DOWN;
>> > -     link->link_speed = ETH_SPEED_NUM_10G;
>> > +     link->link_speed = ETH_SPEED_NUM_NONE;
>> >       link->link_duplex = ETH_LINK_FULL_DUPLEX;
>> >
>> >       return 0;
>> >
>>
>