DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/virtio: deduce IP length for Virtio TSO checksum
@ 2023-02-16 12:35 Boleslav Stankevich
  2023-02-16 12:35 ` Boleslav Stankevich
  0 siblings, 1 reply; 10+ messages in thread
From: Boleslav Stankevich @ 2023-02-16 12:35 UTC (permalink / raw)
  To: dev; +Cc: Boleslav Stankevich, Andrew Rybchenko, Maxime Coquelin, Chenbo Xia

The length of TSO payload could not fit into 16 bits provided by the
IPv4 total length and IPv6 payload length fields. Thus, deduce it
from the length of the packet.

Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 2d0afd3302..e48ff3cca7 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
 	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
 			m->l4_len)) {
 		struct rte_ipv4_hdr *iph;
-		struct rte_ipv6_hdr *ip6h;
 		struct rte_tcp_hdr *th;
-		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
+		uint16_t prev_cksum, new_cksum;
+		uint32_t ip_paylen;
 		uint32_t tmp;
 
 		iph = rte_pktmbuf_mtod_offset(m,
 					struct rte_ipv4_hdr *, m->l2_len);
 		th = RTE_PTR_ADD(iph, m->l3_len);
+
+		/*
+		 * Calculate IPv4 header checksum with current total length value
+		 * (whatever it is) to have correct checksum after update on edits
+		 * done by TSO.
+		 */
 		if ((iph->version_ihl >> 4) == 4) {
 			iph->hdr_checksum = 0;
 			iph->hdr_checksum = rte_ipv4_cksum(iph);
-			ip_len = iph->total_length;
-			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
-				m->l3_len);
-		} else {
-			ip6h = (struct rte_ipv6_hdr *)iph;
-			ip_paylen = ip6h->payload_len;
 		}
 
+		/*
+		 * Do not use IPv4 total length and IPv6 payload length fields to get
+		 * TSO payload length since it could not fit into 16 bits.
+		 */
+		ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - m->l2_len -
+					m->l3_len);
+
 		/* calculate the new phdr checksum not including ip_paylen */
 		prev_cksum = th->cksum;
 		tmp = prev_cksum;
-		tmp += ip_paylen;
+		tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
 		tmp = (tmp & 0xffff) + (tmp >> 16);
 		new_cksum = tmp;
 
-- 
2.30.2


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

* [PATCH] net/virtio: deduce IP length for Virtio TSO checksum
  2023-02-16 12:35 [PATCH] net/virtio: deduce IP length for Virtio TSO checksum Boleslav Stankevich
@ 2023-02-16 12:35 ` Boleslav Stankevich
  2023-03-02 10:25   ` Maxime Coquelin
  2023-03-03 11:19   ` [PATCH v2] " Boleslav Stankevich
  0 siblings, 2 replies; 10+ messages in thread
From: Boleslav Stankevich @ 2023-02-16 12:35 UTC (permalink / raw)
  To: dev; +Cc: Boleslav Stankevich, Andrew Rybchenko, Maxime Coquelin, Chenbo Xia

The length of TSO payload could not fit into 16 bits provided by the
IPv4 total length and IPv6 payload length fields. Thus, deduce it
from the length of the packet.

Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 2d0afd3302..e48ff3cca7 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
 	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
 			m->l4_len)) {
 		struct rte_ipv4_hdr *iph;
-		struct rte_ipv6_hdr *ip6h;
 		struct rte_tcp_hdr *th;
-		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
+		uint16_t prev_cksum, new_cksum;
+		uint32_t ip_paylen;
 		uint32_t tmp;
 
 		iph = rte_pktmbuf_mtod_offset(m,
 					struct rte_ipv4_hdr *, m->l2_len);
 		th = RTE_PTR_ADD(iph, m->l3_len);
+
+		/*
+		 * Calculate IPv4 header checksum with current total length value
+		 * (whatever it is) to have correct checksum after update on edits
+		 * done by TSO.
+		 */
 		if ((iph->version_ihl >> 4) == 4) {
 			iph->hdr_checksum = 0;
 			iph->hdr_checksum = rte_ipv4_cksum(iph);
-			ip_len = iph->total_length;
-			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
-				m->l3_len);
-		} else {
-			ip6h = (struct rte_ipv6_hdr *)iph;
-			ip_paylen = ip6h->payload_len;
 		}
 
