All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geliang Tang <geliangtang@gmail.com>
To: Paolo Abeni <pabeni@redhat.com>
Cc: mptcp@lists.linux.dev, Yonglong Li <liyonglong@chinatelecom.cn>
Subject: Re: [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal"
Date: Sun, 25 Jul 2021 21:40:13 +0800	[thread overview]
Message-ID: <CA+WQbwvLoHcihvPs8pVvTuOXTrVenPFOMR=wy1E0KrT1ur4yFg@mail.gmail.com> (raw)
In-Reply-To: <4206e5473ed255fa08af48a7b5bc0edd00abb99d.1627047074.git.pabeni@redhat.com>

Paolo Abeni <pabeni@redhat.com> 于2021年7月23日周五 下午9:31写道:

>
> We don't need to add a 2nd mptcp_addr_info inside out_options:
> it's quite huge and only one of local or remote is used.
>
> revert back to a single 'addr' field.

Acked-and-tested-by: Geliang Tang <geliangtang@gmail.com>

I think more code needs to be revert too, no need to change the argument
of mptcp_pm_add_addr_signal and no need to modify the code in
mptcp_write_options. And the commit log need to update.

I'll sent another squash-to patch to do these cleanups.

Thanks,
-Geliang

>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  include/net/mptcp.h |  3 +--
>  net/mptcp/options.c | 13 +++++--------
>  net/mptcp/pm.c      |  4 ++--
>  3 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index d0b9e4a7121f..8b5af683a818 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -61,8 +61,7 @@ struct mptcp_out_options {
>         u64 sndr_key;
>         u64 rcvr_key;
>         u64 ahmac;
> -       struct mptcp_addr_info local;
> -       struct mptcp_addr_info remote;
> +       struct mptcp_addr_info addr;
>         struct mptcp_rm_list rm_list;
>         u8 join_id;
>         u8 backup;
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 6803de5d4209..eafdb9408f3a 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -665,7 +665,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
>         unsigned int opt_size = *size;
>         bool echo;
>         bool port;
> -       u8 family;
>         int len;
>
>         if (!mptcp_pm_should_add_signal(msk) ||
> @@ -675,8 +674,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
>
>         if (drop_other_suboptions)
>                 remaining += opt_size;
> -       family = echo ? opts->remote.family : opts->local.family;
> -       len = mptcp_add_addr_len(family, echo, port);
> +       len = mptcp_add_addr_len(opts->addr.family, echo, port);
>         if (remaining < len)
>                 return false;
>
> @@ -692,11 +690,10 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
>         if (!echo) {
>                 opts->ahmac = add_addr_generate_hmac(msk->local_key,
>                                                      msk->remote_key,
> -                                                    &opts->local);
> +                                                    &opts->addr);
>         }
> -       pr_debug("local_id=%d, local_port=%d, remote_id=%d, remote_port=%d, ahmac=%llu, echo=%d",
> -                opts->local.id, ntohs(opts->local.port), opts->remote.id,
> -                ntohs(opts->remote.port), opts->ahmac, echo);
> +       pr_debug("addr_id=%d, addr_port=%d, ahmac=%llu, echo=%d",
> +                opts->addr.id, ntohs(opts->addr.port), opts->ahmac, echo);
>
>         return true;
>  }
> @@ -1253,7 +1250,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
>
>  mp_capable_done:
>         if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) {
> -               struct mptcp_addr_info *addr = opts->ahmac ? &opts->local : &opts->remote;
> +               struct mptcp_addr_info *addr = &opts->addr;
>                 u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
>                 u8 echo = MPTCP_ADDR_ECHO;
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 692614c1f552..4d1828fd2482 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -282,10 +282,10 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
>                 goto out_unlock;
>
>         if (*echo) {
> -               opts->remote = msk->pm.remote;
> +               opts->addr = msk->pm.remote;
>                 add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_ECHO);
>         } else {
> -               opts->local = msk->pm.local;
> +               opts->addr = msk->pm.local;
>                 add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_SIGNAL);
>         }
>         WRITE_ONCE(msk->pm.addr_signal, add_addr);
> --
> 2.26.3
>

  reply	other threads:[~2021-07-25 13:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-23 13:31 [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal" Paolo Abeni
2021-07-25 13:40 ` Geliang Tang [this message]
2021-07-27  0:20 ` Mat Martineau
2021-07-28  7:07 ` Matthieu Baerts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+WQbwvLoHcihvPs8pVvTuOXTrVenPFOMR=wy1E0KrT1ur4yFg@mail.gmail.com' \
    --to=geliangtang@gmail.com \
    --cc=liyonglong@chinatelecom.cn \
    --cc=mptcp@lists.linux.dev \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.