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.