+		/*
+		 * Do not use IPv4 total length and IPv6 payload length fields to get
+		 * TSO payload length since it could not fit into 16 bits.
+		 */
+		ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - m->l2_len -
+					m->l3_len);
+
 		/* calculate the new phdr checksum not including ip_paylen */
 		prev_cksum = th->cksum;
 		tmp = prev_cksum;
-		tmp += ip_paylen;
+		tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
 		tmp = (tmp & 0xffff) + (tmp >> 16);
 		new_cksum = tmp;
 
-- 
2.30.2


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

* Re: [PATCH] net/virtio: deduce IP length for Virtio TSO checksum
  2023-02-16 12:35 ` Boleslav Stankevich
@ 2023-03-02 10:25   ` Maxime Coquelin
  2023-03-03 11:17     ` Boleslav Stankevich
  2023-03-03 11:19   ` [PATCH v2] " Boleslav Stankevich
  1 sibling, 1 reply; 10+ messages in thread
From: Maxime Coquelin @ 2023-03-02 10:25 UTC (permalink / raw)
  To: Boleslav Stankevich, dev; +Cc: Andrew Rybchenko, Chenbo Xia

Hi Boleslav,

On 2/16/23 13:35, Boleslav Stankevich wrote:
> The length of TSO payload could not fit into 16 bits provided by the
> IPv4 total length and IPv6 payload length fields. Thus, deduce it
> from the length of the packet.

My understanding is this is a fix.
If so could you please add the Fixes tag and Cc: stable@dpdk.org?

Thanks,
Maxime

> Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>   drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 2d0afd3302..e48ff3cca7 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
>   	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
>   			m->l4_len)) {
>   		struct rte_ipv4_hdr *iph;
> -		struct rte_ipv6_hdr *ip6h;
>   		struct rte_tcp_hdr *th;
> -		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
> +		uint16_t prev_cksum, new_cksum;
> +		uint32_t ip_paylen;
>   		uint32_t tmp;
>   
>   		iph = rte_pktmbuf_mtod_offset(m,
>   					struct rte_ipv4_hdr *, m->l2_len);
>   		th = RTE_PTR_ADD(iph, m->l3_len);
> +
> +		/*
> +		 * Calculate IPv4 header checksum with current total length value
> +		 * (whatever it is) to have correct checksum after update on edits
> +		 * done by TSO.
> +		 */
>   		if ((iph->version_ihl >> 4) == 4) {
>   			iph->hdr_checksum = 0;
>   			iph->hdr_checksum = rte_ipv4_cksum(iph);
> -			ip_len = iph->total_length;
> -			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
> -				m->l3_len);
> -		} else {
> -			ip6h = (struct rte_ipv6_hdr *)iph;
> -			ip_paylen = ip6h->payload_len;
>   		}
>   
> +		/*
> +		 * Do not use IPv4 total length and IPv6 payload length fields to get
> +		 * TSO payload length since it could not fit into 16 bits.
> +		 */
> +		ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - m->l2_len -
> +					m->l3_len);
> +
>   		/* calculate the new phdr checksum not including ip_paylen */
>   		prev_cksum = th->cksum;
>   		tmp = prev_cksum;
> -		tmp += ip_paylen;
> +		tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
>   		tmp = (tmp & 0xffff) + (tmp >> 16);
>   		new_cksum = tmp;
>   


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

* Re: [PATCH] net/virtio: deduce IP length for Virtio TSO checksum
  2023-03-02 10:25   ` Maxime Coquelin
@ 2023-03-03 11:17     ` Boleslav Stankevich
  0 siblings, 0 replies; 10+ messages in thread
From: Boleslav Stankevich @ 2023-03-03 11:17 UTC (permalink / raw)
  To: Maxime Coquelin, dev; +Cc: Andrew Rybchenko, Chenbo Xia

