All of lore.kernel.org
 help / color / mirror / Atom feed
* [MPTCP] [PATCH 11/18] tcp_extra_option: Pass sock to prepare_req
@ 2017-10-03 16:21 Christoph Paasch
  0 siblings, 0 replies; 3+ messages in thread
From: Christoph Paasch @ 2017-10-03 16:21 UTC (permalink / raw)
  To: mptcp 

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

Will be needed for TCP_MD5.

Signed-off-by: Christoph Paasch <cpaasch(a)apple.com>
---
 include/net/tcp.h     | 6 ++++--
 net/ipv4/tcp.c        | 5 +++--
 net/ipv4/tcp_output.c | 3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index a66671127936..bc3b8f655a43 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2123,7 +2123,8 @@ struct tcp_extra_option_ops {
 	unsigned int (*prepare_req)(struct sk_buff *skb, u8 flags,
 				    unsigned int remaining,
 				    struct tcp_out_options *opts,
-				    struct request_sock *req);
+				    struct request_sock *req,
+				    const struct sock *sk);
 	void (*write)(__be32 *ptr, struct sk_buff *skb,
 		      struct tcp_out_options *opts, struct sock *sk);
 	int (*add_header_len)(const struct sock *listener,
@@ -2144,7 +2145,8 @@ unsigned int tcp_extra_options_prepare(struct sk_buff *skb, u8 flags,
 unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
 					   unsigned int remaining,
 					   struct tcp_out_options *opts,
-					   struct request_sock *req);
+					   struct request_sock *req,
+					   const struct sock *sk);
 
 void tcp_extra_options_write(__be32 *ptr, struct sk_buff *skb,
 			     struct tcp_out_options *opts, struct sock *sk);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 3310e252b325..e6aea011b65d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3433,7 +3433,8 @@ unsigned int tcp_extra_options_prepare(struct sk_buff *skb, u8 flags,
 unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
 					   unsigned int remaining,
 					   struct tcp_out_options *opts,
-					   struct request_sock *req)
+					   struct request_sock *req,
+					   const struct sock *sk)
 {
 	struct tcp_extra_option_ops *entry;
 	unsigned int used = 0;
@@ -3444,7 +3445,7 @@ unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
 			continue;
 
 		used += entry->prepare_req(skb, flags, remaining - used, opts,
-					   req);
+					   req, sk);
 	}
 	rcu_read_unlock();
 
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 522103a43941..1eb00e93c895 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -664,7 +664,8 @@ static unsigned int tcp_synack_options(const struct sock *sk,
 							   TCPHDR_SYN |
 							   TCPHDR_ACK,
 							   remaining, opts,
-							   req);
+							   req,
+							   sk);
 
 	return MAX_TCP_OPTION_SPACE - remaining;
 }
-- 
2.14.1


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

* Re: [MPTCP] [PATCH 11/18] tcp_extra_option: Pass sock to prepare_req
@ 2017-10-05  0:33 Mat Martineau
  0 siblings, 0 replies; 3+ messages in thread
From: Mat Martineau @ 2017-10-05  0:33 UTC (permalink / raw)
  To: mptcp 

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


On Tue, 3 Oct 2017, Christoph Paasch wrote:

> Will be needed for TCP_MD5.

Patches 7-10 look good to me. Some overlap with v2 of patch 6 that I 
posted earlier today, so there will be some conflicts to sort out.


> Signed-off-by: Christoph Paasch <cpaasch(a)apple.com>
> ---
> include/net/tcp.h     | 6 ++++--
> net/ipv4/tcp.c        | 5 +++--
> net/ipv4/tcp_output.c | 3 ++-
> 3 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index a66671127936..bc3b8f655a43 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2123,7 +2123,8 @@ struct tcp_extra_option_ops {
> 	unsigned int (*prepare_req)(struct sk_buff *skb, u8 flags,
> 				    unsigned int remaining,
> 				    struct tcp_out_options *opts,
> -				    struct request_sock *req);
> +				    struct request_sock *req,
> +				    const struct sock *sk);
> 	void (*write)(__be32 *ptr, struct sk_buff *skb,
> 		      struct tcp_out_options *opts, struct sock *sk);
> 	int (*add_header_len)(const struct sock *listener,
> @@ -2144,7 +2145,8 @@ unsigned int tcp_extra_options_prepare(struct sk_buff *skb, u8 flags,
> unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
> 					   unsigned int remaining,
> 					   struct tcp_out_options *opts,
> -					   struct request_sock *req);
> +					   struct request_sock *req,
> +					   const struct sock *sk);

v2 of the option registration patch will cause a problem here, since I 
consolidated the prepare callbacks.

Is it better to keep these callbacks separate to allow for different 
parameters? It might be possible to have one "get_length" callback instead 
of prepare/prepare_req/add_header_len, if we can determine a simple set of 
rules for the callback implementor to follow. This only makes sense if it 
looks like the callbacks are very similar to each other in practice - a 
quick look at the MD5 code makes me think that's the case.


Mat


