On Mon, 30 Nov 2020, Geliang Tang wrote:
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.
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