* [PATCH net-next 0/2] net: introduce and use route hint
@ 2019-11-16 9:14 Paolo Abeni
2019-11-16 9:14 ` [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input Paolo Abeni
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Paolo Abeni @ 2019-11-16 9:14 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Willem de Bruijn, Edward Cree
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input
2019-11-16 9:14 [PATCH net-next 0/2] net: introduce and use route hint Paolo Abeni
@ 2019-11-16 9:14 ` Paolo Abeni
2019-11-16 11:04 ` kbuild test robot
2019-11-17 11:46 ` kbuild test robot
2019-11-16 9:14 ` [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive Paolo Abeni
2019-11-16 20:09 ` [PATCH net-next 0/2] net: introduce and use route hint David Miller
2 siblings, 2 replies; 16+ messages in thread
From: Paolo Abeni @ 2019-11-16 9:14 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Willem de Bruijn, Edward Cree
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive
2019-11-16 9:14 [PATCH net-next 0/2] net: introduce and use route hint Paolo Abeni
2019-11-16 9:14 ` [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input Paolo Abeni
@ 2019-11-16 9:14 ` Paolo Abeni
2019-11-16 10:56 ` kbuild test robot
` (2 more replies)
2019-11-16 20:09 ` [PATCH net-next 0/2] net: introduce and use route hint David Miller
2 siblings, 3 replies; 16+ messages in thread
From: Paolo Abeni @ 2019-11-16 9:14 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Willem de Bruijn, Edward Cree
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive
2019-11-16 9:14 ` [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive Paolo Abeni
@ 2019-11-16 10:56 ` kbuild test robot
2019-11-17 8:58 ` kbuild test robot
2019-11-18 16:15 ` Edward Cree
2 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-11-16 10:56 UTC (permalink / raw)
To: Paolo Abeni
Cc: kbuild-all, netdev, David S. Miller, Willem de Bruijn, Edward Cree
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive
@ 2019-11-16 10:56 ` kbuild test robot
0 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-11-16 10:56 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5008 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(a)lists.01.org Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 25413 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input
2019-11-16 9:14 ` [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input Paolo Abeni
@ 2019-11-16 11:04 ` kbuild test robot
2019-11-17 11:46 ` kbuild test robot
1 sibling, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-11-16 11:04 UTC (permalink / raw)
To: Paolo Abeni
Cc: kbuild-all, netdev, David S. Miller, Willem de Bruijn, Edward Cree
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input
@ 2019-11-16 11:04 ` kbuild test robot
0 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-11-16 11:04 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2891 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(a)lists.01.org Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28138 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 0/2] net: introduce and use route hint
2019-11-16 9:14 [PATCH net-next 0/2] net: introduce and use route hint Paolo Abeni
2019-11-16 9:14 ` [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input Paolo Abeni
2019-11-16 9:14 ` [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive Paolo Abeni
@ 2019-11-16 20:09 ` David Miller
2019-11-18 8:12 ` Paolo Abeni
2 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2019-11-16 20:09 UTC (permalink / raw)
To: pabeni; +Cc: netdev, willemdebruijn.kernel, ecree
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive
2019-11-16 9:14 ` [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive Paolo Abeni
@ 2019-11-17 8:58 ` kbuild test robot
2019-11-17 8:58 ` kbuild test robot
2019-11-18 16:15 ` Edward Cree
2 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-11-17 8:58 UTC (permalink / raw)
To: Paolo Abeni
Cc: kbuild-all, netdev, David S. Miller, Willem de Bruijn, Edward Cree
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive
@ 2019-11-17 8:58 ` kbuild test robot
0 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-11-17 8:58 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5132 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(a)lists.01.org Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31013 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input
2019-11-16 9:14 ` [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input Paolo Abeni
@ 2019-11-17 11:46 ` kbuild test robot
2019-11-17 11:46 ` kbuild test robot
1 sibling, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-11-17 11:46 UTC (permalink / raw)
To: Paolo Abeni
Cc: kbuild-all, netdev, David S. Miller, Willem de Bruijn, Edward Cree
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input
@ 2019-11-17 11:46 ` kbuild test robot
0 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-11-17 11:46 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 7415 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(a)lists.01.org Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31708 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 0/2] net: introduce and use route hint
2019-11-16 20:09 ` [PATCH net-next 0/2] net: introduce and use route hint David Miller
@ 2019-11-18 8:12 ` Paolo Abeni
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Abeni @ 2019-11-18 8:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, willemdebruijn.kernel, ecree
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive
2019-11-16 9:14 ` [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive Paolo Abeni
2019-11-16 10:56 ` kbuild test robot
2019-11-17 8:58 ` kbuild test robot
@ 2019-11-18 16:15 ` Edward Cree
2019-11-18 16:44 ` Paolo Abeni
2 siblings, 1 reply; 16+ messages in thread
From: Edward Cree @ 2019-11-18 16:15 UTC (permalink / raw)
To: Paolo Abeni, netdev; +Cc: David S. Miller, Willem de Bruijn
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive
2019-11-18 16:15 ` Edward Cree
@ 2019-11-18 16:44 ` Paolo Abeni
2019-11-18 17:14 ` Paolo Abeni
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Abeni @ 2019-11-18 16:44 UTC (permalink / raw)
To: Edward Cree, netdev; +Cc: David S. Miller, Willem de Bruijn
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive
2019-11-18 16:44 ` Paolo Abeni
@ 2019-11-18 17:14 ` Paolo Abeni
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Abeni @ 2019-11-18 17:14 UTC (permalink / raw)
To: Edward Cree, netdev; +Cc: David S. Miller, Willem de Bruijn
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
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-11-18 17:15 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-16 9:14 [PATCH net-next 0/2] net: introduce and use route hint Paolo Abeni
2019-11-16 9:14 ` [PATCH net-next 1/2] ipv6: introduce and uses route look hints for list input Paolo Abeni
2019-11-16 11:04 ` kbuild test robot
2019-11-16 11:04 ` kbuild test robot
2019-11-17 11:46 ` kbuild test robot
2019-11-17 11:46 ` kbuild test robot
2019-11-16 9:14 ` [PATCH net-next 2/2] ipv4: use dst hint for ipv4 list receive Paolo Abeni
2019-11-16 10:56 ` kbuild test robot
2019-11-16 10:56 ` kbuild test robot
2019-11-17 8:58 ` kbuild test robot
2019-11-17 8:58 ` kbuild test robot
2019-11-18 16:15 ` Edward Cree
2019-11-18 16:44 ` Paolo Abeni
2019-11-18 17:14 ` Paolo Abeni
2019-11-16 20:09 ` [PATCH net-next 0/2] net: introduce and use route hint David Miller
2019-11-18 8:12 ` Paolo Abeni
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.