[dpdk-dev] [PATCH v3 5/7] net/ixgbe: return unknown speed in status

[Zhao1, Wei]

Hi, Ferruh

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit at intel.com>
> Sent: Thursday, June 18, 2020 7:12 PM
> To: Zhao1, Wei <wei.zhao1 at intel.com>; i.dyukov at samsung.com;
> dev at dpdk.org; v.kuramshin at samsung.com; thomas at monjalon.net;
> david.marchand at redhat.com; arybchenko at solarflare.com; Guo, Jia
> <jia.guo at intel.com>; Xing, Beilei <beilei.xing at intel.com>; Yang, Qiming
> <qiming.yang at intel.com>; Lu, Wenzhuo <wenzhuo.lu at intel.com>
> Subject: Re: [PATCH v3 5/7] net/ixgbe: return unknown speed in status
> 
> On 6/18/2020 2:23 AM, Zhao1, Wei wrote:
> > Hi, ferruh
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh <ferruh.yigit at intel.com>
> >> Sent: Thursday, June 18, 2020 12:51 AM
> >> To: i.dyukov at samsung.com; dev at dpdk.org; v.kuramshin at samsung.com;
> >> thomas at monjalon.net; david.marchand at redhat.com;
> >> arybchenko at solarflare.com; Zhao1, Wei <wei.zhao1 at intel.com>; Guo, Jia
> >> <jia.guo at intel.com>; Xing, Beilei <beilei.xing at intel.com>; Yang,
> >> Qiming <qiming.yang at intel.com>; Lu, Wenzhuo <wenzhuo.lu at intel.com>
> >> Subject: Re: [PATCH v3 5/7] net/ixgbe: return unknown speed in status
> >>
> >> On 6/15/2020 10:01 AM, Ivan Dyukov wrote:
> >>> rte_ethdev has declared new NUM_UNKNOWN speed which could be used
> in
> >>> case when no speed information is available
> >>>
> >>> Signed-off-by: Ivan Dyukov <i.dyukov at samsung.com>
> >>> ---
> >>>  drivers/net/ixgbe/ixgbe_ethdev.c | 6 +-----
> >>>  1 file changed, 1 insertion(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> index a4e5c539d..5b9b13058 100644
> >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> @@ -4311,11 +4311,7 @@ ixgbe_dev_link_update_share(struct
> >> rte_eth_dev *dev,
> >>>  switch (link_speed) {
> >>>  default:
> >>>  case IXGBE_LINK_SPEED_UNKNOWN:
> >>> -if (hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T ||
> >>> -hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T_L)
> >>> -link.link_speed = ETH_SPEED_NUM_10M; -else -link.link_speed =
> >>> ETH_SPEED_NUM_100M;
> >
> > For ixgbe x553(IXGBE_DEV_ID_X550EM_A_1G_T),  we must do some
> adaption, we can not delete these specific code for the kind of ixgbe nic.
> 
> Hi Wei,
> 
> These checks are done when 'link_speed' is 'IXGBE_LINK_SPEED_UNKNOWN'.
> 
> I assume we are setting some default values based on device type when link
> speed is unknown. Using new 'ETH_SPEED_NUM_UNKNOWN' type when link
> speed is unknown can be more accurate.
> 
> For 'IXGBE_DEV_ID_X550EM_A_1G_T', is link speed
> 'IXGBE_LINK_SPEED_UNKNOWN'
> explicitly means 'ETH_SPEED_NUM_10M'?
> If so why it doesn't return 'IXGBE_LINK_SPEED_10_FULL' instead?


After do a double check, it seems base code ixgbe_check_mac_link_generic() has do the adaption, 
		if (hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T ||
		    hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T_L)
			*speed = IXGBE_LINK_SPEED_10_FULL;
so we only need do some upodate in ixgbe_dev_link_update_share(), add the case IXGBE_LINK_SPEED_10_FULL.
So, this patch set is ok now, I think.


> 
> 
> >
> >
> >>> +link.link_speed = ETH_SPEED_NUM_UNKNOWN;
> >>>  break;
> >
> >
> >
> >>>
> >>>  case IXGBE_LINK_SPEED_100_FULL:
> >>>
> >>
> >> looks good to me.