Hi, Maxime!

Yes, it is a fix and will add missing tags in v2.

Thanks,
Boleslav

On 02/03/2023 13:25, Maxime Coquelin wrote:
> Hi Boleslav,
>
> On 2/16/23 13:35, Boleslav Stankevich wrote:
>> The length of TSO payload could not fit into 16 bits provided by the
>> IPv4 total length and IPv6 payload length fields. Thus, deduce it
>> from the length of the packet.
>
> My understanding is this is a fix.
> If so could you please add the Fixes tag and Cc: stable@dpdk.org?
>
> Thanks,
> Maxime
>
>> Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> ---
>>   drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
>>   1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_rxtx.c 
>> b/drivers/net/virtio/virtio_rxtx.c
>> index 2d0afd3302..e48ff3cca7 100644
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
>>       if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
>>               m->l4_len)) {
>>           struct rte_ipv4_hdr *iph;
>> -        struct rte_ipv6_hdr *ip6h;
>>           struct rte_tcp_hdr *th;
>> -        uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
>> +        uint16_t prev_cksum, new_cksum;
>> +        uint32_t ip_paylen;
>>           uint32_t tmp;
>>             iph = rte_pktmbuf_mtod_offset(m,
>>                       struct rte_ipv4_hdr *, m->l2_len);
>>           th = RTE_PTR_ADD(iph, m->l3_len);
>> +
>> +        /*
>> +         * Calculate IPv4 header checksum with current total length 
>> value
>> +         * (whatever it is) to have correct checksum after update on 
>> edits
>> +         * done by TSO.
>> +         */
>>           if ((iph->version_ihl >> 4) == 4) {
>>               iph->hdr_checksum = 0;
>>               iph->hdr_checksum = rte_ipv4_cksum(iph);
>> -            ip_len = iph->total_length;
>> -            ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
>> -                m->l3_len);
>> -        } else {
>> -            ip6h = (struct rte_ipv6_hdr *)iph;
>> -            ip_paylen = ip6h->payload_len;
>>           }
>>   +        /*
>> +         * Do not use IPv4 total length and IPv6 payload length 
>> fields to get
>> +         * TSO payload length since it could not fit into 16 bits.
>> +         */
>> +        ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - 
>> m->l2_len -
>> +                    m->l3_len);
>> +
>>           /* calculate the new phdr checksum not including ip_paylen */
>>           prev_cksum = th->cksum;
>>           tmp = prev_cksum;
>> -        tmp += ip_paylen;
>> +        tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
>>           tmp = (tmp & 0xffff) + (tmp >> 16);
>>           new_cksum = tmp;
>

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

* [PATCH v2] net/virtio: deduce IP length for Virtio TSO checksum
  2023-02-16 12:35 ` Boleslav Stankevich
  2023-03-02 10:25   ` Maxime Coquelin
@ 2023-03-03 11:19   ` Boleslav Stankevich
  2023-03-03 15:13     ` Andrew Rybchenko
                       ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Boleslav Stankevich @ 2023-03-03 11:19 UTC (permalink / raw)
  To: dev; +Cc: Boleslav Stankevich, stable, Andrew Rybchenko

The length of TSO payload could not fit into 16 bits provided by the
IPv4 total length and IPv6 payload length fields. Thus, deduce it
from the length of the packet.

Fixes: 696573046e9 ("net/virtio: support TSO")
Cc: stable@dpdk.org

Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 2d0afd3302..e48ff3cca7 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
 	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
 			m->l4_len)) {
 		struct rte_ipv4_hdr *iph;
-		struct rte_ipv6_hdr *ip6h;
 		struct rte_tcp_hdr *th;
-		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
+		uint16_t prev_cksum, new_cksum;
+		uint32_t ip_paylen;
 		uint32_t tmp;
 
 		iph = rte_pktmbuf_mtod_offset(m,
 					struct rte_ipv4_hdr *, m->l2_len);
 		th = RTE_PTR_ADD(iph, m->l3_len);
+
+		/*
+		 * Calculate IPv4 header checksum with current total length value
+		 * (whatever it is) to have correct checksum after update on edits
+		 * done by TSO.
+		 */
 		if ((iph->version_ihl >> 4) == 4) {
 			iph->hdr_checksum = 0;
 			iph->hdr_checksum = rte_ipv4_cksum(iph);
-			ip_len = iph->total_length;
-			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
-				m->l3_len);
-		} else {
-			ip6h = (struct rte_ipv6_hdr *)iph;
-			ip_paylen = ip6h->payload_len;
 		}
 
