diff mbox series

[v3,2/9] Squash-to: "mptcp: Add path manager interface"

Message ID b8758e0d5d7b8c8955171d59b0d1b5e5712ce726.1582303591.git.pabeni@redhat.com
State Superseded, archived
Delegated to: Matthieu Baerts
Headers show
Series add locking to PM APis, implement PM netlink | expand

Commit Message

Paolo Abeni Feb. 21, 2020, 4:48 p.m. UTC 
Factor out PM addr info to simplify PM data definition and
simplify the PM hooks/APIs.

The PM will carry a single local and remote addrs - the one
currently being processed, if any. It's up to the PM impl,
e.g. netlink, maintain additional per msk data, if needed.

Account the number of created subflow and received addresses,
to enforce limits.

Add a spinlock to protect PM datas, so that we can manipulate
them from subflow BH.

Delegate events handling to a workqueue, so that PM events can
be processed with both the above spinlock and the msk socket
lock held. The PM impl should hook inside the worker.

v1 -> v2:
 - be sure to initialize the address port before calling
   mptcp_pm_add_addr()

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/options.c  |  46 ++++++++++--------
 net/mptcp/pm.c       | 112 ++++++++++++++-----------------------------
 net/mptcp/protocol.c |   2 +
 net/mptcp/protocol.h |  98 ++++++++++++++++++++++---------------
 4 files changed, 123 insertions(+), 135 deletions(-)

Comments

Matthieu Baerts Feb. 21, 2020, 5:59 p.m. UTC | #1 
Hi Paolo,

Thank you for looking at that!

On 21/02/2020 17:48, Paolo Abeni wrote:
> Factor out PM addr info to simplify PM data definition and
> simplify the PM hooks/APIs.
> 
> The PM will carry a single local and remote addrs - the one
> currently being processed, if any. It's up to the PM impl,
> e.g. netlink, maintain additional per msk data, if needed.
> 
> Account the number of created subflow and received addresses,
> to enforce limits.
> 
> Add a spinlock to protect PM datas, so that we can manipulate
> them from subflow BH.
> 
> Delegate events handling to a workqueue, so that PM events can
> be processed with both the above spinlock and the msk socket
> lock held. The PM impl should hook inside the worker.
> 
> v1 -> v2:
>   - be sure to initialize the address port before calling
>     mptcp_pm_add_addr()
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>   net/mptcp/options.c  |  46 ++++++++++--------
>   net/mptcp/pm.c       | 112 ++++++++++++++-----------------------------
>   net/mptcp/protocol.c |   2 +
>   net/mptcp/protocol.h |  98 ++++++++++++++++++++++---------------
>   4 files changed, 123 insertions(+), 135 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index c9f508451f2e..08f00f251838 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -8,38 +8,22 @@

[...]

> +void mptcp_pm_data_init(struct mptcp_sock *msk)
> +{
> +	msk->pm.add_addr_signaled = 0;
> +	msk->pm.add_addr_accepted = 0;
> +	msk->pm.local_addr_used = 0;
> +	WRITE_ONCE(msk->pm.work_pending, false);
> +	WRITE_ONCE(msk->pm.addr_signal, false);
> +	WRITE_ONCE(msk->pm.fully_established, false);
> +	WRITE_ONCE(msk->pm.accept_addr, false);
> +	msk->pm.status = MPTCP_PM_IDLE;
Just by curiosity, why do we need to reset those fields to 0? Is this 
msk not already init to 0? Can we not memset the whole pm part?

>   
> -	return 0;
> +	spin_lock_init(&msk->pm.lock);
> +	INIT_WORK(&msk->pm.work, pm_worker);
>   }

[...]

> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 1d658d9aac36..7e43a2a09a68 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h

[...]

> -int mptcp_pm_addr_signal(struct mptcp_sock *msk, u8 *id,
> -			 struct sockaddr_storage *saddr);
> -int mptcp_pm_get_local_id(struct request_sock *req, struct sock *sk,
> -			  const struct sk_buff *skb);
> +bool mptcp_pm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
> +			  struct mptcp_addr_info *saddr);
> +int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);

I guess I will see that in another patch but why "struct sock_common" 
instead of "struc sock"?

