[MPTCP] [PATCH 1/9] Extend path manager interface

[MPTCP] [PATCH 1/9] Extend path manager interface

[Peter Krystad]

[-- Attachment #1: Type: text/plain, Size: 8245 bytes --]

A little more functionality and some comments clarifying
which routines are for a path manager to call and which are
callbacks.

squashto: Add path manager interface

Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
---
 net/mptcp/options.c  | 19 +++++++++--
 net/mptcp/pm.c       | 77 ++++++++++++++++++++++++++++++++++++++++----
 net/mptcp/protocol.c |  5 +--
 net/mptcp/protocol.h | 35 ++++++++++++++++----
 4 files changed, 117 insertions(+), 19 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 58215f19829a..0d0d47f84adb 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -412,10 +412,10 @@ static bool mptcp_established_options_addr(struct sock *sk,
 	struct subflow_context *subflow = subflow_ctx(sk);
 	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
 
-	if (subflow->fourth_ack)
-		return pm_addr_signal(msk, size, remaining, opts);
+	if (!msk)
+		return false;
 
-	return false;
+	return pm_addr_signal(msk, size, remaining, opts);
 }
 
 bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
@@ -479,11 +479,20 @@ bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
 void mptcp_attach_dss(struct sock *sk, struct sk_buff *skb,
 		      struct tcp_options_received *opt_rx)
 {
+	struct subflow_context *subflow = subflow_ctx(sk);
+	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
 	struct mptcp_options_received *mp_opt;
 	struct mptcp_ext *mpext;
 
 	mp_opt = &opt_rx->mptcp;
 
+	if (mp_opt->add_addr && mp_opt->family == MPTCP_ADDR_IPVERSION_4) {
+		if (msk) {
+			pm_add_addr(msk, &mp_opt->addr, mp_opt->addr_id);
+			mp_opt->add_addr = 0;
+		}
+	}
+
 	if (!mp_opt->dss)
 		return;
 
@@ -510,6 +519,10 @@ void mptcp_attach_dss(struct sock *sk, struct sk_buff *skb,
 	}
 
 	mpext->data_fin = mp_opt->data_fin;
+
+	if (msk)
+		pm_fully_established(msk);
+
 }
 
 void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts)
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 9e9c681a4544..0d53a0f23d2d 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -8,14 +8,42 @@
 #include <net/mptcp.h>
 #include "protocol.h"
 
-void pm_new_connection(struct mptcp_sock *msk)
+/* path manager command handlers */
+
+int pm_announce_addr(u32 token, sa_family_t family, u8 id, struct in_addr *addr)
+{
+	return -ENOTSUPP;
+}
+
+int pm_remove_addr(u32 token, u8 id)
+{
+	return -ENOTSUPP;
+}
+
+int pm_create_subflow(u32 token, u8 local_id, u8 remote_id)
+{
+	return -ENOTSUPP;
+}
+
+int pm_remove_subflow(u32 token, u8 local_id, u8 remote_id)
+{
+	return -ENOTSUPP;
+}
+
+/* path manager event handlers */
+
+void pm_new_connection(struct mptcp_sock *msk, int server_side)
 {
 	pr_debug("msk=%p", msk);
+
+	msk->pm.server_side = server_side;
 }
 
 void pm_fully_established(struct mptcp_sock *msk)
 {
 	pr_debug("msk=%p", msk);
+
+	msk->pm.fully_established = 1;
 }
 
 void pm_connection_closed(struct mptcp_sock *msk)
@@ -35,7 +63,13 @@ void pm_subflow_closed(struct mptcp_sock *msk, u8 id)
 
 void pm_add_addr(struct mptcp_sock *msk, const struct in_addr *addr, u8 id)
 {
-	pr_debug("msk=%p", msk);
+	pr_debug("msk=%p, addr=%x, remote_id=%d", msk, addr->s_addr, id);
+
+	msk->pm.remote_valid = 1;
+	msk->pm.remote_token = msk->token;
+	msk->pm.remote_addr.s_addr = addr->s_addr;
+	msk->pm.remote_id = id;
+	msk->pm.remote_family = AF_INET;
 }
 
 void pm_add_addr6(struct mptcp_sock *msk, const struct in6_addr *addr, u8 id)
