Skip to contentSkip to conversation

net: introduce and use route hint

PATCH Superseded Paolo Abeni 4 participants 16 messages Nov 19, 2019 – Nov 20, 2019

Subsystems (3)
Networking Network drivers IPv4/IPv6

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Paolo Abeni

author maintainer
#0 lore raw

v3 cover net: introduce and use route hint

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. Figures are slightly better then previous iteration because thanks to Willem's suggestion we additionally skip early demux when using the route hint.

v2 -> v3:
- use fib*_has_custom_rules() helpers (David A.)
- add ip*_extract_route_hint() helper (Edward C.)
- use prev skb as hint instead of copying data (Willem )

v1 -> v2:
- fix build issue with !CONFIG_IP*_MULTIPLE_TABLES
- fix potential race in ip6_list_rcv_finish()

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

 include/net/ip6_fib.h   |  9 +++++++++
 include/net/ip_fib.h    | 10 ++++++++++
 include/net/route.h     |  4 ++++
 net/ipv4/fib_frontend.c | 10 ----------
 net/ipv4/ip_input.c     | 35 +++++++++++++++++++++++++++++++----
 net/ipv4/route.c        | 37 +++++++++++++++++++++++++++++++++++++
 net/ipv6/ip6_input.c    | 26 ++++++++++++++++++++++++--
 7 files changed, 115 insertions(+), 16 deletions(-)
Signature · 1 line
2.21.0
author maintainer
#1 lore raw

PATCH v3 1/2 ipv6: introduce and uses route look hints for list input

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 1674 +17

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

v2 -> v3:
- add fib6_has_custom_rules() helpers (David A.)
- add ip6_extract_route_hint() helper (Edward C.)
- use hint directly in ip6_list_rcv_finish() (Willem)

v1 -> v2:
- fix build issue with !CONFIG_IPV6_MULTIPLE_TABLES
- fix potential race when fib6_has_custom_rules is set while processing a packet batch

Signed-off-by Paolo Abeni <[email protected]>
---
 include/net/ip6_fib.h |  9 +++++++++
 net/ipv6/ip6_input.c  | 26 ++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)
include/net/ip6_fib.h +9 -0
index 5d1615463138..9ab60611b97b 100644
@@ -502,6 +502,11 @@ static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
 }
 
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
+static inline bool fib6_has_custom_rules(struct net *net)
+{
+	return net->ipv6.fib6_has_custom_rules;
+}
+
 int fib6_rules_init(void);
 void fib6_rules_cleanup(void);
 bool fib6_rule_default(const struct fib_rule *rule);

@@ -527,6 +532,10 @@ static inline bool fib6_rules_early_flow_dissect(struct net *net,
 	return true;
 }
 #else
+static inline bool fib6_has_custom_rules(struct net *net)
+{
+	return 0;
+}
 static inline int               fib6_rules_init(void)
 {
 	return 0;
net/ipv6/ip6_input.c +24 -2
index ef7f707d9ae3..792b52aa9fc9 100644
@@ -59,6 +59,7 @@ 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))
 		ip6_route_input(skb);
 }

@@ -86,11 +87,26 @@ static void ip6_sublist_rcv_finish(struct list_head *head)
 	}
 }
 
