On Thu, Dec 5, 2024 at 8:31 AM Mo Yuanhao <moyuanhao3676@xxxxxxx> 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@xxxxxxxxxxxxxxx > > > > > > 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@xxxxxxxxxxxxxxx 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.