@@ -48,19 +82,48 @@ void pm_rm_addr(struct mptcp_sock *msk, u8 id)
 	pr_debug("msk=%p", msk);
 }
 
+/* path manager helpers */
+
 bool pm_addr_signal(struct mptcp_sock *msk, unsigned int *size,
 		    unsigned int remaining, struct mptcp_out_options *opts)
 {
-	if (!msk || !msk->addr_signal)
+	if (!msk->pm.fully_established || !msk->addr_signal)
 		return false;
 
-	if (msk->pm.family == AF_INET && remaining < TCPOLEN_MPTCP_ADD_ADDR)
+	if (!msk->pm.local_valid)
 		return false;
 
-	pr_debug("msk=%p", msk);
+	if (msk->pm.local_family == AF_INET &&
+	    remaining < TCPOLEN_MPTCP_ADD_ADDR)
+		return false;
+
+	pr_debug("msk=%p, addr_id=%d", msk, msk->pm.local_id);
 	opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
-	opts->addr_id = msk->pm.addr_id;
-	opts->addr.s_addr = msk->pm.addr.s_addr;
+	opts->addr_id = msk->pm.local_id;
+	opts->addr.s_addr = msk->pm.local_addr.s_addr;
+	*size = TCPOLEN_MPTCP_ADD_ADDR;
+	msk->addr_signal = 0;
 
 	return true;
 }
+
+int pm_get_local_id(struct request_sock *req, struct sock *sk,
+		    const struct sk_buff *skb)
+{
+	struct subflow_request_sock *subflow_req = 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;
+
+	return 0;
+}
+
+void pm_init(void)
+{
+}
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 774ed25d3b6d..a56085742cf7 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -632,7 +632,7 @@ static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
 		token_update_accept(new_sock->sk, new_mptcp_sock);
 		msk->subflow = NULL;
 
-		pm_new_connection(msk);
+		pm_new_connection(msk, 1);
 
 		crypto_key_sha1(msk->remote_key, NULL, &ack_seq);
 		msk->write_seq = subflow->idsn + 1;
@@ -759,7 +759,7 @@ void mptcp_finish_connect(struct sock *sk, int mp_capable)
 		msk->token = subflow->token;
 		pr_debug("msk=%p, token=%u", msk, msk->token);
 
-		pm_new_connection(msk);
+		pm_new_connection(msk, 0);
 
 		crypto_key_sha1(msk->remote_key, NULL, &ack_seq);
 		msk->write_seq = subflow->idsn + 1;
@@ -1008,6 +1008,7 @@ void __init mptcp_init(void)
 	token_init();
 	crypto_init();
 	subflow_init();
+	pm_init();
 
 	if (proto_register(&mptcp_prot, 1) != 0)
 		panic("Failed to register MPTCP proto.\n");
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 7f15f6aab93d..8c04774ea2ec 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -77,15 +77,33 @@ static inline u32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field)
 		     ((nib & 0xF) << 8) | field);
 }
 
-struct pm_data {
-	u8 addr_id;
-	sa_family_t family;
+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_IPV6)
+		struct in6_addr local_addr6;
+#endif
+	};
+	u8	remote_valid;
+	u8	remote_id;
+	sa_family_t remote_family;
 	union {
-		struct in_addr addr;
+		struct in_addr remote_addr;
 #if IS_ENABLED(CONFIG_IPV6)
-		struct in6_addr addr6;
+		struct in6_addr remote_addr6;
 #endif
 	};
