diff mbox

[dpdk-dev] app/testpmd: check if CRC strip offload supported

Message ID 20180509220904.128983-1-ferruh.yigit@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Ferruh Yigit May 9, 2018, 10:09 p.m. UTC 
  Testpmd set CRC_STRIP offload blindly, this is wrong according offload
API definition, and will cause error for the PMDs that doesn't support
CRC_STRIP like virtual PMDs.

Check if underlying device report this capability and don't set it if
not supported.

Fixes: 0074d02fca21 ("app/testpmd: convert to new Rx offloads API")
Cc: shahafs@mellanox.com

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Wei Dai <wei.dai@intel.com>
Cc: Qi Zhang <qi.z.zhang@intel.com>
---
 app/test-pmd/testpmd.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Shahaf Shuler May 10, 2018, 4:39 a.m. UTC | #1 
Thursday, May 10, 2018 1:09 AM, Ferruh Yigit:
> Subject: [PATCH] app/testpmd: check if CRC strip offload supported
> 
> Testpmd set CRC_STRIP offload blindly, this is wrong according offload API
> definition, and will cause error for the PMDs that doesn't support CRC_STRIP
> like virtual PMDs.
> 
> Check if underlying device report this capability and don't set it if not
> supported.
> 
> Fixes: 0074d02fca21 ("app/testpmd: convert to new Rx offloads API")
> Cc: shahafs@mellanox.com
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Yes this is the correct behavior. I hope all the PMDs report the CRC strip correctly. 

Acked-by: Shahaf Shuler < shahafs@mellanox.com>

> ---
> Cc: Wei Dai <wei.dai@intel.com>
> Cc: Qi Zhang <qi.z.zhang@intel.com>
> ---
>  app/test-pmd/testpmd.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> db23f23e5..1938283f6 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -701,6 +701,11 @@ init_config(void)
>  		port->dev_conf.txmode = tx_mode;
>  		port->dev_conf.rxmode = rx_mode;
>  		rte_eth_dev_info_get(pid, &port->dev_info);
> +
> +		if (!(port->dev_info.rx_offload_capa &
> +					DEV_RX_OFFLOAD_CRC_STRIP))
> +			port->dev_conf.rxmode.offloads &=
> +				~DEV_RX_OFFLOAD_CRC_STRIP;
>  		if (!(port->dev_info.tx_offload_capa &
>  		      DEV_TX_OFFLOAD_MBUF_FAST_FREE))
>  			port->dev_conf.txmode.offloads &=
> --
> 2.14.3
Iremonger, Bernard May 11, 2018, 3:36 p.m. UTC | #2 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Thursday, May 10, 2018 5:39 AM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: check if CRC strip offload
> supported
> 
> Thursday, May 10, 2018 1:09 AM, Ferruh Yigit:
> > Subject: [PATCH] app/testpmd: check if CRC strip offload supported
> >
> > Testpmd set CRC_STRIP offload blindly, this is wrong according offload
> > API definition, and will cause error for the PMDs that doesn't support
> > CRC_STRIP like virtual PMDs.
> >
> > Check if underlying device report this capability and don't set it if
> > not supported.
> >
> > Fixes: 0074d02fca21 ("app/testpmd: convert to new Rx offloads API")
> > Cc: shahafs@mellanox.com
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Yes this is the correct behavior. I hope all the PMDs report the CRC strip
> correctly.
> 
> Acked-by: Shahaf Shuler < shahafs@mellanox.com>

Acked-by: Bernard Iremonger <Bernard.iremonger@intel.com>
Ferruh Yigit May 11, 2018, 4:27 p.m. UTC | #3 
On 5/11/2018 4:36 PM, Iremonger, Bernard wrote:
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
>> Sent: Thursday, May 10, 2018 5:39 AM
>> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
>> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: check if CRC strip offload
>> supported
>>
>> Thursday, May 10, 2018 1:09 AM, Ferruh Yigit:
>>> Subject: [PATCH] app/testpmd: check if CRC strip offload supported
>>>
>>> Testpmd set CRC_STRIP offload blindly, this is wrong according offload
>>> API definition, and will cause error for the PMDs that doesn't support
>>> CRC_STRIP like virtual PMDs.
>>>
>>> Check if underlying device report this capability and don't set it if
>>> not supported.
>>>
>>> Fixes: 0074d02fca21 ("app/testpmd: convert to new Rx offloads API")
>>> Cc: shahafs@mellanox.com
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Yes this is the correct behavior. I hope all the PMDs report the CRC strip
>> correctly.
>>
>> Acked-by: Shahaf Shuler < shahafs@mellanox.com>
> 
> Acked-by: Bernard Iremonger <Bernard.iremonger@intel.com>

Applied to dpdk-next-net/master, thanks.
diff mbox

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index db23f23e5..1938283f6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -701,6 +701,11 @@  init_config(void)
 		port->dev_conf.txmode = tx_mode;
 		port->dev_conf.rxmode = rx_mode;
 		rte_eth_dev_info_get(pid, &port->dev_info);
+
+		if (!(port->dev_info.rx_offload_capa &
+					DEV_RX_OFFLOAD_CRC_STRIP))
+			port->dev_conf.rxmode.offloads &=
+				~DEV_RX_OFFLOAD_CRC_STRIP;
 		if (!(port->dev_info.tx_offload_capa &
 		      DEV_TX_OFFLOAD_MBUF_FAST_FREE))
 			port->dev_conf.txmode.offloads &=