+		/*
+		 * Do not use IPv4 total length and IPv6 payload length fields to get
+		 * TSO payload length since it could not fit into 16 bits.
+		 */
+		ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - m->l2_len -
+					m->l3_len);
+
 		/* calculate the new phdr checksum not including ip_paylen */
 		prev_cksum = th->cksum;
 		tmp = prev_cksum;
-		tmp += ip_paylen;
+		tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
 		tmp = (tmp & 0xffff) + (tmp >> 16);
 		new_cksum = tmp;
 
-- 
2.30.2

v2:
* Extend Description with Fixes and CC tags.

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

* Re: [PATCH v2] net/virtio: deduce IP length for Virtio TSO checksum
  2023-03-03 11:19   ` [PATCH v2] " Boleslav Stankevich
@ 2023-03-03 15:13     ` Andrew Rybchenko
  2023-03-06  6:17       ` Xia, Chenbo
  2023-03-06 14:13     ` Maxime Coquelin
  2023-03-06 14:20     ` Maxime Coquelin
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Rybchenko @ 2023-03-03 15:13 UTC (permalink / raw)
  To: Boleslav Stankevich, dev; +Cc: stable, Maxime Coquelin, Chenbo Xia

Cc Maxime and Chenbo

On 3/3/23 14:19, Boleslav Stankevich wrote:
> The length of TSO payload could not fit into 16 bits provided by the
> IPv4 total length and IPv6 payload length fields. Thus, deduce it
> from the length of the packet.
> 
> Fixes: 696573046e9 ("net/virtio: support TSO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>   drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 2d0afd3302..e48ff3cca7 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
>   	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
>   			m->l4_len)) {
>   		struct rte_ipv4_hdr *iph;
> -		struct rte_ipv6_hdr *ip6h;
>   		struct rte_tcp_hdr *th;
> -		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
> +		uint16_t prev_cksum, new_cksum;
> +		uint32_t ip_paylen;
>   		uint32_t tmp;
>   
>   		iph = rte_pktmbuf_mtod_offset(m,
>   					struct rte_ipv4_hdr *, m->l2_len);
>   		th = RTE_PTR_ADD(iph, m->l3_len);
> +
> +		/*
> +		 * Calculate IPv4 header checksum with current total length value
> +		 * (whatever it is) to have correct checksum after update on edits
> +		 * done by TSO.
> +		 */
>   		if ((iph->version_ihl >> 4) == 4) {
>   			iph->hdr_checksum = 0;
>   			iph->hdr_checksum = rte_ipv4_cksum(iph);
> -			ip_len = iph->total_length;
> -			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
> -				m->l3_len);
> -		} else {
> -			ip6h = (struct rte_ipv6_hdr *)iph;
> -			ip_paylen = ip6h->payload_len;
>   		}
>   
> +		/*
> +		 * Do not use IPv4 total length and IPv6 payload length fields to get
> +		 * TSO payload length since it could not fit into 16 bits.
> +		 */
> +		ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - m->l2_len -
> +					m->l3_len);
> +
>   		/* calculate the new phdr checksum not including ip_paylen */
>   		prev_cksum = th->cksum;
>   		tmp = prev_cksum;
> -		tmp += ip_paylen;
> +		tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
>   		tmp = (tmp & 0xffff) + (tmp >> 16);
>   		new_cksum = tmp;
>   


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

* RE: [PATCH v2] net/virtio: deduce IP length for Virtio TSO checksum
  2023-03-03 15:13     ` Andrew Rybchenko
