Re: [PATCH net-next v3 03/15] quic: provide common utilities and data structures

Paolo Abeni <[email protected]> Tue, 23 Sep 2025 13:21:33 +0200
Newsgroups dev.linux.lists.quic,dev.linux.lists.kernel-tls-handshake,org.kernel.vger.linux-cifs,org.kernel.vger.netdev
Message-ID <[email protected]>
On 9/19/25 12:34 AM, Xin Long wrote:
> This patch provides foundational data structures and utilities used
> throughout the QUIC stack.
> 
> It introduces packet header types, connection ID support, and address
> handling. Hash tables are added to manage socket lookup and connection
> ID mapping.
> 
> A flexible binary data type is provided, along with helpers for parsing,
> matching, and memory management. Helpers for encoding and decoding
> transport parameters and frames are also included.
> 
> Signed-off-by: Xin Long <[email protected]>
> ---
> v3:
>   - Rework hashtables: split into two types and size them based on
>     totalram_pages(), similar to SCTP (reported by Paolo).
>   - struct quic_shash_table: use rwlock instead of spinlock.

Why? rwlock usage should be avoided in networking (as it's unfair, see
the many refactors replacing rwlock with rcu/plain spinlock)

[...]
> +
> +static int quic_uhash_table_init(struct quic_uhash_table *ht, u32 max_size, int order)
> +{
> +	int i, max_order, size;
> +
> +	/* Same sizing logic as in quic_shash_table_init(). */
> +	max_order = get_order(max_size * sizeof(struct quic_uhash_head));
> +	order = min(order, max_order);
> +	do {
> +		ht->hash = (struct quic_uhash_head *)
> +			__get_free_pages(GFP_KERNEL | __GFP_NOWARN, order);
> +	} while (!ht->hash && --order > 0);

You can avoid a little complexity, and see more consistent behaviour,
using plain vmalloc() or alloc_large_system_hash() with no fallback.


> +/* rfc9000#section-a.3: DecodePacketNumber()
> + *
> + * Reconstructs the full packet number from a truncated one.
> + */
> +s64 quic_get_num(s64 max_pkt_num, s64 pkt_num, u32 n)
> +{
> +	s64 expected = max_pkt_num + 1;
> +	s64 win = BIT_ULL(n * 8);
> +	s64 hwin = win / 2;
> +	s64 mask = win - 1;
> +	s64 cand;
> +
> +	cand = (expected & ~mask) | pkt_num;
> +	if (cand <= expected - hwin && cand < (1ULL << 62) - win)
> +		return cand + win;
> +	if (cand > expected + hwin && cand >= win)
> +		return cand - win;
> +	return cand;

The above is a bit obscure to me; replacing magic nubers (62) with macro
could help. Some more comments also would do.

/P