+static bool ip6_can_use_hint(struct sk_buff *skb, const struct sk_buff *hint)
+{
+	return hint && !skb_dst(skb) &&
+	       ipv6_addr_equal(&ipv6_hdr(hint)->daddr, &ipv6_hdr(skb)->daddr);
+}
+
+static struct sk_buff *ip6_extract_route_hint(struct net *net,
+					      struct sk_buff *skb)
+{
+	if (IS_ENABLED(IPV6_SUBTREES) || fib6_has_custom_rules(net))
+		return NULL;
+
+	return skb;
+}
+
 static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
 				struct list_head *head)
 {
+	struct sk_buff *skb, *next, *hint = NULL;
 	struct dst_entry *curr_dst = NULL;
-	struct sk_buff *skb, *next;
 	struct list_head sublist;
 
 	INIT_LIST_HEAD(&sublist);

@@ -104,9 +120,15 @@ 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);
+
+		if (ip6_can_use_hint(skb, hint))
+			skb_dst_copy(skb, hint);
+		else
+			ip6_rcv_finish_core(net, sk, skb);
 		dst = skb_dst(skb);
 		if (curr_dst != dst) {
+			hint = ip6_extract_route_hint(net, skb);
+
 			/* dispatch old sublist */
 			if (!list_empty(&sublist))
 				ip6_sublist_rcv_finish(&sublist);
Signature · 1 line
2.21.0
#2 lore raw

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

Quoted 11 lines · 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.

Quoted 6 lines

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

vanilla patched delta
Kpps Kpps %
1431 1674 +17

That's a nice boost...

net/ipv6/ip6_input.c static void ip6_sublist_rcv_finish(struct list_head *head) · line 99

Quoted 4 lines
net/ipv6/ip6_input.c +4 -0
+static struct sk_buff *ip6_extract_route_hint(struct net *net,
+					      struct sk_buff *skb)
+{
+	if (IS_ENABLED(IPV6_SUBTREES) || fib6_has_custom_rules(net))

... but basing on SUBTREES being disabled is going to limit its use. If no routes are source based (fib6_src is not set), you should be able to re-use the hint with SUBTREES enabled. e.g., track fib6_src use with a per-namespace counter - similar to fib6_rules_require_fldissect.

author maintainer
#3 lore raw

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

Hi,

Quoted 9 lines · David Ahern

+static struct sk_buff *ip6_extract_route_hint(struct net *net,

net/ipv6/ip6_input.c static void ip6_sublist_rcv_finish(struct list_head *head) · line 99

Quoted 3 lines
net/ipv6/ip6_input.c +3 -0
+					      struct sk_buff *skb)
+{
+	if (IS_ENABLED(IPV6_SUBTREES) || fib6_has_custom_rules(net))

... but basing on SUBTREES being disabled is going to limit its use. If no routes are source based (fib6_src is not set), you should be able to re-use the hint with SUBTREES enabled. e.g., track fib6_src use with a per-namespace counter - similar to fib6_rules_require_fldissect.

Thank you for the feedback! Would you consider this as an intermediate step? e.g. get these patches in, and then I'll implement subtree support?
I'm asking because I don't have subtree setup handy, it will a little time to get there.

Thanks!

Paolo

#4 lore raw

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

Quoted 13 lines · Paolo Abeni

Hi,

Quoted 9 lines · David Ahern

+static struct sk_buff *ip6_extract_route_hint(struct net *net,

net/ipv6/ip6_input.c static void ip6_sublist_rcv_finish(struct list_head *head) · line 99

Quoted 3 lines
net/ipv6/ip6_input.c +3 -0
+					      struct sk_buff *skb)
+{
+	if (IS_ENABLED(IPV6_SUBTREES) || fib6_has_custom_rules(net))

... but basing on SUBTREES being disabled is going to limit its use. If no routes are source based (fib6_src is not set), you should be able to re-use the hint with SUBTREES enabled. e.g., track fib6_src use with a per-namespace counter - similar to fib6_rules_require_fldissect.

Quoted 6 lines

Thank you for the feedback! Would you consider this as an intermediate step? e.g. get these patches in, and then I'll implement subtree support?
I'm asking because I don't have subtree setup handy, it will a little time to get there.

IPV6_SUBTREES is just a matter of source based routing, so with iproute2 just add 'from <addr>'

If you delay dealing with it, then this patch needs a change: since ip6_extract_route_hint will only return NULL, ip6_can_use_hint will only take NULL as an input so leaving it enabled just adds overhead with no benefit.

#5 lore raw

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

In reply to Paolo Abeni
Quoted 46 lines · 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 1674 +17

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

v2 -> v3:
- add fib6_has_custom_rules() helpers (David A.)
- add ip6_extract_route_hint() helper (Edward C.)
- use hint directly in ip6_list_rcv_finish() (Willem)

v1 -> v2:
- fix build issue with !CONFIG_IPV6_MULTIPLE_TABLES
- fix potential race when fib6_has_custom_rules is set while processing a packet batch

Signed-off-by Paolo Abeni <[email protected]>
---
 include/net/ip6_fib.h |  9 +++++++++
 net/ipv6/ip6_input.c  | 26 ++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)
include/net/ip6_fib.h +0 -0
index 5d1615463138..9ab60611b97b 100644

include/net/ip6_fib.h static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric) · line 505

Quoted 4 lines
include/net/ip6_fib.h +1 -0
@@ -502,6 +502,11 @@ static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
 }
 
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
+static inline bool fib6_has_custom_rules(struct net *net)

const struct net *net

include/net/ip6_fib.h static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric) · line 507

