* Re: [PATCH mptcp-next] mptcp: shrink struct mptcp_options_received
2026-09-04 3:08 [PATCH mptcp-next] mptcp: shrink struct mptcp_options_received Quanye Yang via B4 Relay
@ 2026-09-04 10:09 ` Matthieu Baerts
0 siblings, 0 replies; 2+ messages in thread
From: Matthieu Baerts @ 2026-09-04 10:09 UTC (permalink / raw)
To: quanyeyang, MPTCP Linux
Hi Quanye,
On 04/09/2026 05:08, Quanye Yang via B4 Relay wrote:
> From: Quanye Yang <quanyeyang@proton.me>
>
> struct mptcp_options_received is allocated on the stack while parsing
> incoming MPTCP options.
>
> The sender key, MP_JOIN fields, ADD_ADDR fields and MP_FAIL sequence
> number are mutually exclusive, as enforced by mptcp_parse_option().
> Place them in an anonymous union while keeping the existing field
> access unchanged.
>
> This reduces the structure size from 136 to 88 bytes on x86_64.
Thank you for looking at that.
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/625
> Signed-off-by: Quanye Yang <quanyeyang@proton.me>
> ---
> net/mptcp/protocol.h | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 739f9006c1e1..6646c428dbdf 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -142,7 +142,6 @@ static inline bool before64(__u64 seq1, __u64 seq2)
> #define after64(seq2, seq1) before64(seq1, seq2)
>
> struct mptcp_options_received {
> - u64 sndr_key;
> u64 rcvr_key;
Out of curiosity, can we not also move this one with 'sndr_key' to the
union in a new anonymous struct dedicated to the MP_CAPABLE suboption? I
see it is used by an MPCAPABLE and FASTCLOSE, but maybe we could have a
new fc_recv_key field? It might be clearer to have a dedicated one for
both in the union than re-using this field in two different suboptions, no?
> u64 data_ack;
> u64 data_seq;
It might be useful to have anonymous struct with a comment to explain
that a group is linked to a suboption, e.g. here we have the DSS one,
below we have groups for MPC, MPJ, and ADD_ADDR.
> @@ -166,14 +165,22 @@ struct mptcp_options_received {
> __unused:1;
> );
> u8 join_id;
Can this be moved to the MPJ group?
> - u32 token;
> - u32 nonce;
> - u64 thmac;
> - u8 hmac[MPTCPOPT_HMAC_LEN];
> - struct mptcp_addr_info addr;
> struct mptcp_rm_list rm_list;
> - u64 ahmac;
> - u64 fail_seq;
> + /* Options below are mutually exclusive, see mptcp_parse_option() */
> + union {
> + u64 sndr_key;
> + struct {
> + u32 token;
> + u32 nonce;
> + u64 thmac;
> + u8 hmac[MPTCPOPT_HMAC_LEN];
Maybe yet another anonymous union here for thmac and hmac? I don't think
they can be used at the same time.
> + };
> + struct {
> + struct mptcp_addr_info addr;
> + u64 ahmac;
> + };
> + u64 fail_seq;
> + };
> };
Out of curiosity, did you check the other ideas suggested in the ticket?
If yes, it would be good to add them in the commit message explaining
that they are not worth it for some reason.
>
> static inline __be32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field)
>
> ---
> base-commit: 10d57dc355c99e071a198e50c45503bce944dd5c
> change-id: 20260904-mptcp-shrink-opt-rx-ef8581f125df
> prerequisite-message-id: <20260903-mptcp-mib-inval-opt-v1-0-84a553c552b6@kernel.org>
> prerequisite-patch-id: 7f8decba16ce568283eaa86b23cd93b183aaa675
> prerequisite-patch-id: c8fedc4ff2b5997f716dac0c30ec7620ecf11fc0
> prerequisite-patch-id: 5abea779f2b0af347cd88d6b65bfc3b9eefff405
It looks like your patch doesn't conflict with my series. Do you mind
sending the v2 without the dependencies because it looks like both our
CI and Sashiko were not able to apply your patches. I think that's
because they use 'git am -3' and they don't find the base.
Note that for our CI, it doesn't support b4 dependencies, but it
supports the "Based-on" tag:
https://github.com/multipath-tcp/mptcp_net-next/wiki/CI#base-patches
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 2+ messages in thread