Re: [PATCH net-next v4 14/15] quic: add frame encoder and decoder base

Xin Long <[email protected]> Thu, 13 Nov 2025 16:26:46 -0500
Newsgroups dev.linux.lists.quic,dev.linux.lists.kernel-tls-handshake,org.kernel.vger.linux-cifs,org.kernel.vger.netdev
Message-ID <CADvbK_eEHLbMQ3+OTWdg6_4a_bKFOkO-ZL0y661Vovo9uEFB1g@mail.gmail.com>
On Thu, Nov 6, 2025 at 12:22 PM Xin Long <[email protected]> wrote:
>
> On Tue, Nov 4, 2025 at 7:47 AM Paolo Abeni <[email protected]> wrote:
> >
> > On 10/29/25 3:35 PM, Xin Long wrote:
> > > +static void quic_frame_free(struct quic_frame *frame)
> > > +{
> > > +     struct quic_frame_frag *frag, *next;
> > > +
> > > +     if (!frame->type && frame->skb) { /* RX path frame with skb. */
> >
> > Are RX path frame with !skb expected/possible? such frames will be
> > 'misinterpreted' as TX ones, specifically will do `kfree(frame->data)`
> > which in turn could be a bad thing.
> >
> Yes, when generating and delivering an event to userspace, it
> keeps the frame content into frame->data, instead of frame->skb.
>
> There's no need check !frame->type for RX path,  and I will change it to:
>
>         if (frame->skb) { /* For stream/crypto/dgram frames on RX. */
>                 kfree_skb(frame->skb);
>                 goto out;
>         }
>
> if skb is set, it will go kfree_skb(frame->skb), instead of kfree(frame->data).
> Because if frame->skb is set, it only needs to do kfree_skb(frame->skb).
>
Just realized that frame->skb shares a union with frame->flist, which is
used only on TX. So instead of removing the check !frame->type, I will
put a comment here:

        /* Handle RX stream/crypto/dgram frames. Use !frame->type to detect RX,
         * since frame->skb shares a union with frame->flist, used only on TX.
         */
        if (!frame->type && frame->skb) {

Thanks.