Re: [PATCH] thunderbolt: stream: Make read return framing error to the userspace

Re: [PATCH] thunderbolt: stream: Make read return framing error to the userspace

[Ivy Lopez]

On Mon, Sep 21, 2026 at 11:19 AM UTC, Mika Westerberg wrote:
> +                     if (!nbytes) {
> +                             pr_warn("corrupted frame received, flags %#x\n",
> +                                     sf->frame.flags);
> +                             tbstream_dev_consume_rx(sdev);
> +                             ret = -EIO;
> +                     }
> +                     break;

One question: EPOLLIN is set unconditionally in the rx callback
regardless of frame flags, and tbstream_dev_fops_poll() only checks
tbstream_ring_available() (pure ring depth), never frame flags.
EPOLLERR is used elsewhere in this file but only for device-level
invalidity, not per-frame corruption. So userspace still has no way
to learn a corrupted frame is queued via poll() before calling
read() and getting -EIO. I'm just wondering, is this intentional,
or worth a follow-up?

Also, fwiw, this doesn't apply cleanly against current usb-next for
me, git apply fails on the rx_callback hunk. Looks like it's against
a tree predating the busy-polling changes.

Not a blocker either way.

Reviewed-by: Ivy Lopez <skunkolee@gmail.com>

Re: [PATCH] thunderbolt: stream: Make read return framing error to the userspace

[Mika Westerberg]

Hi,

On Mon, Sep 21, 2026 at 06:17:42PM -0600, Ivy Lopez wrote:
> On Mon, Sep 21, 2026 at 11:19 AM UTC, Mika Westerberg wrote:
> > +                     if (!nbytes) {
> > +                             pr_warn("corrupted frame received, flags %#x\n",
> > +                                     sf->frame.flags);
> > +                             tbstream_dev_consume_rx(sdev);
> > +                             ret = -EIO;
> > +                     }
> > +                     break;
> 
> One question: EPOLLIN is set unconditionally in the rx callback
> regardless of frame flags, and tbstream_dev_fops_poll() only checks
> tbstream_ring_available() (pure ring depth), never frame flags.
> EPOLLERR is used elsewhere in this file but only for device-level
> invalidity, not per-frame corruption. So userspace still has no way
> to learn a corrupted frame is queued via poll() before calling
> read() and getting -EIO. I'm just wondering, is this intentional,
> or worth a follow-up?

It's intentional. EPOLLERR is used to indicate that this file is
"finished". A single framing error would allow the userspace still to
continue.

> Also, fwiw, this doesn't apply cleanly against current usb-next for
> me, git apply fails on the rx_callback hunk. Looks like it's against
> a tree predating the busy-polling changes.

It'a against thunderbolt.git/next [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt.git/log/?h=next

> Not a blocker either way.
> 
> Reviewed-by: Ivy Lopez <skunkolee@gmail.com>

Thanks!