2026-07-30, 09:05:17 -0400, Chuck Lever wrote:
>
>
> On Thu, Jul 30, 2026, at 5:12 AM, Sabrina Dubroca wrote:
> > 2026-07-26, 20:33:29 -0400, Chuck Lever wrote:
>
> >> Bound a run of such records, as net_rx_action() bounds a softirq
> >> poll. The first record that delivers no bytes arms a deadline
> >> TLS_RX_NODATA_NS ahead; any record that delivers bytes disarms it,
> >> so a normal stream never trips it. Breaking out with nothing copied
> >
> > Another thought here: I think a peer that sends "some" 0-length data
> > records followed by one (possibly very small) data record, and then
> > repeats that sequence, will not hit this "rate-limiting" of no-data
> > records. Is that right? And if so, is that a problem?
>
> That occurred to me too. It's right on the cusp between still
> making progress and gumming things up. Neither the packet-count
> limit nor the time-bound address this case.
>
> I don't have a good answer.
I'm not sure that can be addressed in a generic way within ktls. Maybe
the caller needs to do its own accounting of "this read_sock/read_actor
dance has been going on for too long now, let's stop".
> > [I'm guessing that the caller is doing something equivalent to
> > recvmsg(WAITALL), so it could be stuck for a long time even with this
> > new bound]
>
> Still digesting your other feedback. I plan to send a v2 to
> address it.
Thanks. There was also some from sashiko but I couldn't force myself
to read it.
BTW I'm not requesting you to also fix the existing misbehavior I've
mentioned (EOR and timeo).
One small thing I've just noticed in this patch:
> if (rxm->full_len == 0) {
> + err = 0;
> consume_skb(skb);
> + if (!nodata_deadline) {
> + nodata_deadline = ktime_get_ns() +
> + TLS_RX_NODATA_NS;
> + } else if (ktime_get_ns() >= nodata_deadline) {
> + /* Queued records raise no new
> sk_data_ready(),
> + * and tls_rx_reader_release() announces only
> to
> + * saved_data_ready(), not the consumer's own.
> + */
> + sk->sk_data_ready(sk);
> + break;
> + }
> continue;
> }
>
> used = read_actor(desc, skb, rxm->offset, rxm->full_len);
> if (used <= 0) {
> if (!copied)
> err = used;
> goto read_sock_requeue;
> }
If we hit the nodata_deadline, we break out of the loop, read_actor
returns 0 since it's an empty record, and we jump to
read_sock_requeue. It doesn't make much sense to requeue the empty
record? Well, we'll start the next read_sock with a record that
immediately arms the deadline, and it avoids a small additional diff
to handle that case in the "used <= 0" block. Nothing serious, I just
thought I'd mention it. I can live with the current change.
--
Sabrina