Cheers,
Matt
Paolo Abeni Feb. 21, 2020, 6:32 p.m. UTC | #2 
On Fri, 2020-02-21 at 18:59 +0100, Matthieu Baerts wrote:
> Hi Paolo,
> 
> Thank you for looking at that!
> 
> On 21/02/2020 17:48, Paolo Abeni wrote:
> > Factor out PM addr info to simplify PM data definition and
> > simplify the PM hooks/APIs.
> > 
> > The PM will carry a single local and remote addrs - the one
> > currently being processed, if any. It's up to the PM impl,
> > e.g. netlink, maintain additional per msk data, if needed.
> > 
> > Account the number of created subflow and received addresses,
> > to enforce limits.
> > 
> > Add a spinlock to protect PM datas, so that we can manipulate
> > them from subflow BH.
> > 
> > Delegate events handling to a workqueue, so that PM events can
> > be processed with both the above spinlock and the msk socket
> > lock held. The PM impl should hook inside the worker.
> > 
> > v1 -> v2:
> >   - be sure to initialize the address port before calling
> >     mptcp_pm_add_addr()
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> >   net/mptcp/options.c  |  46 ++++++++++--------
> >   net/mptcp/pm.c       | 112 ++++++++++++++-----------------------------
> >   net/mptcp/protocol.c |   2 +
> >   net/mptcp/protocol.h |  98 ++++++++++++++++++++++---------------
> >   4 files changed, 123 insertions(+), 135 deletions(-)
> > 
> > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> > index c9f508451f2e..08f00f251838 100644
> > --- a/net/mptcp/pm.c
> > +++ b/net/mptcp/pm.c
> > @@ -8,38 +8,22 @@
> 
> [...]
> 
> > +void mptcp_pm_data_init(struct mptcp_sock *msk)
> > +{
> > +	msk->pm.add_addr_signaled = 0;
> > +	msk->pm.add_addr_accepted = 0;
> > +	msk->pm.local_addr_used = 0;
> > +	WRITE_ONCE(msk->pm.work_pending, false);
> > +	WRITE_ONCE(msk->pm.addr_signal, false);
> > +	WRITE_ONCE(msk->pm.fully_established, false);
> > +	WRITE_ONCE(msk->pm.accept_addr, false);
> > +	msk->pm.status = MPTCP_PM_IDLE;
> Just by curiosity, why do we need to reset those fields to 0? Is this 
> msk not already init to 0? Can we not memset the whole pm part?

Uhm... we don't need that for sk_alloc(), but we need the above after
mptcp_sk_clone_lock().

memset() could be more efficient than direct assigment if clearing a
wider memory area, see 236222d39347e0e486010f10c1493e83dbbdfba8.

For a bunch of fields we are better off explicitly setting them, I
think.

> > -int mptcp_pm_addr_signal(struct mptcp_sock *msk, u8 *id,
> > -			 struct sockaddr_storage *saddr);
> > -int mptcp_pm_get_local_id(struct request_sock *req, struct sock *sk,
> > -			  const struct sk_buff *skb);
> > +bool mptcp_pm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
> > +			  struct mptcp_addr_info *saddr);
> > +int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
> 
> I guess I will see that in another patch but why "struct sock_common" 
> instead of "struc sock"?

I guess you are already there, anyway mptcp_pm_get_local_id() must be
called on both request_sock and tcp_sock.

Cheers,

Paolo
Matthieu Baerts Feb. 21, 2020, 6:39 p.m. UTC | #3 
Hi Paolo,

