在 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@vger.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@vger.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