All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [MPTCP] [PATCH 2/2] mptcp: Refactor mptcp_established_options() to single hook
@ 2019-06-11 12:59 Matthieu Baerts
  0 siblings, 0 replies; 3+ messages in thread
From: Matthieu Baerts @ 2019-06-11 12:59 UTC (permalink / raw)
  To: mptcp

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

Hi Peter, Paolo, Mat,

On 08/06/2019 02:40, Peter Krystad wrote:
> Make there be only one callout from tcp_output.c for this hook.
> Includes refactoring from the first patch of the MP_JOIN RFC.
> 
> squashto: Write MPTCP DSS headers

Thank you for the new version of the patch and the reviews!

- 98555b04a8d9: "squashed" in "mptcp: Write MPTCP DSS headers to
outgoing data packets"
- da0f223d5698: signed-off
- no conflicts
- 7311dd424a7e..fc816436135c: result

Note that I had to do an extra modification to fix a compilation issue
when CONFIG_MPTCP is not set, see 6e78ea220386 (mptcp: fix compilation
without CONFIG_MPTCP) or below:

> 
> Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
> ---
>  include/net/mptcp.h   | 21 ++++++---------------
>  net/ipv4/tcp_output.c | 20 +++++---------------
>  net/mptcp/options.c   | 40 +++++++++++++++++++++++++++++++---------
>  3 files changed, 42 insertions(+), 39 deletions(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index d84489adbd86..ec3d7f7e5f45 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -57,13 +57,11 @@ void mptcp_parse_option(const unsigned char *ptr, int opsize,
>  bool mptcp_syn_options(struct sock *sk, unsigned int *size,
>  			       struct mptcp_out_options* opts);
>  void mptcp_rcv_synsent(struct sock *sk);
> -bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
> -				   unsigned int *size, unsigned int remaining,
> -				   struct mptcp_out_options* opts);
>  bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
>  			  struct mptcp_out_options *opts);
> -bool mptcp_established_options(struct sock *sk, unsigned int *size,
> -				       struct mptcp_out_options *opts);
> +bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
> +			       unsigned int *size, unsigned int remaining,
> +			       struct mptcp_out_options* opts);
>  
>  void mptcp_attach_dss(struct sock *sk, struct sk_buff *skb,
>  		      struct tcp_options_received *opt_rx);
> @@ -106,15 +104,6 @@ static inline void mptcp_rcv_synsent(struct sock *sk)
>  {
>  }
>  
> -static inline bool mptcp_established_options_dss(struct sock *sk,
> -						 struct sk_buff *skb,
> -						 unsigned int *size,
> -						 unsigned int remaining,
> -						 struct mptcp_out_options* opts)
> -{
> -	return false;
> -}
> -
>  static inline bool mptcp_synack_options(const struct request_sock *req,
>  					unsigned int *size,
>  					struct mptcp_out_options *opts)
> @@ -123,8 +112,10 @@ static inline bool mptcp_synack_options(const struct request_sock *req,
>  }
>  
>  static inline bool mptcp_established_options(struct sock *sk,
> +					     struct sk_buff *skb,
>  					     unsigned int *size,
> -					     struct mptcp_out_options *opts)
> +					     unsigned int remaining,
> +					     struct mptcp_out_options* opts);

I had to remove the semi-colon at the end of this line to fix a
compilation issue.

Please also note that I saw checkpatch.pl forces us to place the star
next to the variable name:

   struct mptcp_out_options *opts

instead of

   struct mptcp_out_options* opts

I will try to fix a maximum of these checkpatch.pl issues once the
MP_JOIN support is added in our repo and before sending patches to Eric.

Cheers,
Matt
-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [MPTCP] [PATCH 2/2] mptcp: Refactor mptcp_established_options() to single hook
@ 2019-06-10 23:24 Mat Martineau
  0 siblings, 0 replies; 3+ messages in thread