+	u32	remote_token;
+	u8	server_side : 1,
+		fully_established : 1;
+
+	/* for interim path manager */
+	struct	work_struct addr_work;
+	struct	work_struct subflow_work;
+	u32	token;
 };
 
 /* MPTCP connection sock */
@@ -99,7 +117,7 @@ struct mptcp_sock {
 	u32		token;
 	struct list_head conn_list;
 	struct socket	*subflow; /* outgoing connect/listener/!mp_capable */
-	struct pm_data	pm;
+	struct mptcp_pm_data	pm;
 	u8		addr_signal;
 };
 
@@ -222,7 +240,8 @@ void crypto_key_sha1(u64 key, u32 *token, u64 *idsn);
 void crypto_hmac_sha1(u64 key1, u64 key2, u32 *hash_out,
 		      int arg_num, ...);
 
-void pm_new_connection(struct mptcp_sock *msk);
+void pm_init(void);
+void pm_new_connection(struct mptcp_sock *msk, int server_side);
 void pm_fully_established(struct mptcp_sock *msk);
 void pm_connection_closed(struct mptcp_sock *msk);
 void pm_subflow_established(struct mptcp_sock *msk, u8 id);
@@ -232,6 +251,8 @@ void pm_add_addr6(struct mptcp_sock *msk, const struct in6_addr *addr, u8 id);
 void pm_rm_addr(struct mptcp_sock *msk, u8 id);
 bool pm_addr_signal(struct mptcp_sock *msk, unsigned int *size,
 		    unsigned int remaining, struct mptcp_out_options *opts);
+int pm_get_local_id(struct request_sock *req, struct sock *sk,
+		    const struct sk_buff *skb);
 
 static inline struct mptcp_ext *mptcp_get_ext(struct sk_buff *skb)
 {
-- 
2.17.2

Re: [MPTCP] [PATCH 1/9] Extend path manager interface

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 2374 bytes --]

Hi,

I have some questions out of sheer ignorance on my side, please see
below ;)

On Wed, 2019-07-17 at 15:12 -0700, Peter Krystad wrote:
> @@ -48,19 +82,48 @@ void pm_rm_addr(struct mptcp_sock *msk, u8 id)
>  	pr_debug("msk=%p", msk);
>  }
>  
> +/* path manager helpers */
> +
>  bool pm_addr_signal(struct mptcp_sock *msk, unsigned int *size,
>  		    unsigned int remaining, struct mptcp_out_options *opts)
>  {
> -	if (!msk || !msk->addr_signal)
> +	if (!msk->pm.fully_established || !msk->addr_signal)
>  		return false;
>  
> -	if (msk->pm.family == AF_INET && remaining < TCPOLEN_MPTCP_ADD_ADDR)
> +	if (!msk->pm.local_valid)
>  		return false;
>  
> -	pr_debug("msk=%p", msk);
> +	if (msk->pm.local_family == AF_INET &&
> +	    remaining < TCPOLEN_MPTCP_ADD_ADDR)
> +		return false;
> +
> +	pr_debug("msk=%p, addr_id=%d", msk, msk->pm.local_id);
>  	opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
> -	opts->addr_id = msk->pm.addr_id;
> -	opts->addr.s_addr = msk->pm.addr.s_addr;
> +	opts->addr_id = msk->pm.local_id;
> +	opts->addr.s_addr = msk->pm.local_addr.s_addr;
> +	*size = TCPOLEN_MPTCP_ADD_ADDR;
> +	msk->addr_signal = 0;

Strictly speaking not related to this patch, but more to the existing
code... perhaps 'opts' updating belong to
options.c/mptcp_established_options_addr() more than the path manager?

> index 7f15f6aab93d..8c04774ea2ec 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -77,15 +77,33 @@ static inline u32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field)
>  		     ((nib & 0xF) << 8) | field);
>  }
>  
> -struct pm_data {
> -	u8 addr_id;
> -	sa_family_t family;
> +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_IPV6)
> +		struct in6_addr local_addr6;
> +#endif
> +	};
> +	u8	remote_valid;
> +	u8	remote_id;
> +	sa_family_t remote_family;
>  	union {
> -		struct in_addr addr;
> +		struct in_addr remote_addr;
>  #if IS_ENABLED(CONFIG_IPV6)
> -		struct in6_addr addr6;
> +		struct in6_addr remote_addr6;
>  #endif
>  	};

