Skip to contentSkip to conversation

generic net and ipv6 minor optimisations

PATCH Applied Pavel Begunkov 2 participants 7 messages Apr 28, 2022 – Apr 30, 2022

Subsystems (4)
Networking Network drivers IPv4/IPv6 Sockets
lore
Share & tools
lore

Pavel Begunkov

author
#0 lore raw

cover generic net and ipv6 minor optimisations

1-3 inline simple functions that only reshuffle arguments possibly adding extra zero args, and call another function. It was benchmarked before with a bunch of extra patches, see for details

https://lore.kernel.org/netdev/[email protected]/

It may increase the binary size, but it's the right thing to do and at least without modules it actually sheds some bytes for some standard-ish config.

text data bss dec hex filename
9627200 0 0 9627200 92e640 ./arch/x86_64/boot/bzImage
text data bss dec hex filename
9627104 0 0 9627104 92e5e0 ./arch/x86_64/boot/bzImage

Pavel Begunkov (5):
net: inline sock_alloc_send_skb
net: inline skb_zerocopy_iter_dgram
net: inline dev_queue_xmit()
ipv6: help __ip6_finish_output() inlining
ipv6: refactor ip6_finish_output2()

 include/linux/netdevice.h | 14 ++++++++++++--
 include/linux/skbuff.h    | 36 ++++++++++++++++++++++--------------
 include/net/sock.h        | 10 ++++++++--
 net/core/datagram.c       |  2 --
 net/core/datagram.h       | 15 ---------------
 net/core/dev.c            | 15 ++-------------
 net/core/skbuff.c         |  7 -------
 net/core/sock.c           |  7 -------
 net/ipv6/ip6_output.c     | 25 +++++++++++++------------
 9 files changed, 57 insertions(+), 74 deletions(-)
 delete mode 100644 net/core/datagram.h
Signature · 1 line
2.36.0
author
#1 lore raw

PATCH 1/5 net: inline sock_alloc_send_skb

sock_alloc_send_skb() is simple and just proxying to another function, so we can inline it and cut associated overhead.

Signed-off-by Pavel Begunkov <[email protected]>
---
 include/net/sock.h | 10 ++++++++--
 net/core/sock.c    |  7 -------
 2 files changed, 8 insertions(+), 9 deletions(-)
include/net/sock.h +8 -2
index a01d6c421aa2..df2e826f67ee 100644
@@ -1824,11 +1824,17 @@ int sock_getsockopt(struct socket *sock, int level, int op,
 		    char __user *optval, int __user *optlen);
 int sock_gettstamp(struct socket *sock, void __user *userstamp,
 		   bool timeval, bool time32);
-struct sk_buff *sock_alloc_send_skb(struct sock *sk, unsigned long size,
-				    int noblock, int *errcode);
 struct sk_buff *sock_alloc_send_pskb(struct sock *sk, unsigned long header_len,
 				     unsigned long data_len, int noblock,
 				     int *errcode, int max_page_order);
+
+static inline struct sk_buff *sock_alloc_send_skb(struct sock *sk,
+						  unsigned long size,
+						  int noblock, int *errcode)
+{
+	return sock_alloc_send_pskb(sk, size, 0, noblock, errcode, 0);
+}
+
 void *sock_kmalloc(struct sock *sk, int size, gfp_t priority);
 void sock_kfree_s(struct sock *sk, void *mem, int size);
 void sock_kzfree_s(struct sock *sk, void *mem, int size);
net/core/sock.c +0 -7
index 29abec3eabd8..fbca35d3749a 100644
@@ -2631,13 +2631,6 @@ struct sk_buff *sock_alloc_send_pskb(struct sock *sk, unsigned long header_len,
 }
 EXPORT_SYMBOL(sock_alloc_send_pskb);
 
-struct sk_buff *sock_alloc_send_skb(struct sock *sk, unsigned long size,
-				    int noblock, int *errcode)
-{
-	return sock_alloc_send_pskb(sk, size, 0, noblock, errcode, 0);
-}
-EXPORT_SYMBOL(sock_alloc_send_skb);
-
 int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,
 		     struct sockcm_cookie *sockc)
 {
Signature · 1 line
2.36.0
author
#2 lore raw

PATCH 2/5 net: inline skb_zerocopy_iter_dgram

In reply to Pavel Begunkov

skb_zerocopy_iter_dgram() is a small proxy function, inline it. For that, move __zerocopy_sg_from_iter into linux/skbuff.h

Signed-off-by Pavel Begunkov <[email protected]>
---
 include/linux/skbuff.h | 36 ++++++++++++++++++++++--------------
 net/core/datagram.c    |  2 --
 net/core/datagram.h    | 15 ---------------
 net/core/skbuff.c      |  7 -------
 4 files changed, 22 insertions(+), 38 deletions(-)
 delete mode 100644 net/core/datagram.h
include/linux/skbuff.h +22 -14
index 84d78df60453..57182947cc80 100644
@@ -684,20 +684,6 @@ struct ubuf_info {
 int mm_account_pinned_pages(struct mmpin *mmp, size_t size);
 void mm_unaccount_pinned_pages(struct mmpin *mmp);
 
-struct ubuf_info *msg_zerocopy_alloc(struct sock *sk, size_t size);
-struct ubuf_info *msg_zerocopy_realloc(struct sock *sk, size_t size,
-				       struct ubuf_info *uarg);
-
-void msg_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref);
-
-void msg_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *uarg,
-			   bool success);
-
-int skb_zerocopy_iter_dgram(struct sk_buff *skb, struct msghdr *msg, int len);
-int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
-			     struct msghdr *msg, int len,
-			     struct ubuf_info *uarg);
-
 /* This data is invariant across clones and lives at
  * the end of the header data, ie. at skb->end.
  */

@@ -1676,6 +1662,28 @@ static inline void skb_set_end_offset(struct sk_buff *skb, unsigned int offset)
 }
 #endif
 
+struct ubuf_info *msg_zerocopy_alloc(struct sock *sk, size_t size);
+struct ubuf_info *msg_zerocopy_realloc(struct sock *sk, size_t size,
+				       struct ubuf_info *uarg);
+
+void msg_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref);
+
+void msg_zerocopy_callback(struct sk_buff *skb, struct ubuf_info *uarg,
+			   bool success);
+
+int __zerocopy_sg_from_iter(struct sock *sk, struct sk_buff *skb,
+			    struct iov_iter *from, size_t length);
+
+static inline int skb_zerocopy_iter_dgram(struct sk_buff *skb,
+					  struct msghdr *msg, int len)
+{
+	return __zerocopy_sg_from_iter(skb->sk, skb, &msg->msg_iter, len);
+}
+
+int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
+			     struct msghdr *msg, int len,
+			     struct ubuf_info *uarg);
+
 /* Internal */
 #define skb_shinfo(SKB)	((struct skb_shared_info *)(skb_end_pointer(SKB)))
net/core/datagram.c +0 -2
index 70126d15ca6e..50f4faeea76c 100644
@@ -62,8 +62,6 @@
 #include <trace/events/skb.h>
 #include <net/busy_poll.h>
 
-#include "datagram.h"
-
 /*
  *	Is a socket 'connection oriented' ?
  */
net/core/datagram.h deleted +0 -15
deleted file mode 100644
index bcfb75bfa3b2..000000000000
@@ -1,15 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-#ifndef _NET_CORE_DATAGRAM_H_
-#define _NET_CORE_DATAGRAM_H_
-
-#include <linux/types.h>
-
-struct sock;
-struct sk_buff;
-struct iov_iter;
-
-int __zerocopy_sg_from_iter(struct sock *sk, struct sk_buff *skb,
-			    struct iov_iter *from, size_t length);
-
-#endif /* _NET_CORE_DATAGRAM_H_ */
net/core/skbuff.c +0 -7
index 30b523fa4ad2..384c6098a5f8 100644
@@ -80,7 +80,6 @@
 #include <linux/user_namespace.h>
 #include <linux/indirect_call_wrapper.h>
 
