On Thu, Dec 5, 2024 at 8:31 AM Mo Yuanhao moyuanhao3676@163.com wrote:

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

We usually do not refactor for a patch targeting a net tree.

Also, please do not add attachments, we need the patch inline as you did in v1.

As you can see, v2 is not avail in https://patchwork.kernel.org/project/netdevbpf/list/

Documentation/process/submitting-patches.rst

No MIME, no links, no compression, no attachments. Just plain text -------------------------------------------------------------------

Linus and other kernel developers need to be able to read and comment on the changes you are submitting. It is important for a kernel developer to be able to "quote" your changes, using standard e-mail tools, so that they may comment on specific portions of your code.

For this reason, all patches should be submitted by e-mail "inline". The easiest way to do this is with ``git send-email``, which is strongly recommended. An interactive tutorial for ``git send-email`` is available at https://git-send-email.io.

If you choose not to use ``git send-email``:

.. warning::

Be wary of your editor's word-wrap corrupting your patch, if you choose to cut-n-paste your patch.

Do not attach the patch as a MIME attachment, compressed or not. Many popular e-mail applications will not always transmit a MIME attachment as plain text, making it impossible to comment on your code. A MIME attachment also takes Linus a bit more time to process, decreasing the likelihood of your MIME-attached change being accepted.