Message ID | 0681c7c5395956c6687108ca4ba7e9daba78a047.1596106606.git.geliangtang@gmail.com |
---|---|
State | Superseded, archived |
Headers | show |
Series | Add REMOVE_ADDR support | expand |
On Thu, 2020-07-30 at 19:06 +0800, Geliang Tang wrote: > The patch adds a new list named anno_list in mptcp_pm_data, to record all > the announced addrs. When the PM netlink removes an address, we check if > it has been recorded. If it has been, we trigger the RM_ADDR signal. > > Suggested-by: Matthieu Baerts <matthieu.baerts@tessares.net> > Suggested-by: Paolo Abeni <pabeni@redhat.com> > Suggested-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > --- > net/mptcp/pm.c | 7 +++++- > net/mptcp/pm_netlink.c | 49 +++++++++++++++++++++++++++++++++++++++++- > net/mptcp/protocol.c | 2 ++ > net/mptcp/protocol.h | 2 ++ > 4 files changed, 58 insertions(+), 2 deletions(-) > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > index 558462d87eb3..b3712771f268 100644 > --- a/net/mptcp/pm.c > +++ b/net/mptcp/pm.c > @@ -24,7 +24,11 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk, > > int mptcp_pm_remove_addr(struct mptcp_sock *msk, u8 local_id) > { > - return -ENOTSUPP; > + pr_debug("msk=%p, local_id=%d", msk, local_id); > + > + msk->pm.rm_id = local_id; > + WRITE_ONCE(msk->pm.rm_addr_signal, true); > + return 0; > } > > int mptcp_pm_remove_subflow(struct mptcp_sock *msk, u8 remote_id) > @@ -229,6 +233,7 @@ void mptcp_pm_data_init(struct mptcp_sock *msk) > msk->pm.status = 0; > > spin_lock_init(&msk->pm.lock); > + INIT_LIST_HEAD(&msk->pm.anno_list); > > mptcp_pm_nl_data_init(msk); > } > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index 74a18e463c3d..b9f0622e4082 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -169,6 +169,19 @@ static void check_work_pending(struct mptcp_sock *msk) > WRITE_ONCE(msk->pm.work_pending, false); > } > > +void mptcp_pm_free_anno_list(struct mptcp_sock *msk) > +{ > + struct mptcp_pm_addr_entry *entry, *tmp; > + > + pr_debug("msk=%p\n", msk); > + spin_lock_bh(&msk->pm.lock); > + list_for_each_entry_safe(entry, tmp, &msk->pm.anno_list, list) { > + list_del(&entry->list); > + kfree(entry); > + } > + spin_unlock_bh(&msk->pm.lock); > +} > + > static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) > { > struct sock *sk = (struct sock *)msk; > @@ -189,8 +202,16 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) > msk->pm.add_addr_signaled); > > if (local) { > + struct mptcp_pm_addr_entry *clone = NULL; > + > msk->pm.add_addr_signaled++; > mptcp_pm_announce_addr(msk, &local->addr); > + > + clone = kmemdup(local, sizeof(*local), GFP_ATOMIC); > + if (!clone) > + return; > + > + list_add(&clone->list, &msk->pm.anno_list); > } else { > /* pick failed, avoid fourther attempts later */ > msk->pm.local_addr_used = msk->pm.add_addr_signal_max; > @@ -548,6 +569,30 @@ __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id) > return NULL; > } > > +static void mptcp_nl_remove_addr(struct sk_buff *skb, struct mptcp_addr_info *addr) > +{ > + struct mptcp_sock *msk; > + long s_slot = 0, s_num = 0; > + struct net *net = sock_net(skb->sk); > + > + while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) { > + struct mptcp_pm_addr_entry *entry, *tmp; > + struct sock *sk = (struct sock *)msk; > + > + pr_debug("msk=%p\n", msk); > + spin_lock_bh(&msk->pm.lock); > + list_for_each_entry_safe(entry, tmp, &msk->pm.anno_list, list) { > + if (addresses_equal(&entry->addr, addr, false)) { > + mptcp_pm_remove_addr(msk, addr->id); > + list_del(&entry->list); > + kfree(entry); > + } > + } > + spin_unlock_bh(&msk->pm.lock); > + sock_put(sk); it would be better add a cond_resched(); here, since traversing the hash table could potentially starve the scheduler for long time otherwise. /P
On Thu, 2020-07-30 at 22:26 +0200, Paolo Abeni wrote: > On Thu, 2020-07-30 at 19:06 +0800, Geliang Tang wrote: > > The patch adds a new list named anno_list in mptcp_pm_data, to record all > > the announced addrs. When the PM netlink removes an address, we check if > > it has been recorded. If it has been, we trigger the RM_ADDR signal. > > > > Suggested-by: Matthieu Baerts <matthieu.baerts@tessares.net> > > Suggested-by: Paolo Abeni <pabeni@redhat.com> > > Suggested-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > > --- > > net/mptcp/pm.c | 7 +++++- > > net/mptcp/pm_netlink.c | 49 +++++++++++++++++++++++++++++++++++++++++- > > net/mptcp/protocol.c | 2 ++ > > net/mptcp/protocol.h | 2 ++ > > 4 files changed, 58 insertions(+), 2 deletions(-) > > > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > > index 558462d87eb3..b3712771f268 100644 > > --- a/net/mptcp/pm.c > > +++ b/net/mptcp/pm.c > > @@ -24,7 +24,11 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk, > > > > int mptcp_pm_remove_addr(struct mptcp_sock *msk, u8 local_id) > > { > > - return -ENOTSUPP; > > + pr_debug("msk=%p, local_id=%d", msk, local_id); > > + > > + msk->pm.rm_id = local_id; > > + WRITE_ONCE(msk->pm.rm_addr_signal, true); > > + return 0; > > } > > > > int mptcp_pm_remove_subflow(struct mptcp_sock *msk, u8 remote_id) > > @@ -229,6 +233,7 @@ void mptcp_pm_data_init(struct mptcp_sock *msk) > > msk->pm.status = 0; > > > > spin_lock_init(&msk->pm.lock); > > + INIT_LIST_HEAD(&msk->pm.anno_list); > > > > mptcp_pm_nl_data_init(msk); > > } > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > > index 74a18e463c3d..b9f0622e4082 100644 > > --- a/net/mptcp/pm_netlink.c > > +++ b/net/mptcp/pm_netlink.c > > @@ -169,6 +169,19 @@ static void check_work_pending(struct mptcp_sock *msk) > > WRITE_ONCE(msk->pm.work_pending, false); > > } > > > > +void mptcp_pm_free_anno_list(struct mptcp_sock *msk) > > +{ > > + struct mptcp_pm_addr_entry *entry, *tmp; > > + > > + pr_debug("msk=%p\n", msk); > > + spin_lock_bh(&msk->pm.lock); > > + list_for_each_entry_safe(entry, tmp, &msk->pm.anno_list, list) { > > + list_del(&entry->list); > > + kfree(entry); > > + } > > + spin_unlock_bh(&msk->pm.lock); > > +} > > + > > static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) > > { > > struct sock *sk = (struct sock *)msk; > > @@ -189,8 +202,16 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) > > msk->pm.add_addr_signaled); > > > > if (local) { > > + struct mptcp_pm_addr_entry *clone = NULL; > > + > > msk->pm.add_addr_signaled++; > > mptcp_pm_announce_addr(msk, &local->addr); > > + > > + clone = kmemdup(local, sizeof(*local), GFP_ATOMIC); > > + if (!clone) > > + return; > > + > > + list_add(&clone->list, &msk->pm.anno_list); > > } else { > > /* pick failed, avoid fourther attempts later */ > > msk->pm.local_addr_used = msk->pm.add_addr_signal_max; > > @@ -548,6 +569,30 @@ __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id) > > return NULL; > > } > > > > +static void mptcp_nl_remove_addr(struct sk_buff *skb, struct mptcp_addr_info *addr) > > +{ > > + struct mptcp_sock *msk; > > + long s_slot = 0, s_num = 0; > > + struct net *net = sock_net(skb->sk); > > + > > + while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) { > > + struct mptcp_pm_addr_entry *entry, *tmp; > > + struct sock *sk = (struct sock *)msk; > > + > > + pr_debug("msk=%p\n", msk); > > + spin_lock_bh(&msk->pm.lock); You can skip acquiring the spinlock and all the following when: if (list_empty(&msk->pm.anno_list)) continue; > > + list_for_each_entry_safe(entry, tmp, &msk->pm.anno_list, list) { > > + if (addresses_equal(&entry->addr, addr, false)) { > > + mptcp_pm_remove_addr(msk, addr->id); > > + list_del(&entry->list); > > + kfree(entry); > > + } > > + } > > + spin_unlock_bh(&msk->pm.lock); > > + sock_put(sk); > > it would be better add a > cond_resched(); > > here, since traversing the hash table could potentially starve the > scheduler for long time otherwise. additionally I think here is necessary also to shutdown the subflow using addr as source, if present - alike what mptcp_nl_remove_subflow() does in the next patch. Perhaps is easier merge the two helpers (mptcp_nl_remove_subflow() and mptcp_nl_remove_addr()) - not sure without writing the actual code. /P
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 558462d87eb3..b3712771f268 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -24,7 +24,11 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk, int mptcp_pm_remove_addr(struct mptcp_sock *msk, u8 local_id) { - return -ENOTSUPP; + pr_debug("msk=%p, local_id=%d", msk, local_id); + + msk->pm.rm_id = local_id; + WRITE_ONCE(msk->pm.rm_addr_signal, true); + return 0; } int mptcp_pm_remove_subflow(struct mptcp_sock *msk, u8 remote_id) @@ -229,6 +233,7 @@ void mptcp_pm_data_init(struct mptcp_sock *msk) msk->pm.status = 0; spin_lock_init(&msk->pm.lock); + INIT_LIST_HEAD(&msk->pm.anno_list); mptcp_pm_nl_data_init(msk); } diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 74a18e463c3d..b9f0622e4082 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -169,6 +169,19 @@ static void check_work_pending(struct mptcp_sock *msk) WRITE_ONCE(msk->pm.work_pending, false); } +void mptcp_pm_free_anno_list(struct mptcp_sock *msk) +{ + struct mptcp_pm_addr_entry *entry, *tmp; + + pr_debug("msk=%p\n", msk); + spin_lock_bh(&msk->pm.lock); + list_for_each_entry_safe(entry, tmp, &msk->pm.anno_list, list) { + list_del(&entry->list); + kfree(entry); + } + spin_unlock_bh(&msk->pm.lock); +} + static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) { struct sock *sk = (struct sock *)msk; @@ -189,8 +202,16 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) msk->pm.add_addr_signaled); if (local) { + struct mptcp_pm_addr_entry *clone = NULL; + msk->pm.add_addr_signaled++; mptcp_pm_announce_addr(msk, &local->addr); + + clone = kmemdup(local, sizeof(*local), GFP_ATOMIC); + if (!clone) + return; + + list_add(&clone->list, &msk->pm.anno_list); } else { /* pick failed, avoid fourther attempts later */ msk->pm.local_addr_used = msk->pm.add_addr_signal_max; @@ -548,6 +569,30 @@ __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id) return NULL; } +static void mptcp_nl_remove_addr(struct sk_buff *skb, struct mptcp_addr_info *addr) +{ + struct mptcp_sock *msk; + long s_slot = 0, s_num = 0; + struct net *net = sock_net(skb->sk); + + while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) { + struct mptcp_pm_addr_entry *entry, *tmp; + struct sock *sk = (struct sock *)msk; + + pr_debug("msk=%p\n", msk); + spin_lock_bh(&msk->pm.lock); + list_for_each_entry_safe(entry, tmp, &msk->pm.anno_list, list) { + if (addresses_equal(&entry->addr, addr, false)) { + mptcp_pm_remove_addr(msk, addr->id); + list_del(&entry->list); + kfree(entry); + } + } + spin_unlock_bh(&msk->pm.lock); + sock_put(sk); + } +} + static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info) { struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR]; @@ -566,8 +611,10 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info) ret = -EINVAL; goto out; } - if (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) + if (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) { pernet->add_addr_signal_max--; + mptcp_nl_remove_addr(skb, &entry->addr); + } if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) pernet->local_addr_max--; diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 2f43d0296951..deb89f3ab3ad 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -1640,6 +1640,8 @@ static void mptcp_close(struct sock *sk, long timeout) __mptcp_close_ssk(sk, ssk, subflow, timeout); } + mptcp_pm_free_anno_list(msk); + mptcp_cancel_work(sk); __skb_queue_purge(&sk->sk_receive_queue); diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 717b3aee776c..a10c53c2c0ee 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -177,6 +177,7 @@ struct mptcp_pm_data { u8 subflows_max; u8 status; u8 rm_id; + struct list_head anno_list; }; struct mptcp_data_frag { @@ -433,6 +434,7 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk, const struct mptcp_addr_info *addr); int mptcp_pm_remove_addr(struct mptcp_sock *msk, u8 local_id); int mptcp_pm_remove_subflow(struct mptcp_sock *msk, u8 remote_id); +void mptcp_pm_free_anno_list(struct mptcp_sock *msk); static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk) {
The patch adds a new list named anno_list in mptcp_pm_data, to record all the announced addrs. When the PM netlink removes an address, we check if it has been recorded. If it has been, we trigger the RM_ADDR signal. Suggested-by: Matthieu Baerts <matthieu.baerts@tessares.net> Suggested-by: Paolo Abeni <pabeni@redhat.com> Suggested-by: Mat Martineau <mathew.j.martineau@linux.intel.com> Signed-off-by: Geliang Tang <geliangtang@gmail.com> --- net/mptcp/pm.c | 7 +++++- net/mptcp/pm_netlink.c | 49 +++++++++++++++++++++++++++++++++++++++++- net/mptcp/protocol.c | 2 ++ net/mptcp/protocol.h | 2 ++ 4 files changed, 58 insertions(+), 2 deletions(-)