Re: [PATCH v3 mptcp-next 1/3] Squash-to: "mptcp: move drop_other_suboptions check under pm lock"

[Geliang Tang <geliangtang@gmail.com>]

Hi Paolo,

Thanks for this patch, it looks good to me.

Paolo Abeni <pabeni@redhat.com> 于2021年7月26日周一 下午6:55写道：
>
> Commit message to be updated appending:
>
> """
> Additionally, always drop the other suboption for TCP pure ack:
> that makes both the code simpler and the MPTCP behaviour more
> consistent.
> """
>
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/mptcp/pm.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 4d1828fd2482..cd2323e5ccdc 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -266,10 +266,11 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
>         if (!mptcp_pm_should_add_signal(msk))
>                 goto out_unlock;
>
> -       if (((msk->pm.addr_signal & BIT(MPTCP_ADD_ADDR_ECHO)) ||
> -            ((msk->pm.addr_signal & BIT(MPTCP_ADD_ADDR_SIGNAL)) &&
> -             (msk->pm.local.family == AF_INET6 || msk->pm.local.port))) &&
> -           skb && skb_is_tcp_pure_ack(skb)) {
> +       /* always drop every other options for pure ack ADD_ADDR; this is a
> +        * plain dup-ack from TCP perspective. The other MPTCP-relevant info,
> +        * if any, will be carried by the 'original' TCP ack
> +        */
> +       if (skb && skb_is_tcp_pure_ack(skb)) {
>                 remaining += opt_size;
>                 *drop_other_suboptions = true;
>         }

If this patch is to be squashed to "mptcp: move drop_other_suboptions check
under pm lock", should it be like this:

@@ -264,10 +264,11 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock
*msk, struct sk_buff *skb,
     if (!mptcp_pm_should_add_signal(msk))
         goto out_unlock;

-    if ((mptcp_pm_should_add_signal_ipv6(msk) ||
-         mptcp_pm_should_add_signal_port(msk) ||
-         mptcp_pm_should_add_signal_echo(msk)) &&
-        skb && skb_is_tcp_pure_ack(skb)) {
+    /* always drop every other options for pure ack ADD_ADDR; this is a
+     * plain dup-ack from TCP perspective. The other MPTCP-relevant info,
+     * if any, will be carried by the 'original' TCP ack
+     */
+    if (skb && skb_is_tcp_pure_ack(skb)) {
         remaining += opt_size;
         *drop_other_suboptions = true;
     }

Thanks,
-Geliang

> --
> 2.26.3
>