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

[Boleslav Stankevich <Boleslav.Stankevich@oktetlabs.ru>]

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