| Message ID | 850406b0f49fa0a76b4825b36c55426a0d033d52.1621002341.git.geliangtang@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mat Martineau |
| Headers | show |
| Series | add MP_CAPABLE 'C' flag | expand |
On Fri, 14 May 2021, Geliang Tang wrote: > Like the anno_list member in struct mptcp_pm_data, this patch added a > new member named add_list in it, to save all the received ADD_ADDRs in > this add_list. Hi Geliang - How to handle MP_JOIN with 'C==1' connections was a topic in today's community meeting, and Paolo made a strong case that reception of ADD_ADDRs from a peer should not be tracked in a linked list (to avoid resource issues in servers with lots of connections). My concern is that preventing any call to __mptcp_subflow_connect() from mptcp_pm_create_subflow_or_signal_addr() when the peer sent C==1, would be a confusing/unexpected silent failure to establish subflows. For v7, could you: 1. Drop patches 3/6 & 4/6 2. Track the most recent received ADD_ADDR information in new struct members of mptcp_pm_data (also, be sure to invalidate if a matching REMOVE_ADDR is received). 3. If C==1 was received, used the stored ADD_ADDR address for the remote address in mptcp_pm_create_subflow_or_signal_addr(). If C==1 and there is no stored address, skip subflow creation. If C==0 was received, keep existing behavior. I've captured what I recall from the discussion - if anyone has corrections, clarifications, or a better proposal, please follow up! -Mat > > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > --- > net/mptcp/pm.c | 1 + > net/mptcp/pm_netlink.c | 89 +++++++++++++++++++++++++++++++++++++++--- > net/mptcp/protocol.c | 1 + > net/mptcp/protocol.h | 2 + > 4 files changed, 87 insertions(+), 6 deletions(-) > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > index 9d00fa6d22e9..9456fe17b6a3 100644 > --- a/net/mptcp/pm.c > +++ b/net/mptcp/pm.c > @@ -324,6 +324,7 @@ void mptcp_pm_data_init(struct mptcp_sock *msk) > > spin_lock_init(&msk->pm.lock); > INIT_LIST_HEAD(&msk->pm.anno_list); > + INIT_LIST_HEAD(&msk->pm.add_list); > > mptcp_pm_nl_data_init(msk); > } > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index 4fff8aef45e4..3fcc167ea702 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -39,6 +39,11 @@ struct mptcp_pm_anno_entry { > u8 retrans_times; > }; > > +struct mptcp_pm_add_entry { > + struct list_head list; > + struct mptcp_addr_info addr; > +}; > + > #define MAX_ADDR_ID 255 > #define BITMAP_SZ DIV_ROUND_UP(MAX_ADDR_ID + 1, BITS_PER_LONG) > > @@ -483,6 +488,69 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk) > mptcp_pm_create_subflow_or_signal_addr(msk); > } > > +struct mptcp_pm_add_entry * > +mptcp_lookup_add_list_by_id(struct mptcp_sock *msk, u8 id) > +{ > + struct mptcp_pm_add_entry *entry; > + > + lockdep_assert_held(&msk->pm.lock); > + > + list_for_each_entry(entry, &msk->pm.add_list, list) { > + if (entry->addr.id == id) > + return entry; > + } > + > + return NULL; > +} > + > +struct mptcp_pm_add_entry * > +mptcp_lookup_add_list_by_saddr(struct mptcp_sock *msk, > + struct mptcp_addr_info *addr) > +{ > + struct mptcp_pm_add_entry *entry; > + > + lockdep_assert_held(&msk->pm.lock); > + > + list_for_each_entry(entry, &msk->pm.add_list, list) { > + if (addresses_equal(&entry->addr, addr, true)) > + return entry; > + } > + > + return NULL; > +} > + > +static bool mptcp_pm_alloc_add_list(struct mptcp_sock *msk, > + struct mptcp_addr_info *addr) > +{ > + struct mptcp_pm_add_entry *add_entry = NULL; > + > + lockdep_assert_held(&msk->pm.lock); > + > + if (mptcp_lookup_add_list_by_saddr(msk, addr)) > + return false; > + > + add_entry = kmalloc(sizeof(*add_entry), GFP_ATOMIC); > + if (!add_entry) > + return false; > + > + list_add(&add_entry->list, &msk->pm.add_list); > + add_entry->addr = *addr; > + > + return true; > +} > + > +void mptcp_pm_free_add_list(struct mptcp_sock *msk) > +{ > + struct mptcp_pm_add_entry *entry, *tmp; > + > + pr_debug("msk=%p", msk); > + > + spin_lock_bh(&msk->pm.lock); > + list_for_each_entry_safe(entry, tmp, &msk->pm.add_list, list) > + kfree(entry); > + spin_unlock_bh(&msk->pm.lock); > +} > + > static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) > { > struct sock *sk = (struct sock *)msk; > @@ -501,12 +569,6 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) > if (lookup_subflow_by_daddr(&msk->conn_list, &msk->pm.remote)) > goto add_addr_echo; > > - msk->pm.add_addr_accepted++; > - msk->pm.subflows++; > - if (msk->pm.add_addr_accepted >= add_addr_accept_max || > - msk->pm.subflows >= subflows_max) > - WRITE_ONCE(msk->pm.accept_addr, false); > - > /* connect to the specified remote address, using whatever > * local address the routing configuration will pick. > */ > @@ -516,6 +578,15 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) > memset(&local, 0, sizeof(local)); > local.family = remote.family; > > + if (!mptcp_pm_alloc_add_list(msk, &remote)) > + return; > + > + msk->pm.add_addr_accepted++; > + msk->pm.subflows++; > + if (msk->pm.add_addr_accepted >= add_addr_accept_max || > + msk->pm.subflows >= subflows_max) > + WRITE_ONCE(msk->pm.accept_addr, false); > + > spin_unlock_bh(&msk->pm.lock); > __mptcp_subflow_connect(sk, &local, &remote, 0, 0); > spin_lock_bh(&msk->pm.lock); > @@ -612,6 +683,7 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk, > list_for_each_entry_safe(subflow, tmp, &msk->conn_list, node) { > struct sock *ssk = mptcp_subflow_tcp_sock(subflow); > int how = RCV_SHUTDOWN | SEND_SHUTDOWN; > + struct mptcp_pm_add_entry *entry; > u8 id = subflow->local_id; > > if (rm_type == MPTCP_MIB_RMADDR) > @@ -631,6 +703,11 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk, > if (rm_type == MPTCP_MIB_RMADDR) { > msk->pm.add_addr_accepted--; > WRITE_ONCE(msk->pm.accept_addr, true); > + entry = mptcp_lookup_add_list_by_id(msk, id); > + if (entry) { > + list_del(&entry->list); > + kfree(entry); > + } > } else if (rm_type == MPTCP_MIB_RMSUBFLOW) { > msk->pm.local_addr_used--; > } > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > index 35c0b1ca95c3..05ceba3972f6 100644 > --- a/net/mptcp/protocol.c > +++ b/net/mptcp/protocol.c > @@ -2888,6 +2888,7 @@ void mptcp_destroy_common(struct mptcp_sock *msk) > skb_rbtree_purge(&msk->out_of_order_queue); > mptcp_token_destroy(msk); > mptcp_pm_free_anno_list(msk); > + mptcp_pm_free_add_list(msk); > } > > static void mptcp_destroy(struct sock *sk) > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h > index 5ccc0d3e5693..1df8da3da695 100644 > --- a/net/mptcp/protocol.h > +++ b/net/mptcp/protocol.h > @@ -185,6 +185,7 @@ struct mptcp_pm_data { > struct mptcp_addr_info local; > struct mptcp_addr_info remote; > struct list_head anno_list; > + struct list_head add_list; > > spinlock_t lock; /*protects the whole PM data */ > > @@ -693,6 +694,7 @@ int 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); > +void mptcp_pm_free_add_list(struct mptcp_sock *msk); > bool mptcp_pm_sport_in_anno_list(struct mptcp_sock *msk, const struct sock *sk); > struct mptcp_pm_anno_entry * > mptcp_pm_del_anno_timer(struct mptcp_sock *msk, > -- > 2.31.1 > > > -- Mat Martineau Intel
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 9d00fa6d22e9..9456fe17b6a3 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -324,6 +324,7 @@ void mptcp_pm_data_init(struct mptcp_sock *msk) spin_lock_init(&msk->pm.lock); INIT_LIST_HEAD(&msk->pm.anno_list); + INIT_LIST_HEAD(&msk->pm.add_list); mptcp_pm_nl_data_init(msk); } diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 4fff8aef45e4..3fcc167ea702 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -39,6 +39,11 @@ struct mptcp_pm_anno_entry { u8 retrans_times; }; +struct mptcp_pm_add_entry { + struct list_head list; + struct mptcp_addr_info addr; +}; + #define MAX_ADDR_ID 255 #define BITMAP_SZ DIV_ROUND_UP(MAX_ADDR_ID + 1, BITS_PER_LONG) @@ -483,6 +488,69 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk) mptcp_pm_create_subflow_or_signal_addr(msk); } +struct mptcp_pm_add_entry * +mptcp_lookup_add_list_by_id(struct mptcp_sock *msk, u8 id) +{ + struct mptcp_pm_add_entry *entry; + + lockdep_assert_held(&msk->pm.lock); + + list_for_each_entry(entry, &msk->pm.add_list, list) { + if (entry->addr.id == id) + return entry; + } + + return NULL; +} + +struct mptcp_pm_add_entry * +mptcp_lookup_add_list_by_saddr(struct mptcp_sock *msk, + struct mptcp_addr_info *addr) +{ + struct mptcp_pm_add_entry *entry; + + lockdep_assert_held(&msk->pm.lock); + + list_for_each_entry(entry, &msk->pm.add_list, list) { + if (addresses_equal(&entry->addr, addr, true)) + return entry; + } + + return NULL; +} + +static bool mptcp_pm_alloc_add_list(struct mptcp_sock *msk, + struct mptcp_addr_info *addr) +{ + struct mptcp_pm_add_entry *add_entry = NULL; + + lockdep_assert_held(&msk->pm.lock); + + if (mptcp_lookup_add_list_by_saddr(msk, addr)) + return false; + + add_entry = kmalloc(sizeof(*add_entry), GFP_ATOMIC); + if (!add_entry) + return false; + + list_add(&add_entry->list, &msk->pm.add_list); + add_entry->addr = *addr; + + return true; +} + +void mptcp_pm_free_add_list(struct mptcp_sock *msk) +{ + struct mptcp_pm_add_entry *entry, *tmp; + + pr_debug("msk=%p", msk); + + spin_lock_bh(&msk->pm.lock); + list_for_each_entry_safe(entry, tmp, &msk->pm.add_list, list) + kfree(entry); + spin_unlock_bh(&msk->pm.lock); +} + static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) { struct sock *sk = (struct sock *)msk; @@ -501,12 +569,6 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) if (lookup_subflow_by_daddr(&msk->conn_list, &msk->pm.remote)) goto add_addr_echo; - msk->pm.add_addr_accepted++; - msk->pm.subflows++; - if (msk->pm.add_addr_accepted >= add_addr_accept_max || - msk->pm.subflows >= subflows_max) - WRITE_ONCE(msk->pm.accept_addr, false); - /* connect to the specified remote address, using whatever * local address the routing configuration will pick. */ @@ -516,6 +578,15 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) memset(&local, 0, sizeof(local)); local.family = remote.family; + if (!mptcp_pm_alloc_add_list(msk, &remote)) + return; + + msk->pm.add_addr_accepted++; + msk->pm.subflows++; + if (msk->pm.add_addr_accepted >= add_addr_accept_max || + msk->pm.subflows >= subflows_max) + WRITE_ONCE(msk->pm.accept_addr, false); + spin_unlock_bh(&msk->pm.lock); __mptcp_subflow_connect(sk, &local, &remote, 0, 0); spin_lock_bh(&msk->pm.lock); @@ -612,6 +683,7 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk, list_for_each_entry_safe(subflow, tmp, &msk->conn_list, node) { struct sock *ssk = mptcp_subflow_tcp_sock(subflow); int how = RCV_SHUTDOWN | SEND_SHUTDOWN; + struct mptcp_pm_add_entry *entry; u8 id = subflow->local_id; if (rm_type == MPTCP_MIB_RMADDR) @@ -631,6 +703,11 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk, if (rm_type == MPTCP_MIB_RMADDR) { msk->pm.add_addr_accepted--; WRITE_ONCE(msk->pm.accept_addr, true); + entry = mptcp_lookup_add_list_by_id(msk, id); + if (entry) { + list_del(&entry->list); + kfree(entry); + } } else if (rm_type == MPTCP_MIB_RMSUBFLOW) { msk->pm.local_addr_used--; } diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 35c0b1ca95c3..05ceba3972f6 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2888,6 +2888,7 @@ void mptcp_destroy_common(struct mptcp_sock *msk) skb_rbtree_purge(&msk->out_of_order_queue); mptcp_token_destroy(msk); mptcp_pm_free_anno_list(msk); + mptcp_pm_free_add_list(msk); } static void mptcp_destroy(struct sock *sk) diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 5ccc0d3e5693..1df8da3da695 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -185,6 +185,7 @@ struct mptcp_pm_data { struct mptcp_addr_info local; struct mptcp_addr_info remote; struct list_head anno_list; + struct list_head add_list; spinlock_t lock; /*protects the whole PM data */ @@ -693,6 +694,7 @@ int 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); +void mptcp_pm_free_add_list(struct mptcp_sock *msk); bool mptcp_pm_sport_in_anno_list(struct mptcp_sock *msk, const struct sock *sk); struct mptcp_pm_anno_entry * mptcp_pm_del_anno_timer(struct mptcp_sock *msk,
Like the anno_list member in struct mptcp_pm_data, this patch added a new member named add_list in it, to save all the received ADD_ADDRs in this add_list. Signed-off-by: Geliang Tang <geliangtang@gmail.com> --- net/mptcp/pm.c | 1 + net/mptcp/pm_netlink.c | 89 +++++++++++++++++++++++++++++++++++++++--- net/mptcp/protocol.c | 1 + net/mptcp/protocol.h | 2 + 4 files changed, 87 insertions(+), 6 deletions(-)