This patch added the outgoing MP_PRIO logic: In mptcp_pm_mp_prio_send_ack, find the related subflow and subsocket according to the input parameter addr. Save the input priority value to suflow's backup, and reschedule mptcp_worker. Then set subflow's send_mp_prio flag to true, and save the input priority value to suflow's request_bkup. Finally, send out a pure ACK on the related subsocket. In mptcp_established_options_mp_prio, check whether the subflow's send_mp_prio is set. If it is, this is the packet for send MP_PRIO. So save subflow->request_bkup value to mptcp_out_options's backup, and change the option type to OPTION_MPTCP_PRIO. In mptcp_write_options, sent out the MP_PRIO suboption with mptcp_out_options's backup value. Signed-off-by: Geliang Tang <geliangtang(a)gmail.com&gt; --- net/mptcp/options.c | 40 ++++++++++++++++++++++++++++++++++++++++ net/mptcp/pm_netlink.c | 39 +++++++++++++++++++++++++++++++++++++++ net/mptcp/protocol.h | 6 ++++++ 3 files changed, 85 insertions(+) diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 9db824b1ed33..e52eda8c1a18 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -666,6 +666,28 @@ static bool mptcp_established_options_rm_addr(struct sock *sk, return true; } +static bool mptcp_established_options_mp_prio(struct sock *sk, + unsigned int *size, + unsigned int remaining, + struct mptcp_out_options *opts) +{ + struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk); + + if (!subflow->send_mp_prio) + return false; + + if (remaining < TCPOLEN_MPTCP_PRIO) + return false; + + *size = TCPOLEN_MPTCP_PRIO; + opts->suboptions |= OPTION_MPTCP_PRIO; + opts->backup = subflow->request_bkup; + + pr_debug("prio=%d", opts->backup); + + return true; +} + bool mptcp_established_options(struct sock *sk, struct sk_buff *skb, unsigned int *size, unsigned int remaining, struct mptcp_out_options *opts) @@ -708,6 +730,12 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb, ret = true; } + if (mptcp_established_options_mp_prio(sk, &opt_size, remaining, opts)) { + *size += opt_size; + remaining -= opt_size; + ret = true; + } + return ret; } @@ -1145,6 +1173,18 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, 0, opts->rm_id); } + if (OPTION_MPTCP_PRIO & opts->suboptions) { + const struct sock *ssk = (const struct sock *)tp; + struct mptcp_subflow_context *subflow; + + subflow = mptcp_subflow_ctx(ssk); + subflow->send_mp_prio = 0; + + *ptr++ = mptcp_option(MPTCPOPT_MP_PRIO, + TCPOLEN_MPTCP_PRIO, + opts->backup, TCPOPT_NOP); + } + if (OPTION_MPTCP_MPJ_SYN & opts->suboptions) { *ptr++ = mptcp_option(MPTCPOPT_MP_JOIN, TCPOLEN_MPTCP_MPJ_SYN, diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 0e7c8a64604f..dd48af3c5ed5 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -513,6 +513,45 @@ void mptcp_pm_nl_add_addr_send_ack(struct mptcp_sock *msk) } } +void mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, + struct mptcp_addr_info *addr, + u8 bkup) +{ + struct mptcp_subflow_context *subflow; + + pr_debug("bkup=%d", bkup); + + mptcp_for_each_subflow(msk, subflow) { + struct sock *ssk = mptcp_subflow_tcp_sock(subflow); + struct sock *sk = (struct sock *)msk; + struct mptcp_addr_info local; + + local_address((struct sock_common *)ssk, &local); + if (!addresses_equal(&local, addr, false)) + continue; + + if (subflow->backup == bkup) { + pr_warn("backup error"); + return; + } + + subflow->backup = bkup; + mptcp_schedule_work(sk); + + subflow->send_mp_prio = 1; + subflow->request_bkup = bkup; + + spin_unlock_bh(&msk->pm.lock); + pr_debug("send ack for mp_prio"); + lock_sock(ssk); + tcp_send_ack(ssk); + release_sock(ssk); + spin_lock_bh(&msk->pm.lock); + + break; + } +} + void mptcp_pm_nl_rm_addr_received(struct mptcp_sock *msk) { struct mptcp_subflow_context *subflow, *tmp; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 737c7c30eadd..99de1f13915d 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -23,6 +23,7 @@ #define OPTION_MPTCP_ADD_ADDR BIT(6) #define OPTION_MPTCP_ADD_ADDR6 BIT(7) #define OPTION_MPTCP_RM_ADDR BIT(8) +#define OPTION_MPTCP_PRIO BIT(9) /* MPTCP option subtypes */ #define MPTCPOPT_MP_CAPABLE 0 @@ -59,6 +60,7 @@ #define TCPOLEN_MPTCP_PORT_LEN 4 #define TCPOLEN_MPTCP_ADD_ADDR_HMAC 8 #define TCPOLEN_MPTCP_RM_ADDR_BASE 4 +#define TCPOLEN_MPTCP_PRIO 4 /* MPTCP MP_JOIN flags */ #define MPTCPOPT_BACKUP BIT(0) @@ -394,6 +396,7 @@ struct mptcp_subflow_context { map_valid : 1, mpc_map : 1, backup : 1, + send_mp_prio : 1, rx_eof : 1, can_ack : 1, /* only after processing the remote a key */ disposable : 1, /* ctx can be free at ulp release time */ @@ -552,6 +555,9 @@ 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_nl_mp_prio_send_ack(struct mptcp_sock *msk, + struct mptcp_addr_info *addr, + u8 bkup); void mptcp_pm_free_anno_list(struct mptcp_sock *msk); bool mptcp_pm_sport_in_anno_list(struct mptcp_sock *msk, const struct sock *sk); struct mptcp_pm_add_entry * -- 2.26.2

This patch added the MP_PRIO support for PM netlink: When PM netlink adds an address, check whether the MPTCP_PM_ADDR_FLAG_BACKUP flag is set. If it is, check whether this address had been added in the local address list. If it had been, then call mptcp_nl_addr_backup to deal with this address. In mptcp_nl_addr_backup, we traverse all the existing msk sockets to find the relevant sockets, and get the backup value according to whether the MPTCP_PM_ADDR_FLAG_BACKUP flag is set, then call mptcp_pm_nl_mp_prio_send_ack to send out a MP_PRIO ACK packet. In mptcp_pm_nl_mp_prio_send_ack, we set or clear the MPTCP_PM_ADDR_FLAG_BACKUP flag. Signed-off-by: Geliang Tang <geliangtang(a)gmail.com&gt; --- net/mptcp/pm_netlink.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index dd48af3c5ed5..58960e1cdc32 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -535,6 +535,11 @@ void mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, return; } + if (addr->flags & MPTCP_PM_ADDR_FLAG_BACKUP) + addr->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP; + else + addr->flags |= MPTCP_PM_ADDR_FLAG_BACKUP; + subflow->backup = bkup; mptcp_schedule_work(sk); @@ -847,17 +852,53 @@ static struct pm_nl_pernet *genl_info_pm_nl(struct genl_info *info) return net_generic(genl_info_net(info), pm_nl_pernet_id); } +static int mptcp_nl_addr_backup(struct net *net, + struct mptcp_addr_info *addr) +{ + long s_slot = 0, s_num = 0; + struct mptcp_sock *msk; + + while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) { + u8 bkup = !(addr->flags & MPTCP_PM_ADDR_FLAG_BACKUP); + struct sock *sk = (struct sock *)msk; + + if (list_empty(&msk->conn_list)) + goto next; + + lock_sock(sk); + spin_lock_bh(&msk->pm.lock); + mptcp_pm_nl_mp_prio_send_ack(msk, addr, bkup); + spin_unlock_bh(&msk->pm.lock); + release_sock(sk); + +next: + sock_put(sk); + cond_resched(); + } + + return 0; +} + static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info) { struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR]; struct pm_nl_pernet *pernet = genl_info_pm_nl(info); struct mptcp_pm_addr_entry addr, *entry; + struct net *net = sock_net(skb->sk); int ret; ret = mptcp_pm_parse_addr(attr, info, true, &addr); if (ret < 0) return ret; + if (addr.addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP) { + list_for_each_entry(entry, &pernet->local_addr_list, list) { + if (addresses_equal(&entry->addr, &addr.addr, false)) + mptcp_nl_addr_backup(net, &entry->addr); + } + return 0; + } + entry = kmalloc(sizeof(*entry), GFP_KERNEL); if (!entry) { GENL_SET_ERR_MSG(info, "can't allocate addr"); -- 2.26.2

This patch added the MP_PRIO testcases: Add a new argument bkup for run_tests and do_transfer to mark how many times we want to execute the 'pm_nl_ctl flags backup' command. Add a new function chk_prio_nr to check the MP_PRIO related MIB counters. The output looks like this: 29 single subflow, backup syn[ ok ] - synack[ ok ] - ack[ ok ] ptx[ ok ] - prx [ ok ] 30 single address, backup syn[ ok ] - synack[ ok ] - ack[ ok ] add[ ok ] - echo [ ok ] ptx[ ok ] - prx [ ok ] Signed-off-by: Geliang Tang <geliangtang(a)gmail.com&gt; --- .../testing/selftests/net/mptcp/mptcp_join.sh | 81 ++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index dd094e3e7f14..84cb4dd0a0eb 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -213,6 +213,7 @@ do_transfer() rm_nr_ns2="$8" speed="$9" with_port="${10}" + bkup="${11}" port=$((10000+$TEST_COUNT)) TEST_COUNT=$((TEST_COUNT+1)) @@ -294,6 +295,27 @@ do_transfer() done fi + if [ $bkup -gt 0 ]; then + counter=1 + + while [ $counter -le $bkup ] + do + sleep 1 + for netns in "$ns1" "$ns2";do + dump=(`ip netns exec $netns ./pm_nl_ctl dump`) + num=${#dump[@]} + while [ $num -gt 0 ] + do + backup="ip netns exec $netns \ + ./pm_nl_ctl add ${dump[$num - 1]} flags backup" + $backup + num=$(($num - 5)) + done + done + let counter+=1 + done + fi + wait $cpid retc=$? wait $spid @@ -361,6 +383,7 @@ run_tests() rm_nr_ns2="${6:-0}" speed="${7:-fast}" with_port="${8:-0}" + bkup=${9:-0} lret=0 oldin="" @@ -375,7 +398,7 @@ run_tests() fi do_transfer ${listener_ns} ${connector_ns} MPTCP MPTCP ${connect_addr} \ - ${test_linkfail} ${rm_nr_ns1} ${rm_nr_ns2} ${speed} ${with_port} + ${test_linkfail} ${rm_nr_ns1} ${rm_nr_ns2} ${speed} ${with_port} ${bkup} lret=$? if [ "$test_linkfail" -eq 1 ];then @@ -564,6 +587,43 @@ chk_rm_nr() fi } +chk_prio_nr() +{ + local mp_prio_nr_tx=$1 + local mp_prio_nr_rx=$2 + local count + local dump_stats + + printf "%-39s %s" " " "ptx" + count=`ip netns exec $ns1 nstat -as | grep MPTcpExtMPPrioTx | awk '{print $2}'` + [ -z "$count" ] && count=0 + if [ "$count" != "$mp_prio_nr_tx" ]; then + echo "[fail] got $count MP_PRIO[s] TX expected $mp_prio_nr_tx" + ret=1 + dump_stats=1 + else + echo -n "[ ok ]" + fi + + echo -n " - prx " + count=`ip netns exec $ns1 nstat -as | grep MPTcpExtMPPrioRx | awk '{print $2}'` + [ -z "$count" ] && count=0 + if [ "$count" != "$mp_prio_nr_rx" ]; then + echo "[fail] got $count MP_PRIO[s] RX expected $mp_prio_nr_rx" + ret=1 + dump_stats=1 + else + echo "[ ok ]" + fi + + if [ "${dump_stats}" = 1 ]; then + echo Server ns stats + ip netns exec $ns1 nstat -as | grep MPTcp + echo Client ns stats + ip netns exec $ns2 nstat -as | grep MPTcp + fi +} + sin=$(mktemp) sout=$(mktemp) cin=$(mktemp) @@ -831,6 +891,25 @@ run_tests $ns1 $ns2 10.0.1.1 0 0 0 fast 10100 chk_join_nr "signal address with port, userspace" 1 0 0 chk_add_nr 1 1 1 0 0 0 +# single subflow, backup +reset +ip netns exec $ns1 ./pm_nl_ctl limits 0 1 +ip netns exec $ns2 ./pm_nl_ctl limits 0 1 +ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow +run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow 0 1 +chk_join_nr "single subflow, backup" 1 1 1 +chk_prio_nr 0 1 + +# single address, backup +reset +ip netns exec $ns1 ./pm_nl_ctl limits 0 1 +ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal +ip netns exec $ns2 ./pm_nl_ctl limits 1 1 +run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow 0 2 +chk_join_nr "single address, backup" 1 1 1 +chk_add_nr 1 1 +chk_prio_nr 2 0 + # single subflow, syncookies reset_with_cookies ip netns exec $ns1 ./pm_nl_ctl limits 0 1 -- 2.26.2

On Fri, 20 Nov 2020, Geliang Tang wrote: Why is the backup bit inverted here? It's not clear why it's changed, and why bkup is passed in based on the value of the bit when read by the calling function. I think a more strict match should be required here that includes the 'id' as well. There may be multiple endpoints with the same address but different port numbers. -- Mat Martineau Intel

On Mon, 30 Nov 2020, Geliang Tang wrote: I would prefer to have the 'backup' bit in the netlink flags set the backup bit directly - if the backup flag is 0 the subflow backup bit gets set to 0, and if the netlink backup flag is 1 the subflow backup bit gets set to 1. Paolo, does that interpretation of the backup flag match what you were thinking too? Ok, yes, addresses_equal() does compare port numbers so that should help. -- Mat Martineau Intel

On Wed, 2020-12-02 at 12:35 +0800, Geliang Tang wrote: This patch changes the semantic of the MPTCP_PM_CMD_ADD_ADDR netlink command depending on the presence of the backup flag. I think is quite confusing. The backup flag should be the only attribute we are allowed to change on an existing endpoint, but that may change in the future - e.g. if we will ever allow setting setsockopt() on the listening socket for port- based endpoint. I think it would be better add another NL command to update the existing address. It may still use the same parsing helper as MPTCP_PM_CMD_ADD_ADDR, than it will lookup the address by ID and will set the address backup flag according to the value provided by the user. The pm_nl_ctl syntax is not extremely important IMHO, but the ip route syntax will be much more interesting. Something alike: ip mptcp endpoint set id <nr> flags backup # the endpoint become a backup one ip mptcp endpoint set id <nr> flags nobackup # the endpoint become a non backup one ip mptcp endpoint set id <nr> flags "" # the endpoint become a non backup one ip mptcp endpoint set id <nr> flags signal # the endpoint become a non backup one, 'signal' is ignored ip mptcp endpoint set id <nr> flags signal,backup # the endpoint become a backup one, 'signal' is ignored pm_nl_ctl could opt for something similar, e.g.: pm_nl_ctl set 10.0.2.1 flags backup pm_nl_ctl set 10.0.2.1 flags nobackup pm_nl_ctl set 10.0.2.1 flags signal,backup pm_nl_ctl set 10.0.2.1 flags signal etc... The only special thing will ne the 'nobackup' keyword, just to be more expressive than leaving the flag list empty. WDYT? Thanks, Paolo

Hi Mat, Thanks for your review. Mat Martineau <mathew.j.martineau(a)linux.intel.com&gt; 于2020年11月21日周六 上午9:06写道： 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