From: Mat Martineau @ 2019-06-10 23:24 UTC (permalink / raw)
  To: mptcp

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


On Fri, 7 Jun 2019, Peter Krystad wrote:

> Make there be only one callout from tcp_output.c for this hook.
> Includes refactoring from the first patch of the MP_JOIN RFC.
>
> squashto: Write MPTCP DSS headers
>
> Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
> ---
> include/net/mptcp.h   | 21 ++++++---------------
> net/ipv4/tcp_output.c | 20 +++++---------------
> net/mptcp/options.c   | 40 +++++++++++++++++++++++++++++++---------
> 3 files changed, 42 insertions(+), 39 deletions(-)

...

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index cad1702323a3..a5a6772f1268 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c

...

> @@ -339,6 +338,29 @@ bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
> 	return true;
> }
>
> +bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
> +			       unsigned int *size, unsigned int remaining,
> +			       struct mptcp_out_options *opts)
> +{
> +	unsigned int opt_size = 0;
> +
> +	if (!subflow_ctx(sk)->mp_capable)
> +		return false;
> +
> +	if (mptcp_established_options_mp(sk, &opt_size, remaining, opts)) {
> +		*size += opt_size;
> +		remaining -= opt_size;
> +		return true;
> +	} else if (mptcp_established_options_dss(sk, skb, &opt_size, remaining,
> +					       opts)) {
> +		*size += opt_size;
> +		remaining -= opt_size;
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
> 			  struct mptcp_out_options *opts)
> {

Given that you're thinking of introducing changes in the mp vs. dss code 
blocks above, and have consolidated the mp_capable check, this looks ready 
to merge. Thanks, Peter.

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [MPTCP] [PATCH 2/2] mptcp: Refactor mptcp_established_options() to single hook
@ 2019-06-08  0:40 Peter Krystad
  0 siblings, 0 replies; 3+ messages in thread
From: Peter Krystad @ 2019-06-08  0:40 UTC (permalink / raw)
  To: mptcp

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

Make there be only one callout from tcp_output.c for this hook.
Includes refactoring from the first patch of the MP_JOIN RFC.

squashto: Write MPTCP DSS headers

Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
---
 include/net/mptcp.h   | 21 ++++++---------------
 net/ipv4/tcp_output.c | 20 +++++---------------
 net/mptcp/options.c   | 40 +++++++++++++++++++++++++++++++---------
 3 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index d84489adbd86..ec3d7f7e5f45 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -57,13 +57,11 @@ void mptcp_parse_option(const unsigned char *ptr, int opsize,
 bool mptcp_syn_options(struct sock *sk, unsigned int *size,
 			       struct mptcp_out_options* opts);
 void mptcp_rcv_synsent(struct sock *sk);
-bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
-				   unsigned int *size, unsigned int remaining,
-				   struct mptcp_out_options* opts);
 bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
 			  struct mptcp_out_options *opts);
-bool mptcp_established_options(struct sock *sk, unsigned int *size,
-				       struct mptcp_out_options *opts);
+bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
+			       unsigned int *size, unsigned int remaining,
+			       struct mptcp_out_options* opts);
 
 void mptcp_attach_dss(struct sock *sk, struct sk_buff *skb,
 		      struct tcp_options_received *opt_rx);
@@ -106,15 +104,6 @@ static inline void mptcp_rcv_synsent(struct sock *sk)
 {
 }
 
-static inline bool mptcp_established_options_dss(struct sock *sk,
-						 struct sk_buff *skb,
-						 unsigned int *size,
-						 unsigned int remaining,
-						 struct mptcp_out_options* opts)
-{
-	return false;
-}
-
 static inline bool mptcp_synack_options(const struct request_sock *req,
 					unsigned int *size,
 					struct mptcp_out_options *opts)
