Re: [RFC] tcp: Replace TCP buffer structure by an iovec array

[Laurent Vivier <lvivier@redhat.com>]

On 3/13/24 12:37, Stefano Brivio wrote:
> On Mon, 11 Mar 2024 14:33:56 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
...
>> @@ -445,133 +413,78 @@ struct tcp_buf_seq_update {
>>   };
>>   
>>   /* Static buffers */
>> -
>>   /**
>> - * tcp4_l2_buf_t - Pre-cooked IPv4 packet buffers for tap connections
>> - * @pad:	Align TCP header to 32 bytes, for AVX2 checksum calculation only
>> - * @taph:	Tap-level headers (partially pre-filled)
>> - * @iph:	Pre-filled IP header (except for tot_len and saddr)
>> - * @uh:		Headroom for TCP header
>> - * @data:	Storage for TCP payload
>> + * tcp_l2_flags_t - TCP header and data to send option flags
> 
> 'struct tcp_l2_flags_t - ...', by the way (pre-existing, I see).
> 
>> + * @th:		TCP header
>> + * @opts	TCP option flags
>>    */
>> -static struct tcp4_l2_buf_t {
>> -#ifdef __AVX2__
>> -	uint8_t pad[26];	/* 0, align th to 32 bytes */
>> -#else
>> -	uint8_t pad[2];		/*	align iph to 4 bytes	0 */
>> -#endif
>> -	struct tap_hdr taph;	/* 26				2 */
>> -	struct iphdr iph;	/* 44				20 */
>> -	struct tcphdr th;	/* 64				40 */
>> -	uint8_t data[MSS4];	/* 84				60 */
>> -				/* 65536			65532 */
>> +struct tcp_l2_flags_t {
>> +	struct tcphdr th;
>> +	char opts[OPT_MSS_LEN + OPT_WS_LEN + 1];
>> +};
> 
> This should still be aligned (when declared below) with:
> 
> #ifdef __AVX2__
> } __attribute__ ((packed, aligned(32)))
> #else
> } __attribute__ ((packed, aligned(__alignof__(unsigned int))))
> #endif
> 
> because yes, now you get the unsigned int alignment for free, but the
> 32-byte boundary for AVX2 (checksums) not really, so that needs to be
> there, and then for clarity I would keep the explicit attribute for the
> non-AVX2 case.
> 
> By the way, I guess you could still combine the definition and
> declaration as it was done earlier.

We can't combine the definition and declaration because the same definition is used with 
IPv4 and IPv6 TCP arrays declaration.

I'm not sure: the __attribute__ must be used on the structure definition or on the array 
of structures declaration?

Thanks,
Laurent