On 21/02/2020 19:32, Paolo Abeni wrote:
> On Fri, 2020-02-21 at 18:59 +0100, Matthieu Baerts wrote:
>> Hi Paolo,
>>
>> Thank you for looking at that!
>>
>> On 21/02/2020 17:48, Paolo Abeni wrote:
>>> Factor out PM addr info to simplify PM data definition and
>>> simplify the PM hooks/APIs.
>>>
>>> The PM will carry a single local and remote addrs - the one
>>> currently being processed, if any. It's up to the PM impl,
>>> e.g. netlink, maintain additional per msk data, if needed.
>>>
>>> Account the number of created subflow and received addresses,
>>> to enforce limits.
>>>
>>> Add a spinlock to protect PM datas, so that we can manipulate
>>> them from subflow BH.
>>>
>>> Delegate events handling to a workqueue, so that PM events can
>>> be processed with both the above spinlock and the msk socket
>>> lock held. The PM impl should hook inside the worker.
>>>
>>> v1 -> v2:
>>>    - be sure to initialize the address port before calling
>>>      mptcp_pm_add_addr()
>>>
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>> ---
>>>    net/mptcp/options.c  |  46 ++++++++++--------
>>>    net/mptcp/pm.c       | 112 ++++++++++++++-----------------------------
>>>    net/mptcp/protocol.c |   2 +
>>>    net/mptcp/protocol.h |  98 ++++++++++++++++++++++---------------
>>>    4 files changed, 123 insertions(+), 135 deletions(-)
>>>
>>> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
>>> index c9f508451f2e..08f00f251838 100644
>>> --- a/net/mptcp/pm.c
>>> +++ b/net/mptcp/pm.c
>>> @@ -8,38 +8,22 @@
>>
>> [...]
>>
>>> +void mptcp_pm_data_init(struct mptcp_sock *msk)
>>> +{
>>> +	msk->pm.add_addr_signaled = 0;
>>> +	msk->pm.add_addr_accepted = 0;
>>> +	msk->pm.local_addr_used = 0;
>>> +	WRITE_ONCE(msk->pm.work_pending, false);
>>> +	WRITE_ONCE(msk->pm.addr_signal, false);
>>> +	WRITE_ONCE(msk->pm.fully_established, false);
>>> +	WRITE_ONCE(msk->pm.accept_addr, false);
>>> +	msk->pm.status = MPTCP_PM_IDLE;
>> Just by curiosity, why do we need to reset those fields to 0? Is this
>> msk not already init to 0? Can we not memset the whole pm part?
> 
> Uhm... we don't need that for sk_alloc(), but we need the above after
> mptcp_sk_clone_lock().

Oh OK, thank you for the clarification!

> memset() could be more efficient than direct assigment if clearing a
> wider memory area, see 236222d39347e0e486010f10c1493e83dbbdfba8.

Thank you for the pointer and for the tests you did before :)

> For a bunch of fields we are better off explicitly setting them, I
> think.

Fine for me, as long as we don't forget one!

>>> -int mptcp_pm_addr_signal(struct mptcp_sock *msk, u8 *id,
>>> -			 struct sockaddr_storage *saddr);
>>> -int mptcp_pm_get_local_id(struct request_sock *req, struct sock *sk,
>>> -			  const struct sk_buff *skb);
>>> +bool mptcp_pm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
>>> +			  struct mptcp_addr_info *saddr);
>>> +int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
>>
>> I guess I will see that in another patch but why "struct sock_common"
>> instead of "struc sock"?
> 
> I guess you are already there, anyway mptcp_pm_get_local_id() must be
> called on both request_sock and tcp_sock.

Yes sorry, I was looking in the new PM netlink but the answer was in the 
next patch :)

Cheers,
Matt
diff mbox series

