Re: [net-next,v10,15/15] quic: add packet parser base

[Paolo Abeni <pabeni@redhat.com>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
quic: add packet parser base

This patch uses 'quic_packet' to handle packing of QUIC packets on the
receive (RX) path. It introduces mechanisms to parse the ALPN from
client Initial packets to determine the correct listener socket.
Received packets are then routed and processed accordingly.

> diff --git a/net/quic/packet.c b/net/quic/packet.c
> index a56edc745bb1..a23ce99a6656 100644
> --- a/net/quic/packet.c
> +++ b/net/quic/packet.c

[ ... ]

> +/* Extract the Destination Connection ID (DCID) from a QUIC Long header packet. */
> +int quic_packet_get_dcid(struct quic_conn_id *dcid, struct sk_buff *skb)
> +{
> +	u32 plen = skb->len;
> +	u8 *p = skb->data;
> +	u64 len;
> +
> +	if (plen < QUIC_HLEN + QUIC_VERSION_LEN)
> +		return -EINVAL;
> +	plen -= (QUIC_HLEN + QUIC_VERSION_LEN);
> +	p += (QUIC_HLEN + QUIC_VERSION_LEN);
> +
> +	if (!quic_get_int(&p, &plen, &len, 1) ||
> +	    len > plen || len > QUIC_CONN_ID_MAX_LEN)
> +		return -EINVAL;
> +	quic_conn_id_update(dcid, p, len);
> +	return 0;
> +}

This parsing logic looks nearly identical to what's already in
quic_packet_get_version_and_connid().  Both skip QUIC_HLEN +
QUIC_VERSION_LEN, read the length byte, validate against
QUIC_CONN_ID_MAX_LEN, and call quic_conn_id_update().

Could quic_packet_get_dcid() call quic_packet_get_version_and_connid()
with dummy parameters, or could both share a helper to extract just the
DCID portion?

> diff --git a/net/quic/path.c b/net/quic/path.c
> index 9414c4f813c5..655f38af4107 100644
> --- a/net/quic/path.c
> +++ b/net/quic/path.c
> @@ -17,6 +17,8 @@
>  #include "family.h"
>  #include "path.h"
>
> +extern int quic_packet_rcv(struct sock *sk, struct sk_buff *skb, u8 err);

Could this include packet.h instead? The canonical declaration already
exists in packet.h (after this patch). Using 'extern' here creates a
duplicate signature.

If the signature in packet.h changes later, this extern could silently
become stale depending on calling conventions. Since packet.h has no
include directives and wouldn't create a circular dependency, including
it would keep the declaration in sync.

> +
>  static int quic_udp_rcv(struct sock *sk, struct sk_buff *skb)
>  {
>  	memset(skb->cb, 0, sizeof(skb->cb));