-#include "datagram.h"
 #include "sock_destructor.h"
 
 struct kmem_cache *skbuff_head_cache __ro_after_init;

@@ -1339,12 +1338,6 @@ void msg_zerocopy_put_abort(struct ubuf_info *uarg, bool have_uref)
 }
 EXPORT_SYMBOL_GPL(msg_zerocopy_put_abort);
 
-int skb_zerocopy_iter_dgram(struct sk_buff *skb, struct msghdr *msg, int len)
-{
-	return __zerocopy_sg_from_iter(skb->sk, skb, &msg->msg_iter, len);
-}
-EXPORT_SYMBOL_GPL(skb_zerocopy_iter_dgram);
-
 int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
 			     struct msghdr *msg, int len,
 			     struct ubuf_info *uarg)
Signature · 1 line
2.36.0
author
#3 lore raw

PATCH 3/5 net: inline dev_queue_xmit()

In reply to Pavel Begunkov

Inline dev_queue_xmit() and dev_queue_xmit_accel(), they both are small proxy functions doing nothing but redirecting the control flow to __dev_queue_xmit().

Signed-off-by Pavel Begunkov <[email protected]>
---
 include/linux/netdevice.h | 14 ++++++++++++--
 net/core/dev.c            | 15 ++-------------
 2 files changed, 14 insertions(+), 15 deletions(-)
include/linux/netdevice.h +12 -2
index 7dccbfd1bf56..3c3bd3381d38 100644
@@ -2940,10 +2940,20 @@ u16 dev_pick_tx_zero(struct net_device *dev, struct sk_buff *skb,
 u16 dev_pick_tx_cpu_id(struct net_device *dev, struct sk_buff *skb,
 		       struct net_device *sb_dev);
 
-int dev_queue_xmit(struct sk_buff *skb);
-int dev_queue_xmit_accel(struct sk_buff *skb, struct net_device *sb_dev);
+int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev);
 int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id);
 
