Re: [PATCH v2] net/virtio: deduce IP length for Virtio TSO checksum

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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;
>>>
>