Quoted 2 lines
include/net/ip6_fib.h +2 -0
+{
+	return net->ipv6.fib6_has_custom_rules;

It would be nice to be able to detect that some custom rules only impact egress routes :/

include/net/ip6_fib.h static inline bool fib6_rules_early_flow_dissect(struct net *net, · line 535

Quoted 10 lines
include/net/ip6_fib.h +3 -0
+}
+
 int fib6_rules_init(void);
 void fib6_rules_cleanup(void);
 bool fib6_rule_default(const struct fib_rule *rule);

@@ -527,6 +532,10 @@ static inline bool fib6_rules_early_flow_dissect(struct net *net,
 	return true;
 }
 #else
+static inline bool fib6_has_custom_rules(struct net *net)

const struct net *net

include/net/ip6_fib.h static inline bool fib6_rules_early_flow_dissect(struct net *net, · line 537

Quoted 2 lines
include/net/ip6_fib.h +2 -0
+{
+	return 0;

	return false;

BTW, this deserves a patch on its own :)

net/ipv6/ip6_input.c static void ip6_rcv_finish_core(struct net *net, struct sock *sk, · line 62

Quoted 13 lines
include/net/ip6_fib.h +1 -0
+}
 static inline int               fib6_rules_init(void)
 {
 	return 0;
net/ipv6/ip6_input.c +1 -0
index ef7f707d9ae3..792b52aa9fc9 100644
@@ -59,6 +59,7 @@ 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);
 	}
+

Why adding a new line ? Please refrain adding noise to a patch.

net/ipv6/ip6_input.c static void ip6_sublist_rcv_finish(struct list_head *head) · line 95

Quoted 13 lines
net/ipv6/ip6_input.c +6 -0
 	if (!skb_valid_dst(skb))
 		ip6_route_input(skb);
 }

@@ -86,11 +87,26 @@ static void ip6_sublist_rcv_finish(struct list_head *head)
 	}
 }
 
+static bool ip6_can_use_hint(struct sk_buff *skb, const struct sk_buff *hint)
+{
+	return hint && !skb_dst(skb) &&
+	       ipv6_addr_equal(&ipv6_hdr(hint)->daddr, &ipv6_hdr(skb)->daddr);
+}
+

Why keeping whole skb as the hint, since all you want is the ipv6_hdr(skb)->daddr ?

Remembering the pointer to daddr would avoid de-referencing many skb fields.

Quoted 36 lines
net/ipv6/ip6_input.c +17 -2
+static struct sk_buff *ip6_extract_route_hint(struct net *net,
+					      struct sk_buff *skb)
+{
+	if (IS_ENABLED(IPV6_SUBTREES) || fib6_has_custom_rules(net))
+		return NULL;
+
+	return skb;
+}
+
 static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
 				struct list_head *head)
 {
+	struct sk_buff *skb, *next, *hint = NULL;
 	struct dst_entry *curr_dst = NULL;
-	struct sk_buff *skb, *next;
 	struct list_head sublist;
 
 	INIT_LIST_HEAD(&sublist);

@@ -104,9 +120,15 @@ 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);
+
+		if (ip6_can_use_hint(skb, hint))
+			skb_dst_copy(skb, hint);
+		else
+			ip6_rcv_finish_core(net, sk, skb);
 		dst = skb_dst(skb);
 		if (curr_dst != dst) {
+			hint = ip6_extract_route_hint(net, skb);
+
 			/* dispatch old sublist */
 			if (!list_empty(&sublist))
 				ip6_sublist_rcv_finish(&sublist);
#6 lore raw

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

Quoted 12 lines · Eric Dumazet

net/ipv6/ip6_input.c static void ip6_sublist_rcv_finish(struct list_head *head) · line 95

Quoted 9 lines
net/ipv6/ip6_input.c +6 -0
+static bool ip6_can_use_hint(struct sk_buff *skb, const struct sk_buff *hint)
+{
+	return hint && !skb_dst(skb) &&
+	       ipv6_addr_equal(&ipv6_hdr(hint)->daddr, &ipv6_hdr(skb)->daddr);
+}
+

Why keeping whole skb as the hint, since all you want is the ipv6_hdr(skb)->daddr ?

Quoted 2 lines

Remembering the pointer to daddr would avoid de-referencing many skb fields.

Ah we also need the hint dst, scrap this then...

#7 lore raw

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

In reply to Eric Dumazet
Quoted 12 lines · Eric Dumazet

include/net/ip6_fib.h static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric) · line 505