@ 2023-03-06  6:17       ` Xia, Chenbo
  2023-03-06 10:57         ` Maxime Coquelin
  0 siblings, 1 reply; 10+ messages in thread
From: Xia, Chenbo @ 2023-03-06  6:17 UTC (permalink / raw)
  To: Andrew Rybchenko, Boleslav Stankevich, dev; +Cc: stable, Maxime Coquelin

Hi Boleslav,

The change seems good, but patchwork is complaining about lack of .mailmap change.

http://mails.dpdk.org/archives/test-report/2023-March/363061.html

Guess this is your first patch? So you need to add name and email in mailmap file.

Thanks,
Chenbo

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Friday, March 3, 2023 11:14 PM
> To: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>; dev@dpdk.org
> Cc: stable@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>; Xia,
> Chenbo <chenbo.xia@intel.com>
> Subject: Re: [PATCH v2] net/virtio: deduce IP length for Virtio TSO
> checksum
> 
> Cc Maxime and Chenbo
> 
> On 3/3/23 14:19, Boleslav Stankevich wrote:
> > The length of TSO payload could not fit into 16 bits provided by the
> > IPv4 total length and IPv6 payload length fields. Thus, deduce it
> > from the length of the packet.
> >
> > Fixes: 696573046e9 ("net/virtio: support TSO")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
> > Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > ---
> >   drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
> >   1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_rxtx.c
> b/drivers/net/virtio/virtio_rxtx.c
> > index 2d0afd3302..e48ff3cca7 100644
> > --- a/drivers/net/virtio/virtio_rxtx.c
> > +++ b/drivers/net/virtio/virtio_rxtx.c
> > @@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
> >   	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
> >   			m->l4_len)) {
> >   		struct rte_ipv4_hdr *iph;
> > -		struct rte_ipv6_hdr *ip6h;
> >   		struct rte_tcp_hdr *th;
> > -		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
> > +		uint16_t prev_cksum, new_cksum;
> > +		uint32_t ip_paylen;
> >   		uint32_t tmp;
> >
> >   		iph = rte_pktmbuf_mtod_offset(m,
> >   					struct rte_ipv4_hdr *, m->l2_len);
> >   		th = RTE_PTR_ADD(iph, m->l3_len);
> > +
> > +		/*
> > +		 * Calculate IPv4 header checksum with current total length
> value
> > +		 * (whatever it is) to have correct checksum after update on
> edits
> > +		 * done by TSO.
> > +		 */
> >   		if ((iph->version_ihl >> 4) == 4) {
> >   			iph->hdr_checksum = 0;
> >   			iph->hdr_checksum = rte_ipv4_cksum(iph);
> > -			ip_len = iph->total_length;
> > -			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
> > -				m->l3_len);
> > -		} else {
> > -			ip6h = (struct rte_ipv6_hdr *)iph;
> > -			ip_paylen = ip6h->payload_len;
> >   		}
> >
> > +		/*
> > +		 * Do not use IPv4 total length and IPv6 payload length fields
> to get
> > +		 * TSO payload length since it could not fit into 16 bits.
> > +		 */
> > +		ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - m-
> >l2_len -
> > +					m->l3_len);
> > +
> >   		/* calculate the new phdr checksum not including ip_paylen */
> >   		prev_cksum = th->cksum;
> >   		tmp = prev_cksum;
> > -		tmp += ip_paylen;
> > +		tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
> >   		tmp = (tmp & 0xffff) + (tmp >> 16);
> >   		new_cksum = tmp;
> >


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

* Re: [PATCH v2] net/virtio: deduce IP length for Virtio TSO checksum
  2023-03-06  6:17       ` Xia, Chenbo
@ 2023-03-06 10:57         ` Maxime Coquelin
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Coquelin @ 2023-03-06 10:57 UTC (permalink / raw)
  To: Xia, Chenbo, Andrew Rybchenko, Boleslav Stankevich, dev; +Cc: stable



