On Wed, Oct 26, 2016 at 05:41:58PM +0200, Thomas Monjalon wrote:
> 2016-10-18 21:11, Rasesh Mody:
> > From: Harish Patil <harish.patil at qlogic.com>
> >
> > This patch fixes the missing 100G link speed advertisement
> > when the 100G support was initially added.
> >
> > Fixes: 2af14ca79c0a ("net/qede: support 100G")
> >
> > Signed-off-by: Harish Patil <harish.patil at qlogic.com>
> [...]
> > [Features]
> > +Speed capabilities = Y
>
> This feature should be checked only when it is fully implemented,
> i.e. when you return the real capabilities of the device.
>
> > --- a/drivers/net/qede/qede_ethdev.c
> > +++ b/drivers/net/qede/qede_ethdev.c
> > @@ -599,7 +599,8 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
> > DEV_TX_OFFLOAD_UDP_CKSUM |
> > DEV_TX_OFFLOAD_TCP_CKSUM);
> >
> > - dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G;
> > + dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G |
> > + ETH_LINK_SPEED_100G;
> > }
>
> It is only faking the capabilities at driver-level.
> You should check if the underlying device is able to achieve 100G
> before advertising this flag to the application.
>
> I suggest to update this patch to remove the doc update.
> The contract is to fill it only when the code is fixed.
> By the way, we must call every other drivers to properly implement
> this feature.
I agree that devices should only advertise speeds they support and the
doc should reflect this ability (or lack of this ability) in a driver.
Regards,
/Bruce
