| Message ID | 33653d8ae0890caa27621c1f40a572ff81a9a9e6.1605855783.git.geliangtang@gmail.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mat Martineau |
| Headers | show |
| Series | MP_PRIO support | expand |
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@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? 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
Hi Mat, Thanks for your review. Mat Martineau <mathew.j.martineau@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@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
On Mon, 30 Nov 2020, Geliang Tang wrote: > Hi Mat, > > Thanks for your review. > > Mat Martineau <mathew.j.martineau@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@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. Thanks for explaining, I had missed that. >> >>> + 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; >>> + } It's not a problem if a duplicate mp_prio is received, no need to do the comparison or print a warning. Let it assign the identical value. >>> + >>> + subflow->backup = bkup; >>> + >>> + spin_lock_bh(&msk->pm.lock); >>> + mptcp_schedule_work((struct sock *)msk); >>> + spin_unlock_bh(&msk->pm.lock); No need to schedule the worker here, there's nothing else to do after subflow->backup is set. Thanks, Mat >>> +} >>> + >>> /* 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
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; + 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);
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@gmail.com> --- net/mptcp/options.c | 15 +++++++++++++++ net/mptcp/pm.c | 19 +++++++++++++++++++ net/mptcp/protocol.h | 5 +++++ 3 files changed, 39 insertions(+)