diff mbox series

[v3,5/7] net/ixgbe: return unknown speed in status

Message ID 20200615090158.18912-6-i.dyukov@samsung.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: allow unknown link speed |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Ivan Dyukov June 15, 2020, 9:01 a.m. UTC 
  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@samsung.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Zhao1, Wei June 15, 2020, 9:28 a.m. UTC | #1 
Hi, dyukov

> -----Original Message-----
> From: Ivan Dyukov <i.dyukov@samsung.com>
> Sent: Monday, June 15, 2020 5:02 PM
> To: dev@dpdk.org; i.dyukov@samsung.com; v.kuramshin@samsung.com;
> thomas@monjalon.net; david.marchand@redhat.com; Yigit, Ferruh
> <ferruh.yigit@intel.com>; arybchenko@solarflare.com; Zhao1, Wei
> <wei.zhao1@intel.com>; Guo, Jia <jia.guo@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [PATCH v3 5/7] net/ixgbe: return unknown speed in status
> 
> 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@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;

You can not delete these specific code for some kind of ixgbe nic!!!


> +		link.link_speed = ETH_SPEED_NUM_UNKNOWN;
>  		break;
> 
>  	case IXGBE_LINK_SPEED_100_FULL:
> --
> 2.17.1
Ferruh Yigit June 17, 2020, 4:50 p.m. UTC | #2 
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@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;
> +		link.link_speed = ETH_SPEED_NUM_UNKNOWN;
>  		break;
>  
>  	case IXGBE_LINK_SPEED_100_FULL:
> 

looks good to me.
Zhao1, Wei June 18, 2020, 1:23 a.m. UTC | #3 
Hi, ferruh

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Thursday, June 18, 2020 12:51 AM
> To: i.dyukov@samsung.com; dev@dpdk.org; v.kuramshin@samsung.com;
> thomas@monjalon.net; david.marchand@redhat.com;
> arybchenko@solarflare.com; Zhao1, Wei <wei.zhao1@intel.com>; Guo, Jia
> <jia.guo@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@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@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.


> > +		link.link_speed = ETH_SPEED_NUM_UNKNOWN;
> >  		break;



> >
> >  	case IXGBE_LINK_SPEED_100_FULL:
> >
> 
> looks good to me.
Ferruh Yigit June 18, 2020, 11:12 a.m. UTC | #4 
On 6/18/2020 2:23 AM, Zhao1, Wei wrote:
> Hi, ferruh
> 
>> -----Original Message-----
>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Sent: Thursday, June 18, 2020 12:51 AM
>> To: i.dyukov@samsung.com; dev@dpdk.org; v.kuramshin@samsung.com;
>> thomas@monjalon.net; david.marchand@redhat.com;
>> arybchenko@solarflare.com; Zhao1, Wei <wei.zhao1@intel.com>; Guo, Jia
>> <jia.guo@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
>> <qiming.yang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@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@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?


> 
> 
>>> +link.link_speed = ETH_SPEED_NUM_UNKNOWN;
>>>  break;
> 
> 
> 
>>>
>>>  case IXGBE_LINK_SPEED_100_FULL:
>>>
>>
>> looks good to me.
Zhao1, Wei June 20, 2020, 3:53 a.m. UTC | #5 
Hi, Ferruh

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Thursday, June 18, 2020 7:12 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; i.dyukov@samsung.com;
> dev@dpdk.org; v.kuramshin@samsung.com; thomas@monjalon.net;
> david.marchand@redhat.com; arybchenko@solarflare.com; Guo, Jia
> <jia.guo@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@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@intel.com>
> >> Sent: Thursday, June 18, 2020 12:51 AM
> >> To: i.dyukov@samsung.com; dev@dpdk.org; v.kuramshin@samsung.com;
> >> thomas@monjalon.net; david.marchand@redhat.com;
> >> arybchenko@solarflare.com; Zhao1, Wei <wei.zhao1@intel.com>; Guo, Jia
> >> <jia.guo@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yang,
> >> Qiming <qiming.yang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@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@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.
Zhao1, Wei June 20, 2020, 3:56 a.m. UTC | #6 
Reviewed-by: Wei Zhao <wei.zhao1@intel.com>

> -----Original Message-----
> From: Ivan Dyukov <i.dyukov@samsung.com>
> Sent: Monday, June 15, 2020 5:02 PM
> To: dev@dpdk.org; i.dyukov@samsung.com; v.kuramshin@samsung.com;
> thomas@monjalon.net; david.marchand@redhat.com; Yigit, Ferruh
> <ferruh.yigit@intel.com>; arybchenko@solarflare.com; Zhao1, Wei
> <wei.zhao1@intel.com>; Guo, Jia <jia.guo@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [PATCH v3 5/7] net/ixgbe: return unknown speed in status
> 
> 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@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;
> +		link.link_speed = ETH_SPEED_NUM_UNKNOWN;
>  		break;
> 
>  	case IXGBE_LINK_SPEED_100_FULL:
> --
> 2.17.1
diff mbox series

Patch

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;
+		link.link_speed = ETH_SPEED_NUM_UNKNOWN;
 		break;
 
 	case IXGBE_LINK_SPEED_100_FULL: