[PATCH net-next 0/3] tcp: tcp_stream_alloc_skb() changes

[PATCH net-next 0/3] tcp: tcp_stream_alloc_skb() changes

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

sk_stream_alloc_skb() is only used by TCP.

Rename it to tcp_stream_alloc_skb() and apply small
optimizations.

Eric Dumazet (3):
  tcp: rename sk_stream_alloc_skb
  tcp: use MAX_TCP_HEADER in tcp_stream_alloc_skb
  tcp: remove unneeded code from tcp_stream_alloc_skb()

 include/net/sock.h    |  3 ---
 include/net/tcp.h     |  2 ++
 net/ipv4/tcp.c        | 19 ++++++++-----------
 net/ipv4/tcp_output.c | 10 +++++-----
 4 files changed, 15 insertions(+), 19 deletions(-)

-- 
2.33.0.1079.g6e70778dc9-goog

[PATCH net-next 1/3] tcp: rename sk_stream_alloc_skb

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

sk_stream_alloc_skb() is only used by TCP.

Rename it to make this clear, and move its declaration
to include/net/tcp.h

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/sock.h    |  3 ---
 include/net/tcp.h     |  2 ++
 net/ipv4/tcp.c        | 12 ++++++------
 net/ipv4/tcp_output.c | 10 +++++-----
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index b76be30674efc88434ed45d46b9c4600261b6271..ff4e62aa62e51a68d086e9e2b8429cba5731be00 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2422,9 +2422,6 @@ static inline void sk_stream_moderate_sndbuf(struct sock *sk)
 	WRITE_ONCE(sk->sk_sndbuf, max_t(u32, val, SOCK_MIN_SNDBUF));
 }
 
-struct sk_buff *sk_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
-				    bool force_schedule);
-
 /**
  * sk_page_frag - return an appropriate page_frag
  * @sk: socket
diff --git a/include/net/tcp.h b/include/net/tcp.h
index d62467a0094fe016ee2f5d9581631e1425e8f201..701587af685296a6b2372fee7b3e94f998c3bbe8 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -337,6 +337,8 @@ void tcp_twsk_destructor(struct sock *sk);
 ssize_t tcp_splice_read(struct socket *sk, loff_t *ppos,
 			struct pipe_inode_info *pipe, size_t len,
 			unsigned int flags);
+struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
+				     bool force_schedule);
 
 void tcp_enter_quickack_mode(struct sock *sk, unsigned int max_quickacks);
 static inline void tcp_dec_quickack_mode(struct sock *sk,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c2d9830136d298a27abc12a5633bf77d1224759c..68dd580dba3d0e04412466868135c49225a4a33b 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -856,8 +856,8 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 }
 EXPORT_SYMBOL(tcp_splice_read);
 
-struct sk_buff *sk_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
-				    bool force_schedule)
+struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
+				     bool force_schedule)
 {
 	struct sk_buff *skb;
 
@@ -960,8 +960,8 @@ static struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
 		if (!sk_stream_memory_free(sk))
 			return NULL;
 
-		skb = sk_stream_alloc_skb(sk, 0, sk->sk_allocation,
-					  tcp_rtx_and_write_queues_empty(sk));
+		skb = tcp_stream_alloc_skb(sk, 0, sk->sk_allocation,
+					   tcp_rtx_and_write_queues_empty(sk));
 		if (!skb)
 			return NULL;
 
@@ -1289,8 +1289,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 					goto restart;
 			}
 			first_skb = tcp_rtx_and_write_queues_empty(sk);
-			skb = sk_stream_alloc_skb(sk, 0, sk->sk_allocation,
-						  first_skb);
+			skb = tcp_stream_alloc_skb(sk, 0, sk->sk_allocation,
+						   first_skb);
 			if (!skb)
 				goto wait_for_space;
 
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 3a01e5593a171d8e8978c11c9880eb9314feeda9..c0c55a8be8f79857e176714f240fddcb0580fa6b 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1564,7 +1564,7 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
 		return -ENOMEM;
 
 	/* Get a new skb... force flag on. */
-	buff = sk_stream_alloc_skb(sk, nsize, gfp, true);
+	buff = tcp_stream_alloc_skb(sk, nsize, gfp, true);
 	if (!buff)
 		return -ENOMEM; /* We'll just try again later. */
 	skb_copy_decrypted(buff, skb);
@@ -2121,7 +2121,7 @@ static int tso_fragment(struct sock *sk, struct sk_buff *skb, unsigned int len,
 		return tcp_fragment(sk, TCP_FRAG_IN_WRITE_QUEUE,
 				    skb, len, mss_now, gfp);
 
-	buff = sk_stream_alloc_skb(sk, 0, gfp, true);
+	buff = tcp_stream_alloc_skb(sk, 0, gfp, true);
 	if (unlikely(!buff))
 		return -ENOMEM;
 	skb_copy_decrypted(buff, skb);
@@ -2388,7 +2388,7 @@ static int tcp_mtu_probe(struct sock *sk)
 		return -1;
 
 	/* We're allowed to probe.  Build it now. */
-	nskb = sk_stream_alloc_skb(sk, probe_size, GFP_ATOMIC, false);
+	nskb = tcp_stream_alloc_skb(sk, probe_size, GFP_ATOMIC, false);
 	if (!nskb)
 		return -1;
 	sk_wmem_queued_add(sk, nskb->truesize);
@@ -3754,7 +3754,7 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
 	/* limit to order-0 allocations */
 	space = min_t(size_t, space, SKB_MAX_HEAD(MAX_TCP_HEADER));
 
-	syn_data = sk_stream_alloc_skb(sk, space, sk->sk_allocation, false);
+	syn_data = tcp_stream_alloc_skb(sk, space, sk->sk_allocation, false);
 	if (!syn_data)
 		goto fallback;
 	syn_data->ip_summed = CHECKSUM_PARTIAL;
@@ -3835,7 +3835,7 @@ int tcp_connect(struct sock *sk)
 		return 0;
 	}
 
-	buff = sk_stream_alloc_skb(sk, 0, sk->sk_allocation, true);
+	buff = tcp_stream_alloc_skb(sk, 0, sk->sk_allocation, true);
 	if (unlikely(!buff))
 		return -ENOBUFS;
 
-- 
2.33.0.1079.g6e70778dc9-goog

[PATCH net-next 2/3] tcp: use MAX_TCP_HEADER in tcp_stream_alloc_skb

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Both IPv4 and IPv6 uses same reserve, no need risking
cache line misses to fetch its value.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 68dd580dba3d0e04412466868135c49225a4a33b..121400557fde898283a8eae3b09d93479c4a089e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -867,7 +867,7 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
 	if (unlikely(tcp_under_memory_pressure(sk)))
 		sk_mem_reclaim_partial(sk);
 
