patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
@ 2020-01-16  1:19 Zhike Wang
  2020-03-12  9:13 ` 王志克
  2020-03-13  4:40 ` Li, Xiaoyun
  0 siblings, 2 replies; 10+ messages in thread
From: Zhike Wang @ 2020-01-16  1:19 UTC (permalink / raw)
  To: dev; +Cc: beilei.xing, qi.z.zhang, stable, Zhike Wang

X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
/I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge
whether it is X722 or not.

Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
Signed-off-by: Zhike Wang <wangzhike@jd.com>
---
 drivers/net/i40e/i40e_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5999c96..85ccb76 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1443,7 +1443,7 @@ static inline void i40e_config_automask(struct i40e_pf *pf)
 		return -EIO;
 	}
 	/* Firmware of SFP x722 does not support adminq option */
-	if (hw->device_id == I40E_DEV_ID_SFP_X722)
+	if (hw->mac.type == I40E_MAC_X722)
 		hw->flags &= ~I40E_HW_FLAG_802_1AD_CAPABLE;
 
 	PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM %02d.%02d.%02d eetrack %04x",
-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-01-16  1:19 [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable adminq operation Zhike Wang
@ 2020-03-12  9:13 ` 王志克
  2020-03-13  4:40 ` Li, Xiaoyun
  1 sibling, 0 replies; 10+ messages in thread
From: 王志克 @ 2020-03-12  9:13 UTC (permalink / raw)
  To: 王志克, dev; +Cc: beilei.xing, qi.z.zhang, stable

Hi,

Any one to review it? I saw there is a build error on 0-day robert, but it seems environment issue (apt get failed).

Br,
Zhike Wang 
JDCloud, Product Development, IaaS   
------------------------------------------------------------------------------------------------
Mobile/+86 13466719566
E- mail/wangzhike@jd.com
Address/5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang District Beijing
Https://JDCloud.com
------------------------------------------------------------------------------------------------


-----Original Message-----
From: wangzk320@163.com [mailto:wangzk320@163.com] On Behalf Of Zhike Wang
Sent: Thursday, January 16, 2020 9:20 AM
To: dev@dpdk.org
Cc: beilei.xing@intel.com; qi.z.zhang@intel.com; stable@dpdk.org; 王志克
Subject: [PATCH] net/i40e: fix X722 judgement when disable adminq operation

X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
/I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge
whether it is X722 or not.

Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
Signed-off-by: Zhike Wang <wangzhike@jd.com>
---
 drivers/net/i40e/i40e_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5999c96..85ccb76 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1443,7 +1443,7 @@ static inline void i40e_config_automask(struct i40e_pf *pf)
 		return -EIO;
 	}
 	/* Firmware of SFP x722 does not support adminq option */
-	if (hw->device_id == I40E_DEV_ID_SFP_X722)
+	if (hw->mac.type == I40E_MAC_X722)
 		hw->flags &= ~I40E_HW_FLAG_802_1AD_CAPABLE;
 
 	PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM %02d.%02d.%02d eetrack %04x",
-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-01-16  1:19 [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable adminq operation Zhike Wang
  2020-03-12  9:13 ` 王志克
@ 2020-03-13  4:40 ` Li, Xiaoyun
  2020-03-13  5:22   ` [dpdk-stable] [dpdk-dev] " Ye Xiaolong
  2020-03-13 19:16   ` Kevin Traynor
  1 sibling, 2 replies; 10+ messages in thread
From: Li, Xiaoyun @ 2020-03-13  4:40 UTC (permalink / raw)
  To: Zhike Wang, dev; +Cc: Xing, Beilei, Zhang, Qi Z, stable

Hi

This issue is already fixed in 20.02 with base code update of the following commit.



commit 37b091c75b13d2f26359be9b77adbc33c55a7581

Author: Xiaolong Ye <xiaolong.ye@intel.com>

Date:   Mon Jan 13 10:39:31 2020 +0800



    net/i40e/base: extend PHY access AQ command



    Currently FW use MDIO I/F number corresponded with current PF for PHY

    access. This code allow to specify used MDIO I/F number.



    Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>

    Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>

    Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Acked-by: Beilei Xing beilei.xing@intel.com<mailto:beilei.xing@intel.com>



But it makes sense to back port this to stable release so it can work with dpdk version <=19.11.

Still some comments inline.



> -----Original Message-----

> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Zhike Wang

> Sent: Thursday, January 16, 2020 09:20

> To: dev@dpdk.org

> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;

> stable@dpdk.org; Zhike Wang <wangzhike@jd.com>

> Subject: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable

> adminq operation

>

> X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722

> /I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge whether it is

> X722 or not.

>

Please be careful about your commit log. If you use mac.type == X722, then that's not just X722 SFP but all X722 devices.

You can just say something like X722 doesn't support adminq operation so disable it.



> Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")

Please follow the DPDK community code style. Fix line needs commit id of 12 bit.

In this case, should be: Fixes: 9efa8d28b4da ("net/i40e: fix SFP X722 with FW4.16")

And since this fix makes sense for <=19.11, please add this line: Cc: stable@dpdk.org



> Signed-off-by: Zhike Wang <wangzhike@jd.com<mailto:wangzhike@jd.com>>

> ---

>  drivers/net/i40e/i40e_ethdev.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c

> index 5999c96..85ccb76 100644

> --- a/drivers/net/i40e/i40e_ethdev.c

> +++ b/drivers/net/i40e/i40e_ethdev.c

> @@ -1443,7 +1443,7 @@ static inline void i40e_config_automask(struct

> i40e_pf *pf)

>                            return -EIO;

>             }

