[PATCH net-next 0/2] net: introduce and use route hint

[PATCH net-next 0/2] net: introduce and use route hint

[Paolo Abeni]

This series leverages the listification infrastructure to avoid
unnecessary route lookup on ingress packets. In absence of policy routing,
packets with equal daddr will usually land on the same dst.

When processing packet bursts (lists) we can easily reference the previous
dst entry. When we hit the 'same destination' condition we can avoid the
route lookup, coping the already available dst.

Detailed performance numbers are available in the individual commit messages.

Paolo Abeni (2):
  ipv6: introduce and uses route look hints for list input
  ipv4: use dst hint for ipv4 list receive

 include/net/route.h  | 11 +++++++++++
 net/ipv4/ip_input.c  | 29 ++++++++++++++++++++++++-----
 net/ipv4/route.c     | 38 ++++++++++++++++++++++++++++++++++++++
 net/ipv6/ip6_input.c | 30 ++++++++++++++++++++++++++----
 4 files changed, 99 insertions(+), 9 deletions(-)

-- 
2.21.0

[PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input

[Paolo Abeni]

When doing RX batch packet processing, we currently always repeat
the route lookup for each ingress packet. If policy routing is
configured, and IPV6_SUBTREES is disabled at build time, we
know that packets with the same destination address will use
the same dst.

This change tries to avoid per packet route lookup caching
the destination address of the latest successful lookup, and
reusing it for the next packet when the above conditions are
in place. Ingress traffic for most servers should fit.

The measured performance delta under UDP flood vs a recvmmsg
receiver is as follow:

vanilla		patched		delta
Kpps		Kpps		%
1431		1664		+14

In the worst-case scenario - each packet has a different
destination address - the performance delta is within noise
range

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/ipv6/ip6_input.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index ef7f707d9ae3..b7b947a9ddc9 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -44,10 +44,16 @@
 #include <net/inet_ecn.h>
 #include <net/dst_metadata.h>
 
+struct ip6_route_input_hint {
+	unsigned long	refdst;
+	struct in6_addr daddr;
+};
+
 INDIRECT_CALLABLE_DECLARE(void udp_v6_early_demux(struct sk_buff *));
 INDIRECT_CALLABLE_DECLARE(void tcp_v6_early_demux(struct sk_buff *));
 static void ip6_rcv_finish_core(struct net *net, struct sock *sk,
-				struct sk_buff *skb)
+				struct sk_buff *skb,
+				struct ip6_route_input_hint *hint)
 {
 	void (*edemux)(struct sk_buff *skb);
 
@@ -59,7 +65,13 @@ static void ip6_rcv_finish_core(struct net *net, struct sock *sk,
 			INDIRECT_CALL_2(edemux, tcp_v6_early_demux,
 					udp_v6_early_demux, skb);
 	}
-	if (!skb_valid_dst(skb))
+
+	if (skb_valid_dst(skb))
+		return;
+
+	if (hint && ipv6_addr_equal(&hint->daddr, &ipv6_hdr(skb)->daddr))
+		__skb_dst_copy(skb, hint->refdst);
+	else
 		ip6_route_input(skb);
 }
 
@@ -71,7 +83,7 @@ int ip6_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 	skb = l3mdev_ip6_rcv(skb);
 	if (!skb)
 		return NET_RX_SUCCESS;
-	ip6_rcv_finish_core(net, sk, skb);
+	ip6_rcv_finish_core(net, sk, skb, NULL);
 
 	return dst_input(skb);
 }