Patch

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 6a2d6cea7d54..08456c8e25b3 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -432,34 +432,30 @@  static bool mptcp_established_options_addr(struct sock *sk,
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
 	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
-	struct sockaddr_storage saddr;
-	u8 id;
+	struct mptcp_addr_info saddr;
+	int len;
 
 	if (!msk)
 		return false;
 
-	if (!msk->pm.fully_established)
+	if (!mptcp_pm_should_signal(msk) ||
+	    !(mptcp_pm_addr_signal(msk, remaining, &saddr)))
 		return false;
 
-	if (mptcp_pm_addr_signal(msk, &id, &saddr))
+	len = mptcp_add_addr_len(saddr.family);
+	if (remaining < len)
 		return false;
 
-	if (saddr.ss_family == AF_INET) {
-		if (remaining < TCPOLEN_MPTCP_ADD_ADDR)
-			return false;
+	*size = len;
+	opts->addr_id = saddr.id;
+	if (saddr.family == AF_INET) {
 		opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
-		opts->addr_id = id;
-		opts->addr = ((struct sockaddr_in *)&saddr)->sin_addr;
-		*size = TCPOLEN_MPTCP_ADD_ADDR;
+		opts->addr = saddr.addr;
 	}
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
-	else if (saddr.ss_family == AF_INET6) {
-		if (remaining < TCPOLEN_MPTCP_ADD_ADDR6)
-			return false;
+	else if (saddr.family == AF_INET6) {
 		opts->suboptions |= OPTION_MPTCP_ADD_ADDR6;
-		opts->addr_id = id;
-		opts->addr6 = ((struct sockaddr_in6 *)&saddr)->sin6_addr;
-		*size = TCPOLEN_MPTCP_ADD_ADDR6;
+		opts->addr6 = saddr.addr6;
 	}
 #endif
 
@@ -557,13 +553,21 @@  void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
 		return;
 
 	if (msk && mp_opt->add_addr) {
-		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4)
-			mptcp_pm_add_addr(msk, &mp_opt->addr, mp_opt->addr_id);
+		struct mptcp_addr_info addr;
+
+		addr.port = 0;
+		addr.id = mp_opt->addr_id;
+		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4) {
+			addr.family = AF_INET;
+			addr.addr = mp_opt->addr;
+		}
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
-		else if (mp_opt->family == MPTCP_ADDR_IPVERSION_6)
-			mptcp_pm_add_addr6(msk, &mp_opt->addr6,
-					   mp_opt->addr_id);
+		else if (mp_opt->family == MPTCP_ADDR_IPVERSION_6) {
+			addr.family = AF_INET6;
+			addr.addr6 = mp_opt->addr6;
+		}
 #endif
+		mptcp_pm_add_addr(msk, &addr);
 		mp_opt->add_addr = 0;
 	}
 
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index c9f508451f2e..08f00f251838 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -8,38 +8,22 @@ 
 #include <net/mptcp.h>
 #include "protocol.h"
 
-/* path manager command handlers */
+static struct workqueue_struct *pm_wq;
 
-int mptcp_pm_announce_addr(u32 token, u8 local_id, struct in_addr *addr)
-{
-	return -ENOTSUPP;
-}
-
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
-int mptcp_pm_announce_addr6(u32 token, u8 local_id, struct in6_addr *addr)
-{
-	return -ENOTSUPP;
-}
-#endif
-
-int mptcp_pm_remove_addr(u32 token, u8 local_id)
-{
-	return -ENOTSUPP;
-}
+/* path manager command handlers */
 
-int mptcp_pm_create_subflow(u32 token, u8 remote_id, struct in_addr *addr)
+int mptcp_pm_announce_addr(struct mptcp_sock *msk,
+			   const struct mptcp_addr_info *addr)
 {
 	return -ENOTSUPP;
 }
 
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
-int mptcp_pm_create_subflow6(u32 token, u8 remote_id, struct in6_addr *addr)
+int mptcp_pm_remove_addr(struct mptcp_sock *msk, u8 local_id)
 {
 	return -ENOTSUPP;
 }
-#endif
 
-int mptcp_pm_remove_subflow(u32 token, u8 remote_id)
+int mptcp_pm_remove_subflow(struct mptcp_sock *msk, u8 remote_id)
 {
 	return -ENOTSUPP;
 }
@@ -50,10 +34,9 @@  void mptcp_pm_new_connection(struct mptcp_sock *msk, int server_side)
 {
 	struct mptcp_pm_data *pm = &msk->pm;
 
-	pr_debug("msk=%p, token=%u", msk, msk->token);
+	pr_debug("msk=%p, token=%u side=%d", msk, msk->token, server_side);
 
-	pm->server_side = server_side;
-	pm->token = msk->token;
+	WRITE_ONCE(pm->server_side, server_side);
 }
 
 void mptcp_pm_fully_established(struct mptcp_sock *msk)
@@ -61,8 +44,6 @@  void mptcp_pm_fully_established(struct mptcp_sock *msk)
 	struct mptcp_pm_data *pm = &msk->pm;
 
 	pr_debug("msk=%p", msk);
-
-	pm->fully_established = 1;
 }
 
 void mptcp_pm_connection_closed(struct mptcp_sock *msk)
@@ -70,7 +51,8 @@  void mptcp_pm_connection_closed(struct mptcp_sock *msk)
 	pr_debug("msk=%p", msk);
 }
 
-void mptcp_pm_subflow_established(struct mptcp_sock *msk, u8 id)
+void mptcp_pm_subflow_established(struct mptcp_sock *msk,
+				  struct mptcp_subflow_context *subflow)
 {
 	pr_debug("msk=%p", msk);
 }
