Re: [PATCH v5 4/5] tcp: Update TCP checksum using an iovec array

[Laurent Vivier <lvivier@redhat.com>]

On 02/10/2024 11:00, Stefano Brivio wrote:
> On 2 October 2024 09:39:23 CEST, Laurent Vivier <lvivier@redhat.com> wrote:
>> On 01/10/2024 20:22, Stefano Brivio wrote:
>>> On Tue, 1 Oct 2024 09:29:28 +0200
>>> Stefano Brivio <sbrivio@redhat.com> wrote:
>>>
>>>> On Fri, 27 Sep 2024 15:53:48 +0200
>>>> Laurent Vivier <lvivier@redhat.com> wrote:
>>>>
>>>>> [...]
>>>>>
>>>>> +void tcp_update_check_tcp4(const struct iphdr *iph,
>>>>> +			   const struct iovec *iov, int iov_cnt,
>>>>> +			   size_t l4offset)
>>>>>    {
>>>>>    	uint16_t l4len = ntohs(iph->tot_len) - sizeof(struct iphdr);
>>>>>    	struct in_addr saddr = { .s_addr = iph->saddr };
>>>>>    	struct in_addr daddr = { .s_addr = iph->daddr };
>>>>> -	uint32_t sum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, saddr, daddr);
>>>>> +	size_t check_ofs;
>>>>> +	__sum16 *check;
>>>>> +	int check_idx;
>>>>> +	uint32_t sum;
>>>>> +	uintptr_t ptr;
>>>>> +
>>>>> +	sum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, saddr, daddr);
>>>>> +
>>>>> +	check_idx = iov_skip_bytes(iov, iov_cnt,
>>>>> +				   l4offset + offsetof(struct tcphdr, check),
>>>>> +				   &check_ofs);
>>>>> +
>>>>> +	if (check_idx >= iov_cnt) {
>>>>> +		err("TCP4 buffer is too small, iov size %zd, check offset %zd",
>>>>> +		    iov_size(iov, iov_cnt),
>>>>> +		    l4offset + offsetof(struct tcphdr, check));
>>>>> +		return;
>>>>> +	}
>>>>>    -	bp->th.check = 0;
>>>>> -	bp->th.check = csum(bp, l4len, sum);
>>>>> +	if (check_ofs + sizeof(*check) > iov[check_idx].iov_len) {
>>>>> +		err("TCP4 checksum field memory is not contiguous "
>>>>> +		    "check_ofs %zd check_idx %d iov_len %zd",
>>>>> +		    check_ofs, check_idx, iov[check_idx].iov_len);
>>>>> +		return;
>>>>> +	}
>>>>> +
>>>>> +	ptr = (uintptr_t)((char *)iov[check_idx].iov_base + check_ofs);
>>>>> +	if (ptr & (__alignof__(*check) - 1)) {
>>>>> +		err("TCP4 checksum field is not correctly aligned in memory");
>>>>> +		return;
>>>>> +	}
>>>>> +
>>>>> +	check = (__sum16 *)ptr;
>>>>
>>>> clang-tidy (14.0.6) says:
>>>>
>>>> /home/sbrivio/passt/tcp.c:803:10: error: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr,-warnings-as-errors]
>>>>           check = (__sum16 *)ptr;
>>>>                   ^
>>>> /home/sbrivio/passt/tcp.c:854:10: error: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr,-warnings-as-errors]
>>>>           check = (__sum16 *)ptr;
>>>>                   ^
>>>>
>>>> ...I'm still trying to figure out what that actually means. If it's
>>>> trivial I can fix that up on merge.
>>>
>>> Something like this?
>>
>> It's ok for me. Do you want I resend a version with it or can you update the patch on the merge?
> 
> No no, thanks, I'll update this on merge.
> 

I also removed the static of tcp_update_check_tcp4() and tcp_update_check_tcp6(). I need 
this later in my series but not here. Could you update or do you want I send a new version?

Thanks,
Laurent