Re: [dpdk-dev] [PATCH v4 18/32] net/qede: add missing 100G link speed capability

[Bruce Richardson <bruce.richardson@intel.com>]

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@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@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