* [MPTCP] Re: [MPTCP][PATCH v4 mptcp-next 4/5] mptcp: trigger the RM_ADDR signal
@ 2020-07-30 20:46 Paolo Abeni
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Abeni @ 2020-07-30 20:46 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 4668 bytes --]
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(a)tessares.net>
> > Suggested-by: Paolo Abeni <pabeni(a)redhat.com>
> > Suggested-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> > Signed-off-by: Geliang Tang <geliangtang(a)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
^ permalink raw reply [flat|nested] 2+ messages in thread
* [MPTCP] Re: [MPTCP][PATCH v4 mptcp-next 4/5] mptcp: trigger the RM_ADDR signal
@ 2020-07-30 20:26 Paolo Abeni
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Abeni @ 2020-07-30 20:26 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3950 bytes --]
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(a)tessares.net>
> Suggested-by: Paolo Abeni <pabeni(a)redhat.com>
> Suggested-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> Signed-off-by: Geliang Tang <geliangtang(a)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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-07-30 20:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 20:46 [MPTCP] Re: [MPTCP][PATCH v4 mptcp-next 4/5] mptcp: trigger the RM_ADDR signal Paolo Abeni
-- strict thread matches above, loose matches on Subject: below --
2020-07-30 20:26 Paolo Abeni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.