-	skb = alloc_skb_fclone(size + sk->sk_prot->max_header, gfp);
+	skb = alloc_skb_fclone(size + MAX_TCP_HEADER, gfp);
 	if (likely(skb)) {
 		bool mem_scheduled;
 
@@ -878,7 +878,7 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
 			mem_scheduled = sk_wmem_schedule(sk, skb->truesize);
 		}
 		if (likely(mem_scheduled)) {
-			skb_reserve(skb, sk->sk_prot->max_header);
+			skb_reserve(skb, MAX_TCP_HEADER);
 			/*
 			 * Make sure that we have exactly size bytes
 			 * available to the caller, no more, no less.
-- 
2.33.0.1079.g6e70778dc9-goog

[PATCH net-next 3/3] tcp: remove unneeded code from tcp_stream_alloc_skb()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Aligning @size argument to 4 bytes is not needed.

The header alignment has nothing to do with @size.

It really depends on skb->head alignment and MAX_TCP_HEADER.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 121400557fde898283a8eae3b09d93479c4a089e..0a27b7ef1a9db8aea8d98cff4b8ab7092994febd 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -861,9 +861,6 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
 {
 	struct sk_buff *skb;
 
-	/* The TCP header must be at least 32-bit aligned.  */
-	size = ALIGN(size, 4);
-
 	if (unlikely(tcp_under_memory_pressure(sk)))
 		sk_mem_reclaim_partial(sk);
 
-- 
2.33.0.1079.g6e70778dc9-goog

Re: [PATCH net-next 2/3] tcp: use MAX_TCP_HEADER in tcp_stream_alloc_skb

[Soheil Hassas Yeganeh]

On Mon, Oct 25, 2021 at 6:13 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> Both IPv4 and IPv6 uses same reserve, no need risking
> cache line misses to fetch its value.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

> ---
>  net/ipv4/tcp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 68dd580dba3d0e04412466868135c49225a4a33b..121400557fde898283a8eae3b09d93479c4a089e 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -867,7 +867,7 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
>         if (unlikely(tcp_under_memory_pressure(sk)))
>                 sk_mem_reclaim_partial(sk);
>
> -       skb = alloc_skb_fclone(size + sk->sk_prot->max_header, gfp);
> +       skb = alloc_skb_fclone(size + MAX_TCP_HEADER, gfp);
>         if (likely(skb)) {
>                 bool mem_scheduled;
>
> @@ -878,7 +878,7 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
>                         mem_scheduled = sk_wmem_schedule(sk, skb->truesize);
>                 }
>                 if (likely(mem_scheduled)) {
> -                       skb_reserve(skb, sk->sk_prot->max_header);
> +                       skb_reserve(skb, MAX_TCP_HEADER);
>                         /*
>                          * Make sure that we have exactly size bytes
>                          * available to the caller, no more, no less.
> --
> 2.33.0.1079.g6e70778dc9-goog
>

Re: [PATCH net-next 3/3] tcp: remove unneeded code from tcp_stream_alloc_skb()

[Soheil Hassas Yeganeh]

On Mon, Oct 25, 2021 at 6:13 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> Aligning @size argument to 4 bytes is not needed.
>
> The header alignment has nothing to do with @size.
>
> It really depends on skb->head alignment and MAX_TCP_HEADER.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

> ---
>  net/ipv4/tcp.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 121400557fde898283a8eae3b09d93479c4a089e..0a27b7ef1a9db8aea8d98cff4b8ab7092994febd 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -861,9 +861,6 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
>  {
>         struct sk_buff *skb;
>
> -       /* The TCP header must be at least 32-bit aligned.  */
> -       size = ALIGN(size, 4);
> -
>         if (unlikely(tcp_under_memory_pressure(sk)))
>                 sk_mem_reclaim_partial(sk);
>
> --
> 2.33.0.1079.g6e70778dc9-goog
>

Re: [PATCH net-next 1/3] tcp: rename sk_stream_alloc_skb

[Soheil Hassas Yeganeh]

On Mon, Oct 25, 2021 at 6:13 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> sk_stream_alloc_skb() is only used by TCP.
>
> Rename it to make this clear, and move its declaration
> to include/net/tcp.h
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

> ---
>  include/net/sock.h    |  3 ---
>  include/net/tcp.h     |  2 ++
>  net/ipv4/tcp.c        | 12 ++++++------
>  net/ipv4/tcp_output.c | 10 +++++-----
>  4 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index b76be30674efc88434ed45d46b9c4600261b6271..ff4e62aa62e51a68d086e9e2b8429cba5731be00 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2422,9 +2422,6 @@ static inline void sk_stream_moderate_sndbuf(struct sock *sk)
>         WRITE_ONCE(sk->sk_sndbuf, max_t(u32, val, SOCK_MIN_SNDBUF));
>  }
>
> -struct sk_buff *sk_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
> -                                   bool force_schedule);
> -
>  /**
>   * sk_page_frag - return an appropriate page_frag
>   * @sk: socket
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index d62467a0094fe016ee2f5d9581631e1425e8f201..701587af685296a6b2372fee7b3e94f998c3bbe8 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -337,6 +337,8 @@ void tcp_twsk_destructor(struct sock *sk);
>  ssize_t tcp_splice_read(struct socket *sk, loff_t *ppos,
>                         struct pipe_inode_info *pipe, size_t len,
>                         unsigned int flags);
> +struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
> +                                    bool force_schedule);
>
>  void tcp_enter_quickack_mode(struct sock *sk, unsigned int max_quickacks);
>  static inline void tcp_dec_quickack_mode(struct sock *sk,
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index c2d9830136d298a27abc12a5633bf77d1224759c..68dd580dba3d0e04412466868135c49225a4a33b 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -856,8 +856,8 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
>  }
>  EXPORT_SYMBOL(tcp_splice_read);
>
> -struct sk_buff *sk_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
> -                                   bool force_schedule)
> +struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp,
> +                                    bool force_schedule)
>  {
>         struct sk_buff *skb;
>
> @@ -960,8 +960,8 @@ static struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
>                 if (!sk_stream_memory_free(sk))
>                         return NULL;
>
> -               skb = sk_stream_alloc_skb(sk, 0, sk->sk_allocation,
> -                                         tcp_rtx_and_write_queues_empty(sk));
> +               skb = tcp_stream_alloc_skb(sk, 0, sk->sk_allocation,
> +                                          tcp_rtx_and_write_queues_empty(sk));
>                 if (!skb)
>                         return NULL;
>
> @@ -1289,8 +1289,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
>                                         goto restart;
>                         }
>                         first_skb = tcp_rtx_and_write_queues_empty(sk);
> -                       skb = sk_stream_alloc_skb(sk, 0, sk->sk_allocation,
> -                                                 first_skb);
> +                       skb = tcp_stream_alloc_skb(sk, 0, sk->sk_allocation,
> +                                                  first_skb);
>                         if (!skb)
>                                 goto wait_for_space;
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 3a01e5593a171d8e8978c11c9880eb9314feeda9..c0c55a8be8f79857e176714f240fddcb0580fa6b 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1564,7 +1564,7 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
>                 return -ENOMEM;
>
>         /* Get a new skb... force flag on. */
> -       buff = sk_stream_alloc_skb(sk, nsize, gfp, true);
> +       buff = tcp_stream_alloc_skb(sk, nsize, gfp, true);
>         if (!buff)
>                 return -ENOMEM; /* We'll just try again later. */
>         skb_copy_decrypted(buff, skb);
> @@ -2121,7 +2121,7 @@ static int tso_fragment(struct sock *sk, struct sk_buff *skb, unsigned int len,
>                 return tcp_fragment(sk, TCP_FRAG_IN_WRITE_QUEUE,
>                                     skb, len, mss_now, gfp);
>
> -       buff = sk_stream_alloc_skb(sk, 0, gfp, true);
> +       buff = tcp_stream_alloc_skb(sk, 0, gfp, true);
>         if (unlikely(!buff))
>                 return -ENOMEM;
>         skb_copy_decrypted(buff, skb);
> @@ -2388,7 +2388,7 @@ static int tcp_mtu_probe(struct sock *sk)
>                 return -1;
>
>         /* We're allowed to probe.  Build it now. */
> -       nskb = sk_stream_alloc_skb(sk, probe_size, GFP_ATOMIC, false);
> +       nskb = tcp_stream_alloc_skb(sk, probe_size, GFP_ATOMIC, false);
>         if (!nskb)
>                 return -1;
>         sk_wmem_queued_add(sk, nskb->truesize);
> @@ -3754,7 +3754,7 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
>         /* limit to order-0 allocations */
>         space = min_t(size_t, space, SKB_MAX_HEAD(MAX_TCP_HEADER));
>
> -       syn_data = sk_stream_alloc_skb(sk, space, sk->sk_allocation, false);
> +       syn_data = tcp_stream_alloc_skb(sk, space, sk->sk_allocation, false);
>         if (!syn_data)
>                 goto fallback;
>         syn_data->ip_summed = CHECKSUM_PARTIAL;
> @@ -3835,7 +3835,7 @@ int tcp_connect(struct sock *sk)
>                 return 0;
>         }
>
> -       buff = sk_stream_alloc_skb(sk, 0, sk->sk_allocation, true);
> +       buff = tcp_stream_alloc_skb(sk, 0, sk->sk_allocation, true);
>         if (unlikely(!buff))
>                 return -ENOBUFS;
>
> --
> 2.33.0.1079.g6e70778dc9-goog
>

Re: [PATCH net-next 0/3] tcp: tcp_stream_alloc_skb() changes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon, 25 Oct 2021 15:13:39 -0700 you wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> sk_stream_alloc_skb() is only used by TCP.
> 
> Rename it to tcp_stream_alloc_skb() and apply small
> optimizations.
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] tcp: rename sk_stream_alloc_skb
    https://git.kernel.org/netdev/net-next/c/f8dd3b8d7020
  - [net-next,2/3] tcp: use MAX_TCP_HEADER in tcp_stream_alloc_skb
    https://git.kernel.org/netdev/net-next/c/8a794df69300
  - [net-next,3/3] tcp: remove unneeded code from tcp_stream_alloc_skb()
    https://git.kernel.org/netdev/net-next/c/c4322884ed21

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html