Re: [PATCH mptcp-next 5/6] mptcp: trim the duplicated skb head at receive enqueue

Hi Paolo,

On Wed, 2026-07-29 at 10:08 +0200, Paolo Abeni wrote:
> On 7/27/26 1:29 PM, Geliang Tang wrote:
> > The linear readers (recvmsg, read_sock, read_done) cope with that
> > by
> > computing a per-skb offset = copied_seq - map_seq and skipping it.
> > But
> > consumers that treat the receive queue as a single contiguous byte
> > stream cannot: the TLS strparser builds an anchor whose frag_list
> > is
> > the receive-queue skbs and reads it with a plain skb_copy_bits(),
> > which
> > has no per-skb offset knowledge. A record spanning such an skb
> > boundary
> > then reads the duplicated prefix and gets corrupted.
> 
> The above raises a question.
> 
> AFAICS, the critical skb layout is also possible with plain TCP -
> possibly is just less likely. How does TLS deal with that? I read the
> above as the TLS stream get corrupted, which sounds suspiciously too
> fragile to me?!? Or did I miss something?

My description was inaccurate. This only occurs in the MPTCP out-of-
order scenario. It does not happen with TCP.

> 
> This change adds a lot of complexity to the rx path, we want to avoid
> it.

I agree with you. This helper mptcp_trim_dup_head() does not need to be
called in __mptcp_move_skb(). It only needs to be called when
overlapping data occurs in __mptcp_ofo_queue(). This way, it won't
affect the efficiency of the rx path.

static bool __mptcp_ofo_queue(struct mptcp_sock *msk)
{
    struct sock *sk = (struct sock *)msk;
    struct sk_buff *skb, *tail;
    u32 seq_delta, ack_seq;
    bool moved = false;
    struct rb_node *p;

    p = rb_first(&msk->out_of_order_queue);
    while (p) {
        ... ... 
        seq_delta = MPTCP_SKB_CB(skb)->end_seq - ack_seq;
        tail = skb_peek_tail(&sk->sk_receive_queue);
        if (!tail || !mptcp_try_coalesce(sk, tail, skb)) {
            int delta = ack_seq - MPTCP_SKB_CB(skb)->map_seq;
        
            /* trim overlapping prefix, if any */
            pr_debug("uncoalesced seq=%x ack seq=%x delta=%d\n",
                     MPTCP_SKB_CB(skb)->map_seq, ack_seq,
                     delta);
            if (mptcp_trim_head(skb, delta)) {
                    mptcp_drop(sk, skb);
                    continue;
            }
            MPTCP_SKB_CB(skb)->map_seq += delta;
            __skb_queue_tail(&sk->sk_receive_queue, skb);
        }
        msk->bytes_received += seq_delta;
        WRITE_ONCE(msk->ack_seq, msk->ack_seq + seq_delta);
        moved = true;
    }
    return moved;
}

This helper is actually a mirror of the tcp_trim_head(). If we could
export and reuse TCP's __pskb_trim_head(), this helper would become
much simpler:

static int mptcp_trim_head(struct sk_buff *skb, int delta)
{
        int eat; 

        if (skb_unclone_keeptruesize(skb, GFP_ATOMIC))
                return -ENOMEM;

        eat = min_t(int, delta, skb_headlen(skb));
        if (eat) {
                __skb_pull(skb, eat);
                delta -= eat; 
        }

        if (delta) {
                __pskb_trim_head(skb, delta);
                skb->len += skb_headlen(skb);
        }

        return 0;
}

Would this implementation be better? Please give me some feedback.

Thanks,
-Geliang

> 
> /P
>