Quoted 9 lines
include/net/ip6_fib.h +1 -0
index 5d1615463138..9ab60611b97b 100644
@@ -502,6 +502,11 @@ static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
 }
 
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
+static inline bool fib6_has_custom_rules(struct net *net)

const struct net *net

Quoted 5 lines

include/net/ip6_fib.h static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric) · line 507

Quoted 2 lines
include/net/ip6_fib.h +2 -0
+{
+	return net->ipv6.fib6_has_custom_rules;

It would be nice to be able to detect that some custom rules only impact egress routes :/

Or vrf. :-)

It's a common problem that needs a better solution - not lumping the full complexity of fib rules into a single boolean.

author maintainer
#8 lore raw

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

In reply to Eric Dumazet
Quoted 8 lines · Eric Dumazet
Quoted 7 lines · Paolo Abeni
include/net/ip6_fib.h +0 -0
index 5d1615463138..9ab60611b97b 100644
@@ -502,6 +502,11 @@ static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
 }
Quoted 4 lines

include/net/ip6_fib.h static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric) · line 505

Quoted 2 lines
include/net/ip6_fib.h +1 -0
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
+static inline bool fib6_has_custom_rules(struct net *net)

const struct net *net

Yep, will do in the next iteration.

Quoted 4 lines

include/net/ip6_fib.h static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric) · line 507

Quoted 2 lines
include/net/ip6_fib.h +2 -0
+{
+	return net->ipv6.fib6_has_custom_rules;

It would be nice to be able to detect that some custom rules only impact egress routes :/

My [mis-] understanding is that addressing correctly the above (and VRF and likely many other use-cases) is beyond these patches scope.

Quoted 4 lines
Quoted 4 lines
include/net/ip6_fib.h +2 -0
+}
+
 int fib6_rules_init(void);
 void fib6_rules_cleanup(void);
Quoted 8 lines

include/net/ip6_fib.h static inline bool fib6_rules_early_flow_dissect(struct net *net, · line 535

Quoted 6 lines
include/net/ip6_fib.h +1 -0
 bool fib6_rule_default(const struct fib_rule *rule);

@@ -527,6 +532,10 @@ static inline bool fib6_rules_early_flow_dissect(struct net *net,
 	return true;
 }
 #else
+static inline bool fib6_has_custom_rules(struct net *net)

const struct net *net

Ditto ;)

Quoted 7 lines

include/net/ip6_fib.h static inline bool fib6_rules_early_flow_dissect(struct net *net, · line 537

Quoted 2 lines
include/net/ip6_fib.h +2 -0
+{
+	return 0;

	return false;

BTW, this deserves a patch on its own :)

Oks, will do

Quoted 7 lines
Quoted 7 lines
include/net/ip6_fib.h +1 -0
+}
 static inline int               fib6_rules_init(void)
 {
 	return 0;
net/ipv6/ip6_input.c +0 -1
index ef7f707d9ae3..792b52aa9fc9 100644
--- a/net/ipv6/ip6_input.c
Quoted 8 lines

net/ipv6/ip6_input.c static void ip6_rcv_finish_core(struct net *net, struct sock *sk, · line 62

Quoted 6 lines

+++ b/net/ipv6/ip6_input.c

net/ipv6/ip6_input.c +1 -0
@@ -59,6 +59,7 @@ 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);
 	}
+

Why adding a new line ? Please refrain adding noise to a patch.

Sorry this is a left-over from previous iterations, will fix in the next one.

Thank you for the detailed feedback!

Paolo

author maintainer
#9 lore raw

PATCH v3 2/2 ipv4: use dst hint for ipv4 list receive

In reply to 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.

To keep the code as simple as possible, use hints for local destination only.

UDP flood performances vs recvmmsg() receiver:

vanilla patched delta
Kpps Kpps %
1683 1871 +11

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