+static inline int dev_queue_xmit(struct sk_buff *skb)
+{
+	return __dev_queue_xmit(skb, NULL);
+}
+
+static inline int dev_queue_xmit_accel(struct sk_buff *skb,
+				       struct net_device *sb_dev)
+{
+	return __dev_queue_xmit(skb, sb_dev);
+}
+
 static inline int dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
 {
 	int ret;
net/core/dev.c +2 -13
index 4a77ebda4fb1..d160c35858df 100644
@@ -4111,7 +4111,7 @@ struct netdev_queue *netdev_core_pick_tx(struct net_device *dev,
  *      the BH enable code must have IRQs enabled so that it will not deadlock.
  *          --BLG
  */
-static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
+int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 {
 	struct net_device *dev = skb->dev;
 	struct netdev_queue *txq = NULL;

@@ -4235,18 +4235,7 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 	rcu_read_unlock_bh();
 	return rc;
 }
-
-int dev_queue_xmit(struct sk_buff *skb)
-{
-	return __dev_queue_xmit(skb, NULL);
-}
-EXPORT_SYMBOL(dev_queue_xmit);
-
-int dev_queue_xmit_accel(struct sk_buff *skb, struct net_device *sb_dev)
-{
-	return __dev_queue_xmit(skb, sb_dev);
-}
-EXPORT_SYMBOL(dev_queue_xmit_accel);
+EXPORT_SYMBOL(__dev_queue_xmit);
 
 int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
 {
Signature · 1 line
2.36.0
author
#4 lore raw

PATCH 4/5 ipv6: help __ip6_finish_output() inlining

In reply to Pavel Begunkov

There are two callers of __ip6_finish_output(), both are in ip6_finish_output(). We can combine the call sites into one and handle return code after, that will inline __ip6_finish_output().

Note, error handling under NET_XMIT_CN will only return 0 if __ip6_finish_output() succeded, and in this case it return 0. Considering that NET_XMIT_SUCCESS is 0, it'll be returning exactly the same result for it as before.

Signed-off-by Pavel Begunkov <[email protected]>
---
 net/ipv6/ip6_output.c | 1 -
 1 file changed, 1 deletion(-)
net/ipv6/ip6_output.c +0 -1
index 1f3d777e7694..bda1d9f76f7e 100644
@@ -198,7 +198,6 @@ static int ip6_finish_output(struct net *net, struct sock *sk, struct sk_buff *s
 	ret = BPF_CGROUP_RUN_PROG_INET_EGRESS(sk, skb);
 	switch (ret) {
 	case NET_XMIT_SUCCESS:
-		return __ip6_finish_output(net, sk, skb);
 	case NET_XMIT_CN:
 		return __ip6_finish_output(net, sk, skb) ? : ret;
 	default:
Signature · 1 line
2.36.0
author
#5 lore raw

PATCH 5/5 ipv6: refactor ip6_finish_output2()

In reply to Pavel Begunkov

Throw neigh checks in ip6_finish_output2() under a single slow path if, so we don't have the overhead in the hot path.

Signed-off-by Pavel Begunkov <[email protected]>
---
 net/ipv6/ip6_output.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
net/ipv6/ip6_output.c +13 -11
index bda1d9f76f7e..afa5bd4ad167 100644
@@ -119,19 +119,21 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
 	rcu_read_lock_bh();
 	nexthop = rt6_nexthop((struct rt6_info *)dst, daddr);
 	neigh = __ipv6_neigh_lookup_noref(dev, nexthop);
-	if (unlikely(!neigh))
-		neigh = __neigh_create(&nd_tbl, nexthop, dev, false);
-	if (!IS_ERR(neigh)) {
-		sock_confirm_neigh(skb, neigh);
-		ret = neigh_output(neigh, skb, false);
-		rcu_read_unlock_bh();
-		return ret;
+
+	if (unlikely(IS_ERR_OR_NULL(neigh))) {
+		if (unlikely(!neigh))
+			neigh = __neigh_create(&nd_tbl, nexthop, dev, false);
+		if (IS_ERR(neigh)) {
+			rcu_read_unlock_bh();
+			IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTNOROUTES);
+			kfree_skb_reason(skb, SKB_DROP_REASON_NEIGH_CREATEFAIL);
+			return -EINVAL;
+		}
 	}
+	sock_confirm_neigh(skb, neigh);
+	ret = neigh_output(neigh, skb, false);
 	rcu_read_unlock_bh();
-
-	IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTNOROUTES);
-	kfree_skb_reason(skb, SKB_DROP_REASON_NEIGH_CREATEFAIL);
-	return -EINVAL;
+	return ret;
 }
 
 static int
Signature · 1 line
2.36.0
CI Applied
#6 lore raw
In reply to Pavel Begunkov
This series was applied to netdev/net-next.git (master) by David S. Miller <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (master) by David S. Miller <[email protected]>:

Quoted 10 lines · On Thu, 28 Apr 2022 11:58:43 +0100 you wrote:

1-3 inline simple functions that only reshuffle arguments possibly adding extra zero args, and call another function. It was benchmarked before with a bunch of extra patches, see for details

https://lore.kernel.org/netdev/[email protected]/

It may increase the binary size, but it's the right thing to do and at least without modules it actually sheds some bytes for some standard-ish config.

[...]

Here is the summary with links:
- [net-next,1/5] net: inline sock_alloc_send_skb

    https://git.kernel.org/netdev/net-next/c/de32bc6aad09
  - [net-next,2/5] net: inline skb_zerocopy_iter_dgram
    https://git.kernel.org/netdev/net-next/c/657dd5f97b2e
  - [net-next,3/5] net: inline dev_queue_xmit()
    https://git.kernel.org/netdev/net-next/c/c526fd8f9f4f
  - [net-next,4/5] ipv6: help __ip6_finish_output() inlining
    https://git.kernel.org/netdev/net-next/c/4b143ed7dde5
  - [net-next,5/5] ipv6: refactor ip6_finish_output2()
    https://git.kernel.org/netdev/net-next/c/58f71be58b87

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html