@@ -123,8 +112,10 @@ static inline bool mptcp_synack_options(const struct request_sock *req,
 }
 
 static inline bool mptcp_established_options(struct sock *sk,
+					     struct sk_buff *skb,
 					     unsigned int *size,
-					     struct mptcp_out_options *opts)
+					     unsigned int remaining,
+					     struct mptcp_out_options* opts);
 {
 	return false;
 }
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 0b4865c55e4e..08675fd0f070 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -796,22 +796,12 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
 	 */
 	if (sk_is_mptcp(sk)) {
 		unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
-		unsigned int opt_size;
+		unsigned int opt_size = 0;
 
-		if (mptcp_established_options(sk, &opt_size, &opts->mptcp)) {
-			if (remaining >= opt_size) {
-				opts->options |= OPTION_MPTCP;
-				size += opt_size;
-			}
-		} else {
-			unsigned int dss_size;
-
-			if (mptcp_established_options_dss(sk, skb, &dss_size,
-							  remaining,
-							  &opts->mptcp)) {
-				opts->options |= OPTION_MPTCP;
-				size += dss_size;
-			}
+		if (mptcp_established_options(sk, skb, &opt_size, remaining,
+					      &opts->mptcp)) {
+			opts->options |= OPTION_MPTCP;
+			size += opt_size;
 		}
 	}
 
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index cad1702323a3..a5a6772f1268 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -250,12 +250,13 @@ void mptcp_rcv_synsent(struct sock *sk)
 	}
 }
 
-bool mptcp_established_options(struct sock *sk, unsigned int *size,
-			       struct mptcp_out_options *opts)
+static bool mptcp_established_options_mp(struct sock *sk, unsigned int *size,
+					 unsigned int remaining,
+					 struct mptcp_out_options *opts)
 {
 	struct subflow_context *subflow = subflow_ctx(sk);
 
-	if (subflow->mp_capable && !subflow->fourth_ack) {
+	if (!subflow->fourth_ack && remaining >= TCPOLEN_MPTCP_MPC_ACK) {
 		opts->suboptions = OPTION_MPTCP_MPC_ACK;
 		opts->sndr_key = subflow->local_key;
 		opts->rcvr_key = subflow->remote_key;
@@ -268,17 +269,15 @@ bool mptcp_established_options(struct sock *sk, unsigned int *size,
 	return false;
 }
 
-bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
-				   unsigned int *size, unsigned int remaining,
-				   struct mptcp_out_options *opts)
+static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
+					  unsigned int *size,
+					  unsigned int remaining,
+					  struct mptcp_out_options *opts)
 {
 	unsigned int dss_size = 0;
 	struct mptcp_ext *mpext;
 	unsigned int ack_size;
 
-	if (!subflow_ctx(sk)->mp_capable)
-		return false;
-
 	mpext = skb ? mptcp_get_ext(skb) : NULL;
 
 	if (!skb || (mpext && mpext->use_map)) {
@@ -339,6 +338,29 @@ bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
 	return true;
 }
 
+bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
+			       unsigned int *size, unsigned int remaining,
+			       struct mptcp_out_options *opts)
+{
+	unsigned int opt_size = 0;
+
+	if (!subflow_ctx(sk)->mp_capable)
+		return false;
+
+	if (mptcp_established_options_mp(sk, &opt_size, remaining, opts)) {
+		*size += opt_size;
+		remaining -= opt_size;
+		return true;
+	} else if (mptcp_established_options_dss(sk, skb, &opt_size, remaining,
+					       opts)) {
+		*size += opt_size;
+		remaining -= opt_size;
+		return true;
+	}
+
+	return false;
+}
+
 bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
 			  struct mptcp_out_options *opts)
 {
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-06-11 12:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-11 12:59 [MPTCP] [PATCH 2/2] mptcp: Refactor mptcp_established_options() to single hook Matthieu Baerts
  -- strict thread matches above, loose matches on Subject: below --
2019-06-10 23:24 Mat Martineau
2019-06-08  0:40 Peter Krystad

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.