Are we going to need a list/container here? (to handle multiples addrs)
I see is likely too early to cope with the extra complexity, just to
have a clearer picture of the road ahead ;)

Cheers,

Paolo

Re: [MPTCP] [PATCH 1/9] Extend path manager interface

[Peter Krystad]

[-- Attachment #1: Type: text/plain, Size: 2776 bytes --]

On Thu, 2019-07-18 at 09:29 +0200, Paolo Abeni wrote:
> Hi,
> 
> I have some questions out of sheer ignorance on my side, please see
> below ;)
> 
> On Wed, 2019-07-17 at 15:12 -0700, Peter Krystad wrote:
> > @@ -48,19 +82,48 @@ void pm_rm_addr(struct mptcp_sock *msk, u8 id)
> >  	pr_debug("msk=%p", msk);
> >  }
> >  
> > +/* path manager helpers */
> > +
> >  bool pm_addr_signal(struct mptcp_sock *msk, unsigned int *size,
> >  		    unsigned int remaining, struct mptcp_out_options *opts)
> >  {
> > -	if (!msk || !msk->addr_signal)
> > +	if (!msk->pm.fully_established || !msk->addr_signal)
> >  		return false;
> >  
> > -	if (msk->pm.family == AF_INET && remaining < TCPOLEN_MPTCP_ADD_ADDR)
> > +	if (!msk->pm.local_valid)
> >  		return false;
> >  
> > -	pr_debug("msk=%p", msk);
> > +	if (msk->pm.local_family == AF_INET &&
> > +	    remaining < TCPOLEN_MPTCP_ADD_ADDR)
> > +		return false;
> > +
> > +	pr_debug("msk=%p, addr_id=%d", msk, msk->pm.local_id);
> >  	opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
> > -	opts->addr_id = msk->pm.addr_id;
> > -	opts->addr.s_addr = msk->pm.addr.s_addr;
> > +	opts->addr_id = msk->pm.local_id;
> > +	opts->addr.s_addr = msk->pm.local_addr.s_addr;
> > +	*size = TCPOLEN_MPTCP_ADD_ADDR;
> > +	msk->addr_signal = 0;
> 
> Strictly speaking not related to this patch, but more to the existing
> code... perhaps 'opts' updating belong to
> options.c/mptcp_established_options_addr() more than the path manager?

Agreed, I'll revise.

> 
> > index 7f15f6aab93d..8c04774ea2ec 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -77,15 +77,33 @@ static inline u32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field)
> >  		     ((nib & 0xF) << 8) | field);
> >  }
> >  
> > -struct pm_data {
> > -	u8 addr_id;
> > -	sa_family_t family;
> > +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_IPV6)
> > +		struct in6_addr local_addr6;
> > +#endif
> > +	};
> > +	u8	remote_valid;
> > +	u8	remote_id;
> > +	sa_family_t remote_family;
> >  	union {
> > -		struct in_addr addr;
> > +		struct in_addr remote_addr;
> >  #if IS_ENABLED(CONFIG_IPV6)
> > -		struct in6_addr addr6;
> > +		struct in6_addr remote_addr6;
> >  #endif
> >  	};
> 
> Are we going to need a list/container here? (to handle multiples addrs)
> I see is likely too early to cope with the extra complexity, just to
> have a clearer picture of the road ahead ;)
> 

Indeed. For now this is a list of length one. :) 

Although you night argue for some of our primary use cases this is probably
enough...

Peter.


> Cheers,
> 
> Paolo
>