v2 -> v3:
- really fix build (sic) and hint usage check
- use fib4_has_custom_rules() helpers (David A.)
- add ip_extract_route_hint() helper (Edward C.)
- use prev skb as hint instead of copying data (Willem)

v1 -> v2:
- fix build issue with !CONFIG_IP_MULTIPLE_TABLES

Signed-off-by Paolo Abeni <[email protected]>
---
 include/net/ip_fib.h    | 10 ++++++++++
 include/net/route.h     |  4 ++++
 net/ipv4/fib_frontend.c | 10 ----------
 net/ipv4/ip_input.c     | 35 +++++++++++++++++++++++++++++++----
 net/ipv4/route.c        | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 82 insertions(+), 14 deletions(-)
include/net/ip_fib.h +10 -0
index 52b2406a5dfc..8e65e3e0a948 100644
@@ -311,6 +311,11 @@ static inline int fib_lookup(struct net *net, const struct flowi4 *flp,
 	return err;
 }
 
+static inline bool fib4_has_custom_rules(struct net *net)
+{
+	return false;
+}
+
 static inline bool fib4_rule_default(const struct fib_rule *rule)
 {
 	return true;

@@ -378,6 +383,11 @@ static inline int fib_lookup(struct net *net, struct flowi4 *flp,
 	return err;
 }
 
+static inline bool fib4_has_custom_rules(struct net *net)
+{
+	return net->ipv4.fib_has_custom_rules;
+}
+
 bool fib4_rule_default(const struct fib_rule *rule);
 int fib4_rules_dump(struct net *net, struct notifier_block *nb,
 		    struct netlink_ext_ack *extack);
include/net/route.h +4 -0
index 6c516840380d..a9c60fc68e36 100644
@@ -185,6 +185,10 @@ int ip_route_input_rcu(struct sk_buff *skb, __be32 dst, __be32 src,
 		       u8 tos, struct net_device *devin,
 		       struct fib_result *res);
 
+int ip_route_use_hint(struct sk_buff *skb, __be32 dst, __be32 src,
+		      u8 tos, struct net_device *devin,
+		      const struct sk_buff *hint);
+
 static inline int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
 				 u8 tos, struct net_device *devin)
 {
net/ipv4/fib_frontend.c +0 -10
index 71c78d223dfd..577db1d50a24 100644
@@ -70,11 +70,6 @@ static int __net_init fib4_rules_init(struct net *net)
 	fib_free_table(main_table);
 	return -ENOMEM;
 }
-
-static bool fib4_has_custom_rules(struct net *net)
-{
-	return false;
-}
 #else
 
 struct fib_table *fib_new_table(struct net *net, u32 id)

@@ -131,11 +126,6 @@ struct fib_table *fib_get_table(struct net *net, u32 id)
 	}
 	return NULL;
 }
-
-static bool fib4_has_custom_rules(struct net *net)
-{
-	return net->ipv4.fib_has_custom_rules;
-}
 #endif /* CONFIG_IP_MULTIPLE_TABLES */
 
 static void fib_replace_table(struct net *net, struct fib_table *old,
net/ipv4/ip_input.c +31 -4
index 24a95126e698..e992f90586f3 100644
@@ -302,16 +302,31 @@ static inline bool ip_rcv_options(struct sk_buff *skb, struct net_device *dev)
 	return true;
 }
 
+static bool ip_can_use_hint(struct sk_buff *skb, const struct iphdr *iph,
+			    const struct sk_buff *hint)
+{
+	return hint && !skb_dst(skb) && ip_hdr(hint)->daddr == iph->daddr &&
+	       ip_hdr(hint)->tos == iph->tos;
+}
+
 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,
+			      const struct sk_buff *hint)
 {
 	const struct iphdr *iph = ip_hdr(skb);
 	int (*edemux)(struct sk_buff *skb);
 	struct rtable *rt;
 	int err;
 
+	if (ip_can_use_hint(skb, iph, hint)) {
+		err = ip_route_use_hint(skb, iph->daddr, iph->saddr, iph->tos,
+					dev, hint);
+		if (unlikely(err))
+			goto drop_error;
+	}
+
 	if (net->ipv4.sysctl_ip_early_demux &&
 	    !skb_dst(skb) &&
 	    !skb->sk &&

@@ -408,7 +423,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;

@@ -535,11 +550,20 @@ static void ip_sublist_rcv_finish(struct list_head *head)
 	}
 }
 
