From: gang.yan@linux.dev
To: "Matthieu Baerts" <matttbe@kernel.org>, mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-next] mptcp: preserve MSG_EOR semantics in sendmsg path
Date: Mon, 30 Mar 2026 08:19:13 +0000 [thread overview]
Message-ID: <be87d58dcca25715b3f992acd152a6778de44dec@linux.dev> (raw)
In-Reply-To: <c572547f-449b-43e9-bd76-e0ffe2ebf735@kernel.org>
March 27, 2026 at 12:42 AM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:
>
> Hi Gang,
>
> Thank you for the new version.
>
> On 09/03/2026 03:54, Gang Yan wrote:
>
> >
> > From: Gang Yan <yangang@kylinos.cn>
> >
> > Extend MPTCP's sendmsg handling to recognize and honor the MSG_EOR flag,
> > which marks the end of a record for application-level message boundaries.
> >
> > Data fragments tagged with MSG_EOR are explicitly marked in the
> > mptcp_data_frag structure and skb context to prevent unintended
> > coalescing with subsequent data chunks. This ensures the intent of
> > applications using MSG_EOR is preserved across MPTCP subflows,
> > maintaining consistent message segmentation behavior.
> >
> > Signed-off-by: Gang Yan <yangang@kylinos.cn>
> > ---
> >
> > Notes:
> > - This patch incorporates feedback and suggestions from Paolo Abeni
> > and Geliang Tang, including memory alignment optimizations for the
> > mptcp_data_frag struct (shrinking overhead to u8 and using bitfield
> > for eor to avoid size increase) and compile-time checks with BUILD_BUG_ON.
> >
> Please mention why you shrank "overhead" to a u8 (not to increase the
> struct size), and why it is OK to do so (u16 not needed because ...) +
> explaining the BUILD_BUG_ON().
The ‘u8’ is one of Paolo's suggestions[1]. I think 'u16' is not needed because:
- 'offset = ALIGN(orig_offset, sizeof(long));'
- 'dfrag->offset = offset - origin_offset + sizeof(struct mptcp_data_frag);',
the max value of offset is 7, and sizeof(struct mptcp_data_frag)) is
usually 40, so the overhead is 47, far less than 255.
Another suggestion from Paolo[1] is a build time check on the max 'overhead'
value. So I use 'ALIGN(1, sizeof(long)) + sizeof(struct mptcp_data_frag)' to
represent the max_val of 'overhead'.
But Paolo also mention it's probably too conservative. WDYT?
[1] https://patchwork.kernel.org/project/mptcp/patch/20260203023029.855434-1-gang.yan@linux.dev/
>
> >
> > - Packetdrill test cases validating this feature are available at:
> > https://github.com/multipath-tcp/packetdrill/pull/189/changes/d6ce92a4786704fe749bbd848ced0c047632282e
> >
> Thank you, I just reviewed it.
Thanks, I'll try to fix them.
>
> Do you mind checking the AI review there please:
>
> https://netdev-ai.bots.linux.dev/ai-review.html?id=22434689-7326-48c8-af75-273d99fbef55
>
> I think it is valid, but better to double-check.
Yes, I think it's a good catch, and we should fix it as follows:
@@ -1032,7 +1032,8 @@ static bool mptcp_frag_can_collapse_to(const struct mptcp_sock *msk,
const struct page_frag *pfrag,
const struct mptcp_data_frag *df)
{
- return df && pfrag->page == df->page &&
+ return df && !df->eor &&
+ pfrag->page == df->page &&
pfrag->size - pfrag->offset > 0 &&
pfrag->offset == (df->offset + df->data_len) &&
df->data_seq + df->data_len == msk->write_seq;
If OK, I'll apply it when sending v2.
>
> >
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index 17e43aff4459..3e574c87301b 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> >
> (...)
>
> >
> > @@ -4621,6 +4638,9 @@ void __init mptcp_proto_init(void)
> > inet_register_protosw(&mptcp_protosw);
> >
> > BUILD_BUG_ON(sizeof(struct mptcp_skb_cb) > sizeof_field(struct sk_buff, cb));
> > + /* Compile-time check: ensure 'overhead' (alignment + struct size) fits in u8 */
> > + BUILD_BUG_ON(ALIGN(1, sizeof(long)) + sizeof(struct mptcp_data_frag) > U8_MAX);
> >
> Sorry, I'm not sure what you are checking here. Do you mind explaining
> it please?
>
The 'BUILD_BUG_ON' is explained at the beginning of the reply, thanks.
Cheers,
Gang
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>
next prev parent reply other threads:[~2026-03-30 8:19 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-09 2:54 [PATCH mptcp-next] mptcp: preserve MSG_EOR semantics in sendmsg path Gang Yan
2026-03-09 4:07 ` MPTCP CI
2026-03-26 16:42 ` Matthieu Baerts
2026-03-30 8:19 ` gang.yan [this message]
2026-03-30 9:50 ` Matthieu Baerts
2026-03-31 6:54 ` gang.yan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=be87d58dcca25715b3f992acd152a6778de44dec@linux.dev \
--to=gang.yan@linux.dev \
--cc=matttbe@kernel.org \
--cc=mptcp@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.