DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1 0/2] fix some csum errors
@ 2020-10-31  3:38 Xiaoyun wang
  2020-10-31  3:38 ` [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error Xiaoyun wang
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Xiaoyun wang @ 2020-10-31  3:38 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, bluca, stable, luoxingyu, luoxianjun, yin.yinshi,
	zhouguoyang, Xiaoyun wang

This patch fixes outer_l3_len parse error when
PKT_TX_OUTER_IP_CKSUM is not set, which does not affect 
checksum function, just be consistent with mbuf meta 
information description, and fixes SCTP checksum 
errors because driver doesn't pass payload offset info
to hardware, which may cause SCTP checksum error.

--

v1:
  - fix outer_l3_len parse error
  - fix SCTP checksum error

Xiaoyun wang (2):
  net/hinic: fix outer_l3_len parse error
  net/hinic: fix SCTP checksum error

 drivers/net/hinic/hinic_pmd_tx.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error
  2020-10-31  3:38 [dpdk-dev] [PATCH v1 0/2] fix some csum errors Xiaoyun wang
@ 2020-10-31  3:38 ` Xiaoyun wang
  2020-11-02 17:08   ` Ferruh Yigit
  2020-10-31  3:38 ` [dpdk-dev] [PATCH v1 2/2] net/hinic: fix SCTP checksum error Xiaoyun wang
  2020-11-02 17:47 ` [dpdk-dev] [PATCH v1 0/2] fix some csum errors Ferruh Yigit
  2 siblings, 1 reply; 8+ messages in thread
From: Xiaoyun wang @ 2020-10-31  3:38 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, bluca, stable, luoxingyu, luoxianjun, yin.yinshi,
	zhouguoyang, Xiaoyun wang

This patch fixes outer_l3_len parse error when
PKT_TX_OUTER_IP_CKSUM is not set, which does not affect
checksum function, just be consistent with mbuf meta
information description.

Fixes: 8c8b61234ffd ("net/hinic: refactor checksum functions")
Cc: stable@dpdk.org
Signed-off-by: Xiaoyun wang <cloud.wangxiaoyun@huawei.com>
---
 drivers/net/hinic/hinic_pmd_tx.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/net/hinic/hinic_pmd_tx.c b/drivers/net/hinic/hinic_pmd_tx.c
index 2dd4fe1..125627e 100644
--- a/drivers/net/hinic/hinic_pmd_tx.c
+++ b/drivers/net/hinic/hinic_pmd_tx.c
@@ -779,26 +779,25 @@ static inline void hinic_analyze_tx_info(struct rte_mbuf *mbuf,
 {
 	struct rte_ether_hdr *eth_hdr;
 	struct rte_vlan_hdr *vlan_hdr;
-	struct rte_ipv4_hdr *ip4h;
-	u16 pkt_type;
-	u8 *hdr;
+	struct rte_ipv4_hdr *ipv4_hdr;
+	u16 eth_type;
 
-	hdr = (u8 *)rte_pktmbuf_mtod(mbuf, u8*);
-	eth_hdr = (struct rte_ether_hdr *)hdr;
-	pkt_type = rte_be_to_cpu_16(eth_hdr->ether_type);
+	eth_hdr = rte_pktmbuf_mtod(mbuf, struct rte_ether_hdr *);
+	eth_type = rte_be_to_cpu_16(eth_hdr->ether_type);
 
-	if (pkt_type == RTE_ETHER_TYPE_VLAN) {
+	if (eth_type == RTE_ETHER_TYPE_VLAN) {
 		off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
-		vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
-		pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
+		vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
+		eth_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
 	} else {
 		off_info->outer_l2_len = ETHER_LEN_NO_VLAN;
 	}
 
-	if (pkt_type == RTE_ETHER_TYPE_IPV4) {
-		ip4h = (struct rte_ipv4_hdr *)(hdr + off_info->outer_l2_len);
-		off_info->outer_l3_len = rte_ipv4_hdr_len(ip4h);
-	} else if (pkt_type == RTE_ETHER_TYPE_IPV6) {
+	if (eth_type == RTE_ETHER_TYPE_IPV4) {
+		ipv4_hdr = rte_pktmbuf_mtod_offset(mbuf, struct rte_ipv4_hdr *,
+						   off_info->outer_l2_len);
+		off_info->outer_l3_len = rte_ipv4_hdr_len(ipv4_hdr);
+	} else if (eth_type == RTE_ETHER_TYPE_IPV6) {
 		/* not support ipv6 extension header */
 		off_info->outer_l3_len = sizeof(struct rte_ipv6_hdr);
 	}
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v1 2/2] net/hinic: fix SCTP checksum error
  2020-10-31  3:38 [dpdk-dev] [PATCH v1 0/2] fix some csum errors Xiaoyun wang
  2020-10-31  3:38 ` [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error Xiaoyun wang
@ 2020-10-31  3:38 ` Xiaoyun wang
  2020-11-02 17:47 ` [dpdk-dev] [PATCH v1 0/2] fix some csum errors Ferruh Yigit
  2 siblings, 0 replies; 8+ messages in thread
From: Xiaoyun wang @ 2020-10-31  3:38 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, bluca, stable, luoxingyu, luoxianjun, yin.yinshi,
	zhouguoyang, Xiaoyun wang

For SCTP checksum offload, pmd driver does not parse payload offset
info, which may cause hardware calculate SCTP checksum failed.

Fixes: 8c8b61234ffd ("net/hinic: refactor checksum functions")
Cc: stable@dpdk.org
Signed-off-by: Xiaoyun wang <cloud.wangxiaoyun@huawei.com>
---
 drivers/net/hinic/hinic_pmd_tx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hinic/hinic_pmd_tx.c b/drivers/net/hinic/hinic_pmd_tx.c
index 125627e..9d0264e 100644
--- a/drivers/net/hinic/hinic_pmd_tx.c
+++ b/drivers/net/hinic/hinic_pmd_tx.c
@@ -767,7 +767,8 @@ static inline void hinic_get_pld_offset(struct rte_mbuf *m,
 {
 	uint64_t ol_flags = m->ol_flags;
 
-	if ((ol_flags & PKT_TX_L4_MASK) == PKT_TX_UDP_CKSUM)
+	if (((ol_flags & PKT_TX_L4_MASK) == PKT_TX_UDP_CKSUM) ||
+	    ((ol_flags & PKT_TX_L4_MASK) == PKT_TX_SCTP_CKSUM))
 		off_info->payload_offset = m->l2_len + m->l3_len;
 	else if ((ol_flags & PKT_TX_TCP_CKSUM) || (ol_flags & PKT_TX_TCP_SEG))
 		off_info->payload_offset = m->l2_len + m->l3_len +
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error
  2020-10-31  3:38 ` [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error Xiaoyun wang
@ 2020-11-02 17:08   ` Ferruh Yigit
  2020-11-02 17:26     ` Ferruh Yigit
  2020-11-04  2:19     ` Wangxiaoyun (Cloud)
  0 siblings, 2 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-11-02 17:08 UTC (permalink / raw)
  To: Xiaoyun wang, dev
  Cc: bluca, stable, luoxingyu, luoxianjun, yin.yinshi, zhouguoyang

On 10/31/2020 3:38 AM, Xiaoyun wang wrote:
> This patch fixes outer_l3_len parse error when
> PKT_TX_OUTER_IP_CKSUM is not set, which does not affect
> checksum function, just be consistent with mbuf meta
> information description.
> 
> Fixes: 8c8b61234ffd ("net/hinic: refactor checksum functions")
> Cc: stable@dpdk.org
> Signed-off-by: Xiaoyun wang <cloud.wangxiaoyun@huawei.com>
> ---
>   drivers/net/hinic/hinic_pmd_tx.c | 25 ++++++++++++-------------
>   1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/hinic/hinic_pmd_tx.c b/drivers/net/hinic/hinic_pmd_tx.c
> index 2dd4fe1..125627e 100644
> --- a/drivers/net/hinic/hinic_pmd_tx.c
> +++ b/drivers/net/hinic/hinic_pmd_tx.c
> @@ -779,26 +779,25 @@ static inline void hinic_analyze_tx_info(struct rte_mbuf *mbuf,
>   {
>   	struct rte_ether_hdr *eth_hdr;
>   	struct rte_vlan_hdr *vlan_hdr;
> -	struct rte_ipv4_hdr *ip4h;
> -	u16 pkt_type;
> -	u8 *hdr;
> +	struct rte_ipv4_hdr *ipv4_hdr;
> +	u16 eth_type;
>   
> -	hdr = (u8 *)rte_pktmbuf_mtod(mbuf, u8*);
> -	eth_hdr = (struct rte_ether_hdr *)hdr;
> -	pkt_type = rte_be_to_cpu_16(eth_hdr->ether_type);
> +	eth_hdr = rte_pktmbuf_mtod(mbuf, struct rte_ether_hdr *);
> +	eth_type = rte_be_to_cpu_16(eth_hdr->ether_type);
>   
> -	if (pkt_type == RTE_ETHER_TYPE_VLAN) {
> +	if (eth_type == RTE_ETHER_TYPE_VLAN) {
>   		off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
> -		vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
> -		pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
> +		vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
> +		eth_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>   	} else {
>   		off_info->outer_l2_len = ETHER_LEN_NO_VLAN;
>   	}
>   
> -	if (pkt_type == RTE_ETHER_TYPE_IPV4) {
> -		ip4h = (struct rte_ipv4_hdr *)(hdr + off_info->outer_l2_len);
> -		off_info->outer_l3_len = rte_ipv4_hdr_len(ip4h);
> -	} else if (pkt_type == RTE_ETHER_TYPE_IPV6) {
> +	if (eth_type == RTE_ETHER_TYPE_IPV4) {
> +		ipv4_hdr = rte_pktmbuf_mtod_offset(mbuf, struct rte_ipv4_hdr *,
> +						   off_info->outer_l2_len);
> +		off_info->outer_l3_len = rte_ipv4_hdr_len(ipv4_hdr);
> +	} else if (eth_type == RTE_ETHER_TYPE_IPV6) {
>   		/* not support ipv6 extension header */
>   		off_info->outer_l3_len = sizeof(struct rte_ipv6_hdr);
>   	}
> 


The actual fix is following [1] and rest is refactoring, right?
It is hard to catch the actual fix with refactoring, can you please describe the 
actual problem and fix in the commit log to clarify it?



[1]
  @@ -789,7 +789,7 @@ static inline void hinic_analyze_tx_info(struct rte_mbuf 
*mbuf,

          if (pkt_type == RTE_ETHER_TYPE_VLAN) {
                  off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
  -               vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
  +               vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
                  pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
          } else {
                  off_info->outer_l2_len = ETHER_LEN_NO_VLAN;

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

* Re: [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error
  2020-11-02 17:08   ` Ferruh Yigit
@ 2020-11-02 17:26     ` Ferruh Yigit
  2020-11-04  2:19     ` Wangxiaoyun (Cloud)
  1 sibling, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-11-02 17:26 UTC (permalink / raw)
  To: Xiaoyun wang, dev
  Cc: bluca, stable, luoxingyu, luoxianjun, yin.yinshi, zhouguoyang

On 11/2/2020 5:08 PM, Ferruh Yigit wrote:
> On 10/31/2020 3:38 AM, Xiaoyun wang wrote:
>> This patch fixes outer_l3_len parse error when
>> PKT_TX_OUTER_IP_CKSUM is not set, which does not affect
>> checksum function, just be consistent with mbuf meta
>> information description.
>>
>> Fixes: 8c8b61234ffd ("net/hinic: refactor checksum functions")
>> Cc: stable@dpdk.org
>> Signed-off-by: Xiaoyun wang <cloud.wangxiaoyun@huawei.com>
>> ---
>>   drivers/net/hinic/hinic_pmd_tx.c | 25 ++++++++++++-------------
>>   1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/hinic/hinic_pmd_tx.c b/drivers/net/hinic/hinic_pmd_tx.c
>> index 2dd4fe1..125627e 100644
>> --- a/drivers/net/hinic/hinic_pmd_tx.c
>> +++ b/drivers/net/hinic/hinic_pmd_tx.c
>> @@ -779,26 +779,25 @@ static inline void hinic_analyze_tx_info(struct rte_mbuf 
>> *mbuf,
>>   {
>>       struct rte_ether_hdr *eth_hdr;
>>       struct rte_vlan_hdr *vlan_hdr;
>> -    struct rte_ipv4_hdr *ip4h;
>> -    u16 pkt_type;
>> -    u8 *hdr;
>> +    struct rte_ipv4_hdr *ipv4_hdr;
>> +    u16 eth_type;
>> -    hdr = (u8 *)rte_pktmbuf_mtod(mbuf, u8*);
>> -    eth_hdr = (struct rte_ether_hdr *)hdr;
>> -    pkt_type = rte_be_to_cpu_16(eth_hdr->ether_type);
>> +    eth_hdr = rte_pktmbuf_mtod(mbuf, struct rte_ether_hdr *);
>> +    eth_type = rte_be_to_cpu_16(eth_hdr->ether_type);
>> -    if (pkt_type == RTE_ETHER_TYPE_VLAN) {
>> +    if (eth_type == RTE_ETHER_TYPE_VLAN) {
>>           off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
>> -        vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
>> -        pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>> +        vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
>> +        eth_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>>       } else {
>>           off_info->outer_l2_len = ETHER_LEN_NO_VLAN;
>>       }
>> -    if (pkt_type == RTE_ETHER_TYPE_IPV4) {
>> -        ip4h = (struct rte_ipv4_hdr *)(hdr + off_info->outer_l2_len);
>> -        off_info->outer_l3_len = rte_ipv4_hdr_len(ip4h);
>> -    } else if (pkt_type == RTE_ETHER_TYPE_IPV6) {
>> +    if (eth_type == RTE_ETHER_TYPE_IPV4) {
>> +        ipv4_hdr = rte_pktmbuf_mtod_offset(mbuf, struct rte_ipv4_hdr *,
>> +                           off_info->outer_l2_len);
>> +        off_info->outer_l3_len = rte_ipv4_hdr_len(ipv4_hdr);
>> +    } else if (eth_type == RTE_ETHER_TYPE_IPV6) {
>>           /* not support ipv6 extension header */
>>           off_info->outer_l3_len = sizeof(struct rte_ipv6_hdr);
>>       }
>>
> 
> 
> The actual fix is following [1] and rest is refactoring, right?
> It is hard to catch the actual fix with refactoring, can you please describe the 
> actual problem and fix in the commit log to clarify it?
> 
> 
> 
> [1]
>   @@ -789,7 +789,7 @@ static inline void hinic_analyze_tx_info(struct rte_mbuf 
> *mbuf,
> 
>           if (pkt_type == RTE_ETHER_TYPE_VLAN) {
>                   off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
>   -               vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
>   +               vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
>                   pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>           } else {
>                   off_info->outer_l2_len = ETHER_LEN_NO_VLAN;

Added following to the commit log while merging, please correct it if it is 
wrong/missing:

     The outer_l3_len is calculated wrong because 'vlan_hdr' is calculated
     wrong, 'vlan_hdr' fixed and code refactored.


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

* Re: [dpdk-dev] [PATCH v1 0/2] fix some csum errors
  2020-10-31  3:38 [dpdk-dev] [PATCH v1 0/2] fix some csum errors Xiaoyun wang
  2020-10-31  3:38 ` [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error Xiaoyun wang
  2020-10-31  3:38 ` [dpdk-dev] [PATCH v1 2/2] net/hinic: fix SCTP checksum error Xiaoyun wang
@ 2020-11-02 17:47 ` Ferruh Yigit
  2 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-11-02 17:47 UTC (permalink / raw)
  To: Xiaoyun wang, dev
  Cc: bluca, stable, luoxingyu, luoxianjun, yin.yinshi, zhouguoyang

On 10/31/2020 3:38 AM, Xiaoyun wang wrote:
> This patch fixes outer_l3_len parse error when
> PKT_TX_OUTER_IP_CKSUM is not set, which does not affect
> checksum function, just be consistent with mbuf meta
> information description, and fixes SCTP checksum
> errors because driver doesn't pass payload offset info
> to hardware, which may cause SCTP checksum error.
> 
> --
> 
> v1:
>    - fix outer_l3_len parse error
>    - fix SCTP checksum error
> 
> Xiaoyun wang (2):
>    net/hinic: fix outer_l3_len parse error
>    net/hinic: fix SCTP checksum error
> 

Series applied to dpdk-next-net/main, thanks.


Also can you please review a long waiting [1] hinic patch:
https://patches.dpdk.org/patch/83421/


[1]
Current patch doesn't have version information but it is continuation of the 
patch from early August:
https://patches.dpdk.org/project/dpdk/list/?series=&submitter=1866&state=*&q=net%2Fhinic&archive=&delegate=

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

* Re: [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error
  2020-11-02 17:08   ` Ferruh Yigit
  2020-11-02 17:26     ` Ferruh Yigit
@ 2020-11-04  2:19     ` Wangxiaoyun (Cloud)
  2020-11-04 11:05       ` Ferruh Yigit
  1 sibling, 1 reply; 8+ messages in thread
From: Wangxiaoyun (Cloud) @ 2020-11-04  2:19 UTC (permalink / raw)
  To: Ferruh Yigit, dev
  Cc: bluca, stable, luoxingyu, luoxianjun, yin.yinshi, zhouguoyang

Yes,the actual fix is following as this patch:

commit 8c8b61234ffd9a283cecbf9751942cbdb87d68f6
Author: Xiaoyun Wang <cloud.wangxiaoyun@huawei.com>
Date:   Sat Jul 25 16:15:33 2020 +0800

     net/hinic: refactor checksum functions

     Encapsulate different types of packet checksum preprocessing
     into functions.


在 2020/11/3 1:08, Ferruh Yigit 写道:
> On 10/31/2020 3:38 AM, Xiaoyun wang wrote:
>> This patch fixes outer_l3_len parse error when
>> PKT_TX_OUTER_IP_CKSUM is not set, which does not affect
>> checksum function, just be consistent with mbuf meta
>> information description.
>>
>> Fixes: 8c8b61234ffd ("net/hinic: refactor checksum functions")
>> Cc: stable@dpdk.org
>> Signed-off-by: Xiaoyun wang <cloud.wangxiaoyun@huawei.com>
>> ---
>>   drivers/net/hinic/hinic_pmd_tx.c | 25 ++++++++++++-------------
>>   1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/hinic/hinic_pmd_tx.c b/drivers/net/hinic/hinic_pmd_tx.c
>> index 2dd4fe1..125627e 100644
>> --- a/drivers/net/hinic/hinic_pmd_tx.c
>> +++ b/drivers/net/hinic/hinic_pmd_tx.c
>> @@ -779,26 +779,25 @@ static inline void hinic_analyze_tx_info(struct rte_mbuf *mbuf,
>>   {
>>       struct rte_ether_hdr *eth_hdr;
>>       struct rte_vlan_hdr *vlan_hdr;
>> -    struct rte_ipv4_hdr *ip4h;
>> -    u16 pkt_type;
>> -    u8 *hdr;
>> +    struct rte_ipv4_hdr *ipv4_hdr;
>> +    u16 eth_type;
>> -    hdr = (u8 *)rte_pktmbuf_mtod(mbuf, u8*);
>> -    eth_hdr = (struct rte_ether_hdr *)hdr;
>> -    pkt_type = rte_be_to_cpu_16(eth_hdr->ether_type);
>> +    eth_hdr = rte_pktmbuf_mtod(mbuf, struct rte_ether_hdr *);
>> +    eth_type = rte_be_to_cpu_16(eth_hdr->ether_type);
>> -    if (pkt_type == RTE_ETHER_TYPE_VLAN) {
>> +    if (eth_type == RTE_ETHER_TYPE_VLAN) {
>>           off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
>> -        vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
>> -        pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>> +        vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
>> +        eth_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>>       } else {
>>           off_info->outer_l2_len = ETHER_LEN_NO_VLAN;
>>       }
>> -    if (pkt_type == RTE_ETHER_TYPE_IPV4) {
>> -        ip4h = (struct rte_ipv4_hdr *)(hdr + off_info->outer_l2_len);
>> -        off_info->outer_l3_len = rte_ipv4_hdr_len(ip4h);
>> -    } else if (pkt_type == RTE_ETHER_TYPE_IPV6) {
>> +    if (eth_type == RTE_ETHER_TYPE_IPV4) {
>> +        ipv4_hdr = rte_pktmbuf_mtod_offset(mbuf, struct rte_ipv4_hdr *,
>> +                           off_info->outer_l2_len);
>> +        off_info->outer_l3_len = rte_ipv4_hdr_len(ipv4_hdr);
>> +    } else if (eth_type == RTE_ETHER_TYPE_IPV6) {
>>           /* not support ipv6 extension header */
>>           off_info->outer_l3_len = sizeof(struct rte_ipv6_hdr);
>>       }
>>
> 
> 
> The actual fix is following [1] and rest is refactoring, right?
> It is hard to catch the actual fix with refactoring, can you please describe the actual problem and fix in the commit log to clarify it?
> 
> 
> 
> [1]
>   @@ -789,7 +789,7 @@ static inline void hinic_analyze_tx_info(struct rte_mbuf *mbuf,
> 
>           if (pkt_type == RTE_ETHER_TYPE_VLAN) {
>                   off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
>   -               vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
>   +               vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
>                   pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>           } else {
>                   off_info->outer_l2_len = ETHER_LEN_NO_VLAN;
> .

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

* Re: [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error
  2020-11-04  2:19     ` Wangxiaoyun (Cloud)
@ 2020-11-04 11:05       ` Ferruh Yigit
  0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-11-04 11:05 UTC (permalink / raw)
  To: Wangxiaoyun (Cloud), dev
  Cc: bluca, stable, luoxingyu, luoxianjun, yin.yinshi, zhouguoyang

On 11/4/2020 2:19 AM, Wangxiaoyun (Cloud) wrote:

<please don't top post, moved reply to down>

> 在 2020/11/3 1:08, Ferruh Yigit 写道:
>> On 10/31/2020 3:38 AM, Xiaoyun wang wrote:
>>> This patch fixes outer_l3_len parse error when
>>> PKT_TX_OUTER_IP_CKSUM is not set, which does not affect
>>> checksum function, just be consistent with mbuf meta
>>> information description.
>>>
>>> Fixes: 8c8b61234ffd ("net/hinic: refactor checksum functions")
>>> Cc: stable@dpdk.org
>>> Signed-off-by: Xiaoyun wang <cloud.wangxiaoyun@huawei.com>
>>> ---
>>>   drivers/net/hinic/hinic_pmd_tx.c | 25 ++++++++++++-------------
>>>   1 file changed, 12 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/hinic/hinic_pmd_tx.c b/drivers/net/hinic/hinic_pmd_tx.c
>>> index 2dd4fe1..125627e 100644
>>> --- a/drivers/net/hinic/hinic_pmd_tx.c
>>> +++ b/drivers/net/hinic/hinic_pmd_tx.c
>>> @@ -779,26 +779,25 @@ static inline void hinic_analyze_tx_info(struct 
>>> rte_mbuf *mbuf,
>>>   {
>>>       struct rte_ether_hdr *eth_hdr;
>>>       struct rte_vlan_hdr *vlan_hdr;
>>> -    struct rte_ipv4_hdr *ip4h;
>>> -    u16 pkt_type;
>>> -    u8 *hdr;
>>> +    struct rte_ipv4_hdr *ipv4_hdr;
>>> +    u16 eth_type;
>>> -    hdr = (u8 *)rte_pktmbuf_mtod(mbuf, u8*);
>>> -    eth_hdr = (struct rte_ether_hdr *)hdr;
>>> -    pkt_type = rte_be_to_cpu_16(eth_hdr->ether_type);
>>> +    eth_hdr = rte_pktmbuf_mtod(mbuf, struct rte_ether_hdr *);
>>> +    eth_type = rte_be_to_cpu_16(eth_hdr->ether_type);
>>> -    if (pkt_type == RTE_ETHER_TYPE_VLAN) {
>>> +    if (eth_type == RTE_ETHER_TYPE_VLAN) {
>>>           off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
>>> -        vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
>>> -        pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>>> +        vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
>>> +        eth_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>>>       } else {
>>>           off_info->outer_l2_len = ETHER_LEN_NO_VLAN;
>>>       }
>>> -    if (pkt_type == RTE_ETHER_TYPE_IPV4) {
>>> -        ip4h = (struct rte_ipv4_hdr *)(hdr + off_info->outer_l2_len);
>>> -        off_info->outer_l3_len = rte_ipv4_hdr_len(ip4h);
>>> -    } else if (pkt_type == RTE_ETHER_TYPE_IPV6) {
>>> +    if (eth_type == RTE_ETHER_TYPE_IPV4) {
>>> +        ipv4_hdr = rte_pktmbuf_mtod_offset(mbuf, struct rte_ipv4_hdr *,
>>> +                           off_info->outer_l2_len);
>>> +        off_info->outer_l3_len = rte_ipv4_hdr_len(ipv4_hdr);
>>> +    } else if (eth_type == RTE_ETHER_TYPE_IPV6) {
>>>           /* not support ipv6 extension header */
>>>           off_info->outer_l3_len = sizeof(struct rte_ipv6_hdr);
>>>       }
>>>
>>
>>
>> The actual fix is following [1] and rest is refactoring, right?
>> It is hard to catch the actual fix with refactoring, can you please describe 
>> the actual problem and fix in the commit log to clarify it?
>>
>>
>>
>> [1]
>>   @@ -789,7 +789,7 @@ static inline void hinic_analyze_tx_info(struct rte_mbuf 
>> *mbuf,
>>
>>           if (pkt_type == RTE_ETHER_TYPE_VLAN) {
>>                   off_info->outer_l2_len = ETHER_LEN_WITH_VLAN;
>>   -               vlan_hdr = (struct rte_vlan_hdr *)(hdr + 1);
>>   +               vlan_hdr = (struct rte_vlan_hdr *)(eth_hdr + 1);
>>                   pkt_type = rte_be_to_cpu_16(vlan_hdr->eth_proto);
>>           } else {
>>                   off_info->outer_l2_len = ETHER_LEN_NO_VLAN;
>> .
 > Yes,the actual fix is following as this patch:
 >
 > commit 8c8b61234ffd9a283cecbf9751942cbdb87d68f6
 > Author: Xiaoyun Wang <cloud.wangxiaoyun@huawei.com>
 > Date:   Sat Jul 25 16:15:33 2020 +0800
 >
 >      net/hinic: refactor checksum functions
 >
 >      Encapsulate different types of packet checksum preprocessing
 >      into functions.
 >
 >

I am not asking the original patch that introduced the problem.
In this patch, the single line I put above is the only required change, right?

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

end of thread, other threads:[~2020-11-04 11:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-31  3:38 [dpdk-dev] [PATCH v1 0/2] fix some csum errors Xiaoyun wang
2020-10-31  3:38 ` [dpdk-dev] [PATCH v1 1/2] net/hinic: fix outer_l3_len parse error Xiaoyun wang
2020-11-02 17:08   ` Ferruh Yigit
2020-11-02 17:26     ` Ferruh Yigit
2020-11-04  2:19     ` Wangxiaoyun (Cloud)
2020-11-04 11:05       ` Ferruh Yigit
2020-10-31  3:38 ` [dpdk-dev] [PATCH v1 2/2] net/hinic: fix SCTP checksum error Xiaoyun wang
2020-11-02 17:47 ` [dpdk-dev] [PATCH v1 0/2] fix some csum errors Ferruh Yigit

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git