>             /* Firmware of SFP x722 does not support adminq option */

> -          if (hw->device_id == I40E_DEV_ID_SFP_X722)

> +          if (hw->mac.type == I40E_MAC_X722)

>                            hw->flags &= ~I40E_HW_FLAG_802_1AD_CAPABLE;

>

>             PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM %02d.%02d.%02d

> eetrack %04x",

> --

> 1.8.3.1

>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-03-13  4:40 ` Li, Xiaoyun
@ 2020-03-13  5:22   ` Ye Xiaolong
  2020-03-13  5:57     ` Li, Xiaoyun
  2020-03-13 19:16   ` Kevin Traynor
  1 sibling, 1 reply; 10+ messages in thread
From: Ye Xiaolong @ 2020-03-13  5:22 UTC (permalink / raw)
  To: Li, Xiaoyun; +Cc: Zhike Wang, dev, Xing, Beilei, Zhang, Qi Z, stable

Hi, Xiaoyun

Thanks for the review, just want to double check, are you suggesting that
this fix should just be applied to LTS? Latest code doesn't need it, right?  

Thanks,
Xiaolong

On 03/13, Li, Xiaoyun wrote:
>Hi
>
>This issue is already fixed in 20.02 with base code update of the following commit.
>
>
>
>commit 37b091c75b13d2f26359be9b77adbc33c55a7581
>
>Author: Xiaolong Ye <xiaolong.ye@intel.com>
>
>Date:   Mon Jan 13 10:39:31 2020 +0800
>
>
>
>    net/i40e/base: extend PHY access AQ command
>
>
>
>    Currently FW use MDIO I/F number corresponded with current PF for PHY
>
>    access. This code allow to specify used MDIO I/F number.
>
>
>
>    Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
>
>    Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>
>    Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>
>Acked-by: Beilei Xing beilei.xing@intel.com<mailto:beilei.xing@intel.com>
>
>
>
>But it makes sense to back port this to stable release so it can work with dpdk version <=19.11.
>
>Still some comments inline.
>
>
>
>> -----Original Message-----
>
>> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Zhike Wang
>
>> Sent: Thursday, January 16, 2020 09:20
>
>> To: dev@dpdk.org
>
>> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>
>> stable@dpdk.org; Zhike Wang <wangzhike@jd.com>
>
>> Subject: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable
>
>> adminq operation
>
>>
>
>> X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
>
>> /I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge whether it is
>
>> X722 or not.
>
>>
>
>Please be careful about your commit log. If you use mac.type == X722, then that's not just X722 SFP but all X722 devices.
>
>You can just say something like X722 doesn't support adminq operation so disable it.
>
>
>
>> Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
>
>Please follow the DPDK community code style. Fix line needs commit id of 12 bit.
>
>In this case, should be: Fixes: 9efa8d28b4da ("net/i40e: fix SFP X722 with FW4.16")
>
>And since this fix makes sense for <=19.11, please add this line: Cc: stable@dpdk.org
>
>
>
>> Signed-off-by: Zhike Wang <wangzhike@jd.com<mailto:wangzhike@jd.com>>
>
>> ---
>
>>  drivers/net/i40e/i40e_ethdev.c | 2 +-
>
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>>
>
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>
>> index 5999c96..85ccb76 100644
>
>> --- a/drivers/net/i40e/i40e_ethdev.c
>
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>
>> @@ -1443,7 +1443,7 @@ static inline void i40e_config_automask(struct
>
>> i40e_pf *pf)
>
>>                            return -EIO;
>
>>             }
>
>>             /* Firmware of SFP x722 does not support adminq option */
>
>> -          if (hw->device_id == I40E_DEV_ID_SFP_X722)
>
>> +          if (hw->mac.type == I40E_MAC_X722)
>
>>                            hw->flags &= ~I40E_HW_FLAG_802_1AD_CAPABLE;
>
>>
>
>>             PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM %02d.%02d.%02d
>
>> eetrack %04x",
>
>> --
>
>> 1.8.3.1
>
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-03-13  5:22   ` [dpdk-stable] [dpdk-dev] " Ye Xiaolong
@ 2020-03-13  5:57     ` Li, Xiaoyun
  0 siblings, 0 replies; 10+ messages in thread
From: Li, Xiaoyun @ 2020-03-13  5:57 UTC (permalink / raw)
  To: Ye, Xiaolong; +Cc: Zhike Wang, dev, Xing, Beilei, Zhang, Qi Z, stable

Yes. But I'm OK if it's applied to later since it won't take effect anyway with the share code change in 20.02.

> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Friday, March 13, 2020 13:22
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: Zhike Wang <wangzhike@jd.com>; dev@dpdk.org; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix X722 judgement
> when disable adminq operation
> 
> Hi, Xiaoyun
> 
> Thanks for the review, just want to double check, are you suggesting that this fix
> should just be applied to LTS? Latest code doesn't need it, right?
> 
> Thanks,
> Xiaolong
> 
> On 03/13, Li, Xiaoyun wrote:
> >Hi
> >
> >This issue is already fixed in 20.02 with base code update of the following
> commit.
> >
> >
> >
> >commit 37b091c75b13d2f26359be9b77adbc33c55a7581
> >
> >Author: Xiaolong Ye <xiaolong.ye@intel.com>
> >
> >Date:   Mon Jan 13 10:39:31 2020 +0800
> >
> >
> >
> >    net/i40e/base: extend PHY access AQ command
> >
> >
> >
> >    Currently FW use MDIO I/F number corresponded with current PF for
> > PHY
> >
> >    access. This code allow to specify used MDIO I/F number.
> >
> >
> >
> >    Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
> >
> >    Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >
> >    Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >
> >Acked-by: Beilei Xing
> >beilei.xing@intel.com<mailto:beilei.xing@intel.com>
> >
> >
> >
> >But it makes sense to back port this to stable release so it can work with dpdk
> version <=19.11.
> >
> >Still some comments inline.
> >
> >
> >
> >> -----Original Message-----
> >
> >> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Zhike Wang
> >
> >> Sent: Thursday, January 16, 2020 09:20
> >
> >> To: dev@dpdk.org
> >
> >> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>;
> >
> >> stable@dpdk.org; Zhike Wang <wangzhike@jd.com>
> >
> >> Subject: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when
> >> disable
> >
> >> adminq operation
> >
> >>
> >
> >> X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
> >
> >> /I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge whether
> >> it is
> >
> >> X722 or not.
> >
> >>
> >
> >Please be careful about your commit log. If you use mac.type == X722, then
> that's not just X722 SFP but all X722 devices.
> >
> >You can just say something like X722 doesn't support adminq operation so
> disable it.
> >
> >
> >
> >> Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
> >
> >Please follow the DPDK community code style. Fix line needs commit id of 12
> bit.
> >
> >In this case, should be: Fixes: 9efa8d28b4da ("net/i40e: fix SFP X722
> >with FW4.16")
> >
> >And since this fix makes sense for <=19.11, please add this line: Cc:
> >stable@dpdk.org
> >
> >
> >
> >> Signed-off-by: Zhike Wang <wangzhike@jd.com<mailto:wangzhike@jd.com>>
> >
> >> ---
> >
> >>  drivers/net/i40e/i40e_ethdev.c | 2 +-
> >
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >>
> >
> >> diff --git a/drivers/net/i40e/i40e_ethdev.c
> >> b/drivers/net/i40e/i40e_ethdev.c
> >
> >> index 5999c96..85ccb76 100644
> >
> >> --- a/drivers/net/i40e/i40e_ethdev.c
> >
> >> +++ b/drivers/net/i40e/i40e_ethdev.c
> >
> >> @@ -1443,7 +1443,7 @@ static inline void i40e_config_automask(struct
> >
> >> i40e_pf *pf)
> >
> >>                            return -EIO;
> >
> >>             }
> >
> >>             /* Firmware of SFP x722 does not support adminq option */
> >
> >> -          if (hw->device_id == I40E_DEV_ID_SFP_X722)
> >
> >> +          if (hw->mac.type == I40E_MAC_X722)
> >
> >>                            hw->flags &=
> >> ~I40E_HW_FLAG_802_1AD_CAPABLE;
> >
> >>
> >
> >>             PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM %02d.%02d.%02d
> >
> >> eetrack %04x",
> >
> >> --
> >
> >> 1.8.3.1
> >
> >>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-03-13  4:40 ` Li, Xiaoyun
  2020-03-13  5:22   ` [dpdk-stable] [dpdk-dev] " Ye Xiaolong
@ 2020-03-13 19:16   ` Kevin Traynor
  2020-03-16  2:02     ` Li, Xiaoyun
  1 sibling, 1 reply; 10+ messages in thread
From: Kevin Traynor @ 2020-03-13 19:16 UTC (permalink / raw)
  To: Li, Xiaoyun, Zhike Wang, dev; +Cc: Xing, Beilei, Zhang, Qi Z, stable

On 13/03/2020 04:40, Li, Xiaoyun wrote:
> Hi
> 
> This issue is already fixed in 20.02 with base code update of the following commit.
> 
> 
> 
> commit 37b091c75b13d2f26359be9b77adbc33c55a7581
> 
> Author: Xiaolong Ye <xiaolong.ye@intel.com>
> 
> Date:   Mon Jan 13 10:39:31 2020 +0800
> 
> 
> 
>     net/i40e/base: extend PHY access AQ command
> 
> 
> 
>     Currently FW use MDIO I/F number corresponded with current PF for PHY
> 
>     access. This code allow to specify used MDIO I/F number.
> 
> 
> 
>     Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
> 
>     Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> 
>     Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Acked-by: Beilei Xing beilei.xing@intel.com<mailto:beilei.xing@intel.com>
> 
> 
> 
> But it makes sense to back port this to stable release so it can work with dpdk version <=19.11.
> 
> Still some comments inline.
> 
> 
> 
>> -----Original Message-----
> 
>> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Zhike Wang
> 
>> Sent: Thursday, January 16, 2020 09:20
> 
>> To: dev@dpdk.org
> 
>> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> 
>> stable@dpdk.org; Zhike Wang <wangzhike@jd.com>
> 
>> Subject: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable
> 
>> adminq operation
> 
>>
> 
>> X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
> 
>> /I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge whether it is
> 
>> X722 or not.
> 
>>
> 
> Please be careful about your commit log. If you use mac.type == X722, then that's not just X722 SFP but all X722 devices.
> 
> You can just say something like X722 doesn't support adminq operation so disable it.
> 

You say commit log, but isn't that what is done in the code?

> 
> 
>> Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
> 
> Please follow the DPDK community code style. Fix line needs commit id of 12 bit.
> 
> In this case, should be: Fixes: 9efa8d28b4da ("net/i40e: fix SFP X722 with FW4.16")
> 
> And since this fix makes sense for <=19.11, please add this line: Cc: stable@dpdk.org
> 

