[MPTCP] Re: [MPTCP][PATCH mptcp-next 2/5] mptcp: add the incoming MP_PRIO support

[Geliang Tang <geliangtang at gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5082 bytes --]

Hi Mat,

Thanks for your review.

Mat Martineau <mathew.j.martineau(a)linux.intel.com> 于2020年11月21日周六 上午9:06写道：
>
> On Fri, 20 Nov 2020, Geliang Tang wrote:
>
> > This patch added the incoming MP_PRIO logic:
> >
> > Added a flag named mp_prio in struct mptcp_options_received, to mark the
> > MP_PRIO is received, and save the priority value to struct
> > mptcp_options_received's backup member. Then invoke
> > mptcp_pm_mp_prio_received with the receiving subsocket and
> > the backup value.
> >
> > In mptcp_pm_mp_prio_received, get the subflow context according the input
> > subsocket, and change the subflow's backup as the incoming priority value
> > and reschedule mptcp_worker.
> >
> > Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
> > ---
> > net/mptcp/options.c  | 15 +++++++++++++++
> > net/mptcp/pm.c       | 19 +++++++++++++++++++
> > net/mptcp/protocol.h |  5 +++++
> > 3 files changed, 39 insertions(+)
> >
> > diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> > index e52eda8c1a18..b1cb43c76be8 100644
> > --- a/net/mptcp/options.c
> > +++ b/net/mptcp/options.c
> > @@ -282,6 +282,15 @@ static void mptcp_parse_option(const struct sk_buff *skb,
> >               pr_debug("RM_ADDR: id=%d", mp_opt->rm_id);
> >               break;
> >
> > +     case MPTCPOPT_MP_PRIO:
> > +             if (opsize != TCPOLEN_MPTCP_PRIO)
> > +                     break;
> > +
> > +             mp_opt->mp_prio = 1;
> > +             mp_opt->backup = *ptr++ & MPTCP_PRIO_BKUP;
>
> Hi Geliang. Thank you for the updated patch set.
>
> Why are both mp_opt->mp_prio and mp_opt->backup needed? Why not use only
> mp_opt->backup?

I think mp_opt->backup could be 0 when we want to change the subflow's
backup value from 1 to 0. So I added another flag mp_prio to mark we get
an MP_PRIO packet.


-Geliang

>
> Regards,
>
> Mat
>
>
> > +             pr_debug("MP_PRIO: prio=%d", mp_opt->backup);
> > +             break;
> > +
> >       default:
> >               break;
> >       }
> > @@ -302,6 +311,7 @@ void mptcp_get_options(const struct sk_buff *skb,
> >       mp_opt->port = 0;
> >       mp_opt->rm_addr = 0;
> >       mp_opt->dss = 0;
> > +     mp_opt->mp_prio = 0;
> >
> >       length = (th->doff * 4) - sizeof(struct tcphdr);
> >       ptr = (const unsigned char *)(th + 1);
> > @@ -999,6 +1009,11 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
> >               mp_opt.rm_addr = 0;
> >       }
> >
> > +     if (mp_opt.mp_prio) {
> > +             mptcp_pm_mp_prio_received(sk, mp_opt.backup);
> > +             mp_opt.mp_prio = 0;
> > +     }
> > +
> >       if (!mp_opt.dss)
> >               return;
> >
> > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> > index ea9e498903e4..ba5b6c4b6fea 100644
> > --- a/net/mptcp/pm.c
> > +++ b/net/mptcp/pm.c
> > @@ -201,6 +201,25 @@ void mptcp_pm_rm_addr_received(struct mptcp_sock *msk, u8 rm_id)
> >       spin_unlock_bh(&pm->lock);
> > }
> >
> > +void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
> > +{
> > +     struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
> > +     struct mptcp_sock *msk = mptcp_sk(subflow->conn);
> > +
> > +     pr_debug("bkup=%d", bkup);
> > +
> > +     if (subflow->backup == bkup) {
> > +             pr_warn("backup error.");
> > +             return;
> > +     }
> > +
> > +     subflow->backup = bkup;
> > +
> > +     spin_lock_bh(&msk->pm.lock);
> > +     mptcp_schedule_work((struct sock *)msk);
> > +     spin_unlock_bh(&msk->pm.lock);
> > +}
> > +
> > /* path manager helpers */
> >
> > bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index 99de1f13915d..09a8885a0cc4 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -87,6 +87,9 @@
> > #define MPTCP_ADDR_IPVERSION_4        4
> > #define MPTCP_ADDR_IPVERSION_6        6
> >
> > +/* MPTCP MP_PRIO flags */
> > +#define MPTCP_PRIO_BKUP              BIT(0)
> > +
> > /* MPTCP socket flags */
> > #define MPTCP_DATA_READY      0
> > #define MPTCP_NOSPACE         1
> > @@ -116,6 +119,7 @@ struct mptcp_options_received {
> >               dss : 1,
> >               add_addr : 1,
> >               rm_addr : 1,
> > +             mp_prio : 1,
> >               family : 4,
> >               echo : 1,
> >               backup : 1;
> > @@ -555,6 +559,7 @@ void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
> >                               const struct mptcp_addr_info *addr);
> > void mptcp_pm_add_addr_send_ack(struct mptcp_sock *msk);
> > void mptcp_pm_rm_addr_received(struct mptcp_sock *msk, u8 rm_id);
> > +void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup);
> > void mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
> >                                 struct mptcp_addr_info *addr,
> >                                 u8 bkup);
> > --
> > 2.26.2
>
> --
> Mat Martineau
> Intel