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

On Tue, Mar 3, 2026 at 3:33=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 builder base
>
> This patch introduces 'quic_packet' to handle packing of QUIC packets on
> the transmit path, including frame packing, packet construction,
> encryption, bundling, and transmission logic.
>
> > diff --git a/net/quic/Makefile b/net/quic/Makefile
> > index 2ccf01ad9e22..0f903f4a7ff1 100644
> > --- a/net/quic/Makefile
> > +++ b/net/quic/Makefile
> > @@ -6,4 +6,4 @@
> >  obj-$(CONFIG_IP_QUIC) +=3D quic.o
> >
> >  quic-y :=3D common.o family.o protocol.o socket.o stream.o connid.o pa=
th.o \
> > -       cong.o pnspace.o crypto.o timer.o
> > +       cong.o pnspace.o crypto.o timer.o packet.o
> > diff --git a/net/quic/packet.c b/net/quic/packet.c
> > new file mode 100644
> > index 000000000000..a56edc745bb1
> > --- /dev/null
> > +++ b/net/quic/packet.c
>
> [ ... ]
>
> > +/* Perform routing for the QUIC packet on the specified path, update h=
eader length and MSS
> > + * accordingly, reset path and start PMTU timer.
> > + */
> > +int quic_packet_route(struct sock *sk)
> > +{
> > +     struct quic_path_group *paths =3D quic_paths(sk);
> > +     struct quic_packet *packet =3D quic_packet(sk);
> > +     union quic_addr *sa, *da;
> > +     u32 pmtu;
> > +     int err;
> > +
> > +     da =3D quic_path_daddr(paths, packet->path);
> > +     sa =3D quic_path_saddr(paths, packet->path);
> > +     err =3D quic_flow_route(sk, da, sa, &paths->fl);
> > +     if (err)
> > +             return err;
> > +
> > +     packet->hlen =3D quic_encap_len(da);
> > +     pmtu =3D min_t(u32, dst_mtu(__sk_dst_get(sk)), QUIC_PATH_MAX_PMTU=
);
> > +     quic_packet_mss_update(sk, pmtu - packet->hlen);
> > +
> > +     quic_path_pl_reset(paths);
> > +     quic_timer_reset(sk, QUIC_TIMER_PMTU, paths->plpmtud_interval);
> > +     return 0;
> > +}
> > +
> > +/* Configure the QUIC packet header and routing based on encryption le=
vel and path. */
> > +int quic_packet_config(struct sock *sk, u8 level, u8 path)
> > +{
>
> [ ... ]
>
> > +
> > +     /* Perform routing and MSS update for the configured packet. */
> > +     if (quic_packet_route(sk) < 0)
> > +             return -1;
> > +     return 0;
> > +}
>
> Should quic_packet_config() propagate the error code from
> quic_packet_route() instead of returning -1?
>
> The function quic_packet_route() returns proper negative errno values
> (via quic_flow_route()), but quic_packet_config() replaces any error
> with -1 (which is -EPERM). This could be misleading when diagnosing
> routing failures.
Currently quic_packet_config() callers will either skip the process or
return -ENETUNREACH on the failures. But maybe it's better to use the
errno from quic_packet_route(). I will try to improve this.

>
> > diff --git a/net/quic/packet.h b/net/quic/packet.h
> > new file mode 100644
> > index 000000000000..8c23be386207
> > --- /dev/null
> > +++ b/net/quic/packet.h
>
> [ ... ]
>
> > +static inline u8 quic_packet_taglen(struct quic_packet *packet)
> > +{
> > +     return packet->taglen[!!packet->level];
> > +}
> > +
> > +static inline void quic_packet_set_taglen(struct quic_packet *packet, =
u8 taglen)
> > +{
> > +     packet->taglen[0] =3D taglen;
> > +}
> > +
> > +static inline u32 quic_packet_mss(struct quic_packet *packet)
> > +{
> > +     return packet->mss[0] - packet->taglen[!!packet->level];
> > +}
> > +
> > +static inline u32 quic_packet_max_payload(struct quic_packet *packet)
> > +{
> > +     return packet->mss[0] - packet->overhead - packet->taglen[!!packe=
t->level];
> > +}
> > +
> > +static inline u32 quic_packet_max_payload_dgram(struct quic_packet *pa=
cket)
> > +{
> > +     return packet->mss[1] - packet->overhead - packet->taglen[!!packe=
t->level];
> > +}
>
> This isn't a bug, but quic_packet_mss(), quic_packet_max_payload(),
> and quic_packet_max_payload_dgram() all repeat the expression
> 'packet->taglen[!!packet->level]'.
>
> Could these use quic_packet_taglen(packet) instead for consistency?
>
OK.

Thanks.