Please add the stable branches requested in the usual format.

http://doc.dpdk.org/guides/contributing/patches.html#backporting-patches-for-stable-releases

> 
> 
>> Signed-off-by: Zhike Wang <wangzhike@jd.com<mailto:wangzhike@jd.com>>
> 
>> ---
> 
>>  drivers/net/i40e/i40e_ethdev.c | 2 +-
> 
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>>
> 
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> 
>> index 5999c96..85ccb76 100644
> 
>> --- a/drivers/net/i40e/i40e_ethdev.c
> 
>> +++ b/drivers/net/i40e/i40e_ethdev.c
> 
>> @@ -1443,7 +1443,7 @@ static inline void i40e_config_automask(struct
> 
>> i40e_pf *pf)
> 
>>                            return -EIO;
> 
>>             }
> 
>>             /* Firmware of SFP x722 does not support adminq option */
> 
>> -          if (hw->device_id == I40E_DEV_ID_SFP_X722)
> 
>> +          if (hw->mac.type == I40E_MAC_X722)
> 
>>                            hw->flags &= ~I40E_HW_FLAG_802_1AD_CAPABLE;
> 
>>
> 
>>             PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM %02d.%02d.%02d
> 
>> eetrack %04x",
> 
>> --
> 
>> 1.8.3.1
> 
>>
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-03-13 19:16   ` Kevin Traynor
@ 2020-03-16  2:02     ` Li, Xiaoyun
  2020-03-16 10:15       ` Kevin Traynor
  0 siblings, 1 reply; 10+ messages in thread
From: Li, Xiaoyun @ 2020-03-16  2:02 UTC (permalink / raw)
  To: Kevin Traynor, Zhike Wang, dev; +Cc: Xing, Beilei, Zhang, Qi Z, stable



> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Saturday, March 14, 2020 03:16
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhike Wang <wangzhike@jd.com>;
> dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix X722 judgement
> when disable adminq operation
> 
> On 13/03/2020 04:40, Li, Xiaoyun wrote:
> > Hi
> >
> > This issue is already fixed in 20.02 with base code update of the following
> commit.
> >
> >
> >
> > commit 37b091c75b13d2f26359be9b77adbc33c55a7581
> >
> > Author: Xiaolong Ye <xiaolong.ye@intel.com>
> >
> > Date:   Mon Jan 13 10:39:31 2020 +0800
> >
> >
> >
> >     net/i40e/base: extend PHY access AQ command
> >
> >
> >
> >     Currently FW use MDIO I/F number corresponded with current PF for PHY
> >
> >     access. This code allow to specify used MDIO I/F number.
> >
> >
> >
> >     Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
> >
> >     Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >
> >     Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >
> > Acked-by: Beilei Xing beilei.xing@intel.com<mailto:beilei.xing@intel.com>
> >
> >
> >
> > But it makes sense to back port this to stable release so it can work with dpdk
> version <=19.11.
> >
> > Still some comments inline.
> >
> >
> >
> >> -----Original Message-----
> >
> >> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Zhike Wang
> >
> >> Sent: Thursday, January 16, 2020 09:20
> >
> >> To: dev@dpdk.org
> >
> >> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >
> >> stable@dpdk.org; Zhike Wang <wangzhike@jd.com>
> >
> >> Subject: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable
> >
> >> adminq operation
> >
> >>
> >
> >> X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
> >
> >> /I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge whether it is
> >
> >> X722 or not.
> >
> >>
> >
> > Please be careful about your commit log. If you use mac.type == X722, then
> that's not just X722 SFP but all X722 devices.
> >
> > You can just say something like X722 doesn't support adminq operation so
> disable it.
> >
> 
> You say commit log, but isn't that what is done in the code?

His commit log only says about X722 SFPs but mac.type == X722 means all X722 devices. SFPs are only a part of X722.
So I prefer his commit log can explain the root cause.

> 
> >
> >
> >> Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
> >
> > Please follow the DPDK community code style. Fix line needs commit id of 12
> bit.
> >
> > In this case, should be: Fixes: 9efa8d28b4da ("net/i40e: fix SFP X722 with
> FW4.16")
> >
> > And since this fix makes sense for <=19.11, please add this line: Cc:
> stable@dpdk.org
> >
> 
> Please add the stable branches requested in the usual format.
> 
> http://doc.dpdk.org/guides/contributing/patches.html#backporting-patches-
> for-stable-releases
> 
> >
> >
> >> Signed-off-by: Zhike Wang <wangzhike@jd.com<mailto:wangzhike@jd.com>>
> >
> >> ---
> >
> >>  drivers/net/i40e/i40e_ethdev.c | 2 +-
> >
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >>
> >
> >> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> >
> >> index 5999c96..85ccb76 100644
> >
> >> --- a/drivers/net/i40e/i40e_ethdev.c
> >
> >> +++ b/drivers/net/i40e/i40e_ethdev.c
> >
> >> @@ -1443,7 +1443,7 @@ static inline void i40e_config_automask(struct
> >
> >> i40e_pf *pf)
> >
> >>                            return -EIO;
> >
> >>             }
> >
> >>             /* Firmware of SFP x722 does not support adminq option */
> >
> >> -          if (hw->device_id == I40E_DEV_ID_SFP_X722)
> >
> >> +          if (hw->mac.type == I40E_MAC_X722)
> >
> >>                            hw->flags &= ~I40E_HW_FLAG_802_1AD_CAPABLE;
> >
> >>
> >
> >>             PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM %02d.%02d.%02d
> >
> >> eetrack %04x",
> >
> >> --
> >
> >> 1.8.3.1
> >
> >>
> >


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-03-16  2:02     ` Li, Xiaoyun
@ 2020-03-16 10:15       ` Kevin Traynor
  2020-03-17  1:35         ` Li, Xiaoyun
  0 siblings, 1 reply; 10+ messages in thread