On 3/6/23 07:17, Xia, Chenbo wrote:
> Hi Boleslav,
> 
> The change seems good, but patchwork is complaining about lack of .mailmap change.
> 
> http://mails.dpdk.org/archives/test-report/2023-March/363061.html
> 
> Guess this is your first patch? So you need to add name and email in mailmap file.


Thanks for the heads-up, I will update the mailmap when applying.
No need to submit a new revision.

Thanks,
Maxime

> Thanks,
> Chenbo
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Friday, March 3, 2023 11:14 PM
>> To: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>; dev@dpdk.org
>> Cc: stable@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>; Xia,
>> Chenbo <chenbo.xia@intel.com>
>> Subject: Re: [PATCH v2] net/virtio: deduce IP length for Virtio TSO
>> checksum
>>
>> Cc Maxime and Chenbo
>>
>> On 3/3/23 14:19, Boleslav Stankevich wrote:
>>> The length of TSO payload could not fit into 16 bits provided by the
>>> IPv4 total length and IPv6 payload length fields. Thus, deduce it
>>> from the length of the packet.
>>>
>>> Fixes: 696573046e9 ("net/virtio: support TSO")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
>>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>> ---
>>>    drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
>>>    1 file changed, 16 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_rxtx.c
>> b/drivers/net/virtio/virtio_rxtx.c
>>> index 2d0afd3302..e48ff3cca7 100644
>>> --- a/drivers/net/virtio/virtio_rxtx.c
>>> +++ b/drivers/net/virtio/virtio_rxtx.c
>>> @@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
>>>    	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
>>>    			m->l4_len)) {
>>>    		struct rte_ipv4_hdr *iph;
>>> -		struct rte_ipv6_hdr *ip6h;
>>>    		struct rte_tcp_hdr *th;
>>> -		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
>>> +		uint16_t prev_cksum, new_cksum;
>>> +		uint32_t ip_paylen;
>>>    		uint32_t tmp;
>>>
>>>    		iph = rte_pktmbuf_mtod_offset(m,
>>>    					struct rte_ipv4_hdr *, m->l2_len);
>>>    		th = RTE_PTR_ADD(iph, m->l3_len);
>>> +
>>> +		/*
>>> +		 * Calculate IPv4 header checksum with current total length
>> value
>>> +		 * (whatever it is) to have correct checksum after update on
>> edits
>>> +		 * done by TSO.
>>> +		 */
>>>    		if ((iph->version_ihl >> 4) == 4) {
>>>    			iph->hdr_checksum = 0;
>>>    			iph->hdr_checksum = rte_ipv4_cksum(iph);
>>> -			ip_len = iph->total_length;
>>> -			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
>>> -				m->l3_len);
>>> -		} else {
>>> -			ip6h = (struct rte_ipv6_hdr *)iph;
>>> -			ip_paylen = ip6h->payload_len;
>>>    		}
>>>
>>> +		/*
>>> +		 * Do not use IPv4 total length and IPv6 payload length fields
>> to get
>>> +		 * TSO payload length since it could not fit into 16 bits.
>>> +		 */
>>> +		ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - m-
>>> l2_len -
>>> +					m->l3_len);
>>> +
>>>    		/* calculate the new phdr checksum not including ip_paylen */
>>>    		prev_cksum = th->cksum;
>>>    		tmp = prev_cksum;
>>> -		tmp += ip_paylen;
>>> +		tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
>>>    		tmp = (tmp & 0xffff) + (tmp >> 16);
>>>    		new_cksum = tmp;
>>>
> 


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

* Re: [PATCH v2] net/virtio: deduce IP length for Virtio TSO checksum
  2023-03-03 11:19   ` [PATCH v2] " Boleslav Stankevich
  2023-03-03 15:13     ` Andrew Rybchenko
@ 2023-03-06 14:13     ` Maxime Coquelin
  2023-03-06 14:20     ` Maxime Coquelin
  2 siblings, 0 replies; 10+ messages in thread
From: Maxime Coquelin @ 2023-03-06 14:13 UTC (permalink / raw)
  To: Boleslav Stankevich, dev; +Cc: stable, Andrew Rybchenko



