Re: [PATCH v6 3/4] mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal

[Geliang Tang <geliangtang@gmail.com>]

Yonglong Li <liyonglong@chinatelecom.cn> 于2021年6月30日周三 上午9:30写道：
>
>
>
> On 2021/6/29 16:25, Geliang Tang wrote:
> > Yonglong Li <liyonglong@chinatelecom.cn> 于2021年6月29日周二 下午3:54写道：
> >>
> >>
> >>
> >> On 2021/6/29 15:35, Geliang Tang wrote:
> >>> Yonglong Li <liyonglong@chinatelecom.cn> 于2021年6月29日周二 下午3:02写道：
> >>>>
> >>>>
> >>>> Hi Geiliang, Thanks for your reviews.
> >>>>
> >>>> On 2021/6/29 13:58, Geliang Tang wrote:
> >>>>> Yonglong Li <liyonglong@chinatelecom.cn> 于2021年6月29日周二 上午9:42写道：
> >>>>>>
> >>>>>> according MPTCP_ADD_ADDR_SIGNAL and MPTCP_ADD_ADDR_ECHO flag build
> >>>>>> ADD_ADDR/echo-ADD_ADDR option
> >>>>>>
> >>>>>> add a suboptions type OPTION_MPTCP_ADD_ECHO to mark as echo option
> >>>>>>
> >>>>>> Signed-off-by: Yonglong Li <liyonglong@chinatelecom.cn>
> >>>>>> ---
> >>>>>>  include/net/mptcp.h  |  3 ++-
> >>>>>>  net/mptcp/options.c  | 65 +++++++++++++++++++++++++++++++---------------------
> >>>>>>  net/mptcp/pm.c       | 33 +++++++++++---------------
> >>>>>>  net/mptcp/protocol.h | 23 ++++++++++++-------
> >>>>>>  4 files changed, 69 insertions(+), 55 deletions(-)
> >>>>>>
> >>>>>> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> >>>>>> index d61bbbf..d2c6ebe 100644
> >>>>>> --- a/include/net/mptcp.h
> >>>>>> +++ b/include/net/mptcp.h
> >>>>>> @@ -61,7 +61,8 @@ struct mptcp_out_options {
> >>>>>>         u64 sndr_key;
> >>>>>>         u64 rcvr_key;
> >>>>>>         u64 ahmac;
> >>>>>> -       struct mptcp_addr_info addr;
> >>>>>> +       struct mptcp_addr_info local;
> >>>>>> +       struct mptcp_addr_info remote;
> >>>>>>         struct mptcp_rm_list rm_list;
> >>>>>>         u8 join_id;
> >>>>>>         u8 backup;
> >>>>>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> >>>>>> index 1aec016..1707bec 100644
> >>>>>> --- a/net/mptcp/options.c
> >>>>>> +++ b/net/mptcp/options.c
> >>>>>> @@ -655,13 +655,15 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> >>>>>>         struct mptcp_sock *msk = mptcp_sk(subflow->conn);
> >>>>>>         bool drop_other_suboptions = false;
> >>>>>>         unsigned int opt_size = *size;
> >>>>>> -       bool echo;
> >>>>>> -       bool port;
> >>>>>> -       int len;
> >>>>>> +       u8 add_addr, flags = 0xff;
> >>>>>> +       int len = 0;
> >>>>>>
> >>>>>> -       if ((mptcp_pm_should_add_signal_ipv6(msk) ||
> >>>>>> -            mptcp_pm_should_add_signal_port(msk) ||
> >>>>>> -            mptcp_pm_should_add_signal_echo(msk)) &&
> >>>>>> +       if (!mptcp_pm_add_addr_signal(msk, opts, &add_addr))
> >>>>>> +               return false;
> >>>>>
> >>>>> This add_addr argument is useless, let's drop it.
> >>>>>
> >>>> we can use add_addr use in debug log.
> >>>
> >>> I think it's not worth adding a new argument just for debugging.
> >> agree.
> >>
> >>>
> >>>>
> >>>>> And here add back mptcp_pm_should_add_signal check here. The original code
> >>>>> called mptcp_pm_should_add_signal twice for double check, once out of pm
> >>>>> lock, once under pm lock. We should keep it.
> >>>> Sorry, I think double check is not necessary. does we need double check?
> >>>
> >>> I think we should keep the original logic here. If we want to drop this
> >>> double check or something, we should do it in another patch, don't mix too
> >>> much things in one patch.
> >> agree.
> >>
> >>>
> >>>>
> >>>>>
> >>>>>> +
> >>>>>> +       if ((mptcp_pm_should_add_signal_echo(msk) ||
> >>>>>> +            (mptcp_pm_should_add_signal_addr(msk) &&
> >>>>>> +             (opts->local.family == AF_INET6 || opts->local.port))) &&
> >>>>>>             skb && skb_is_tcp_pure_ack(skb)) {
> >>>>>>                 pr_debug("drop other suboptions");
> >>>>>>                 opts->suboptions = 0;
> >>>>>> @@ -671,11 +673,17 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> >>>>>>                 drop_other_suboptions = true;
> >>>>>>         }
> >>>>>>
> >>>>>> -       if (!mptcp_pm_should_add_signal(msk) ||
> >>>>>> -           !(mptcp_pm_add_addr_signal(msk, remaining, &opts->addr, &echo, &port)))
> >>>>>> -               return false;
> >>>>>>
> >>>>>> -       len = mptcp_add_addr_len(opts->addr.family, echo, port);
> >>>>>> +       if (mptcp_pm_should_add_signal_echo(msk)) {
> >>>>>> +               flags = (u8)~BIT(MPTCP_ADD_ADDR_ECHO);
> >>>>>> +       } else {
> >>>>>> +               opts->ahmac = add_addr_generate_hmac(msk->local_key,
> >>>>>> +                                                    msk->remote_key,
> >>>>>> +                                                    &opts->local);
> >>>>>
> >>>>> Keep this ahmac generating code after opts->suboptions set just like the
> >>>>> original code, since ahmac is the more expensive to populate. If remaining
> >>>>> length isn't enough, no need to set ahmac.
> >>>>
> >>>> because mptcp_add_addr_len(opts) will use ahmac to calculate len of opts, so I think Keep this ahmac
> >>>> generating code after opts->suboptions set is not ok.
> >>>
> >>> So we should use mptcp_pm_should_add_signal_addr instead of opts->ahmac in
> >>> mptcp_add_addr_len.
> >> agree.
> >>
> >>>
> >>>>
> >>>>>
> >>>>>> +               flags = (u8)~BIT(MPTCP_ADD_ADDR_SIGNAL);
> >>>>>> +       }
> >>>>>> +
> >>>>>> +       len = mptcp_add_addr_len(opts);
> >>>>>>         if (remaining < len)
> >>>>>>                 return false;
> >>>>>>
> >>>>>> @@ -683,13 +691,14 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> >>>>>>         if (drop_other_suboptions)
> >>>>>>                 *size -= opt_size;
> >>>>>>         opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
> >>>>>> -       if (!echo) {
> >>>>>> -               opts->ahmac = add_addr_generate_hmac(msk->local_key,
> >>>>>> -                                                    msk->remote_key,
> >>>>>> -                                                    &opts->addr);
> >>>>>> -       }
> >>>>>> -       pr_debug("addr_id=%d, ahmac=%llu, echo=%d, port=%d",
> >>>>>> -                opts->addr.id, opts->ahmac, echo, ntohs(opts->addr.port));
> >>>>>> +
> >>>>>> +       spin_lock_bh(&msk->pm.lock);
> >>>>>> +       WRITE_ONCE(msk->pm.addr_signal, flags & msk->pm.addr_signal);
> >>>>>> +       spin_unlock_bh(&msk->pm.lock);
> >>>>>
> >>>>> addr_signal has been set in mptcp_pm_add_addr_signal in patch 1, no need to
> >>>>> set it again. I thinks this trunk and all the flags set above should be
> >>>>> dropped.
> >>>>
> >>>> Because MPTCP_ADD_ADDR_SIGNAL and MPTCP_ADD_ADDR_ECHO maybe set at the same time.
> >>>> So i think we should only unset one flag.
> >>>
> >>> We can only unset one flag in mptcp_pm_add_addr_signal, see my comment in
> >>> patch 1.
> >>
> >> if change like this. there is a issue: if remaining len checking is not ok and return false, The ADD_ADDR/ECHO event will
> >> be clear. So I think we should make sure ADD_ADDR/ECHO option will add in packet before clean flags. WDYT?
> >>
> >
> > You're right, let's clear it in mptcp_established_options_add_addr.
> > Furthermore, we should do the same thing for RM_ADDR, clear rm_addr in
> > mptcp_established_options_rm_addr too.
> >
> > If so, patch 1 will become useless. Let's drop it.
> >
> > -Geliang
> > I think RM_ADDR doesn't have this issue. Because mptcp_pm_rm_addr_signal() check the failed case.

If so, how about doing the same thing as RM_ADDR to check the failed case
in mptcp_pm_add_addr_signal too.

I think we should use the same logic for ADD_ADDR and RM_ADDR.

>
> >
> >
> >>>
> >>> -Geliang
> >>>
> >>>>
> >>>>>
> >>>>>> +
> >>>>>> +       pr_debug("addr_signal:%x, echo=%d, local_addr_id=%d, ahmac=%llu, local_port=%d, remote_addr_id=%d, remote_port=%d",
> >>>>>> +                add_addr, (opts->ahmac == 0), opts->local.id,
> >>>>>> +                opts->ahmac, ntohs(opts->local.port), opts->remote.id, ntohs(opts->remote.port));
> >>>>>>
> >>>>>>         return true;
> >>>>>>  }
> >>>>>
> >>>>> The whole function is something like this:
> >>>>> '''
> >>>>>         struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
> >>>>>         struct mptcp_sock *msk = mptcp_sk(subflow->conn);
> >>>>>         bool drop_other_suboptions = false;
> >>>>>         unsigned int opt_size = *size;
> >>>>>         int len;
> >>>>>
> >>>>>         if (!mptcp_pm_should_add_signal(msk) ||
> >>>>>             !mptcp_pm_add_addr_signal(msk, remaining, opts))
> >>>>>                 return false;
> >>>>>
> >>>>>         if ((mptcp_pm_should_add_signal_echo(msk) ||
> >>>>>              (mptcp_pm_should_add_signal_addr(msk) &&
> >>>>>               (opts->local.family == AF_INET6 || opts->local.port))) &&
> >>>>>             skb && skb_is_tcp_pure_ack(skb)) {
> >>>>>                 pr_debug("drop other suboptions");
> >>>>>                 opts->suboptions = 0;
> >>>>>                 opts->ext_copy.use_ack = 0;
> >>>>>                 opts->ext_copy.use_map = 0;
> >>>>>                 remaining += opt_size;
> >>>>>                 drop_other_suboptions = true;
> >>>>>         }
> >>>>>
> >>>>>         len = mptcp_add_addr_len(opts);
> >>>>>         if (remaining < len)
> >>>>>                 return false;
> >>>>>
> >>>>>         *size = len;
> >>>>>         if (drop_other_suboptions)
> >>>>>                 *size -= opt_size;
> >>>>>         opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
> >>>>>         if (mptcp_pm_should_add_signal_addr(msk)) {
> >>>>>                 opts->ahmac = add_addr_generate_hmac(msk->local_key,
> >>>>>                                                      msk->remote_key,
> >>>>>                                                      &opts->local);
> >>>>>         }
> >>>>>
> >>>>>         pr_debug("addr_signal:%x, echo=%d, local_addr_id=%d,
> >>>>> ahmac=%llu, local_port=%d, remote_addr_id=%d, remote_port=%d",
> >>>>>                  msk->pm.addr_signal, (opts->ahmac == 0), opts->local.id,
> >>>>>                  opts->ahmac, ntohs(opts->local.port),
> >>>>> opts->remote.id, ntohs(opts->remote.port));
> >>>>>
> >>>>>         return true;
> >>>>> '''
> >>>>>
> >>>>>> @@ -1229,15 +1238,19 @@ 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->remote;
> >>>>>
> >>>>> We can simplify it like this:
> >>>>>          struct mptcp_addr_info *addr = opts->ahmac ? &opts->local :
> >>>>> &opts->remote;
> >>>>>
> >>>>>>                 u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
> >>>>>>                 u8 echo = MPTCP_ADDR_ECHO;
> >>>>>>
> >>>>>> +               if (opts->ahmac)
> >>>>>> +                       addr = &opts->local;
> >>>>>
> >>>>> And this trunk can be dropped.
> >>>>>
> >>>>>> +
> >>>>>>  #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> >>>>>> -               if (opts->addr.family == AF_INET6)
> >>>>>> +               if (addr->family == AF_INET6)
> >>>>>>                         len = TCPOLEN_MPTCP_ADD_ADDR6_BASE;
> >>>>>>  #endif
> >>>>>>
> >>>>>> -               if (opts->addr.port)
> >>>>>> +               if (addr->port)
> >>>>>>                         len += TCPOLEN_MPTCP_PORT_LEN;
> >>>>>>
> >>>>>>                 if (opts->ahmac) {
> >>>>>> @@ -1246,25 +1259,25 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
> >>>>>>                 }
> >>>>>>
> >>>>>>                 *ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> >>>>>> -                                     len, echo, opts->addr.id);
> >>>>>> -               if (opts->addr.family == AF_INET) {
> >>>>>> -                       memcpy((u8 *)ptr, (u8 *)&opts->addr.addr.s_addr, 4);
> >>>>>> +                                     len, echo, addr->id);
> >>>>>> +               if (addr->family == AF_INET) {
> >>>>>> +                       memcpy((u8 *)ptr, (u8 *)&addr->addr.s_addr, 4);
> >>>>>>                         ptr += 1;
> >>>>>>                 }
> >>>>>>  #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> >>>>>> -               else if (opts->addr.family == AF_INET6) {
> >>>>>> -                       memcpy((u8 *)ptr, opts->addr.addr6.s6_addr, 16);
> >>>>>> +               else if (addr->family == AF_INET6) {
> >>>>>> +                       memcpy((u8 *)ptr, addr->addr6.s6_addr, 16);
> >>>>>>                         ptr += 4;
> >>>>>>                 }
> >>>>>>  #endif
> >>>>>>
> >>>>>> -               if (!opts->addr.port) {
> >>>>>> +               if (!addr->port) {
> >>>>>>                         if (opts->ahmac) {
> >>>>>>                                 put_unaligned_be64(opts->ahmac, ptr);
> >>>>>>                                 ptr += 2;
> >>>>>>                         }
> >>>>>>                 } else {
> >>>>>> -                       u16 port = ntohs(opts->addr.port);
> >>>>>> +                       u16 port = ntohs(addr->port);
> >>>>>>
> >>>>>>                         if (opts->ahmac) {
> >>>>>>                                 u8 *bptr = (u8 *)ptr;
> >>>>>> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> >>>>>> index cf873e9..9c621293 100644
> >>>>>> --- a/net/mptcp/pm.c
> >>>>>> +++ b/net/mptcp/pm.c
> >>>>>> @@ -253,32 +253,25 @@ void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
> >>>>>>
> >>>>>>  /* path manager helpers */
> >>>>>>
> >>>>>> -bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
> >>>>>> -                             struct mptcp_addr_info *saddr, bool *echo, bool *port)
> >>>>>> +bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct mptcp_out_options *opts,
> >>>>>> +                             u8 *add_addr)
> >>>>>
> >>>>> Drop this add_addr argument.
> >>>>>
> >>>>>>  {
> >>>>>> -       u8 add_addr;
> >>>>>> -       int ret = false;
> >>>>>> -
> >>>>>>         spin_lock_bh(&msk->pm.lock);
> >>>>>>
> >>>>>> -       /* double check after the lock is acquired */
> >>>>>> -       if (!mptcp_pm_should_add_signal(msk))
> >>>>>> -               goto out_unlock;
> >>>>>
> >>>>> Keep this double check code.
> >>>>>
> >>>>>> -
> >>>>>> -       *echo = mptcp_pm_should_add_signal_echo(msk);
> >>>>>> -       *port = mptcp_pm_should_add_signal_port(msk);
> >>>>>> -
> >>>>>> -       if (remaining < mptcp_add_addr_len(msk->pm.local.family, *echo, *port))
> >>>>>> -               goto out_unlock;
> >>>>>
> >>>>> Keep this length double check code too.
> >>>>>
> >>>>>> +       if (!mptcp_pm_should_add_signal(msk)) {
> >>>>>> +               spin_unlock_bh(&msk->pm.lock);
> >>>>>> +               return false;
> >>>>>> +       }
> >>>>>>
> >>>>>> -       *saddr = msk->pm.local;
> >>>>>> -       add_addr = msk->pm.addr_signal & ~(BIT(MPTCP_ADD_ADDR_SIGNAL) | BIT(MPTCP_ADD_ADDR_ECHO));
> >>>>> -       WRITE_ONCE(msk->pm.addr_signal, add_addr);
> >>>>>
> >>>>> This code is just added in patch 1, I think we should keep it. And no need
> >>>>> to write addr_signal again in mptcp_established_options_add_addr.
> >>>>>
> >>>>>> -       ret = true;
> >>>>>> +       opts->local = msk->pm.local;
> >>>>>> +       opts->remote = msk->pm.remote;
> >>>>>> +       *add_addr = msk->pm.addr_signal;
> >>>>>>
> >>>>>> -out_unlock:
> >>>>>>         spin_unlock_bh(&msk->pm.lock);
> >>>>>> -       return ret;
> >>>>>
> >>>>> Keep this out_unlock code.
> >>>>>
> >>>>>> +
> >>>>>> +       if ((mptcp_pm_should_add_signal_echo(msk)) && (mptcp_pm_should_add_signal_addr(msk)))
> >>>>>> +               mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_SEND_ACK);
> >>>>>
> >>>>> Could we use mptcp_pm_add_addr_send_ack here instead of open coding?
> >>>>>
> >>>>> I'm no sure why we need this two lines, and why you use '&&' here. Do you
> >>>>> mean set MPTCP_ADD_ADDR_SIGNAL and MPTCP_ADD_ADDR_ECHO at the same time?

Please move these two lines into a new patch, and describe why we need it
in the commit log.

Thanks.
-Geliang

> >>>>>
> >>>>>> +       return true;
> >>>>>>  }
> >>>>>
> >>>>> The whole function is something like this:
> >>>>> '''
> >>>>>         int ret = false;
> >>>>>         u8 add_addr;
> >>>>>
> >>>>>         spin_lock_bh(&msk->pm.lock);
> >>>>>
> >>>>>         /* double check after the lock is acquired */
> >>>>>         if (!mptcp_pm_should_add_signal(msk))
> >>>>>                 goto out_unlock;
> >>>>>
> >>>>>         if (remaining < mptcp_add_addr_len(opts))
> >>>>>                 goto out_unlock;
> >>>>>
> >>>>>         opts->local = msk->pm.local;
> >>>>>         opts->remote = msk->pm.remote;
> >>>>>         if (mptcp_pm_should_add_signal_echo(msk))
> >>>>>                 add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_ECHO);
> >>>>>         else
> >>>>>                 add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_SIGNAL);
> >>>>>         WRITE_ONCE(msk->pm.addr_signal, add_addr);
> >>>>>         ret = true;
> >>>>>
> >>>>> out_unlock:
> >>>>>         spin_unlock_bh(&msk->pm.lock);
> >>>>>         if (ret && mptcp_pm_should_add_signal_echo(msk) &&
> >>>>> mptcp_pm_should_add_signal_addr(msk))
> >>>>>                 mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_SEND_ACK);
> >>>>>         return ret;
> >>>>> '''
> >>>>>
> >>>>>>
> >>>>>>  bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
> >>>>>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> >>>>>> index a0b0ec0..0bfbbdef 100644
> >>>>>> --- a/net/mptcp/protocol.h
> >>>>>> +++ b/net/mptcp/protocol.h
> >>>>>> @@ -737,16 +737,23 @@ static inline bool mptcp_pm_should_rm_signal(struct mptcp_sock *msk)
> >>>>>>         return READ_ONCE(msk->pm.addr_signal) & BIT(MPTCP_RM_ADDR_SIGNAL);
> >>>>>>  }
> >>>>>>
> >>>>>> -static inline unsigned int mptcp_add_addr_len(int family, bool echo, bool port)
> >>>>>> +static inline unsigned int mptcp_add_addr_len(struct mptcp_out_options *opts)
> >>>>>>  {
> >>>>>> -       u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
> >>>>>> +       u8 len = 0;
> >>>>>> +       struct mptcp_addr_info *addr = &opts->remote;
> >>>>>
> >>>>> We can simplify it like this:
> >>>>>          struct mptcp_addr_info *addr = opts->ahmac ? &opts->local :
> >>>>> &opts->remote;
> >>>>>
> >>>>> And keep the orignal code unchanged.
> >>>>>
> >>>>>>
> >>>>>> -       if (family == AF_INET6)
> >>>>>> -               len = TCPOLEN_MPTCP_ADD_ADDR6_BASE;
> >>>>>> -       if (!echo)
> >>>>>> +       if (opts->ahmac) {
> >>>>>> +               addr = &opts->local;
> >>>>>>                 len += MPTCPOPT_THMAC_LEN;
> >>>>>> +       }
> >>>>>> +
> >>>>>> +       if (addr->family == AF_INET6)
> >>>>>> +               len += TCPOLEN_MPTCP_ADD_ADDR6_BASE;
> >>>>>> +       else
> >>>>>> +               len += TCPOLEN_MPTCP_ADD_ADDR_BASE;
> >>>>>> +
> >>>>>>         /* account for 2 trailing 'nop' options */
> >>>>>> -       if (port)
> >>>>>> +       if (addr->port)
> >>>>>>                 len += TCPOLEN_MPTCP_PORT_LEN + TCPOLEN_MPTCP_PORT_ALIGN;
> >>>>>>
> >>>>>>         return len;
> >>>>>
> >>>>> The whole function is something like this:
> >>>>> '''
> >>>>>         struct mptcp_addr_info *addr = opts->ahmac ? &opts->local :
> >>>>> &opts->remote;
> >>>>>         u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
> >>>>>
> >>>>>         if (addr->family == AF_INET6)
> >>>>>                 len = TCPOLEN_MPTCP_ADD_ADDR6_BASE;
> >>>>>         if (opts->ahmac)
> >>>>>                 len += MPTCPOPT_THMAC_LEN;
> >>>>>         /* account for 2 trailing 'nop' options */
> >>>>>         if (addr->port)
> >>>>>                 len += TCPOLEN_MPTCP_PORT_LEN + TCPOLEN_MPTCP_PORT_ALIGN;
> >>>>>
> >>>>>         return len;
> >>>>> '''
> >>>>>
> >>>>> Thanks.
> >>>>> -Geliang
> >>>>>
> >>>>>> @@ -760,8 +767,8 @@ static inline int mptcp_rm_addr_len(const struct mptcp_rm_list *rm_list)
> >>>>>>         return TCPOLEN_MPTCP_RM_ADDR_BASE + roundup(rm_list->nr - 1, 4) + 1;
> >>>>>>  }
> >>>>>>
> >>>>>> -bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
> >>>>>> -                             struct mptcp_addr_info *saddr, bool *echo, bool *port);
> >>>>>> +bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct mptcp_out_options *opts,
> >>>>>> +                             u8 *add_addr);
> >>>>>>  bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
> >>>>>>                              struct mptcp_rm_list *rm_list);
> >>>>>>  int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
> >>>>>> --
> >>>>>> 1.8.3.1
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>> --
> >>>> Li YongLong
> >>>
> >>
> >> --
> >> Li YongLong
> >>
> >
>
> --
> Li YongLong