@@ -89,6 +101,7 @@ static void ip6_sublist_rcv_finish(struct list_head *head)
 static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
 				struct list_head *head)
 {
+	struct ip6_route_input_hint _hint, *hint = NULL;
 	struct dst_entry *curr_dst = NULL;
 	struct sk_buff *skb, *next;
 	struct list_head sublist;
@@ -104,9 +117,18 @@ static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
 		skb = l3mdev_ip6_rcv(skb);
 		if (!skb)
 			continue;
-		ip6_rcv_finish_core(net, sk, skb);
+		ip6_rcv_finish_core(net, sk, skb, hint);
 		dst = skb_dst(skb);
 		if (curr_dst != dst) {
+#ifndef CONFIG_IPV6_SUBTREES
+			if (!net->ipv6.fib6_has_custom_rules) {
+				_hint.refdst = skb->_skb_refdst;
+				memcpy(&_hint.daddr, &ipv6_hdr(skb)->daddr,
+				       sizeof(_hint.daddr));
+				hint = &_hint;
+			}
+#endif
+
 			/* dispatch old sublist */
 			if (!list_empty(&sublist))
 				ip6_sublist_rcv_finish(&sublist);
-- 
2.21.0

Re: [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input

[kbuild test robot]

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

Hi Paolo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on v5.4-rc7 next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/net-introduce-and-use-route-hint/20191116-172108
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 20021578ba226bda1f0ddf50e4d4a12ea1c6c6c1
config: i386-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/ipv6/ip6_input.c: In function 'ip6_list_rcv_finish':
>> net/ipv6/ip6_input.c:124:18: error: 'struct netns_ipv6' has no member named 'fib6_has_custom_rules'
       if (!net->ipv6.fib6_has_custom_rules) {
                     ^

vim +124 net/ipv6/ip6_input.c

   100	
   101	static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
   102					struct list_head *head)
   103	{
   104		struct ip6_route_input_hint _hint, *hint = NULL;
   105		struct dst_entry *curr_dst = NULL;
   106		struct sk_buff *skb, *next;
   107		struct list_head sublist;
   108	
   109		INIT_LIST_HEAD(&sublist);
   110		list_for_each_entry_safe(skb, next, head, list) {
   111			struct dst_entry *dst;
   112	
   113			skb_list_del_init(skb);
   114			/* if ingress device is enslaved to an L3 master device pass the
   115			 * skb to its handler for processing
   116			 */
   117			skb = l3mdev_ip6_rcv(skb);
   118			if (!skb)
   119				continue;
   120			ip6_rcv_finish_core(net, sk, skb, hint);
   121			dst = skb_dst(skb);
   122			if (curr_dst != dst) {
   123	#ifndef CONFIG_IPV6_SUBTREES
 > 124				if (!net->ipv6.fib6_has_custom_rules) {
   125					_hint.refdst = skb->_skb_refdst;
   126					memcpy(&_hint.daddr, &ipv6_hdr(skb)->daddr,
   127					       sizeof(_hint.daddr));
   128					hint = &_hint;
   129				}
   130	#endif
   131	
   132				/* dispatch old sublist */
   133				if (!list_empty(&sublist))
   134					ip6_sublist_rcv_finish(&sublist);
   135				/* start new sublist */
   136				INIT_LIST_HEAD(&sublist);
   137				curr_dst = dst;
   138			}
   139			list_add_tail(&skb->list, &sublist);
   140		}
   141		/* dispatch final sublist */
   142		ip6_sublist_rcv_finish(&sublist);
   143	}
   144	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28138 bytes --]

Re: [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input

[kbuild test robot]

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

Hi Paolo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v5.4-rc7 next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/net-introduce-and-use-route-hint/20191116-172108
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 20021578ba226bda1f0ddf50e4d4a12ea1c6c6c1
config: i386-randconfig-h003-20191117 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/export.h:42:0,
                    from include/linux/linkage.h:7,
                    from include/linux/kernel.h:8,
                    from include/linux/uio.h:8,
                    from include/linux/socket.h:8,
                    from net/ipv6/ip6_input.c:20:
   net/ipv6/ip6_input.c: In function 'ip6_list_rcv_finish':
   net/ipv6/ip6_input.c:124:18: error: 'struct netns_ipv6' has no member named 'fib6_has_custom_rules'
       if (!net->ipv6.fib6_has_custom_rules) {
                     ^
   include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                       ^~~~
>> net/ipv6/ip6_input.c:124:4: note: in expansion of macro 'if'
       if (!net->ipv6.fib6_has_custom_rules) {
       ^~
   net/ipv6/ip6_input.c:124:18: error: 'struct netns_ipv6' has no member named 'fib6_has_custom_rules'
       if (!net->ipv6.fib6_has_custom_rules) {
                     ^
   include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                                ^~~~
>> net/ipv6/ip6_input.c:124:4: note: in expansion of macro 'if'
       if (!net->ipv6.fib6_has_custom_rules) {
       ^~
   net/ipv6/ip6_input.c:124:18: error: 'struct netns_ipv6' has no member named 'fib6_has_custom_rules'
       if (!net->ipv6.fib6_has_custom_rules) {
                     ^
   include/linux/compiler.h:69:3: note: in definition of macro '__trace_if_value'
     (cond) ?     \
      ^~~~
   include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
    #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                               ^~~~~~~~~~~~~~
>> net/ipv6/ip6_input.c:124:4: note: in expansion of macro 'if'
       if (!net->ipv6.fib6_has_custom_rules) {
       ^~

vim +/if +124 net/ipv6/ip6_input.c

  > 20	#include <linux/socket.h>
    21	#include <linux/sockios.h>
    22	#include <linux/net.h>
    23	#include <linux/netdevice.h>
    24	#include <linux/in6.h>
    25	#include <linux/icmpv6.h>
    26	#include <linux/mroute6.h>
    27	#include <linux/slab.h>
    28	#include <linux/indirect_call_wrapper.h>
    29	
    30	#include <linux/netfilter.h>
    31	#include <linux/netfilter_ipv6.h>
    32	
    33	#include <net/sock.h>
    34	#include <net/snmp.h>
    35	
    36	#include <net/ipv6.h>
    37	#include <net/protocol.h>
    38	#include <net/transp_v6.h>
    39	#include <net/rawv6.h>
    40	#include <net/ndisc.h>
    41	#include <net/ip6_route.h>
    42	#include <net/addrconf.h>
    43	#include <net/xfrm.h>
    44	#include <net/inet_ecn.h>
    45	#include <net/dst_metadata.h>
    46	
    47	struct ip6_route_input_hint {
    48		unsigned long	refdst;
    49		struct in6_addr daddr;
    50	};
    51	
    52	INDIRECT_CALLABLE_DECLARE(void udp_v6_early_demux(struct sk_buff *));
    53	INDIRECT_CALLABLE_DECLARE(void tcp_v6_early_demux(struct sk_buff *));
    54	static void ip6_rcv_finish_core(struct net *net, struct sock *sk,
    55					struct sk_buff *skb,
    56					struct ip6_route_input_hint *hint)
    57	{
    58		void (*edemux)(struct sk_buff *skb);
    59	
    60		if (net->ipv4.sysctl_ip_early_demux && !skb_dst(skb) && skb->sk == NULL) {
    61			const struct inet6_protocol *ipprot;
    62	
    63			ipprot = rcu_dereference(inet6_protos[ipv6_hdr(skb)->nexthdr]);
    64			if (ipprot && (edemux = READ_ONCE(ipprot->early_demux)))
    65				INDIRECT_CALL_2(edemux, tcp_v6_early_demux,
    66						udp_v6_early_demux, skb);
    67		}
    68	
    69		if (skb_valid_dst(skb))
    70			return;
    71	
    72		if (hint && ipv6_addr_equal(&hint->daddr, &ipv6_hdr(skb)->daddr))
    73			__skb_dst_copy(skb, hint->refdst);
    74		else
    75			ip6_route_input(skb);
    76	}
    77	
    78	int ip6_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
    79	{
    80		/* if ingress device is enslaved to an L3 master device pass the
    81		 * skb to its handler for processing
    82		 */
    83		skb = l3mdev_ip6_rcv(skb);
    84		if (!skb)
    85			return NET_RX_SUCCESS;
    86		ip6_rcv_finish_core(net, sk, skb, NULL);
    87	
    88		return dst_input(skb);
    89	}
    90	
    91	static void ip6_sublist_rcv_finish(struct list_head *head)
    92	{
    93		struct sk_buff *skb, *next;
    94	
    95		list_for_each_entry_safe(skb, next, head, list) {
    96			skb_list_del_init(skb);
    97			dst_input(skb);
    98		}
    99	}
   100	
   101	static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
   102					struct list_head *head)
   103	{
   104		struct ip6_route_input_hint _hint, *hint = NULL;
   105		struct dst_entry *curr_dst = NULL;
   106		struct sk_buff *skb, *next;
   107		struct list_head sublist;
   108	
   109		INIT_LIST_HEAD(&sublist);
   110		list_for_each_entry_safe(skb, next, head, list) {
   111			struct dst_entry *dst;
   112	
   113			skb_list_del_init(skb);
   114			/* if ingress device is enslaved to an L3 master device pass the
   115			 * skb to its handler for processing
   116			 */
   117			skb = l3mdev_ip6_rcv(skb);
   118			if (!skb)
   119				continue;
   120			ip6_rcv_finish_core(net, sk, skb, hint);
   121			dst = skb_dst(skb);
   122			if (curr_dst != dst) {
   123	#ifndef CONFIG_IPV6_SUBTREES
 > 124				if (!net->ipv6.fib6_has_custom_rules) {
   125					_hint.refdst = skb->_skb_refdst;
   126					memcpy(&_hint.daddr, &ipv6_hdr(skb)->daddr,
   127					       sizeof(_hint.daddr));
   128					hint = &_hint;
   129				}
   130	#endif
   131	
   132				/* dispatch old sublist */
   133				if (!list_empty(&sublist))
   134					ip6_sublist_rcv_finish(&sublist);
   135				/* start new sublist */
   136				INIT_LIST_HEAD(&sublist);
   137				curr_dst = dst;
   138			}
   139			list_add_tail(&skb->list, &sublist);
   140		}
   141		/* dispatch final sublist */
   142		ip6_sublist_rcv_finish(&sublist);
   143	}
   144	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31708 bytes --]

[PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive

[Paolo Abeni]

This is alike the previous change, with some additional ipv4 specific
quirk. Even when using the route hint we still have to do perform
additional per packet checks about source address validity: a new
helper is added to wrap them.

Moreover, the ipv4 route lookup, even in the absence of policy routing,
may depend on pkts ToS, so we cache that values, too.

Explicitly avoid hints for local broadcast: this simplify the code
and broadcasts are slower path anyway.

UDP flood performances vs recvmmsg() receiver:

vanilla		patched		delta
Kpps		Kpps		%
1683		1833		+8

In the worst case scenario - each packet has a different
destination address - the performance delta is within noise
range.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/net/route.h | 11 +++++++++++
 net/ipv4/ip_input.c | 29 ++++++++++++++++++++++++-----
 net/ipv4/route.c    | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/include/net/route.h b/include/net/route.h
index 6c516840380d..f7a8a52318cd 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -185,6 +185,17 @@ int ip_route_input_rcu(struct sk_buff *skb, __be32 dst, __be32 src,
 		       u8 tos, struct net_device *devin,
 		       struct fib_result *res);
 
+struct ip_route_input_hint {
+	unsigned long	refdst;
+	__be32		daddr;
+	char		tos;
+	bool		local;
+};
+
+int ip_route_use_hint(struct sk_buff *skb, __be32 dst, __be32 src,
+		      u8 tos, struct net_device *devin,
+		      struct ip_route_input_hint *hint);
+
 static inline int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
 				 u8 tos, struct net_device *devin)
 {
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 24a95126e698..78fd60bf1c8a 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -305,7 +305,8 @@ static inline bool ip_rcv_options(struct sk_buff *skb, struct net_device *dev)
 INDIRECT_CALLABLE_DECLARE(int udp_v4_early_demux(struct sk_buff *));
 INDIRECT_CALLABLE_DECLARE(int tcp_v4_early_demux(struct sk_buff *));
 static int ip_rcv_finish_core(struct net *net, struct sock *sk,
-			      struct sk_buff *skb, struct net_device *dev)
+			      struct sk_buff *skb, struct net_device *dev,
+			      struct ip_route_input_hint *hint)
 {
 	const struct iphdr *iph = ip_hdr(skb);
 	int (*edemux)(struct sk_buff *skb);
@@ -335,8 +336,12 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
 	 *	how the packet travels inside Linux networking.
 	 */
 	if (!skb_valid_dst(skb)) {
-		err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
-					   iph->tos, dev);
+		if (hint && hint->daddr == iph->daddr && hint->tos == iph->tos)
+			err = ip_route_use_hint(skb, iph->daddr, iph->saddr,
+						iph->tos, dev, hint);
+		else
+			err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
+						   iph->tos, dev);
 		if (unlikely(err))
 			goto drop_error;
 	}
@@ -408,7 +413,7 @@ static int ip_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 	if (!skb)
 		return NET_RX_SUCCESS;
 
-	ret = ip_rcv_finish_core(net, sk, skb, dev);
+	ret = ip_rcv_finish_core(net, sk, skb, dev, NULL);
 	if (ret != NET_RX_DROP)
 		ret = dst_input(skb);
 	return ret;
@@ -538,6 +543,7 @@ static void ip_sublist_rcv_finish(struct list_head *head)
 static void ip_list_rcv_finish(struct net *net, struct sock *sk,
 			       struct list_head *head)
 {
+	struct ip_route_input_hint _hint, *hint = NULL;
 	struct dst_entry *curr_dst = NULL;
 	struct sk_buff *skb, *next;
 	struct list_head sublist;
@@ -554,11 +560,24 @@ static void ip_list_rcv_finish(struct net *net, struct sock *sk,
 		skb = l3mdev_ip_rcv(skb);
 		if (!skb)
 			continue;
-		if (ip_rcv_finish_core(net, sk, skb, dev) == NET_RX_DROP)
+		if (ip_rcv_finish_core(net, sk, skb, dev, hint) == NET_RX_DROP)
 			continue;
 
 		dst = skb_dst(skb);
 		if (curr_dst != dst) {
+			struct rtable *rt = (struct rtable *)dst;
+
+			if (!net->ipv4.fib_has_custom_rules &&
+			    rt->rt_type != RTN_BROADCAST) {
+				_hint.refdst = skb->_skb_refdst;
+				_hint.daddr = ip_hdr(skb)->daddr;
+				_hint.tos = ip_hdr(skb)->tos;
+				_hint.local = rt->rt_type == RTN_LOCAL;
+				hint = &_hint;
+			} else {
+				hint = NULL;
+			}
+
 			/* dispatch old sublist */
 			if (!list_empty(&sublist))
 				ip_sublist_rcv_finish(&sublist);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index dcc4fa10138d..b0ddff17db80 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2019,6 +2019,44 @@ static int ip_mkroute_input(struct sk_buff *skb,
 	return __mkroute_input(skb, res, in_dev, daddr, saddr, tos);
 }
 
+/* Implements all the saddr-related checks as ip_route_input_slow(),
+ * assuming daddr is valid and this is not a local broadcast.
+ * Uses the provided hint instead of performing a route lookup.
+ */
+int ip_route_use_hint(struct sk_buff *skb, __be32 daddr, __be32 saddr,
+		      u8 tos, struct net_device *dev,
+		      struct ip_route_input_hint *hint)
+{
+	struct in_device *in_dev = __in_dev_get_rcu(dev);
+	struct net *net = dev_net(dev);
+	int err = -EINVAL;
+	u32 itag = 0;
+
+	if (ipv4_is_multicast(saddr) || ipv4_is_lbcast(saddr))
+		goto martian_source;
+
+	if (ipv4_is_zeronet(saddr))
+		goto martian_source;
+
+	if (ipv4_is_loopback(saddr) && !IN_DEV_NET_ROUTE_LOCALNET(in_dev, net))
+		goto martian_source;
+
+	if (hint->local) {
+		err = fib_validate_source(skb, saddr, daddr, tos, 0, dev,
+					  in_dev, &itag);
+		if (err < 0)
+			goto martian_source;
+	}
+
+	err = 0;
+	__skb_dst_copy(skb, hint->refdst);
+	return err;
+
+martian_source:
+	ip_handle_martian_source(dev, in_dev, skb, daddr, saddr);
+	return err;
+}
+
 /*
  *	NOTE. We drop all the packets that has local source
  *	addresses, because every properly looped back packet
-- 
2.21.0

Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive

[kbuild test robot]

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

Hi Paolo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on v5.4-rc7 next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/net-introduce-and-use-route-hint/20191116-172108
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 20021578ba226bda1f0ddf50e4d4a12ea1c6c6c1
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from ./arch/powerpc/include/generated/asm/local64.h:1:0,
                    from include/linux/u64_stats_sync.h:72,
                    from include/net/snmp.h:47,
                    from include/net/netns/mib.h:5,
                    from include/net/net_namespace.h:17,
                    from include/linux/inet.h:42,
                    from net/ipv4/ip_input.c:122:
   include/linux/u64_stats_sync.h: In function 'u64_stats_read':
   include/asm-generic/local64.h:30:37: warning: passing argument 1 of 'local_read' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
    #define local64_read(l)  local_read(&(l)->a)
                                        ^
   include/linux/u64_stats_sync.h:80:9: note: in expansion of macro 'local64_read'
     return local64_read(&p->v);
            ^~~~~~~~~~~~
   In file included from include/asm-generic/local64.h:22:0,
                    from ./arch/powerpc/include/generated/asm/local64.h:1,
                    from include/linux/u64_stats_sync.h:72,
                    from include/net/snmp.h:47,
                    from include/net/netns/mib.h:5,
                    from include/net/net_namespace.h:17,
                    from include/linux/inet.h:42,
                    from net/ipv4/ip_input.c:122:
   arch/powerpc/include/asm/local.h:20:24: note: expected 'local_t * {aka struct <anonymous> *}' but argument is of type 'const local_t * {aka const struct <anonymous> *}'
    static __inline__ long local_read(local_t *l)
                           ^~~~~~~~~~
   net/ipv4/ip_input.c: In function 'ip_list_rcv_finish':
>> net/ipv4/ip_input.c:570:19: error: 'struct netns_ipv4' has no member named 'fib_has_custom_rules'; did you mean 'fib_has_custom_local_routes'?
       if (!net->ipv4.fib_has_custom_rules &&
                      ^~~~~~~~~~~~~~~~~~~~
                      fib_has_custom_local_routes

vim +570 net/ipv4/ip_input.c

   542	
   543	static void ip_list_rcv_finish(struct net *net, struct sock *sk,
   544				       struct list_head *head)
   545	{
   546		struct ip_route_input_hint _hint, *hint = NULL;
   547		struct dst_entry *curr_dst = NULL;
   548		struct sk_buff *skb, *next;
   549		struct list_head sublist;
   550	
   551		INIT_LIST_HEAD(&sublist);
   552		list_for_each_entry_safe(skb, next, head, list) {
   553			struct net_device *dev = skb->dev;
   554			struct dst_entry *dst;
   555	
   556			skb_list_del_init(skb);
   557			/* if ingress device is enslaved to an L3 master device pass the
   558			 * skb to its handler for processing
   559			 */
   560			skb = l3mdev_ip_rcv(skb);
   561			if (!skb)
   562				continue;
   563			if (ip_rcv_finish_core(net, sk, skb, dev, hint) == NET_RX_DROP)
   564				continue;
   565	
   566			dst = skb_dst(skb);
   567			if (curr_dst != dst) {
   568				struct rtable *rt = (struct rtable *)dst;
   569	
 > 570				if (!net->ipv4.fib_has_custom_rules &&
   571				    rt->rt_type != RTN_BROADCAST) {
   572					_hint.refdst = skb->_skb_refdst;
   573					_hint.daddr = ip_hdr(skb)->daddr;
   574					_hint.tos = ip_hdr(skb)->tos;
   575					_hint.local = rt->rt_type == RTN_LOCAL;
   576					hint = &_hint;
   577				} else {
   578					hint = NULL;
   579				}
   580	
   581				/* dispatch old sublist */
   582				if (!list_empty(&sublist))
   583					ip_sublist_rcv_finish(&sublist);
   584				/* start new sublist */
   585				INIT_LIST_HEAD(&sublist);
   586				curr_dst = dst;
   587			}
   588			list_add_tail(&skb->list, &sublist);
   589		}
   590		/* dispatch final sublist */
   591		ip_sublist_rcv_finish(&sublist);
   592	}
   593	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25413 bytes --]

Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive

[kbuild test robot]

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

Hi Paolo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v5.4-rc7 next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/net-introduce-and-use-route-hint/20191116-172108
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 20021578ba226bda1f0ddf50e4d4a12ea1c6c6c1
config: i386-randconfig-f001-20191117 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/export.h:42:0,
                    from include/linux/linkage.h:7,
                    from include/linux/kernel.h:8,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from net/ipv4/ip_input.c:111:
   net/ipv4/ip_input.c: In function 'ip_list_rcv_finish':
   net/ipv4/ip_input.c:570:19: error: 'struct netns_ipv4' has no member named 'fib_has_custom_rules'; did you mean 'fib_has_custom_local_routes'?
       if (!net->ipv4.fib_has_custom_rules &&
                      ^
   include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                       ^~~~
>> net/ipv4/ip_input.c:570:4: note: in expansion of macro 'if'
       if (!net->ipv4.fib_has_custom_rules &&
       ^~
   net/ipv4/ip_input.c:570:19: error: 'struct netns_ipv4' has no member named 'fib_has_custom_rules'; did you mean 'fib_has_custom_local_routes'?
       if (!net->ipv4.fib_has_custom_rules &&
                      ^
   include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                                ^~~~
>> net/ipv4/ip_input.c:570:4: note: in expansion of macro 'if'
       if (!net->ipv4.fib_has_custom_rules &&
       ^~
   net/ipv4/ip_input.c:570:19: error: 'struct netns_ipv4' has no member named 'fib_has_custom_rules'; did you mean 'fib_has_custom_local_routes'?
       if (!net->ipv4.fib_has_custom_rules &&
                      ^
   include/linux/compiler.h:69:3: note: in definition of macro '__trace_if_value'
     (cond) ?     \
      ^~~~
   include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
    #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                               ^~~~~~~~~~~~~~
>> net/ipv4/ip_input.c:570:4: note: in expansion of macro 'if'
       if (!net->ipv4.fib_has_custom_rules &&
       ^~

vim +/if +570 net/ipv4/ip_input.c

   542	
   543	static void ip_list_rcv_finish(struct net *net, struct sock *sk,
   544				       struct list_head *head)
   545	{
   546		struct ip_route_input_hint _hint, *hint = NULL;
   547		struct dst_entry *curr_dst = NULL;
   548		struct sk_buff *skb, *next;
   549		struct list_head sublist;
   550	
   551		INIT_LIST_HEAD(&sublist);
   552		list_for_each_entry_safe(skb, next, head, list) {
   553			struct net_device *dev = skb->dev;
   554			struct dst_entry *dst;
   555	
   556			skb_list_del_init(skb);
   557			/* if ingress device is enslaved to an L3 master device pass the
   558			 * skb to its handler for processing
   559			 */
   560			skb = l3mdev_ip_rcv(skb);
   561			if (!skb)
   562				continue;
   563			if (ip_rcv_finish_core(net, sk, skb, dev, hint) == NET_RX_DROP)
   564				continue;
   565	
   566			dst = skb_dst(skb);
   567			if (curr_dst != dst) {
   568				struct rtable *rt = (struct rtable *)dst;
   569	
 > 570				if (!net->ipv4.fib_has_custom_rules &&
   571				    rt->rt_type != RTN_BROADCAST) {
   572					_hint.refdst = skb->_skb_refdst;
   573					_hint.daddr = ip_hdr(skb)->daddr;
   574					_hint.tos = ip_hdr(skb)->tos;
   575					_hint.local = rt->rt_type == RTN_LOCAL;
   576					hint = &_hint;
   577				} else {
   578					hint = NULL;
   579				}
   580	
   581				/* dispatch old sublist */
   582				if (!list_empty(&sublist))
   583					ip_sublist_rcv_finish(&sublist);
   584				/* start new sublist */
   585				INIT_LIST_HEAD(&sublist);
   586				curr_dst = dst;
   587			}
   588			list_add_tail(&skb->list, &sublist);
   589		}
   590		/* dispatch final sublist */
   591		ip_sublist_rcv_finish(&sublist);
   592	}
   593	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31013 bytes --]

Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive

[Edward Cree]

On 16/11/2019 09:14, Paolo Abeni wrote:
> This is alike the previous change, with some additional ipv4 specific
> quirk. Even when using the route hint we still have to do perform
> additional per packet checks about source address validity: a new
> helper is added to wrap them.
>
> Moreover, the ipv4 route lookup, even in the absence of policy routing,
> may depend on pkts ToS, so we cache that values, too.
>
> Explicitly avoid hints for local broadcast: this simplify the code
> and broadcasts are slower path anyway.
>
> UDP flood performances vs recvmmsg() receiver:
>
> vanilla		patched		delta
> Kpps		Kpps		%
> 1683		1833		+8
>
> In the worst case scenario - each packet has a different
> destination address - the performance delta is within noise
> range.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  include/net/route.h | 11 +++++++++++
>  net/ipv4/ip_input.c | 29 ++++++++++++++++++++++++-----
>  net/ipv4/route.c    | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/route.h b/include/net/route.h
> index 6c516840380d..f7a8a52318cd 100644
> --- a/include/net/route.h
> +++ b/include/net/route.h
> @@ -185,6 +185,17 @@ int ip_route_input_rcu(struct sk_buff *skb, __be32 dst, __be32 src,
>  		       u8 tos, struct net_device *devin,
>  		       struct fib_result *res);
>  
> +struct ip_route_input_hint {
> +	unsigned long	refdst;
> +	__be32		daddr;
> +	char		tos;
Why isn't this a u8?

> +	bool		local;
> +};
> +
> +int ip_route_use_hint(struct sk_buff *skb, __be32 dst, __be32 src,
> +		      u8 tos, struct net_device *devin,
> +		      struct ip_route_input_hint *hint);
> +
>  static inline int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
>  				 u8 tos, struct net_device *devin)
>  {
> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> index 24a95126e698..78fd60bf1c8a 100644
> --- a/net/ipv4/ip_input.c
> +++ b/net/ipv4/ip_input.c
> @@ -305,7 +305,8 @@ static inline bool ip_rcv_options(struct sk_buff *skb, struct net_device *dev)
>  INDIRECT_CALLABLE_DECLARE(int udp_v4_early_demux(struct sk_buff *));
>  INDIRECT_CALLABLE_DECLARE(int tcp_v4_early_demux(struct sk_buff *));
>  static int ip_rcv_finish_core(struct net *net, struct sock *sk,
> -			      struct sk_buff *skb, struct net_device *dev)
> +			      struct sk_buff *skb, struct net_device *dev,
> +			      struct ip_route_input_hint *hint)
>  {
>  	const struct iphdr *iph = ip_hdr(skb);
>  	int (*edemux)(struct sk_buff *skb);
> @@ -335,8 +336,12 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
>  	 *	how the packet travels inside Linux networking.
>  	 */
>  	if (!skb_valid_dst(skb)) {
> -		err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
> -					   iph->tos, dev);
> +		if (hint && hint->daddr == iph->daddr && hint->tos == iph->tos)
> +			err = ip_route_use_hint(skb, iph->daddr, iph->saddr,
> +						iph->tos, dev, hint);
> +		else
> +			err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
> +						   iph->tos, dev);
>  		if (unlikely(err))
>  			goto drop_error;
>  	}
> @@ -408,7 +413,7 @@ static int ip_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
>  	if (!skb)
>  		return NET_RX_SUCCESS;
>  
> -	ret = ip_rcv_finish_core(net, sk, skb, dev);
> +	ret = ip_rcv_finish_core(net, sk, skb, dev, NULL);
>  	if (ret != NET_RX_DROP)
>  		ret = dst_input(skb);
>  	return ret;
> @@ -538,6 +543,7 @@ static void ip_sublist_rcv_finish(struct list_head *head)
>  static void ip_list_rcv_finish(struct net *net, struct sock *sk,
>  			       struct list_head *head)
>  {
> +	struct ip_route_input_hint _hint, *hint = NULL;
>  	struct dst_entry *curr_dst = NULL;
>  	struct sk_buff *skb, *next;
>  	struct list_head sublist;
> @@ -554,11 +560,24 @@ static void ip_list_rcv_finish(struct net *net, struct sock *sk,
>  		skb = l3mdev_ip_rcv(skb);
>  		if (!skb)
>  			continue;
> -		if (ip_rcv_finish_core(net, sk, skb, dev) == NET_RX_DROP)
> +		if (ip_rcv_finish_core(net, sk, skb, dev, hint) == NET_RX_DROP)
>  			continue;
>  
>  		dst = skb_dst(skb);
>  		if (curr_dst != dst) {
> +			struct rtable *rt = (struct rtable *)dst;
> +
> +			if (!net->ipv4.fib_has_custom_rules &&
> +			    rt->rt_type != RTN_BROADCAST) {
> +				_hint.refdst = skb->_skb_refdst;
> +				_hint.daddr = ip_hdr(skb)->daddr;
> +				_hint.tos = ip_hdr(skb)->tos;
> +				_hint.local = rt->rt_type == RTN_LOCAL;
> +				hint = &_hint;
> +			} else {
> +				hint = NULL;
> +			}
Perhaps factor this block out into a function?  Just because it's getting
 deeply indented and giving it a name would make it more obvious what it's
 for.  hint = ipv4_extract_route_hint(skb, &_hint)?

> +
>  			/* dispatch old sublist */
>  			if (!list_empty(&sublist))
>  				ip_sublist_rcv_finish(&sublist);
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index dcc4fa10138d..b0ddff17db80 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2019,6 +2019,44 @@ static int ip_mkroute_input(struct sk_buff *skb,
>  	return __mkroute_input(skb, res, in_dev, daddr, saddr, tos);
>  }
>  
> +/* Implements all the saddr-related checks as ip_route_input_slow(),
> + * assuming daddr is valid and this is not a local broadcast.
> + * Uses the provided hint instead of performing a route lookup.
> + */
> +int ip_route_use_hint(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> +		      u8 tos, struct net_device *dev,
> +		      struct ip_route_input_hint *hint)
Mostly I like the idea of these patches, but it bugs me that this seems
 to be reimplementing a little bit, and might get out of sync.  Is it
 possible to factor out the checks from ip_route_input_slow() and just
 call them here?
Otherwise maybe stick something in the comment to ip_route_input_slow()
 reminding to propagate changes to ip_route_use_hint()?

Or perhaps better still would be to come up with a single function that
 always takes a hint, that may be NULL, in which case it performs normal
 routing; and use that in all paths?  (Plumbing the hint through from
 ip_route_input_noref() etc.)

-Ed

> +{
> +	struct in_device *in_dev = __in_dev_get_rcu(dev);
> +	struct net *net = dev_net(dev);
> +	int err = -EINVAL;
> +	u32 itag = 0;
> +
> +	if (ipv4_is_multicast(saddr) || ipv4_is_lbcast(saddr))
> +		goto martian_source;
> +
> +	if (ipv4_is_zeronet(saddr))
> +		goto martian_source;
> +
> +	if (ipv4_is_loopback(saddr) && !IN_DEV_NET_ROUTE_LOCALNET(in_dev, net))
> +		goto martian_source;
> +
> +	if (hint->local) {
> +		err = fib_validate_source(skb, saddr, daddr, tos, 0, dev,
> +					  in_dev, &itag);
> +		if (err < 0)
> +			goto martian_source;
> +	}
> +
> +	err = 0;
> +	__skb_dst_copy(skb, hint->refdst);
> +	return err;
> +
> +martian_source:
> +	ip_handle_martian_source(dev, in_dev, skb, daddr, saddr);
> +	return err;
> +}
> +
>  /*
>   *	NOTE. We drop all the packets that has local source
>   *	addresses, because every properly looped back packet

Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive

[Paolo Abeni]

On Mon, 2019-11-18 at 16:15 +0000, Edward Cree wrote:
> @@ -538,6 +543,7 @@ static void ip_sublist_rcv_finish(struct list_head *head)
> >  static void ip_list_rcv_finish(struct net *net, struct sock *sk,
> >  			       struct list_head *head)
> >  {
> > +	struct ip_route_input_hint _hint, *hint = NULL;
> >  	struct dst_entry *curr_dst = NULL;
> >  	struct sk_buff *skb, *next;
> >  	struct list_head sublist;
> > @@ -554,11 +560,24 @@ static void ip_list_rcv_finish(struct net *net, struct sock *sk,
> >  		skb = l3mdev_ip_rcv(skb);
> >  		if (!skb)
> >  			continue;
> > -		if (ip_rcv_finish_core(net, sk, skb, dev) == NET_RX_DROP)
> > +		if (ip_rcv_finish_core(net, sk, skb, dev, hint) == NET_RX_DROP)
> >  			continue;
> >  
> >  		dst = skb_dst(skb);
> >  		if (curr_dst != dst) {
> > +			struct rtable *rt = (struct rtable *)dst;
> > +
> > +			if (!net->ipv4.fib_has_custom_rules &&
> > +			    rt->rt_type != RTN_BROADCAST) {
> > +				_hint.refdst = skb->_skb_refdst;
> > +				_hint.daddr = ip_hdr(skb)->daddr;
> > +				_hint.tos = ip_hdr(skb)->tos;
> > +				_hint.local = rt->rt_type == RTN_LOCAL;
> > +				hint = &_hint;
> > +			} else {
> > +				hint = NULL;
> > +			}
> Perhaps factor this block out into a function?  Just because it's getting
>  deeply indented and giving it a name would make it more obvious what it's
>  for.  hint = ipv4_extract_route_hint(skb, &_hint)?

yep, I like the idea, will do in the next iteration.
> 
> > +
> >  			/* dispatch old sublist */
> >  			if (!list_empty(&sublist))
> >  				ip_sublist_rcv_finish(&sublist);
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index dcc4fa10138d..b0ddff17db80 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -2019,6 +2019,44 @@ static int ip_mkroute_input(struct sk_buff *skb,
> >  	return __mkroute_input(skb, res, in_dev, daddr, saddr, tos);
> >  }
> >  
> > +/* Implements all the saddr-related checks as ip_route_input_slow(),
> > + * assuming daddr is valid and this is not a local broadcast.
> > + * Uses the provided hint instead of performing a route lookup.
> > + */
> > +int ip_route_use_hint(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> > +		      u8 tos, struct net_device *dev,
> > +		      struct ip_route_input_hint *hint)
> Mostly I like the idea of these patches, but it bugs me that this seems
>  to be reimplementing a little bit, and might get out of sync.  Is it
>  possible to factor out the checks from ip_route_input_slow() and just
>  call them here?
> Otherwise maybe stick something in the comment to ip_route_input_slow()
>  reminding to propagate changes to ip_route_use_hint()?
> 
> Or perhaps better still would be to come up with a single function that
>  always takes a hint, that may be NULL, in which case it performs normal
>  routing; and use that in all paths?  (Plumbing the hint through from
>  ip_route_input_noref() etc.)

I experimented a bit with the latter option before restricting to
!RTN_BROADCAST, and it make the code quite uglier. Anyhow, preserving
the !RTN_BROADCAST restriction for hint usage, I think it could work.
Let me try that.

Thank you for the feedback!

Paolo

Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive

[Paolo Abeni]

On Mon, 2019-11-18 at 17:44 +0100, Paolo Abeni wrote:
> On Mon, 2019-11-18 at 16:15 +0000, Edward Cree wrote:
> > @@ -538,6 +543,7 @@ static void ip_sublist_rcv_finish(struct list_head *head)
> > >  static void ip_list_rcv_finish(struct net *net, struct sock *sk,
> > >  			       struct list_head *head)
> > >  {
> > > +	struct ip_route_input_hint _hint, *hint = NULL;
> > >  	struct dst_entry *curr_dst = NULL;
> > >  	struct sk_buff *skb, *next;
> > >  	struct list_head sublist;
> > > @@ -554,11 +560,24 @@ static void ip_list_rcv_finish(struct net *net, struct sock *sk,
> > >  		skb = l3mdev_ip_rcv(skb);
> > >  		if (!skb)
> > >  			continue;
> > > -		if (ip_rcv_finish_core(net, sk, skb, dev) == NET_RX_DROP)
> > > +		if (ip_rcv_finish_core(net, sk, skb, dev, hint) == NET_RX_DROP)
> > >  			continue;
> > >  
> > >  		dst = skb_dst(skb);
> > >  		if (curr_dst != dst) {
> > > +			struct rtable *rt = (struct rtable *)dst;
> > > +
> > > +			if (!net->ipv4.fib_has_custom_rules &&
> > > +			    rt->rt_type != RTN_BROADCAST) {
> > > +				_hint.refdst = skb->_skb_refdst;
> > > +				_hint.daddr = ip_hdr(skb)->daddr;
> > > +				_hint.tos = ip_hdr(skb)->tos;
> > > +				_hint.local = rt->rt_type == RTN_LOCAL;
> > > +				hint = &_hint;
> > > +			} else {
> > > +				hint = NULL;
> > > +			}
> > Perhaps factor this block out into a function?  Just because it's getting
> >  deeply indented and giving it a name would make it more obvious what it's
> >  for.  hint = ipv4_extract_route_hint(skb, &_hint)?
> 
> yep, I like the idea, will do in the next iteration.
> > > +
> > >  			/* dispatch old sublist */
> > >  			if (!list_empty(&sublist))
> > >  				ip_sublist_rcv_finish(&sublist);
> > > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > > index dcc4fa10138d..b0ddff17db80 100644
> > > --- a/net/ipv4/route.c
> > > +++ b/net/ipv4/route.c
> > > @@ -2019,6 +2019,44 @@ static int ip_mkroute_input(struct sk_buff *skb,
> > >  	return __mkroute_input(skb, res, in_dev, daddr, saddr, tos);
> > >  }
> > >  
> > > +/* Implements all the saddr-related checks as ip_route_input_slow(),
> > > + * assuming daddr is valid and this is not a local broadcast.
> > > + * Uses the provided hint instead of performing a route lookup.
> > > + */
> > > +int ip_route_use_hint(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> > > +		      u8 tos, struct net_device *dev,
> > > +		      struct ip_route_input_hint *hint)
> > Mostly I like the idea of these patches, but it bugs me that this seems
> >  to be reimplementing a little bit, and might get out of sync.  Is it
> >  possible to factor out the checks from ip_route_input_slow() and just
> >  call them here?
> > Otherwise maybe stick something in the comment to ip_route_input_slow()
> >  reminding to propagate changes to ip_route_use_hint()?
> > 
> > Or perhaps better still would be to come up with a single function that
> >  always takes a hint, that may be NULL, in which case it performs normal
> >  routing; and use that in all paths?  (Plumbing the hint through from
> >  ip_route_input_noref() etc.)
> 
> I experimented a bit with the latter option before restricting to
> !RTN_BROADCAST, and it make the code quite uglier. Anyhow, preserving
> the !RTN_BROADCAST restriction for hint usage, I think it could work.
> Let me try that.

Uhm... all the other options lead to significant code uglification, so
unless someone has strong optinion against it, I would go for some
additional comment to ip_route_input_slow().

Cheers,

Paolo

Re: [PATCH net-next 0/2] net: introduce and use route hint

[David Miller]

From: Paolo Abeni <pabeni@redhat.com>
Date: Sat, 16 Nov 2019 10:14:49 +0100

> This series leverages the listification infrastructure to avoid
> unnecessary route lookup on ingress packets. In absence of policy routing,
> packets with equal daddr will usually land on the same dst.
> 
> When processing packet bursts (lists) we can easily reference the previous
> dst entry. When we hit the 'same destination' condition we can avoid the
> route lookup, coping the already available dst.
> 
> Detailed performance numbers are available in the individual commit messages.

Looks like there are some problems with the unconditional use of
fib{6}_has_custom_rules in this series.

Re: [PATCH net-next 0/2] net: introduce and use route hint

[Paolo Abeni]

On Sat, 2019-11-16 at 12:09 -0800, David Miller wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> Date: Sat, 16 Nov 2019 10:14:49 +0100
> 
> > This series leverages the listification infrastructure to avoid
> > unnecessary route lookup on ingress packets. In absence of policy routing,
> > packets with equal daddr will usually land on the same dst.
> > 
> > When processing packet bursts (lists) we can easily reference the previous
> > dst entry. When we hit the 'same destination' condition we can avoid the
> > route lookup, coping the already available dst.
> > 
> > Detailed performance numbers are available in the individual commit messages.
> 
> Looks like there are some problems with the unconditional use of
> fib{6}_has_custom_rules in this series.

Whoops... I forgot the dependency on CONFIG_IP*_MULTIPLE_TABLES. I'll
fix that in the next iteration.

Thanks,

Paolo