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