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

On Tue, Mar 3, 2026 at 3:34=E2=80=AFAM Paolo Abeni <[email protected]> wrot=
e:
>
> 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-loca=
l.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 heade=
r packet. */
> > +int quic_packet_get_dcid(struct quic_conn_id *dcid, struct sk_buff *sk=
b)
> > +{
> > +     u32 plen =3D skb->len;
> > +     u8 *p =3D skb->data;
> > +     u64 len;
> > +
> > +     if (plen < QUIC_HLEN + QUIC_VERSION_LEN)
> > +             return -EINVAL;
> > +     plen -=3D (QUIC_HLEN + QUIC_VERSION_LEN);
> > +     p +=3D (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?
>
No, they work on different forms of QUIC packet, one for short header and
the other for long header packets.

> > 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 er=
r);
>
> 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.
>
No, let's not have this dependency for now.

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