> void tcp_extra_options_write(__be32 *ptr, struct sk_buff *skb,
> 			     struct tcp_out_options *opts, struct sock *sk);
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 3310e252b325..e6aea011b65d 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3433,7 +3433,8 @@ unsigned int tcp_extra_options_prepare(struct sk_buff *skb, u8 flags,
> unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
> 					   unsigned int remaining,
> 					   struct tcp_out_options *opts,
> -					   struct request_sock *req)
> +					   struct request_sock *req,
> +					   const struct sock *sk)
> {
> 	struct tcp_extra_option_ops *entry;
> 	unsigned int used = 0;
> @@ -3444,7 +3445,7 @@ unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
> 			continue;
>
> 		used += entry->prepare_req(skb, flags, remaining - used, opts,
> -					   req);
> +					   req, sk);
> 	}
> 	rcu_read_unlock();
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 522103a43941..1eb00e93c895 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -664,7 +664,8 @@ static unsigned int tcp_synack_options(const struct sock *sk,
> 							   TCPHDR_SYN |
> 							   TCPHDR_ACK,
> 							   remaining, opts,
> -							   req);
> +							   req,
> +							   sk);
>
> 	return MAX_TCP_OPTION_SPACE - remaining;
> }
> -- 
> 2.14.1
>
>

--
Mat Martineau
Intel OTC

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

* Re: [MPTCP] [PATCH 11/18] tcp_extra_option: Pass sock to prepare_req
@ 2017-10-05  7:46 Christoph Paasch
  0 siblings, 0 replies; 3+ messages in thread
From: Christoph Paasch @ 2017-10-05  7:46 UTC (permalink / raw)
  To: mptcp 

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

On 04/10/17 - 17:33:24, Mat Martineau wrote:
> 
> On Tue, 3 Oct 2017, Christoph Paasch wrote:
> 
> > Will be needed for TCP_MD5.
> 
> Patches 7-10 look good to me. Some overlap with v2 of patch 6 that I posted
> earlier today, so there will be some conflicts to sort out.

Ok, if 7-10 are fine, I will squash them into your patch so that we have a
single one for the extra options.

> 
> 
> > Signed-off-by: Christoph Paasch <cpaasch(a)apple.com>
> > ---
> > include/net/tcp.h     | 6 ++++--
> > net/ipv4/tcp.c        | 5 +++--
> > net/ipv4/tcp_output.c | 3 ++-
> > 3 files changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index a66671127936..bc3b8f655a43 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -2123,7 +2123,8 @@ struct tcp_extra_option_ops {
> > 	unsigned int (*prepare_req)(struct sk_buff *skb, u8 flags,
> > 				    unsigned int remaining,
> > 				    struct tcp_out_options *opts,
> > -				    struct request_sock *req);
> > +				    struct request_sock *req,
> > +				    const struct sock *sk);
> > 	void (*write)(__be32 *ptr, struct sk_buff *skb,
> > 		      struct tcp_out_options *opts, struct sock *sk);
> > 	int (*add_header_len)(const struct sock *listener,
> > @@ -2144,7 +2145,8 @@ unsigned int tcp_extra_options_prepare(struct sk_buff *skb, u8 flags,
> > unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
> > 					   unsigned int remaining,
> > 					   struct tcp_out_options *opts,
> > -					   struct request_sock *req);
> > +					   struct request_sock *req,
> > +					   const struct sock *sk);
> 
> v2 of the option registration patch will cause a problem here, since I
> consolidated the prepare callbacks.
> 
> Is it better to keep these callbacks separate to allow for different
> parameters? It might be possible to have one "get_length" callback instead
> of prepare/prepare_req/add_header_len, if we can determine a simple set of
> rules for the callback implementor to follow. This only makes sense if it
> looks like the callbacks are very similar to each other in practice - a
> quick look at the MD5 code makes me think that's the case.

prepare and prepare_req are fine to merge IMO.

add_header_len has quite different semantics in my opinion in that it gets
called during connect() and tcp_create_openreq_child(). And is rather static.
For example, for MPTCP prepare and add_header_len would be quite different.


Christoph

> 
> 
> Mat
> 
> 
> > void tcp_extra_options_write(__be32 *ptr, struct sk_buff *skb,
> > 			     struct tcp_out_options *opts, struct sock *sk);
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index 3310e252b325..e6aea011b65d 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -3433,7 +3433,8 @@ unsigned int tcp_extra_options_prepare(struct sk_buff *skb, u8 flags,
> > unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
> > 					   unsigned int remaining,
> > 					   struct tcp_out_options *opts,
> > -					   struct request_sock *req)
> > +					   struct request_sock *req,
> > +					   const struct sock *sk)
> > {
> > 	struct tcp_extra_option_ops *entry;
> > 	unsigned int used = 0;
> > @@ -3444,7 +3445,7 @@ unsigned int tcp_extra_options_prepare_req(struct sk_buff *skb, u8 flags,
> > 			continue;
> > 
> > 		used += entry->prepare_req(skb, flags, remaining - used, opts,
> > -					   req);
> > +					   req, sk);
> > 	}
> > 	rcu_read_unlock();
> > 
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index 522103a43941..1eb00e93c895 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -664,7 +664,8 @@ static unsigned int tcp_synack_options(const struct sock *sk,
> > 							   TCPHDR_SYN |
> > 							   TCPHDR_ACK,
> > 							   remaining, opts,
> > -							   req);
> > +							   req,
> > +							   sk);
> > 
> > 	return MAX_TCP_OPTION_SPACE - remaining;
> > }
> > -- 
> > 2.14.1
> > 
> > 
> 
> --
> Mat Martineau
> Intel OTC

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

end of thread, other threads:[~2017-10-05  7:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-05  7:46 [MPTCP] [PATCH 11/18] tcp_extra_option: Pass sock to prepare_req Christoph Paasch
  -- strict thread matches above, loose matches on Subject: below --
2017-10-05  0:33 Mat Martineau
2017-10-03 16:21 Christoph Paasch

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.