On 3/3/23 12:19, Boleslav Stankevich wrote:
> The length of TSO payload could not fit into 16 bits provided by the
> IPv4 total length and IPv6 payload length fields. Thus, deduce it
> from the length of the packet.
> 
> Fixes: 696573046e9 ("net/virtio: support TSO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>   drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 2d0afd3302..e48ff3cca7 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -404,29 +404,36 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
>   	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
>   			m->l4_len)) {
>   		struct rte_ipv4_hdr *iph;
> -		struct rte_ipv6_hdr *ip6h;
>   		struct rte_tcp_hdr *th;
> -		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
> +		uint16_t prev_cksum, new_cksum;
> +		uint32_t ip_paylen;
>   		uint32_t tmp;
>   
>   		iph = rte_pktmbuf_mtod_offset(m,
>   					struct rte_ipv4_hdr *, m->l2_len);
>   		th = RTE_PTR_ADD(iph, m->l3_len);
> +
> +		/*
> +		 * Calculate IPv4 header checksum with current total length value
> +		 * (whatever it is) to have correct checksum after update on edits
> +		 * done by TSO.
> +		 */
>   		if ((iph->version_ihl >> 4) == 4) {
>   			iph->hdr_checksum = 0;
>   			iph->hdr_checksum = rte_ipv4_cksum(iph);
> -			ip_len = iph->total_length;
> -			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
> -				m->l3_len);
> -		} else {
> -			ip6h = (struct rte_ipv6_hdr *)iph;
> -			ip_paylen = ip6h->payload_len;
>   		}
>   
> +		/*
> +		 * Do not use IPv4 total length and IPv6 payload length fields to get
> +		 * TSO payload length since it could not fit into 16 bits.
> +		 */
> +		ip_paylen = rte_cpu_to_be_32(rte_pktmbuf_pkt_len(m) - m->l2_len -
> +					m->l3_len);
> +
>   		/* calculate the new phdr checksum not including ip_paylen */
>   		prev_cksum = th->cksum;
>   		tmp = prev_cksum;
> -		tmp += ip_paylen;
> +		tmp += (ip_paylen & 0xffff) + (ip_paylen >> 16);
>   		tmp = (tmp & 0xffff) + (tmp >> 16);
>   		new_cksum = tmp;
>   

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


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

* Re: [PATCH v2] net/virtio: deduce IP length for Virtio TSO checksum
  2023-03-03 11:19   ` [PATCH v2] " Boleslav Stankevich
  2023-03-03 15:13     ` Andrew Rybchenko
  2023-03-06 14:13     ` Maxime Coquelin
@ 2023-03-06 14:20     ` Maxime Coquelin
  2 siblings, 0 replies; 10+ messages in thread
From: Maxime Coquelin @ 2023-03-06 14:20 UTC (permalink / raw)
  To: Boleslav Stankevich, dev; +Cc: stable, Andrew Rybchenko



On 3/3/23 12:19, Boleslav Stankevich wrote:
> The length of TSO payload could not fit into 16 bits provided by the
> IPv4 total length and IPv6 payload length fields. Thus, deduce it
> from the length of the packet.
> 
> Fixes: 696573046e9 ("net/virtio: support TSO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Boleslav Stankevich <boleslav.stankevich@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>   drivers/net/virtio/virtio_rxtx.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime


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

end of thread, other threads:[~2023-03-06 14:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-16 12:35 [PATCH] net/virtio: deduce IP length for Virtio TSO checksum Boleslav Stankevich
2023-02-16 12:35 ` Boleslav Stankevich
2023-03-02 10:25   ` Maxime Coquelin
2023-03-03 11:17     ` Boleslav Stankevich
2023-03-03 11:19   ` [PATCH v2] " Boleslav Stankevich
2023-03-03 15:13     ` Andrew Rybchenko
2023-03-06  6:17       ` Xia, Chenbo
2023-03-06 10:57         ` Maxime Coquelin
2023-03-06 14:13     ` Maxime Coquelin
2023-03-06 14:20     ` Maxime Coquelin

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).