From: Kevin Traynor @ 2020-03-16 10:15 UTC (permalink / raw)
  To: Li, Xiaoyun, Zhike Wang, dev; +Cc: Xing, Beilei, Zhang, Qi Z, stable

On 16/03/2020 02:02, Li, Xiaoyun wrote:
> 
> 
>> -----Original Message-----
>> From: Kevin Traynor [mailto:ktraynor@redhat.com]
>> Sent: Saturday, March 14, 2020 03:16
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhike Wang <wangzhike@jd.com>;
>> dev@dpdk.org
>> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>> stable@dpdk.org
>> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix X722 judgement
>> when disable adminq operation
>>
>> On 13/03/2020 04:40, Li, Xiaoyun wrote:
>>> Hi
>>>
>>> This issue is already fixed in 20.02 with base code update of the following
>> commit.
>>>
>>>
>>>
>>> commit 37b091c75b13d2f26359be9b77adbc33c55a7581
>>>
>>> Author: Xiaolong Ye <xiaolong.ye@intel.com>
>>>
>>> Date:   Mon Jan 13 10:39:31 2020 +0800
>>>
>>>
>>>
>>>     net/i40e/base: extend PHY access AQ command
>>>
>>>
>>>
>>>     Currently FW use MDIO I/F number corresponded with current PF for PHY
>>>
>>>     access. This code allow to specify used MDIO I/F number.
>>>
>>>
>>>
>>>     Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
>>>
>>>     Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>>
>>>     Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>>
>>> Acked-by: Beilei Xing beilei.xing@intel.com<mailto:beilei.xing@intel.com>
>>>
>>>
>>>
>>> But it makes sense to back port this to stable release so it can work with dpdk
>> version <=19.11.
>>>
>>> Still some comments inline.
>>>
>>>
>>>
>>>> -----Original Message-----
>>>
>>>> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Zhike Wang
>>>
>>>> Sent: Thursday, January 16, 2020 09:20
>>>
>>>> To: dev@dpdk.org
>>>
>>>> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>>>
>>>> stable@dpdk.org; Zhike Wang <wangzhike@jd.com>
>>>
>>>> Subject: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable
>>>
>>>> adminq operation
>>>
>>>>
>>>
>>>> X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
>>>
>>>> /I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge whether it is
>>>
>>>> X722 or not.
>>>
>>>>
>>>
>>> Please be careful about your commit log. If you use mac.type == X722, then
>> that's not just X722 SFP but all X722 devices.
>>>
>>> You can just say something like X722 doesn't support adminq operation so
>> disable it.
>>>
>>
>> You say commit log, but isn't that what is done in the code?
> 
> His commit log only says about X722 SFPs but mac.type == X722 means all X722 devices. SFPs are only a part of X722.
> So I prefer his commit log can explain the root cause.
> 

Yes, my comment/question is similar about the code.

"if (hw->mac.type == I40E_MAC_X722)"

is this correct?