+static struct sk_buff *ip_extract_route_hint(struct net *net,
+					     struct sk_buff *skb, int rt_type)
+{
+	if (fib4_has_custom_rules(net) || rt_type != RTN_LOCAL)
+		return NULL;
+
+	return skb;
+}
+
 static void ip_list_rcv_finish(struct net *net, struct sock *sk,
 			       struct list_head *head)
 {
+	struct sk_buff *skb, *next, *hint = NULL;
 	struct dst_entry *curr_dst = NULL;
-	struct sk_buff *skb, *next;
 	struct list_head sublist;
 
 	INIT_LIST_HEAD(&sublist);

@@ -554,11 +578,14 @@ 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) {
+			hint = ip_extract_route_hint(net, skb,
+					       ((struct rtable *)dst)->rt_type);
+
 			/* dispatch old sublist */
 			if (!list_empty(&sublist))
 				ip_sublist_rcv_finish(&sublist);
net/ipv4/route.c +37 -0
index dcc4fa10138d..7083cfa9f0a5 100644
@@ -2019,10 +2019,47 @@ 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 destination is local.
+ * 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,
+		      const struct sk_buff *hint)
+{
+	struct in_device *in_dev = __in_dev_get_rcu(dev);
+	struct net *net = dev_net(dev);
+	int err = -EINVAL;
+	u32 tag = 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;
+
+	tos &= IPTOS_RT_MASK;
+	err = fib_validate_source(skb, saddr, daddr, tos, 0, dev, in_dev, &tag);
+	if (err < 0)
+		goto martian_source;
+
+	skb_dst_copy(skb, hint);
+	return 0;
+
+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
  *	must have correct destination already attached by output routine.
+ *	Changes in the enforced policies must be applied also to
+ *	ip_route_use_hint().
  *
  *	Such approach solves two big problems:
  *	1. Not simplex devices are handled properly.
Signature · 1 line
2.21.0
#10 lore raw

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

net/ipv4/ip_input.c static void ip_sublist_rcv_finish(struct list_head *head) · line 556

Quoted 8 lines · Paolo Abeni
net/ipv4/ip_input.c +4 -0
@@ -535,11 +550,20 @@ static void ip_sublist_rcv_finish(struct list_head *head)
 	}
 }
 
+static struct sk_buff *ip_extract_route_hint(struct net *net,
+					     struct sk_buff *skb, int rt_type)
+{
+	if (fib4_has_custom_rules(net) || rt_type != RTN_LOCAL)

Why the local only limitation for v4 but not v6? Really, why limit this to LOCAL at all? same destination with same tos and no custom rules means even for forwarding the lookup should be the same and you can re-use the dst.

author maintainer
#11 lore raw

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

Quoted 5 lines · David Ahern
Quoted 4 lines · Paolo Abeni
(quoted diff) +0 -0
@@ -535,11 +550,20 @@ static void ip_sublist_rcv_finish(struct list_head *head)
 	}
 }
Quoted 7 lines

net/ipv4/ip_input.c static void ip_sublist_rcv_finish(struct list_head *head) · line 556

Quoted 4 lines
net/ipv4/ip_input.c +4 -0
+static struct sk_buff *ip_extract_route_hint(struct net *net,
+					     struct sk_buff *skb, int rt_type)
+{
+	if (fib4_has_custom_rules(net) || rt_type != RTN_LOCAL)

Why the local only limitation for v4 but not v6? Really, why limit this to LOCAL at all?

The goal here was to simplify as much as possible the ipv4 ip_route_use_hint() helper, as its complexity raised some eyebrown.

Yes, hints can be used also for forwarding. I'm unsure how much will help, given the daddr contraint. If there is agreement I can re-add it.

Thanks,

Paolo

author maintainer
#12 lore raw

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

Quoted 14 lines · Paolo Abeni
Quoted 5 lines · David Ahern
Quoted 4 lines · Paolo Abeni
(quoted diff) +0 -0
@@ -535,11 +550,20 @@ static void ip_sublist_rcv_finish(struct list_head *head)
 	}
 }
Quoted 7 lines

net/ipv4/ip_input.c static void ip_sublist_rcv_finish(struct list_head *head) · line 556