@@ -80,71 +62,49 @@  void mptcp_pm_subflow_closed(struct mptcp_sock *msk, u8 id)
 	pr_debug("msk=%p", msk);
 }
 
-void mptcp_pm_add_addr(struct mptcp_sock *msk, const struct in_addr *addr,
-		       u8 id)
+void mptcp_pm_add_addr(struct mptcp_sock *msk,
+		       const struct mptcp_addr_info *addr)
 {
 	struct mptcp_pm_data *pm = &msk->pm;
 
-	pr_debug("msk=%p, addr=%x, remote_id=%d", msk, addr->s_addr, id);
-
-	pm->remote_addr = *addr;
-	pm->remote_id = id;
-	pm->remote_family = AF_INET;
-	pm->remote_valid = 1;
-}
-
-void mptcp_pm_add_addr6(struct mptcp_sock *msk, const struct in6_addr *addr,
-			u8 id)
-{
-	pr_debug("msk=%p", msk);
+	pr_debug("msk=%p, remote_id=%d", msk, addr->id);
 }
 
 /* path manager helpers */
 
-int mptcp_pm_addr_signal(struct mptcp_sock *msk, u8 *id,
-			 struct sockaddr_storage *saddr)
+bool mptcp_pm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
+			  struct mptcp_addr_info *saddr)
 {
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
-	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)saddr;
-#endif
-	struct sockaddr_in *addr = (struct sockaddr_in *)saddr;
-
-	if (!msk->pm.local_valid)
-		return -1;
-
-	if (msk->pm.local_family == AF_INET) {
-		addr->sin_family = msk->pm.local_family;
-		addr->sin_addr = msk->pm.local_addr;
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
-	} else if (msk->pm.local_family == AF_INET6) {
-		addr6->sin6_family = msk->pm.local_family;
-		addr6->sin6_addr = msk->pm.local_addr6;
-#endif
-	} else {
-		return -1;
-	}
-	*id = msk->pm.local_id;
+	return false;
+}
 
+int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
+{
 	return 0;
 }
 
-int mptcp_pm_get_local_id(struct request_sock *req, struct sock *sk,
-			  const struct sk_buff *skb)
+static void pm_worker(struct work_struct *work)
 {
-	struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
-	struct mptcp_sock *msk = mptcp_sk(sk);
-
-	if (!msk->pm.local_valid)
-		return -1;
-
-	/* @@ check if address actually matches... */
+}
 
-	pr_debug("msk=%p, addr_id=%d", msk, msk->pm.local_id);
-	subflow_req->local_id = msk->pm.local_id;
+void mptcp_pm_data_init(struct mptcp_sock *msk)
+{
+	msk->pm.add_addr_signaled = 0;
+	msk->pm.add_addr_accepted = 0;
+	msk->pm.local_addr_used = 0;
+	WRITE_ONCE(msk->pm.work_pending, false);
+	WRITE_ONCE(msk->pm.addr_signal, false);
+	WRITE_ONCE(msk->pm.fully_established, false);
+	WRITE_ONCE(msk->pm.accept_addr, false);
+	msk->pm.status = MPTCP_PM_IDLE;
 
-	return 0;
+	spin_lock_init(&msk->pm.lock);
+	INIT_WORK(&msk->pm.work, pm_worker);
 }
 
 void mptcp_pm_init(void)
 {
+	pm_wq = alloc_workqueue("pm_wq", WQ_UNBOUND | WQ_MEM_RECLAIM, 8);
+	if (!pm_wq)
+		panic("Failed to allocate workqueue");
 }
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index dfb7fe3352de..fff8f2701385 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -552,6 +552,8 @@  static int __mptcp_init_sock(struct sock *sk)
 
 	msk->first = NULL;
 
+	mptcp_pm_data_init(msk);
+
 	return 0;
 }
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 1d658d9aac36..7e43a2a09a68 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -76,32 +76,47 @@  static inline __be32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field)
 		     ((nib & 0xF) << 8) | field);
 }
 