>>
>>>
>>>
>>>> Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
>>>
>>> Please follow the DPDK community code style. Fix line needs commit id of 12
>> bit.
>>>
>>> In this case, should be: Fixes: 9efa8d28b4da ("net/i40e: fix SFP X722 with
>> FW4.16")
>>>
>>> And since this fix makes sense for <=19.11, please add this line: Cc:
>> stable@dpdk.org
>>>
>>
>> Please add the stable branches requested in the usual format.
>>
>> http://doc.dpdk.org/guides/contributing/patches.html#backporting-patches-
>> for-stable-releases
>>
>>>
>>>
>>>> Signed-off-by: Zhike Wang <wangzhike@jd.com<mailto:wangzhike@jd.com>>
>>>
>>>> ---
>>>
>>>>  drivers/net/i40e/i40e_ethdev.c | 2 +-
>>>
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>>
>>>
>>>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>>>
>>>> index 5999c96..85ccb76 100644
>>>
>>>> --- a/drivers/net/i40e/i40e_ethdev.c
>>>
>>>> +++ b/drivers/net/i40e/i40e_ethdev.c
>>>
>>>> @@ -1443,7 +1443,7 @@ static inline void i40e_config_automask(struct
>>>
>>>> i40e_pf *pf)
>>>
>>>>                            return -EIO;
>>>
>>>>             }
>>>
>>>>             /* Firmware of SFP x722 does not support adminq option */
>>>
>>>> -          if (hw->device_id == I40E_DEV_ID_SFP_X722)
>>>
>>>> +          if (hw->mac.type == I40E_MAC_X722)
>>>
>>>>                            hw->flags &= ~I40E_HW_FLAG_802_1AD_CAPABLE;
>>>
>>>>
>>>
>>>>             PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM %02d.%02d.%02d
>>>
>>>> eetrack %04x",
>>>
>>>> --
>>>
>>>> 1.8.3.1
>>>
>>>>
>>>
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-03-16 10:15       ` Kevin Traynor
@ 2020-03-17  1:35         ` Li, Xiaoyun
  2020-03-18  3:25           ` 王志克
  0 siblings, 1 reply; 10+ messages in thread
From: Li, Xiaoyun @ 2020-03-17  1:35 UTC (permalink / raw)
  To: Kevin Traynor, Zhike Wang, dev; +Cc: Xing, Beilei, Zhang, Qi Z, stable



> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Monday, March 16, 2020 18:16
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhike Wang <wangzhike@jd.com>;
> dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix X722 judgement
> when disable adminq operation
> 
> On 16/03/2020 02:02, Li, Xiaoyun wrote:
> >
> >
> >> -----Original Message-----
> >> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> >> Sent: Saturday, March 14, 2020 03:16
> >> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhike Wang
> >> <wangzhike@jd.com>; dev@dpdk.org
> >> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; stable@dpdk.org
> >> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix X722
> >> judgement when disable adminq operation
> >>
> >> On 13/03/2020 04:40, Li, Xiaoyun wrote:
> >>> Hi
> >>>
> >>> This issue is already fixed in 20.02 with base code update of the
> >>> following
> >> commit.
> >>>
> >>>
> >>>
> >>> commit 37b091c75b13d2f26359be9b77adbc33c55a7581
> >>>
> >>> Author: Xiaolong Ye <xiaolong.ye@intel.com>
> >>>
> >>> Date:   Mon Jan 13 10:39:31 2020 +0800
> >>>
> >>>
> >>>
> >>>     net/i40e/base: extend PHY access AQ command
> >>>
> >>>
> >>>
> >>>     Currently FW use MDIO I/F number corresponded with current PF
> >>> for PHY
> >>>
> >>>     access. This code allow to specify used MDIO I/F number.
> >>>
> >>>
> >>>
> >>>     Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
> >>>
> >>>     Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >>>
> >>>     Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>
> >>> Acked-by: Beilei Xing
> >>> beilei.xing@intel.com<mailto:beilei.xing@intel.com>
> >>>
> >>>
> >>>
> >>> But it makes sense to back port this to stable release so it can
> >>> work with dpdk
> >> version <=19.11.
> >>>
> >>> Still some comments inline.
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>
> >>>> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Zhike
> >>>> Wang
> >>>
> >>>> Sent: Thursday, January 16, 2020 09:20
> >>>
> >>>> To: dev@dpdk.org
> >>>
> >>>> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >>>> <qi.z.zhang@intel.com>;
> >>>
> >>>> stable@dpdk.org; Zhike Wang <wangzhike@jd.com>
> >>>
> >>>> Subject: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when
> >>>> disable
> >>>
> >>>> adminq operation
> >>>
> >>>>
> >>>
> >>>> X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
> >>>
> >>>> /I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge
> >>>> whether it is
> >>>
> >>>> X722 or not.
> >>>
> >>>>
> >>>
> >>> Please be careful about your commit log. If you use mac.type ==
> >>> X722, then
> >> that's not just X722 SFP but all X722 devices.
> >>>
> >>> You can just say something like X722 doesn't support adminq
> >>> operation so
> >> disable it.
> >>>
> >>
> >> You say commit log, but isn't that what is done in the code?
> >
> > His commit log only says about X722 SFPs but mac.type == X722 means all
> X722 devices. SFPs are only a part of X722.
> > So I prefer his commit log can explain the root cause.
> >
> 
> Yes, my comment/question is similar about the code.
> 
> "if (hw->mac.type == I40E_MAC_X722)"
> 
> is this correct?
I've checked the base code update in 20.02. When the mac type is X722, this flag is not set. So I think it should be correct.
The following is the related base code.
	switch (hw->mac.type) {
	case I40E_MAC_XL710:
		if (aq->api_maj_ver > 1 ||
		    (aq->api_maj_ver == 1 &&
		     aq->api_min_ver >= I40E_MINOR_VER_GET_LINK_INFO_XL710)) {
			hw->flags |= I40E_HW_FLAG_AQ_PHY_ACCESS_CAPABLE;
			hw->flags |= I40E_HW_FLAG_FW_LLDP_STOPPABLE;
			/* The ability to RX (not drop) 802.1ad frames */
			hw->flags |= I40E_HW_FLAG_802_1AD_CAPABLE;
		}
		break;
	case I40E_MAC_X722:
		hw->flags |= I40E_HW_FLAG_AQ_SRCTL_ACCESS_ENABLE |
			     I40E_HW_FLAG_NVM_READ_REQUIRES_LOCK;

		if (aq->api_maj_ver > 1 ||
		    (aq->api_maj_ver == 1 &&
		     aq->api_min_ver >= I40E_MINOR_VER_FW_LLDP_STOPPABLE_X722))
			hw->flags |= I40E_HW_FLAG_FW_LLDP_STOPPABLE;

		if (aq->api_maj_ver > 1 ||
		    (aq->api_maj_ver == 1 &&
		     aq->api_min_ver >= I40E_MINOR_VER_GET_LINK_INFO_X722))
			hw->flags |= I40E_HW_FLAG_AQ_PHY_ACCESS_CAPABLE;
		/* fall through */
	default:
		break;
	}

> 
> >>
> >>>
> >>>
> >>>> Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
> >>>
> >>> Please follow the DPDK community code style. Fix line needs commit
> >>> id of 12
> >> bit.
> >>>
> >>> In this case, should be: Fixes: 9efa8d28b4da ("net/i40e: fix SFP
> >>> X722 with
> >> FW4.16")
> >>>
> >>> And since this fix makes sense for <=19.11, please add this line: Cc:
> >> stable@dpdk.org
> >>>
> >>
> >> Please add the stable branches requested in the usual format.
> >>
> >> http://doc.dpdk.org/guides/contributing/patches.html#backporting-patc
> >> hes-
> >> for-stable-releases
> >>
> >>>
> >>>
> >>>> Signed-off-by: Zhike Wang
> >>>> <wangzhike@jd.com<mailto:wangzhike@jd.com>>
> >>>
> >>>> ---
> >>>
> >>>>  drivers/net/i40e/i40e_ethdev.c | 2 +-
> >>>
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>>
> >>>
> >>>> diff --git a/drivers/net/i40e/i40e_ethdev.c
> >>>> b/drivers/net/i40e/i40e_ethdev.c
> >>>
> >>>> index 5999c96..85ccb76 100644
> >>>
> >>>> --- a/drivers/net/i40e/i40e_ethdev.c
> >>>
> >>>> +++ b/drivers/net/i40e/i40e_ethdev.c
> >>>
> >>>> @@ -1443,7 +1443,7 @@ static inline void
> >>>> i40e_config_automask(struct
> >>>
> >>>> i40e_pf *pf)
> >>>
> >>>>                            return -EIO;
> >>>
> >>>>             }
> >>>
> >>>>             /* Firmware of SFP x722 does not support adminq option
> >>>> */
> >>>
> >>>> -          if (hw->device_id == I40E_DEV_ID_SFP_X722)
> >>>
> >>>> +          if (hw->mac.type == I40E_MAC_X722)
> >>>
> >>>>                            hw->flags &=
> >>>> ~I40E_HW_FLAG_802_1AD_CAPABLE;
> >>>
> >>>>
> >>>
> >>>>             PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM
> >>>> %02d.%02d.%02d
> >>>
> >>>> eetrack %04x",
> >>>
> >>>> --
> >>>
> >>>> 1.8.3.1
> >>>
> >>>>
> >>>
> >


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/i40e: fix X722 judgement when disable adminq operation
  2020-03-17  1:35         ` Li, Xiaoyun
