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

[Thomas Monjalon]

2016-10-26 21:28, Harish Patil:
> 
> >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.
> >
> 
> Hi Thomas,
> Its not really a faking. The same driver supports all three link speeds.
> The required support for 100G was already present in the 16.07 inbox
> driver.
> We just had missed out advertising 100G link speed via
> dev_info->speed_capa.
> Hence it is - Fixes: 2af14ca79c0a ("net/qede: support 100G”).
> Hope it is okay.

Yes the code is okay. But it is not complete.
Please tell me you understand that you must fill speed_capa depending
of the device capabilities.
Then you will be able to fill "Speed capabilities" box in the doc.