-struct mptcp_pm_data {
-	u8	local_valid;
-	u8	local_id;
-	sa_family_t local_family;
-	union {
-		struct in_addr local_addr;
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
-		struct in6_addr local_addr6;
-#endif
-	};
-	u8	remote_valid;
-	u8	remote_id;
-	sa_family_t remote_family;
+#define MPTCP_PM_MAX_ADDR	4
+
+struct mptcp_addr_info {
+	sa_family_t		family;
+	__be16			port;
+	u8			id;
 	union {
-		struct in_addr remote_addr;
+		struct in_addr addr;
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
-		struct in6_addr remote_addr6;
+		struct in6_addr addr6;
 #endif
 	};
-	u8	server_side : 1,
-		fully_established : 1;
+};
 
-	/* for interim path manager */
-	struct	work_struct addr_work;
-	struct	work_struct subflow_work;
-	u32	token;
+enum mptcp_pm_status {
+	MPTCP_PM_IDLE,
+	MPTCP_PM_ADD_ADDR,
+	MPTCP_PM_ESTABLISHED,
+	MPTCP_PM_SUBFLOW_ESTABLISHED,
+};
+
+struct mptcp_pm_data {
+	struct mptcp_addr_info local;
+	struct mptcp_addr_info remote;
+
+	spinlock_t	lock;		/*protects the whole PM data */
+
+	bool		addr_signal;
+	bool		server_side;
+	bool		fully_established;
+	bool		work_pending;
+	bool		accept_addr;
+	u8		add_addr_signaled;
+	u8		add_addr_accepted;
+	u8		local_addr_used;
+	u8		add_addr_signal_max;
+	u8		add_addr_accept_max;
+	u8		local_addr_max;
+	enum mptcp_pm_status status;
+
+	struct		work_struct work;
 };
 
 /* MPTCP connection sock */
@@ -273,29 +288,36 @@  void mptcp_crypto_hmac_sha(u64 key1, u64 key2, u32 nonce1, u32 nonce2,
 			   void *hash_out);
 
 void mptcp_pm_init(void);
+void mptcp_pm_data_init(struct mptcp_sock *msk);
 void mptcp_pm_new_connection(struct mptcp_sock *msk, int server_side);
 void mptcp_pm_fully_established(struct mptcp_sock *msk);
 void mptcp_pm_connection_closed(struct mptcp_sock *msk);
-void mptcp_pm_subflow_established(struct mptcp_sock *msk, u8 id);
+void mptcp_pm_subflow_established(struct mptcp_sock *msk,
+				  struct mptcp_subflow_context *subflow);
 void mptcp_pm_subflow_closed(struct mptcp_sock *msk, u8 id);
-void mptcp_pm_add_addr(struct mptcp_sock *msk, const struct in_addr *addr,
-		       u8 id);
-void mptcp_pm_add_addr6(struct mptcp_sock *msk, const struct in6_addr *addr,
-			u8 id);
+void mptcp_pm_add_addr(struct mptcp_sock *msk,
+		       const struct mptcp_addr_info *addr);
 
-int mptcp_pm_announce_addr(u32 token, u8 local_id, struct in_addr *addr);
-int mptcp_pm_create_subflow(u32 token, u8 remote_id, struct in_addr *addr);
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
-int mptcp_pm_announce_addr6(u32 token, u8 local_id, struct in6_addr *addr);
-int mptcp_pm_create_subflow6(u32 token, u8 remote_id, struct in6_addr *addr);
-#endif
-int mptcp_pm_remove_addr(u32 token, u8 local_id);
-int mptcp_pm_remove_subflow(u32 token, u8 remote_id);
+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);
+
+static inline bool mptcp_pm_should_signal(struct mptcp_sock *msk)
+{
+	return READ_ONCE(msk->pm.addr_signal);
+}
+
+static inline unsigned int mptcp_add_addr_len(int family)
+{
+	if (family == AF_INET)
+		return TCPOLEN_MPTCP_ADD_ADDR;
+	return TCPOLEN_MPTCP_ADD_ADDR6;
+}
 
-int mptcp_pm_addr_signal(struct mptcp_sock *msk, u8 *id,
-			 struct sockaddr_storage *saddr);
-int mptcp_pm_get_local_id(struct request_sock *req, struct sock *sk,
-			  const struct sk_buff *skb);
+bool mptcp_pm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
+			  struct mptcp_addr_info *saddr);
+int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
 
 static inline struct mptcp_ext *mptcp_get_ext(struct sk_buff *skb)
 {