Message-ID: <dcdeaf17-c3ce-4677-a0c0-c391d8bd951f@163.com>
Date: Thu, 5 Dec 2024 15:31:47 +0800
From: Mo Yuanhao <moyuanhao3676@....com>
To: matttbe@...nel.org, martineau@...nel.org, geliang@...nel.org,
 edumazet@...gle.com, davem@...emloft.net, dsahern@...nel.org,
 kuba@...nel.org, pabeni@...hat.com, horms@...nel.org
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 mptcp@...ts.linux.dev, stable@...r.kernel.org
Subject: Re: [PATCH net-next] tcp: Check space before adding MPTCP options

在 2024/12/4 19:01, Matthieu Baerts 写道:
> Hi MoYuanhao,
> 
> +Cc MPTCP mailing list.
> 
> (Please cc the MPTCP list next time)
> 
> On 04/12/2024 09:58, MoYuanhao wrote:
>> Ensure enough space before adding MPTCP options in tcp_syn_options()
>> Added a check to verify sufficient remaining space
>> before inserting MPTCP options in SYN packets.
>> This prevents issues when space is insufficient.
> 
> Thank you for this patch. I'm surprised we all missed this check, but
> yes it is missing.
> 
> As mentioned by Eric in his previous email, please add a 'Fixes' tag.
> For bug-fixes, you should also Cc stable and target 'net', not 'net-next':
> 
> Fixes: cec37a6e41aa ("mptcp: Handle MP_CAPABLE options for outgoing
> connections")
> Cc: stable@...r.kernel.org
> 
> 
> Regarding the code, it looks OK to me, as we did exactly that with
> mptcp_synack_options(). In mptcp_established_options(), we pass
> 'remaining' because many MPTCP options can be set, but not here. So I
> guess that's fine to keep the code like that, especially for the 'net' tree.
> 
> 
> Also, and linked to Eric's email, did you have an issue with that, or is
> it to prevent issues in the future?
> 
> 
> One last thing, please don’t repost your patches within one 24h period, see:
> 
>    https://docs.kernel.org/process/maintainer-netdev.html
> 
> 
> Because the code is OK to me, and the same patch has already been sent
> twice to the netdev ML within a few hours, I'm going to apply this patch
> in our MPTCP tree with the suggested modifications. Later on, we will
> send it for inclusion in the net tree.
> 
> pw-bot: awaiting-upstream
> 
> (Not sure this pw-bot instruction will work as no net/mptcp/* files have
> been modified)
> 
> Cheers,
> Matt
Hi Matt,

Thank you for your feedback!

I have made the suggested updates to the patch (version 2):

I’ve added the Fixes tag and Cc'd the stable@...r.kernel.org list.
The target branch has been adjusted to net as per your suggestion.
I will make sure to Cc the MPTCP list in future submissions.

Regarding your question, this patch was created to prevent potential 
issues related to insufficient space for MPTCP options in the future. I 
didn't encounter a specific issue, but it seemed like a necessary 
safeguard to ensure robustness when handling SYN packets with MPTCP options.

Additionally, I have made further optimizations to the patch, which are 
included in the attached version. I believe it would be more elegant to 
introduce a new function, mptcp_set_option(), similar to 
mptcp_set_option_cond(), to handle MPTCP options.

This is my first time replying to a message in a Linux mailing list, so 
if there are any formatting issues or mistakes, please point them out 
and I will make sure to correct them in future submissions.

Thanks again for your review and suggestions. Looking forward to seeing 
the patch applied to the MPTCP tree and later inclusion in the net tree.

Best regards,

MoYuanhao
View attachment "0001-tcp-Check-space-before-adding-MPTCP-options.patch" of type "text/x-patch" (1854 bytes)

Powered by blists - more mailing lists