@ 2020-03-18  3:25           ` 王志克
  0 siblings, 0 replies; 10+ messages in thread
From: 王志克 @ 2020-03-18  3:25 UTC (permalink / raw)
  To: Li, Xiaoyun, Kevin Traynor, dev; +Cc: Xing, Beilei, Zhang, Qi Z, stable

Hi,

Thanks for comment. I sent out v3.

BTW, I do not use --subject-prefix='PATCH branch.x' since it seems do not take effect (and my git does not show such option)

Br,
Zhike Wang 
JDCloud, Product Development, IaaS   
------------------------------------------------------------------------------------------------
Mobile/+86 13466719566
E- mail/wangzhike@jd.com
Address/5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang District Beijing
Https://JDCloud.com
------------------------------------------------------------------------------------------------


-----Original Message-----
From: Li, Xiaoyun [mailto:xiaoyun.li@intel.com] 
Sent: Tuesday, March 17, 2020 9:35 AM
To: Kevin Traynor; 王志克; dev@dpdk.org
Cc: Xing, Beilei; Zhang, Qi Z; stable@dpdk.org
Subject: RE: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable adminq operation




> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Monday, March 16, 2020 18:16
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhike Wang <wangzhike@jd.com>;
> dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix X722 judgement
> when disable adminq operation
> 
> On 16/03/2020 02:02, Li, Xiaoyun wrote:
> >
> >
> >> -----Original Message-----
> >> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> >> Sent: Saturday, March 14, 2020 03:16
> >> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Zhike Wang
> >> <wangzhike@jd.com>; dev@dpdk.org
> >> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; stable@dpdk.org
> >> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix X722
> >> judgement when disable adminq operation
> >>
> >> On 13/03/2020 04:40, Li, Xiaoyun wrote:
> >>> Hi
> >>>
> >>> This issue is already fixed in 20.02 with base code update of the
> >>> following
> >> commit.
> >>>
> >>>
> >>>
> >>> commit 37b091c75b13d2f26359be9b77adbc33c55a7581
> >>>
> >>> Author: Xiaolong Ye <xiaolong.ye@intel.com>
> >>>
> >>> Date:   Mon Jan 13 10:39:31 2020 +0800
> >>>
> >>>
> >>>
> >>>     net/i40e/base: extend PHY access AQ command
> >>>
> >>>
> >>>
> >>>     Currently FW use MDIO I/F number corresponded with current PF
> >>> for PHY
> >>>
> >>>     access. This code allow to specify used MDIO I/F number.
> >>>
> >>>
> >>>
> >>>     Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
> >>>
> >>>     Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >>>
> >>>     Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>
> >>> Acked-by: Beilei Xing
> >>> beilei.xing@intel.com<mailto:beilei.xing@intel.com>
> >>>
> >>>
> >>>
> >>> But it makes sense to back port this to stable release so it can
> >>> work with dpdk
> >> version <=19.11.
> >>>
> >>> Still some comments inline.
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>
> >>>> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Zhike
> >>>> Wang
> >>>
> >>>> Sent: Thursday, January 16, 2020 09:20
> >>>
> >>>> To: dev@dpdk.org
> >>>
> >>>> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >>>> <qi.z.zhang@intel.com>;
> >>>
> >>>> stable@dpdk.org; Zhike Wang <wangzhike@jd.com>
> >>>
> >>>> Subject: [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when
> >>>> disable
> >>>
> >>>> adminq operation
> >>>
> >>>>
> >>>
> >>>> X722 SFPs have different flavors, eg I40E_DEV_ID_SFP_X722
> >>>
> >>>> /I40E_DEV_ID_SFP_I_X722. So instead we use mac.type to judge
> >>>> whether it is
> >>>
> >>>> X722 or not.
> >>>
> >>>>
> >>>
> >>> Please be careful about your commit log. If you use mac.type ==
> >>> X722, then
> >> that's not just X722 SFP but all X722 devices.
> >>>
> >>> You can just say something like X722 doesn't support adminq
> >>> operation so
> >> disable it.
> >>>
> >>
> >> You say commit log, but isn't that what is done in the code?
> >
> > His commit log only says about X722 SFPs but mac.type == X722 means all
> X722 devices. SFPs are only a part of X722.
> > So I prefer his commit log can explain the root cause.
> >
> 
> Yes, my comment/question is similar about the code.
> 
> "if (hw->mac.type == I40E_MAC_X722)"
> 
> is this correct?
I've checked the base code update in 20.02. When the mac type is X722, this flag is not set. So I think it should be correct.
The following is the related base code.
	switch (hw->mac.type) {
	case I40E_MAC_XL710:
		if (aq->api_maj_ver > 1 ||
		    (aq->api_maj_ver == 1 &&
		     aq->api_min_ver >= I40E_MINOR_VER_GET_LINK_INFO_XL710)) {
			hw->flags |= I40E_HW_FLAG_AQ_PHY_ACCESS_CAPABLE;
			hw->flags |= I40E_HW_FLAG_FW_LLDP_STOPPABLE;
			/* The ability to RX (not drop) 802.1ad frames */
			hw->flags |= I40E_HW_FLAG_802_1AD_CAPABLE;
		}
		break;
	case I40E_MAC_X722:
		hw->flags |= I40E_HW_FLAG_AQ_SRCTL_ACCESS_ENABLE |
			     I40E_HW_FLAG_NVM_READ_REQUIRES_LOCK;

		if (aq->api_maj_ver > 1 ||
		    (aq->api_maj_ver == 1 &&
		     aq->api_min_ver >= I40E_MINOR_VER_FW_LLDP_STOPPABLE_X722))
			hw->flags |= I40E_HW_FLAG_FW_LLDP_STOPPABLE;

		if (aq->api_maj_ver > 1 ||
		    (aq->api_maj_ver == 1 &&
		     aq->api_min_ver >= I40E_MINOR_VER_GET_LINK_INFO_X722))
			hw->flags |= I40E_HW_FLAG_AQ_PHY_ACCESS_CAPABLE;
		/* fall through */
	default:
		break;
	}

> 
> >>
> >>>
> >>>
> >>>> Fixes: 9efa8d28b ("net/i40e: fix SFP X722 with FW4.16")
> >>>
> >>> Please follow the DPDK community code style. Fix line needs commit
> >>> id of 12
> >> bit.
> >>>
> >>> In this case, should be: Fixes: 9efa8d28b4da ("net/i40e: fix SFP
> >>> X722 with
> >> FW4.16")
> >>>
> >>> And since this fix makes sense for <=19.11, please add this line: Cc:
> >> stable@dpdk.org
> >>>
> >>
> >> Please add the stable branches requested in the usual format.
> >>
> >> http://doc.dpdk.org/guides/contributing/patches.html#backporting-patc
> >> hes-
> >> for-stable-releases
> >>
> >>>
> >>>
> >>>> Signed-off-by: Zhike Wang
> >>>> <wangzhike@jd.com<mailto:wangzhike@jd.com>>
> >>>
> >>>> ---
> >>>
> >>>>  drivers/net/i40e/i40e_ethdev.c | 2 +-
> >>>
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>>
> >>>
> >>>> diff --git a/drivers/net/i40e/i40e_ethdev.c
> >>>> b/drivers/net/i40e/i40e_ethdev.c
> >>>
> >>>> index 5999c96..85ccb76 100644
> >>>
> >>>> --- a/drivers/net/i40e/i40e_ethdev.c
> >>>
> >>>> +++ b/drivers/net/i40e/i40e_ethdev.c
> >>>
> >>>> @@ -1443,7 +1443,7 @@ static inline void
> >>>> i40e_config_automask(struct
> >>>
> >>>> i40e_pf *pf)
> >>>
> >>>>                            return -EIO;
> >>>
> >>>>             }
> >>>
> >>>>             /* Firmware of SFP x722 does not support adminq option
> >>>> */
> >>>
> >>>> -          if (hw->device_id == I40E_DEV_ID_SFP_X722)
> >>>
> >>>> +          if (hw->mac.type == I40E_MAC_X722)
> >>>
> >>>>                            hw->flags &=
> >>>> ~I40E_HW_FLAG_802_1AD_CAPABLE;
> >>>
> >>>>
> >>>
> >>>>             PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM
> >>>> %02d.%02d.%02d
> >>>
> >>>> eetrack %04x",
> >>>
> >>>> --
> >>>
> >>>> 1.8.3.1
> >>>
> >>>>
> >>>
> >


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-03-18  3:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16  1:19 [dpdk-stable] [PATCH] net/i40e: fix X722 judgement when disable adminq operation Zhike Wang
2020-03-12  9:13 ` 王志克
2020-03-13  4:40 ` Li, Xiaoyun
2020-03-13  5:22   ` [dpdk-stable] [dpdk-dev] " Ye Xiaolong
2020-03-13  5:57     ` Li, Xiaoyun
2020-03-13 19:16   ` Kevin Traynor
2020-03-16  2:02     ` Li, Xiaoyun
2020-03-16 10:15       ` Kevin Traynor
2020-03-17  1:35         ` Li, Xiaoyun
2020-03-18  3:25           ` 王志克

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).