Quoted 4 lines
net/ipv4/ip_input.c +4 -0
+static struct sk_buff *ip_extract_route_hint(struct net *net,
+					     struct sk_buff *skb, int rt_type)
+{
+	if (fib4_has_custom_rules(net) || rt_type != RTN_LOCAL)

Why the local only limitation for v4 but not v6? Really, why limit this to LOCAL at all?

Quoted 5 lines

The goal here was to simplify as much as possible the ipv4 ip_route_use_hint() helper, as its complexity raised some eyebrown.

Yes, hints can be used also for forwarding. I'm unsure how much will help, given the daddr contraint. If there is agreement I can re-add it.

Sorry, I forgot to ask: would you be ok enabling the route hint for !RTN_BROADCAST, as in the previous iteration? Covering RTN_BROADCAST will add quite a bit of complexity to ip_route_use_hint(), likely with no relevant use-case.

Thanks!

Paolo

#13 lore raw

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

Quoted 18 lines · Paolo Abeni
Quoted 17 lines · Paolo Abeni
Quoted 5 lines · David Ahern
Quoted 4 lines · Paolo Abeni
(quoted diff) +0 -0
@@ -535,11 +550,20 @@ static void ip_sublist_rcv_finish(struct list_head *head)
 	}
 }
Quoted 7 lines

net/ipv4/ip_input.c static void ip_sublist_rcv_finish(struct list_head *head) · line 556

Quoted 4 lines
net/ipv4/ip_input.c +4 -0
+static struct sk_buff *ip_extract_route_hint(struct net *net,
+					     struct sk_buff *skb, int rt_type)
+{
+	if (fib4_has_custom_rules(net) || rt_type != RTN_LOCAL)

Why the local only limitation for v4 but not v6? Really, why limit this to LOCAL at all?

The goal here was to simplify as much as possible the ipv4 ip_route_use_hint() helper, as its complexity raised some eyebrown.

Quoted 8 lines
Quoted 2 lines

Yes, hints can be used also for forwarding. I'm unsure how much will help, given the daddr contraint. If there is agreement I can re-add it.

Sorry, I forgot to ask: would you be ok enabling the route hint for !RTN_BROADCAST, as in the previous iteration? Covering RTN_BROADCAST will add quite a bit of complexity to ip_route_use_hint(), likely with no relevant use-case.

It is a trade-off of too many checks which just add overhead to the packets that can not benefit from re-use. I was trying to understand why local delivery was given preference.

maintainer
#14 lore raw
In reply to Paolo Abeni

From: Paolo Abeni <[email protected]>
Date: Tue, 19 Nov 2019 15:38:35 +0100

Quoted 13 lines

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. Figures are slightly better then previous iteration because thanks to Willem's suggestion we additionally skip early demux when using the route hint.

Quoted 8 lines

v2 -> v3:
- use fib*_has_custom_rules() helpers (David A.)
- add ip*_extract_route_hint() helper (Edward C.)
- use prev skb as hint instead of copying data (Willem )

v1 -> v2:
- fix build issue with !CONFIG_IP*_MULTIPLE_TABLES
- fix potential race in ip6_list_rcv_finish()

To reiterate David A.'s feedback, having this depend upon IP_MULTIPLE_TABLES being disabled is %100 a non-starter.

No distribution will benefit from these changes at all.

author maintainer
#15 lore raw
Quoted 21 lines · David Miller

From: Paolo Abeni <[email protected]>
Date: Tue, 19 Nov 2019 15:38:35 +0100

Quoted 18 lines

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. Figures are slightly better then previous iteration because thanks to Willem's suggestion we additionally skip early demux when using the route hint.

v2 -> v3:
- use fib*_has_custom_rules() helpers (David A.)
- add ip*_extract_route_hint() helper (Edward C.)
- use prev skb as hint instead of copying data (Willem )

Quoted 5 lines
Quoted 3 lines

v1 -> v2:
- fix build issue with !CONFIG_IP*_MULTIPLE_TABLES
- fix potential race in ip6_list_rcv_finish()

To reiterate David A.'s feedback,

Yep, I'm working to address it...

Quoted 2 lines

having this depend upon
IP_MULTIPLE_TABLES being disabled is %100 a non-starter.

...anyway in its current form it 'just' depends on CONFIG_IPV6_SUBTREE being disabled. Next iteration will remove such dep, as per David's feedback.

Thanks,

Paolo