bpf: Add helper to do FIB lookups

PATCH Applied David Ahern 5 participants 22 messages May 4, 2018 – May 9, 2018

Subsystems (9)

Networking Network drivers Kselftest BPF +5 more subsystems

lore

Share & tools

lore

David Ahern

author

2018-05-04 02:54 UTC #0 lore raw

v2 cover bpf: Add helper to do FIB lookups

Provide a helper for doing a FIB and neighbor lookup in the kernel tables from an XDP program. The helper provides a fastpath for forwarding packets. If the packet is a local delivery or for any reason is not a simple lookup and forward, the packet is expected to continue up the stack for full processing.

The response from a FIB and neighbor lookup is either the egress index with the bpf_fib_lookup struct filled in with dmac and gateway or 0 meaning the packet should continue up the stack. In time we can revisit this to return the FIB lookup result errno if it is one of the special RTN_'s such as RTN_BLACKHOLE (-EINVAL) so that the XDP programs can do an early drop if desired.

Patches 1-6 do some more refactoring to IPv6 with the end goal of extracting a FIB lookup function that aligns with fib_lookup for IPv4, basically returning a fib6_info without creating a dst based entry.

Patch 7 adds lookup functions to the ipv6 stub. These are needed since bpf is built into the kernel and ipv6 may not be built or loaded.

Patch 8 adds the bpf helper and 9 adds a sample program.

v2
- removed pkt_access from bpf_func_proto as noticed by Daniel
- added check in that IPv6 forwarding is enabled
- added DaveM's ack on patches 1-7 and 9 based on v1 response and fact that no changes were made to them in v2

v1
- updated commit messages and cover letter
- added comment to sample program noting lack of verification on egress device supporting XDP

RFC v2
- fixed use of foward helper from cls_act as noted by Daniel
- in patch 1 rename fib6_lookup_1 as well for consistency

David Ahern (9):
net/ipv6: Rename fib6_lookup to fib6_node_lookup net/ipv6: Rename rt6_multipath_select
net/ipv6: Extract table lookup from ip6_pol_route net/ipv6: Refactor fib6_rule_action
net/ipv6: Add fib6_lookup
net/ipv6: Update fib6 tracepoint to take fib6_info net/ipv6: Add fib lookup stubs for use in bpf helper
bpf: Provide helper to do forwarding lookups in kernel FIB table samples/bpf: Add example of ipv4 and ipv6 forwarding in XDP

 include/net/addrconf.h                    |  14 ++
 include/net/ip6_fib.h                     |  21 ++-
 include/trace/events/fib6.h               |  14 +-
 include/uapi/linux/bpf.h                  |  83 +++++++++-
 net/core/filter.c                         | 266 ++++++++++++++++++++++++++++++
 net/ipv6/addrconf_core.c                  |  33 +++-
 net/ipv6/af_inet6.c                       |   6 +-
 net/ipv6/fib6_rules.c                     | 138 +++++++++++++---
 net/ipv6/ip6_fib.c                        |  21 ++-
 net/ipv6/route.c                          |  76 +++++----
 samples/bpf/Makefile                      |   4 +
 samples/bpf/xdp_fwd_kern.c                | 113 +++++++++++++
 samples/bpf/xdp_fwd_user.c                | 136 +++++++++++++++
 tools/testing/selftests/bpf/bpf_helpers.h |   3 +
 14 files changed, 853 insertions(+), 75 deletions(-)
 create mode 100644 samples/bpf/xdp_fwd_kern.c
 create mode 100644 samples/bpf/xdp_fwd_user.c

Signature · 1 line

2.11.0

David Ahern

author

2018-05-04 02:54 UTC #1 lore raw

PATCH v2 1/9 net/ipv6: Rename fib6_lookup to fib6_node_lookup

Rename fib6_lookup to fib6_node_lookup to better reflect what it returns. The fib6_lookup name will be used in a later patch for an IPv6 equivalent to IPv4's fib_lookup.

Signed-off-by David Ahern <[email protected]>

---
 include/net/ip6_fib.h |  6 +++---
 net/ipv6/ip6_fib.c    | 14 ++++++++------
 net/ipv6/route.c      |  8 ++++----
 3 files changed, 15 insertions(+), 13 deletions(-)

include/net/ip6_fib.h +3 -3

index 1af450d4e923..5a16630179cb 100644
@@ -376,9 +376,9 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 				   const struct sk_buff *skb,
 				   int flags, pol_lookup_t lookup);
 
-struct fib6_node *fib6_lookup(struct fib6_node *root,
-			      const struct in6_addr *daddr,
-			      const struct in6_addr *saddr);
+struct fib6_node *fib6_node_lookup(struct fib6_node *root,
+				   const struct in6_addr *daddr,
+				   const struct in6_addr *saddr);
 
 struct fib6_node *fib6_locate(struct fib6_node *root,
 			      const struct in6_addr *daddr, int dst_len,

net/ipv6/ip6_fib.c +8 -6

index 6421c893466e..4cfffa0f676e 100644
@@ -1354,8 +1354,8 @@ struct lookup_args {
 	const struct in6_addr	*addr;		/* search key			*/
 };
 
-static struct fib6_node *fib6_lookup_1(struct fib6_node *root,
-				       struct lookup_args *args)
+static struct fib6_node *fib6_node_lookup_1(struct fib6_node *root,
+					    struct lookup_args *args)
 {
 	struct fib6_node *fn;
 	__be32 dir;

@@ -1400,7 +1400,8 @@ static struct fib6_node *fib6_lookup_1(struct fib6_node *root,
 #ifdef CONFIG_IPV6_SUBTREES
 				if (subtree) {
 					struct fib6_node *sfn;
-					sfn = fib6_lookup_1(subtree, args + 1);
+					sfn = fib6_node_lookup_1(subtree,
+								 args + 1);
 					if (!sfn)
 						goto backtrack;
 					fn = sfn;

@@ -1422,8 +1423,9 @@ static struct fib6_node *fib6_lookup_1(struct fib6_node *root,
 
 /* called with rcu_read_lock() held
  */
-struct fib6_node *fib6_lookup(struct fib6_node *root, const struct in6_addr *daddr,
-			      const struct in6_addr *saddr)
+struct fib6_node *fib6_node_lookup(struct fib6_node *root,
+				   const struct in6_addr *daddr,
+				   const struct in6_addr *saddr)
 {
 	struct fib6_node *fn;
 	struct lookup_args args[] = {

@@ -1442,7 +1444,7 @@ struct fib6_node *fib6_lookup(struct fib6_node *root, const struct in6_addr *dad
 		}
 	};
 
-	fn = fib6_lookup_1(root, daddr ? args : args + 1);
+	fn = fib6_node_lookup_1(root, daddr ? args : args + 1);
 	if (!fn || fn->fn_flags & RTN_TL_ROOT)
 		fn = root;

net/ipv6/route.c +4 -4

index 7ee0a34fba46..d903db30dfff 100644
@@ -1006,7 +1006,7 @@ static struct fib6_node* fib6_backtrack(struct fib6_node *fn,
 		pn = rcu_dereference(fn->parent);
 		sn = FIB6_SUBTREE(pn);
 		if (sn && sn != fn)
-			fn = fib6_lookup(sn, NULL, saddr);
+			fn = fib6_node_lookup(sn, NULL, saddr);
 		else
 			fn = pn;
 		if (fn->fn_flags & RTN_RTINFO)

@@ -1059,7 +1059,7 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 		flags &= ~RT6_LOOKUP_F_IFACE;
 
 	rcu_read_lock();
-	fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
+	fn = fib6_node_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
 restart:
 	f6i = rcu_dereference(fn->leaf);
 	if (!f6i) {

@@ -1815,7 +1815,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 
 	rcu_read_lock();
 
-	fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
+	fn = fib6_node_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
 	saved_fn = fn;
 
 	if (fl6->flowi6_flags & FLOWI_FLAG_SKIP_NH_OIF)

@@ -2420,7 +2420,7 @@ static struct rt6_info *__ip6_route_redirect(struct net *net,
 	 */
 
 	rcu_read_lock();
-	fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
+	fn = fib6_node_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
 restart:
 	for_each_fib6_node_rt_rcu(fn) {
 		if (rt->fib6_nh.nh_flags & RTNH_F_DEAD)

Signature · 1 line

2.11.0

David Ahern

author

2018-05-04 02:54 UTC #2 lore raw

PATCH v2 2/9 net/ipv6: Rename rt6_multipath_select

In reply to David Ahern

Rename rt6_multipath_select to fib6_multipath_select and export it. A later patch wants access to it similar to IPv4's fib_select_path.

Signed-off-by David Ahern <[email protected]>

Acked-by David S. Miller <[email protected]>

---
 include/net/ip6_fib.h |  5 +++++
 net/ipv6/route.c      | 17 +++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

include/net/ip6_fib.h +5 -0

index 5a16630179cb..80d76d8dc683 100644
@@ -376,6 +376,11 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 				   const struct sk_buff *skb,
 				   int flags, pol_lookup_t lookup);
 
+struct fib6_info *fib6_multipath_select(const struct net *net,
+					struct fib6_info *match,
+					struct flowi6 *fl6, int oif,
+					const struct sk_buff *skb, int strict);
+
 struct fib6_node *fib6_node_lookup(struct fib6_node *root,
 				   const struct in6_addr *daddr,
 				   const struct in6_addr *saddr);

net/ipv6/route.c +9 -8

index d903db30dfff..58af969f3a2c 100644
@@ -419,11 +419,11 @@ static bool rt6_check_expired(const struct rt6_info *rt)
 	return false;
 }
 
-static struct fib6_info *rt6_multipath_select(const struct net *net,
-					      struct fib6_info *match,
-					     struct flowi6 *fl6, int oif,
-					     const struct sk_buff *skb,
-					     int strict)
+struct fib6_info *fib6_multipath_select(const struct net *net,
+					struct fib6_info *match,
+					struct flowi6 *fl6, int oif,
+					const struct sk_buff *skb,
+					int strict)
 {
 	struct fib6_info *sibling, *next_sibling;

@@ -1068,8 +1068,9 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 		f6i = rt6_device_match(net, f6i, &fl6->saddr,
 				      fl6->flowi6_oif, flags);
 		if (f6i->fib6_nsiblings && fl6->flowi6_oif == 0)
-			f6i = rt6_multipath_select(net, f6i, fl6,
-						   fl6->flowi6_oif, skb, flags);
+			f6i = fib6_multipath_select(net, f6i, fl6,
+						    fl6->flowi6_oif, skb,
+						    flags);
 	}
 	if (f6i == net->ipv6.fib6_null_entry) {
 		fn = fib6_backtrack(fn, &fl6->saddr);

@@ -1824,7 +1825,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 redo_rt6_select:
 	f6i = rt6_select(net, fn, oif, strict);
 	if (f6i->fib6_nsiblings)
-		f6i = rt6_multipath_select(net, f6i, fl6, oif, skb, strict);
+		f6i = fib6_multipath_select(net, f6i, fl6, oif, skb, strict);
 	if (f6i == net->ipv6.fib6_null_entry) {
 		fn = fib6_backtrack(fn, &fl6->saddr);
 		if (fn)

Signature · 1 line

2.11.0

David Ahern

author

2018-05-04 02:54 UTC #3 lore raw

PATCH v2 3/9 net/ipv6: Extract table lookup from ip6_pol_route

In reply to David Ahern

ip6_pol_route is used for ingress and egress FIB lookups. Refactor it moving the table lookup into a separate fib6_table_lookup that can be invoked separately and export the new function.

ip6_pol_route now calls fib6_table_lookup and uses the result to generate a dst based rt6_info.

Signed-off-by David Ahern <[email protected]>

Acked-by David S. Miller <[email protected]>

---
 include/net/ip6_fib.h |  4 ++++
 net/ipv6/route.c      | 39 +++++++++++++++++++++++++--------------
 2 files changed, 29 insertions(+), 14 deletions(-)

include/net/ip6_fib.h +4 -0

index 80d76d8dc683..4f7b8f59ea6d 100644
@@ -376,6 +376,10 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 				   const struct sk_buff *skb,
 				   int flags, pol_lookup_t lookup);
 
+/* called with rcu lock held; caller needs to select path */
+struct fib6_info *fib6_table_lookup(struct net *net, struct fib6_table *table,
+				    int oif, struct flowi6 *fl6, int strict);
+
 struct fib6_info *fib6_multipath_select(const struct net *net,
 					struct fib6_info *match,
 					struct flowi6 *fl6, int oif,

net/ipv6/route.c +25 ~~-14~~

index 58af969f3a2c..d0ace0c5c3e9 100644
@@ -1800,21 +1800,12 @@ void rt6_age_exceptions(struct fib6_info *rt,
 	rcu_read_unlock_bh();
 }
 
-struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
-			       int oif, struct flowi6 *fl6,
-			       const struct sk_buff *skb, int flags)
+/* must be called with rcu lock held */
+struct fib6_info *fib6_table_lookup(struct net *net, struct fib6_table *table,
+				    int oif, struct flowi6 *fl6, int strict)
 {
 	struct fib6_node *fn, *saved_fn;
 	struct fib6_info *f6i;
-	struct rt6_info *rt;
-	int strict = 0;
-
-	strict |= flags & RT6_LOOKUP_F_IFACE;
-	strict |= flags & RT6_LOOKUP_F_IGNORE_LINKSTATE;
-	if (net->ipv6.devconf_all->forwarding == 0)
-		strict |= RT6_LOOKUP_F_REACHABLE;
-
-	rcu_read_lock();
 
 	fn = fib6_node_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
 	saved_fn = fn;

@@ -1824,8 +1815,6 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 
 redo_rt6_select:
 	f6i = rt6_select(net, fn, oif, strict);
-	if (f6i->fib6_nsiblings)
-		f6i = fib6_multipath_select(net, f6i, fl6, oif, skb, strict);
 	if (f6i == net->ipv6.fib6_null_entry) {
 		fn = fib6_backtrack(fn, &fl6->saddr);
 		if (fn)

@@ -1838,6 +1827,28 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 		}
 	}
 
+	return f6i;
+}
+
+struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
+			       int oif, struct flowi6 *fl6,
+			       const struct sk_buff *skb, int flags)
+{
+	struct fib6_info *f6i;
+	struct rt6_info *rt;
+	int strict = 0;
+
+	strict |= flags & RT6_LOOKUP_F_IFACE;
+	strict |= flags & RT6_LOOKUP_F_IGNORE_LINKSTATE;
+	if (net->ipv6.devconf_all->forwarding == 0)
+		strict |= RT6_LOOKUP_F_REACHABLE;
+
+	rcu_read_lock();
+
+	f6i = fib6_table_lookup(net, table, oif, fl6, strict);
+	if (f6i->fib6_nsiblings)
+		f6i = fib6_multipath_select(net, f6i, fl6, oif, skb, strict);
+
 	if (f6i == net->ipv6.fib6_null_entry) {
 		rt = net->ipv6.ip6_null_entry;
 		rcu_read_unlock();

Signature · 1 line

2.11.0

David Ahern

author

2018-05-04 02:54 UTC #4 lore raw

PATCH v2 4/9 net/ipv6: Refactor fib6_rule_action

In reply to David Ahern

Move source address lookup from fib6_rule_action to a helper. It will be used in a later patch by a second variant for fib6_rule_action.

Signed-off-by David Ahern <[email protected]>

Acked-by David S. Miller <[email protected]>

---
 net/ipv6/fib6_rules.c | 52 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 21 deletions(-)

net/ipv6/fib6_rules.c +31 ~~-21~~

index 6547fc6491a6..d040c4bff3a0 100644
@@ -96,6 +96,31 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 	return &net->ipv6.ip6_null_entry->dst;
 }
 
+static int fib6_rule_saddr(struct net *net, struct fib_rule *rule, int flags,
+			   struct flowi6 *flp6, const struct net_device *dev)
+{
+	struct fib6_rule *r = (struct fib6_rule *)rule;
+
+	/* If we need to find a source address for this traffic,
+	 * we check the result if it meets requirement of the rule.
+	 */
+	if ((rule->flags & FIB_RULE_FIND_SADDR) &&
+	    r->src.plen && !(flags & RT6_LOOKUP_F_HAS_SADDR)) {
+		struct in6_addr saddr;
+
+		if (ipv6_dev_get_saddr(net, dev, &flp6->daddr,
+				       rt6_flags2srcprefs(flags), &saddr))
+			return -EAGAIN;
+
+		if (!ipv6_prefix_equal(&saddr, &r->src.addr, r->src.plen))
+			return -EAGAIN;
+
+		flp6->saddr = saddr;
+	}
+
+	return 0;
+}
+
 static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
 			    int flags, struct fib_lookup_arg *arg)
 {

@@ -134,27 +159,12 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
 
 	rt = lookup(net, table, flp6, arg->lookup_data, flags);
 	if (rt != net->ipv6.ip6_null_entry) {
-		struct fib6_rule *r = (struct fib6_rule *)rule;
-
-		/*
-		 * If we need to find a source address for this traffic,
-		 * we check the result if it meets requirement of the rule.
-		 */
-		if ((rule->flags & FIB_RULE_FIND_SADDR) &&
-		    r->src.plen && !(flags & RT6_LOOKUP_F_HAS_SADDR)) {
-			struct in6_addr saddr;
-
-			if (ipv6_dev_get_saddr(net,
-					       ip6_dst_idev(&rt->dst)->dev,
-					       &flp6->daddr,
-					       rt6_flags2srcprefs(flags),
-					       &saddr))
-				goto again;
-			if (!ipv6_prefix_equal(&saddr, &r->src.addr,
-					       r->src.plen))
-				goto again;
-			flp6->saddr = saddr;
-		}
+		err = fib6_rule_saddr(net, rule, flags, flp6,
+				      ip6_dst_idev(&rt->dst)->dev);
+
+		if (err == -EAGAIN)
+			goto again;
+
 		err = rt->dst.error;
 		if (err != -EAGAIN)
 			goto out;

Signature · 1 line

2.11.0

David Ahern

author

2018-05-04 02:54 UTC #5 lore raw

PATCH v2 5/9 net/ipv6: Add fib6_lookup

In reply to David Ahern

Add IPv6 equivalent to fib_lookup. Does a fib lookup, including rules, but returns a FIB entry, fib6_info, rather than a dst based rt6_info. fib6_lookup is any where from 140% (MULTIPLE_TABLES config disabled) to 60% faster than any of the dst based lookup methods (without custom rules) and 25% faster with custom rules (e.g., l3mdev rule).

Since the lookup function has a completely different signature, fib6_rule_action is split into 2 paths: the existing one is renamed __fib6_rule_action and a new one for the fib6_info path is added. fib6_rule_action decides which to call based on the lookup_ptr. If it is fib6_table_lookup then the new path is taken.

Caller must hold rcu lock as no reference is taken on the returned fib entry.

Signed-off-by David Ahern <[email protected]>

Acked-by David S. Miller <[email protected]>

---
 include/net/ip6_fib.h |  6 ++++
 net/ipv6/fib6_rules.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++--
 net/ipv6/ip6_fib.c    |  7 +++++
 3 files changed, 97 insertions(+), 2 deletions(-)

include/net/ip6_fib.h +6 -0

index 4f7b8f59ea6d..d920dd00139b 100644
@@ -376,6 +376,12 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 				   const struct sk_buff *skb,
 				   int flags, pol_lookup_t lookup);
 
+/* called with rcu lock held; can return error pointer
+ * caller needs to select path
+ */
+struct fib6_info *fib6_lookup(struct net *net, int oif, struct flowi6 *fl6,
+			      int flags);
+
 /* called with rcu lock held; caller needs to select path */
 struct fib6_info *fib6_table_lookup(struct net *net, struct fib6_table *table,
 				    int oif, struct flowi6 *fl6, int strict);

net/ipv6/fib6_rules.c +84 -2

104 lines not on this page · raw message

net/ipv6/ip6_fib.c +7 -0

index 4cfffa0f676e..0b94c0a631cb 100644
@@ -354,6 +354,13 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 	return &rt->dst;
 }
 
+/* called with rcu lock held; no reference taken on fib6_info */
+struct fib6_info *fib6_lookup(struct net *net, int oif, struct flowi6 *fl6,
+			      int flags)
+{
+	return fib6_table_lookup(net, net->ipv6.fib6_main_tbl, oif, fl6, flags);
+}
+
 static void __net_init fib6_tables_init(struct net *net)
 {
 	fib6_link_table(net, net->ipv6.fib6_main_tbl);

Signature · 1 line

2.11.0

David Ahern

author

2018-05-04 02:54 UTC #6 lore raw

PATCH v2 6/9 net/ipv6: Update fib6 tracepoint to take fib6_info

In reply to David Ahern

Similar to IPv4, IPv6 should use the FIB lookup result in the tracepoint.

Signed-off-by David Ahern <[email protected]>

Acked-by David S. Miller <[email protected]>

---
 include/trace/events/fib6.h | 14 +++++++-------
 net/ipv6/route.c            | 14 ++++++--------
 2 files changed, 13 insertions(+), 15 deletions(-)

include/trace/events/fib6.h +7 -7

index 7e8d48a81b91..1b8d951e3c12 100644
@@ -12,10 +12,10 @@
 
 TRACE_EVENT(fib6_table_lookup,
 
-	TP_PROTO(const struct net *net, const struct rt6_info *rt,
+	TP_PROTO(const struct net *net, const struct fib6_info *f6i,
 		 struct fib6_table *table, const struct flowi6 *flp),
 
-	TP_ARGS(net, rt, table, flp),
+	TP_ARGS(net, f6i, table, flp),
 
 	TP_STRUCT__entry(
 		__field(	u32,	tb_id		)

@@ -48,20 +48,20 @@ TRACE_EVENT(fib6_table_lookup,
 		in6 = (struct in6_addr *)__entry->dst;
 		*in6 = flp->daddr;
 
-		if (rt->rt6i_idev) {
-			__assign_str(name, rt->rt6i_idev->dev->name);
+		if (f6i->fib6_nh.nh_dev) {
+			__assign_str(name, f6i->fib6_nh.nh_dev);
 		} else {
 			__assign_str(name, "");
 		}
-		if (rt == net->ipv6.ip6_null_entry) {
+		if (f6i == net->ipv6.fib6_null_entry) {
 			struct in6_addr in6_zero = {};
 
 			in6 = (struct in6_addr *)__entry->gw;
 			*in6 = in6_zero;
 
-		} else if (rt) {
+		} else if (f6i) {
 			in6 = (struct in6_addr *)__entry->gw;
-			*in6 = rt->rt6i_gateway;
+			*in6 = f6i->fib6_nh.nh_gw;
 		}
 	),

net/ipv6/route.c +6 -8

index d0ace0c5c3e9..cf8de6899581 100644
@@ -1078,6 +1078,8 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 			goto restart;
 	}
 
+	trace_fib6_table_lookup(net, f6i, table, fl6);
+
 	/* Search through exception table */
 	rt = rt6_find_cached_rt(f6i, &fl6->daddr, &fl6->saddr);
 	if (rt) {

@@ -1096,8 +1098,6 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 
 	rcu_read_unlock();
 
-	trace_fib6_table_lookup(net, rt, table, fl6);
-
 	return rt;
 }

@@ -1827,6 +1827,8 @@ struct fib6_info *fib6_table_lookup(struct net *net, struct fib6_table *table,
 		}
 	}
 
+	trace_fib6_table_lookup(net, f6i, table, fl6);
+
 	return f6i;
 }

@@ -1853,7 +1855,6 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 		rt = net->ipv6.ip6_null_entry;
 		rcu_read_unlock();
 		dst_hold(&rt->dst);
-		trace_fib6_table_lookup(net, rt, table, fl6);
 		return rt;
 	}

@@ -1864,7 +1865,6 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 			dst_use_noref(&rt->dst, jiffies);
 
 		rcu_read_unlock();
-		trace_fib6_table_lookup(net, rt, table, fl6);
 		return rt;
 	} else if (unlikely((fl6->flowi6_flags & FLOWI_FLAG_KNOWN_NH) &&
 			    !(f6i->fib6_flags & RTF_GATEWAY))) {

@@ -1890,9 +1890,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 			dst_hold(&uncached_rt->dst);
 		}
 
-		trace_fib6_table_lookup(net, uncached_rt, table, fl6);
 		return uncached_rt;
-
 	} else {
 		/* Get a percpu copy */

@@ -1906,7 +1904,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 
 		local_bh_enable();
 		rcu_read_unlock();
-		trace_fib6_table_lookup(net, pcpu_rt, table, fl6);
+
 		return pcpu_rt;
 	}
 }

@@ -2486,7 +2484,7 @@ static struct rt6_info *__ip6_route_redirect(struct net *net,
 
 	rcu_read_unlock();
 
-	trace_fib6_table_lookup(net, ret, table, fl6);
+	trace_fib6_table_lookup(net, rt, table, fl6);
 	return ret;
 };

Signature · 1 line

2.11.0

David Ahern

author

2018-05-04 02:54 UTC #7 lore raw

PATCH v2 7/9 net/ipv6: Add fib lookup stubs for use in bpf helper

In reply to David Ahern

Add stubs to retrieve a handle to an IPv6 FIB table, fib6_get_table, a stub to do a lookup in a specific table, fib6_table_lookup, and a stub for a full route lookup.

The stubs are needed for core bpf code to handle the case when the IPv6 module is not builtin.

Signed-off-by David Ahern <[email protected]>

Acked-by David S. Miller <[email protected]>

---
 include/net/addrconf.h   | 14 ++++++++++++++
 net/ipv6/addrconf_core.c | 33 ++++++++++++++++++++++++++++++++-
 net/ipv6/af_inet6.c      |  6 +++++-
 3 files changed, 51 insertions(+), 2 deletions(-)

include/net/addrconf.h +14 -0

index 8312cc25a3af..ff766ab207e0 100644
@@ -223,6 +223,20 @@ struct ipv6_stub {
 				 const struct in6_addr *addr);
 	int (*ipv6_dst_lookup)(struct net *net, struct sock *sk,
 			       struct dst_entry **dst, struct flowi6 *fl6);
+
+	struct fib6_table *(*fib6_get_table)(struct net *net, u32 id);
+	struct fib6_info *(*fib6_lookup)(struct net *net, int oif,
+					 struct flowi6 *fl6, int flags);
+	struct fib6_info *(*fib6_table_lookup)(struct net *net,
+					      struct fib6_table *table,
+					      int oif, struct flowi6 *fl6,
+					      int flags);
+	struct fib6_info *(*fib6_multipath_select)(const struct net *net,
+						   struct fib6_info *f6i,
+						   struct flowi6 *fl6, int oif,
+						   const struct sk_buff *skb,
+						   int strict);
+
 	void (*udpv6_encap_enable)(void);
 	void (*ndisc_send_na)(struct net_device *dev, const struct in6_addr *daddr,
 			      const struct in6_addr *solicited_addr,

net/ipv6/addrconf_core.c +32 -1

index 32b564dfd02a..2fe754fd4f5e 100644
@@ -134,8 +134,39 @@ static int eafnosupport_ipv6_dst_lookup(struct net *net, struct sock *u1,
 	return -EAFNOSUPPORT;
 }
 
+static struct fib6_table *eafnosupport_fib6_get_table(struct net *net, u32 id)
+{
+	return NULL;
+}
+
+static struct fib6_info *
+eafnosupport_fib6_table_lookup(struct net *net, struct fib6_table *table,
+			       int oif, struct flowi6 *fl6, int flags)
+{
+	return NULL;
+}
+
+static struct fib6_info *
+eafnosupport_fib6_lookup(struct net *net, int oif, struct flowi6 *fl6,
+			 int flags)
+{
+	return NULL;
+}
+
+static struct fib6_info *
+eafnosupport_fib6_multipath_select(const struct net *net, struct fib6_info *f6i,
+				   struct flowi6 *fl6, int oif,
+				   const struct sk_buff *skb, int strict)
+{
+	return f6i;
+}
+
 const struct ipv6_stub *ipv6_stub __read_mostly = &(struct ipv6_stub) {
-	.ipv6_dst_lookup = eafnosupport_ipv6_dst_lookup,
+	.ipv6_dst_lookup   = eafnosupport_ipv6_dst_lookup,
+	.fib6_get_table    = eafnosupport_fib6_get_table,
+	.fib6_table_lookup = eafnosupport_fib6_table_lookup,
+	.fib6_lookup       = eafnosupport_fib6_lookup,
+	.fib6_multipath_select = eafnosupport_fib6_multipath_select,
 };
 EXPORT_SYMBOL_GPL(ipv6_stub);

net/ipv6/af_inet6.c +5 -1

index 36d622c477b1..c0e8255d50bb 100644
@@ -887,7 +887,11 @@ static struct pernet_operations inet6_net_ops = {
 static const struct ipv6_stub ipv6_stub_impl = {
 	.ipv6_sock_mc_join = ipv6_sock_mc_join,
 	.ipv6_sock_mc_drop = ipv6_sock_mc_drop,
-	.ipv6_dst_lookup = ip6_dst_lookup,
+	.ipv6_dst_lookup   = ip6_dst_lookup,
+	.fib6_get_table	   = fib6_get_table,
+	.fib6_table_lookup = fib6_table_lookup,
+	.fib6_lookup       = fib6_lookup,
+	.fib6_multipath_select = fib6_multipath_select,
 	.udpv6_encap_enable = udpv6_encap_enable,
 	.ndisc_send_na = ndisc_send_na,
 	.nd_tbl	= &nd_tbl,

Signature · 1 line

2.11.0

David Ahern

author

2018-05-04 02:54 UTC #8 lore raw

PATCH v2 8/9 bpf: Provide helper to do forwarding lookups in kernel FIB table

In reply to David Ahern

If it is to be forwarded, the forwarding can be done directly if the neighbor is already known. If the neighbor does not exist, the first few packets go up the stack for neighbor resolution. Once resolved, the xdp program provides the fast path.

On successful lookup the nexthop dmac, current device smac and egress device index are returned.

The API supports IPv4, IPv6 and MPLS protocols, but only IPv4 and IPv6 are implemented in this patch. The API includes layer 4 parameters if the XDP program chooses to do deep packet inspection to allow compare against ACLs implemented as FIB rules.

Header rewrite is left to the XDP program.

The lookup takes 2 flags:
- BPF_FIB_LOOKUP_DIRECT to do a lookup that bypasses FIB rules and goes straight to the table associated with the device (expert setting for those looking to maximize throughput)

- BPF_FIB_LOOKUP_OUTPUT to do a lookup from the egress perspective. Default is an ingress lookup.

Initial performance numbers collected by Jesper, forwarded packets/sec:

Full stack XDP FIB lookup XDP Direct lookup

IPv4   1,947,969       7,074,156          7,415,333
IPv6   1,728,000       6,165,504          7,262,720

These number are single CPU core forwarding on a Broadwell E5-1650 v4 @ 3.60GHz.

Signed-off-by David Ahern <[email protected]>

---
 include/uapi/linux/bpf.h |  83 ++++++++++++++-
 net/core/filter.c        | 266 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 348 insertions(+), 1 deletion(-)

include/uapi/linux/bpf.h +82 -1

index 93d5a4eeec2a..ddc566cb7492 100644
@@ -10,6 +10,8 @@
 
 #include <linux/types.h>
 #include <linux/bpf_common.h>
+#include <linux/if_ether.h>
+#include <linux/in6.h>
 
 /* Extended instruction set based on top of classic BPF */

@@ -1826,6 +1828,33 @@ union bpf_attr {
  * 	Return
  * 		0 on success, or a negative error in case of failure.
  *
+ *
+ * int bpf_fib_lookup(void *ctx, struct bpf_fib_lookup *params, int plen, u32 flags)
+ *	Description
+ *		Do FIB lookup in kernel tables using parameters in *params*.
+ *		If lookup is successful and result shows packet is to be
+ *		forwarded, the neighbor tables are searched for the nexthop.
+ *		If successful (ie., FIB lookup shows forwarding and nexthop
+ *		is resolved), the nexthop address is returned in ipv4_dst,
+ *		ipv6_dst or mpls_out based on family, smac is set to mac
+ *		address of egress device, dmac is set to nexthop mac address,
+ *		rt_metric is set to metric from route.
+ *
+ *             *plen* argument is the size of the passed in struct.
+ *             *flags* argument can be one or more BPF_FIB_LOOKUP_ flags:
+ *
+ *             **BPF_FIB_LOOKUP_DIRECT** means do a direct table lookup vs
+ *             full lookup using FIB rules
+ *             **BPF_FIB_LOOKUP_OUTPUT** means do lookup from an egress
+ *             perspective (default is ingress)
+ *
+ *             *ctx* is either **struct xdp_md** for XDP programs or
+ *             **struct sk_buff** tc cls_act programs.
+ *
+ *     Return
+ *             Egress device index on success, 0 if packet needs to continue
+ *             up the stack for further processing or a negative error in case
+ *             of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\

@@ -1896,7 +1925,8 @@ union bpf_attr {
 	FN(xdp_adjust_tail),		\
 	FN(skb_get_xfrm_state),		\
 	FN(get_stack),			\
-	FN(skb_load_bytes_relative),
+	FN(skb_load_bytes_relative),	\
+	FN(fib_lookup),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call

@@ -2310,4 +2340,55 @@ struct bpf_raw_tracepoint_args {
 	__u64 args[0];
 };
 
+/* DIRECT:  Skip the FIB rules and go to FIB table associated with device
+ * OUTPUT:  Do lookup from egress perspective; default is ingress
+ */
+#define BPF_FIB_LOOKUP_DIRECT  BIT(0)
+#define BPF_FIB_LOOKUP_OUTPUT  BIT(1)
+
+struct bpf_fib_lookup {
+	/* input */
+	__u8	family;   /* network family, AF_INET, AF_INET6, AF_MPLS */
+
+	/* set if lookup is to consider L4 data - e.g., FIB rules */
+	__u8	l4_protocol;
+	__be16	sport;
+	__be16	dport;
+
+	/* total length of packet from network header - used for MTU check */
+	__u16	tot_len;
+	__u32	ifindex;  /* L3 device index for lookup */
+
+	union {
+		/* inputs to lookup */
+		__u8	tos;		/* AF_INET  */
+		__be32	flowlabel;	/* AF_INET6 */
+
+		/* output: metric of fib result */
+		__u32 rt_metric;
+	};
+
+	union {
+		__be32		mpls_in;
+		__be32		ipv4_src;
+		struct in6_addr	ipv6_src;
+	};
+
+	/* input to bpf_fib_lookup, *dst is destination address.
+	 * output: bpf_fib_lookup sets to gateway address
+	 */
+	union {
+		/* return for MPLS lookups */
+		__be32		mpls_out[4];  /* support up to 4 labels */
+		__be32		ipv4_dst;
+		struct in6_addr	ipv6_dst;
+	};
+
+	/* output */
+	__be16	h_vlan_proto;
+	__be16	h_vlan_TCI;
+	__u8	smac[ETH_ALEN];
+	__u8	dmac[ETH_ALEN];
+};
+
 #endif /* _UAPI__LINUX_BPF_H__ */

net/core/filter.c +266 -0

290 lines not on this page · raw message

Signature · 1 line

2.11.0

Jesper Dangaard Brouer

2018-05-07 13:36 UTC #9 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

Quoted 4 lines · On Thu, 3 May 2018 19:54:31 -0700 David Ahern <[email protected]> wrote:

net/core/filter.c +0 -0
index 6877426c23a6..cf0d27acf1d1 100644

[...]

net/core/filter.c static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = { · line 4260

Quoted 3 lines

net/core/filter.c +3 -0

+static const struct bpf_func_proto bpf_xdp_fib_lookup_proto = {
+	.func		= bpf_xdp_fib_lookup,
+	.gpl_only	= true,

Is it a deliberate choice to require BPF-progs using this helper to be GPL licensed?

Asking as this seems to be the first network related helper with this requirement, while this is typical for tracing related helpers.

Quoted 6 lines

net/core/filter.c +6 -0

+	.ret_type	= RET_INTEGER,
+	.arg1_type      = ARG_PTR_TO_CTX,
+	.arg2_type      = ARG_PTR_TO_MEM,
+	.arg3_type      = ARG_CONST_SIZE,
+	.arg4_type	= ARG_ANYTHING,
+};

Signature · 4 lines

Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Daniel Borkmann

maintainer

2018-05-07 14:10 UTC #10 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

Quoted 14 lines · Jesper Dangaard Brouer

Quoted 4 lines · On Thu, 3 May 2018 19:54:31 -0700 David Ahern <[email protected]> wrote:
net/core/filter.c +0 -0
index 6877426c23a6..cf0d27acf1d1 100644
[...]
net/core/filter.c static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = { · line 4260
Quoted 3 lines
net/core/filter.c +3 -0
+static const struct bpf_func_proto bpf_xdp_fib_lookup_proto = {
+	.func		= bpf_xdp_fib_lookup,
+	.gpl_only	= true,
Is it a deliberate choice to require BPF-progs using this helper to be GPL licensed?

Quoted 2 lines

Asking as this seems to be the first network related helper with this requirement, while this is typical for tracing related helpers.

Good point, we should remove that. In networking it's only the perf event output helpers tying into tracing bits. After all, if you do a route lookup via netlink from user space there's no such restriction at all.

David Ahern

author

2018-05-07 14:26 UTC #11 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

Quoted 15 lines · Daniel Borkmann

Quoted 14 lines · Jesper Dangaard Brouer
Quoted 4 lines · On Thu, 3 May 2018 19:54:31 -0700 David Ahern <[email protected]> wrote:
net/core/filter.c +0 -0
index 6877426c23a6..cf0d27acf1d1 100644
[...]
net/core/filter.c static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = { · line 4260
Quoted 3 lines
net/core/filter.c +3 -0
+static const struct bpf_func_proto bpf_xdp_fib_lookup_proto = {
+	.func		= bpf_xdp_fib_lookup,
+	.gpl_only	= true,
Is it a deliberate choice to require BPF-progs using this helper to be GPL licensed?

Quoted 7 lines

Quoted 2 lines
Asking as this seems to be the first network related helper with this requirement, while this is typical for tracing related helpers.
Good point, we should remove that. In networking it's only the perf event output helpers tying into tracing bits. After all, if you do a route lookup via netlink from user space there's no such restriction at all.

Networking symbols are typically exported GPL for modules. The person writing the code and exporting GPL is specifying a desire that only GPL licensed modules can link to the symbol.

Given the common analogy of modules and bpf programs, why can't a writer of a bpf helper specify a preference that only GPL licensed programs leverage a BPF helper?

David Miller

maintainer

2018-05-07 15:36 UTC #12 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

From: David Ahern <[email protected]>
Date: Mon, 7 May 2018 08:26:47 -0600

Quoted 24 lines · Daniel Borkmann

Quoted 15 lines · Jesper Dangaard Brouer
Quoted 14 lines · On Thu, 3 May 2018 19:54:31 -0700 David Ahern <[email protected]> wrote:
Quoted 5 lines
net/core/filter.c +0 -0
index 6877426c23a6..cf0d27acf1d1 100644
[...]
net/core/filter.c static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = { · line 4260
Quoted 3 lines
net/core/filter.c +3 -0
+static const struct bpf_func_proto bpf_xdp_fib_lookup_proto = {
+	.func		= bpf_xdp_fib_lookup,
+	.gpl_only	= true,
Is it a deliberate choice to require BPF-progs using this helper to be GPL licensed?
Quoted 7 lines
Quoted 2 lines
Asking as this seems to be the first network related helper with this requirement, while this is typical for tracing related helpers.
Good point, we should remove that. In networking it's only the perf event output helpers tying into tracing bits. After all, if you do a route lookup via netlink from user space there's no such restriction at all.

Quoted 7 lines

Networking symbols are typically exported GPL for modules. The person writing the code and exporting GPL is specifying a desire that only GPL licensed modules can link to the symbol.
Given the common analogy of modules and bpf programs, why can't a writer of a bpf helper specify a preference that only GPL licensed programs leverage a BPF helper?

I also think that for this particular set of helpers GPL is appropriate.

Yes, via netlink the same lookup can happen, but not with the same level of performance and microsecond tuning we've done over the years on this sophisticated trie lookup code.

Therefore, I think David's choice is very appropriate.

Daniel Borkmann

maintainer

2018-05-09 08:15 UTC #13 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

In reply to David Ahern

Quoted 30 lines · David Ahern

Provide a helper for doing a FIB and neighbor lookup in the kernel tables from an XDP program. The helper provides a fastpath for forwarding packets. If the packet is a local delivery or for any reason is not a simple lookup and forward, the packet continues up the stack.
If it is to be forwarded, the forwarding can be done directly if the neighbor is already known. If the neighbor does not exist, the first few packets go up the stack for neighbor resolution. Once resolved, the xdp program provides the fast path.
On successful lookup the nexthop dmac, current device smac and egress device index are returned.
The API supports IPv4, IPv6 and MPLS protocols, but only IPv4 and IPv6 are implemented in this patch. The API includes layer 4 parameters if the XDP program chooses to do deep packet inspection to allow compare against ACLs implemented as FIB rules.
Header rewrite is left to the XDP program.
The lookup takes 2 flags:
- BPF_FIB_LOOKUP_DIRECT to do a lookup that bypasses FIB rules and goes straight to the table associated with the device (expert setting for those looking to maximize throughput)
- BPF_FIB_LOOKUP_OUTPUT to do a lookup from the egress perspective. Default is an ingress lookup.
Initial performance numbers collected by Jesper, forwarded packets/sec:

Quoted 8 lines

Full stack XDP FIB lookup XDP Direct lookup
IPv4   1,947,969       7,074,156          7,415,333
IPv6   1,728,000       6,165,504          7,262,720
These number are single CPU core forwarding on a Broadwell E5-1650 v4 @ 3.60GHz.
Signed-off-by David Ahern <[email protected]>

Ohh well, this is causing allmodconfig build warnings (e.g. on x86) as reported today:

In file included from include/linux/dma-mapping.h:5:0,

                 from include/linux/skbuff.h:34,
                 from include/linux/if_ether.h:23,
                 from include/uapi/linux/bpf.h:13,
                 from include/linux/bpf-cgroup.h:6,
                 from include/linux/cgroup-defs.h:22,
                 from include/linux/cgroup.h:28,
                 from include/linux/perf_event.h:57,
                 from include/linux/trace_events.h:10,
                 from include/trace/trace_events.h:20,
                 from include/trace/define_trace.h:96,
                 from drivers/android/binder_trace.h:387,
                 from drivers/android/binder.c:5702:

include/linux/sizes.h:24:0: warning: "SZ_1K" redefined
#define SZ_1K 0x00000400
drivers/android/binder.c:116:0: note: this is the location of the previous definition
#define SZ_1K 0x400
In file included from include/linux/dma-mapping.h:5:0,

                 from include/linux/skbuff.h:34,
                 from include/linux/if_ether.h:23,
                 from include/uapi/linux/bpf.h:13,
                 from include/linux/bpf-cgroup.h:6,
                 from include/linux/cgroup-defs.h:22,
                 from include/linux/cgroup.h:28,
                 from include/linux/perf_event.h:57,
                 from include/linux/trace_events.h:10,
                 from include/trace/trace_events.h:20,
                 from include/trace/define_trace.h:96,
                 from drivers/android/binder_trace.h:387,
                 from drivers/android/binder.c:5702:

include/linux/sizes.h:37:0: warning: "SZ_4M" redefined
#define SZ_4M 0x00400000
drivers/android/binder.c:120:0: note: this is the location of the previous definition
#define SZ_4M 0x400000
fs/ecryptfs/miscdev.c:206:0: warning: "PKT_TYPE_OFFSET" redefined
#define PKT_TYPE_OFFSET 0
In file included from include/linux/if_ether.h:23:0,
from include/uapi/linux/bpf.h:13,
from include/linux/bpf-cgroup.h:6,
from include/linux/cgroup-defs.h:22,
from include/linux/cgroup.h:28,
from include/linux/writeback.h:183,
from include/linux/backing-dev.h:16,
from fs/ecryptfs/ecryptfs_kernel.h:41,
from fs/ecryptfs/miscdev.c:30:
include/linux/skbuff.h:753:0: note: this is the location of the previous definition
#define PKT_TYPE_OFFSET() offsetof(struct sk_buff, __pkt_type_offset)

Lets get a clean, proper version of the whole series into bpf-next. I've dropped it from there right now and waiting for your v3 respin to apply with the above fixed.

Thank you.

David Ahern

author

2018-05-09 16:05 UTC #14 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

Quoted 2 lines · Daniel Borkmann

Ohh well, this is causing allmodconfig build warnings (e.g. on x86) as reported today:

lovely.

Quoted 14 lines

In file included from include/linux/dma-mapping.h:5:0,

                 from include/linux/skbuff.h:34,
                 from include/linux/if_ether.h:23,
                 from include/uapi/linux/bpf.h:13,
                 from include/linux/bpf-cgroup.h:6,
                 from include/linux/cgroup-defs.h:22,
                 from include/linux/cgroup.h:28,
                 from include/linux/perf_event.h:57,
                 from include/linux/trace_events.h:10,
                 from include/trace/trace_events.h:20,
                 from include/trace/define_trace.h:96,
                 from drivers/android/binder_trace.h:387,
                 from drivers/android/binder.c:5702:

Quoted 4 lines

include/linux/sizes.h:24:0: warning: "SZ_1K" redefined
#define SZ_1K 0x00000400
drivers/android/binder.c:116:0: note: this is the location of the previous definition
#define SZ_1K 0x400

binder.c has very few recent commits to it. Are you ok with me submitting the change with the others in this set (with proper cc's of course)?

Quoted 5 lines

fs/ecryptfs/miscdev.c:206:0: warning: "PKT_TYPE_OFFSET" redefined
#define PKT_TYPE_OFFSET 0
In file included from include/linux/if_ether.h:23:0,
from include/uapi/linux/bpf.h:13,
from include/linux/bpf-cgroup.h:6,

Quoted 8 lines

from include/linux/cgroup-defs.h:22,
from include/linux/cgroup.h:28,
from include/linux/writeback.h:183,
from include/linux/backing-dev.h:16,
from fs/ecryptfs/ecryptfs_kernel.h:41,
from fs/ecryptfs/miscdev.c:30:
include/linux/skbuff.h:753:0: note: this is the location of the previous definition
#define PKT_TYPE_OFFSET() offsetof(struct sk_buff, __pkt_type_offset)

And this one I renamed to SKB_PKT_TYPE_OFFSET

With that it compiles cleanly.

Quoted 6 lines

Lets get a clean, proper version of the whole series into bpf-next. I've dropped it from there right now and waiting for your v3 respin to apply with the above fixed.
Thank you.

Daniel Borkmann

maintainer

2018-05-09 20:44 UTC #15 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

Quoted 42 lines · David Ahern

Quoted 2 lines · Daniel Borkmann
Ohh well, this is causing allmodconfig build warnings (e.g. on x86) as reported today:
lovely.
Quoted 13 lines
In file included from include/linux/dma-mapping.h:5:0,
                 from include/linux/skbuff.h:34,
                 from include/linux/if_ether.h:23,
                 from include/uapi/linux/bpf.h:13,
                 from include/linux/bpf-cgroup.h:6,
                 from include/linux/cgroup-defs.h:22,
                 from include/linux/cgroup.h:28,
                 from include/linux/perf_event.h:57,
                 from include/linux/trace_events.h:10,
                 from include/trace/trace_events.h:20,
                 from include/trace/define_trace.h:96,
                 from drivers/android/binder_trace.h:387,
                 from drivers/android/binder.c:5702:
Quoted 4 lines
include/linux/sizes.h:24:0: warning: "SZ_1K" redefined
#define SZ_1K 0x00000400
drivers/android/binder.c:116:0: note: this is the location of the previous definition
#define SZ_1K 0x400
binder.c has very few recent commits to it. Are you ok with me submitting the change with the others in this set (with proper cc's of course)?
Quoted 5 lines
fs/ecryptfs/miscdev.c:206:0: warning: "PKT_TYPE_OFFSET" redefined
#define PKT_TYPE_OFFSET 0
In file included from include/linux/if_ether.h:23:0,
from include/uapi/linux/bpf.h:13,
from include/linux/bpf-cgroup.h:6,
Quoted 8 lines
from include/linux/cgroup-defs.h:22,
from include/linux/cgroup.h:28,
from include/linux/writeback.h:183,
from include/linux/backing-dev.h:16,
from fs/ecryptfs/ecryptfs_kernel.h:41,
from fs/ecryptfs/miscdev.c:30:
include/linux/skbuff.h:753:0: note: this is the location of the previous definition
#define PKT_TYPE_OFFSET() offsetof(struct sk_buff, __pkt_type_offset)

Quoted 3 lines

And this one I renamed to SKB_PKT_TYPE_OFFSET
With that it compiles cleanly.

Generally, no objection. However, could we get rid of the two extra includes altogether to avoid running into any such dependency issue? Right now the only includes we have in the bpf uapi header is linux/types.h and linux/bpf_common.h (latter has no extra deps by itself). Both the ETH_ALEN and struct in6_addr are in uapi and therefore never allowed to change so we can e.g. avoid to use ETH_ALEN and just have the value instead. In the other places of the header we use __u32 remote_ipv6[4], __u32 src_ip6[4] etc to denote a v6 address, we could do the same here and should be all good then.

David Ahern

author

2018-05-09 21:29 UTC #16 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

Quoted 7 lines · Daniel Borkmann

Generally, no objection. However, could we get rid of the two extra includes altogether to avoid running into any such dependency issue? Right now the only includes we have in the bpf uapi header is linux/types.h and linux/bpf_common.h (latter has no extra deps by itself). Both the ETH_ALEN and struct in6_addr are in uapi and therefore never allowed to change so we can e.g. avoid to use ETH_ALEN and just have the value instead. In the other places of the header we use __u32 remote_ipv6[4], __u32 src_ip6[4] etc to denote a v6 address, we could do the same here and should be all good then.

I was able to drop the include of linux/in6.h and still use in6_addr. I would prefer to keep in6_addr since it works and avoid the need to add typecasts.

As for ETH_ALEN, I could redefine it but it just kicks the can down the road. If if_ether.h is included after bpf.h, it will cause redefinition warnings.

David Ahern

author

2018-05-09 21:39 UTC #17 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

Quoted 9 lines · David Ahern

Quoted 7 lines · Daniel Borkmann
Generally, no objection. However, could we get rid of the two extra includes altogether to avoid running into any such dependency issue? Right now the only includes we have in the bpf uapi header is linux/types.h and linux/bpf_common.h (latter has no extra deps by itself). Both the ETH_ALEN and struct in6_addr are in uapi and therefore never allowed to change so we can e.g. avoid to use ETH_ALEN and just have the value instead. In the other places of the header we use __u32 remote_ipv6[4], __u32 src_ip6[4] etc to denote a v6 address, we could do the same here and should be all good then.

Quoted 3 lines

I was able to drop the include of linux/in6.h and still use in6_addr. I would prefer to keep in6_addr since it works and avoid the need to add typecasts.

Never mind; that was working because if_ether.h was pulling in skbuff.h which included in6.h.

Quoted 5 lines

As for ETH_ALEN, I could redefine it but it just kicks the can down the road. If if_ether.h is included after bpf.h, it will cause redefinition warnings.

I guess I will continue the open coded magic numbers for mac and ipv6 addresses.

Alexei Starovoitov

2018-05-09 21:49 UTC #18 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

Quoted 17 lines · David Ahern

Quoted 9 lines · David Ahern
Quoted 7 lines · Daniel Borkmann
Generally, no objection. However, could we get rid of the two extra includes altogether to avoid running into any such dependency issue? Right now the only includes we have in the bpf uapi header is linux/types.h and linux/bpf_common.h (latter has no extra deps by itself). Both the ETH_ALEN and struct in6_addr are in uapi and therefore never allowed to change so we can e.g. avoid to use ETH_ALEN and just have the value instead. In the other places of the header we use __u32 remote_ipv6[4], __u32 src_ip6[4] etc to denote a v6 address, we could do the same here and should be all good then.
Quoted 3 lines
I was able to drop the include of linux/in6.h and still use in6_addr. I would prefer to keep in6_addr since it works and avoid the need to add typecasts.
Never mind; that was working because if_ether.h was pulling in skbuff.h which included in6.h.
Quoted 1 line

Quoted 7 lines

Quoted 4 lines
As for ETH_ALEN, I could redefine it but it just kicks the can down the road. If if_ether.h is included after bpf.h, it will cause redefinition warnings.
I guess I will continue the open coded magic numbers for mac and ipv6 addresses.

That's the only way.
Adding
+#include <linux/if_ether.h>
+#include <linux/in6.h>
to uapi/bpf.h is no-go. It will cause all sorts of breakage
not only to kernel build as we realized, but to various user space apps too. Please use be32 ipv6[4] and hard coded mac instead.

Daniel Borkmann

maintainer

2018-05-09 21:49 UTC #19 lore raw

Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

In reply to David Ahern

Quoted 17 lines · David Ahern

Quoted 9 lines · David Ahern
Quoted 7 lines · Daniel Borkmann
Generally, no objection. However, could we get rid of the two extra includes altogether to avoid running into any such dependency issue? Right now the only includes we have in the bpf uapi header is linux/types.h and linux/bpf_common.h (latter has no extra deps by itself). Both the ETH_ALEN and struct in6_addr are in uapi and therefore never allowed to change so we can e.g. avoid to use ETH_ALEN and just have the value instead. In the other places of the header we use __u32 remote_ipv6[4], __u32 src_ip6[4] etc to denote a v6 address, we could do the same here and should be all good then.
Quoted 3 lines
I was able to drop the include of linux/in6.h and still use in6_addr. I would prefer to keep in6_addr since it works and avoid the need to add typecasts.
Never mind; that was working because if_ether.h was pulling in skbuff.h which included in6.h.

Quoted 6 lines

Quoted 3 lines
As for ETH_ALEN, I could redefine it but it just kicks the can down the road. If if_ether.h is included after bpf.h, it will cause redefinition warnings.
I guess I will continue the open coded magic numbers for mac and ipv6 addresses.

Agree, it will avoid breakage. We cannot assume that every BPF prog out there has one specific ordering of if_ether.h and bpf.h includes. Open coding the numbers seems best here.

David Ahern

author

2018-05-04 02:55 UTC #20 lore raw

PATCH v2 9/9 samples/bpf: Add example of ipv4 and ipv6 forwarding in XDP

In reply to David Ahern

Simple example of fast-path forwarding. It has a serious flaw in not verifying the egress device index supports XDP forwarding. If the egress device does not packets are dropped.

Take this only as a simple example of fast-path forwarding.

Signed-off-by David Ahern <[email protected]>

Acked-by David S. Miller <[email protected]>

---
 samples/bpf/Makefile                      |   4 +
 samples/bpf/xdp_fwd_kern.c                | 113 +++++++++++++++++++++++++
 samples/bpf/xdp_fwd_user.c                | 136 ++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/bpf_helpers.h |   3 +
 4 files changed, 256 insertions(+)
 create mode 100644 samples/bpf/xdp_fwd_kern.c
 create mode 100644 samples/bpf/xdp_fwd_user.c

samples/bpf/Makefile +4 -0

index 8e0c7fb6d7cc..28513d6be1bf 100644
@@ -46,6 +46,7 @@ hostprogs-y += syscall_tp
 hostprogs-y += cpustat
 hostprogs-y += xdp_adjust_tail
 hostprogs-y += xdpsock
+hostprogs-y += xdp_fwd
 
 # Libbpf dependencies
 LIBBPF := ../../tools/lib/bpf/bpf.o ../../tools/lib/bpf/nlattr.o

@@ -100,6 +101,7 @@ syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o
 cpustat-objs := bpf_load.o $(LIBBPF) cpustat_user.o
 xdp_adjust_tail-objs := bpf_load.o $(LIBBPF) xdp_adjust_tail_user.o
 xdpsock-objs := bpf_load.o $(LIBBPF) xdpsock_user.o
+xdp_fwd-objs := bpf_load.o $(LIBBPF) xdp_fwd_user.o
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)

@@ -154,6 +156,7 @@ always += syscall_tp_kern.o
 always += cpustat_kern.o
 always += xdp_adjust_tail_kern.o
 always += xdpsock_kern.o
+always += xdp_fwd_kern.o
 
 HOSTCFLAGS += -I$(objtree)/usr/include
 HOSTCFLAGS += -I$(srctree)/tools/lib/

@@ -201,6 +204,7 @@ HOSTLOADLIBES_syscall_tp += -lelf
 HOSTLOADLIBES_cpustat += -lelf
 HOSTLOADLIBES_xdp_adjust_tail += -lelf
 HOSTLOADLIBES_xdpsock += -lelf -pthread
+HOSTLOADLIBES_xdp_fwd += -lelf
 
 # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
 #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang

samples/bpf/xdp_fwd_kern.c new file +113 -0

113 lines not on this page · raw message

samples/bpf/xdp_fwd_user.c new file +136 -0

136 lines not on this page · raw message

tools/testing/selftests/bpf/bpf_helpers.h +3 -0

index 265f8e0e8ada..2375d06c706b 100644
@@ -103,6 +103,9 @@ static int (*bpf_skb_get_xfrm_state)(void *ctx, int index, void *state,
 	(void *) BPF_FUNC_skb_get_xfrm_state;
 static int (*bpf_get_stack)(void *ctx, void *buf, int size, int flags) =
 	(void *) BPF_FUNC_get_stack;
+static int (*bpf_fib_lookup)(void *ctx, struct bpf_fib_lookup *params,
+			     int plen, __u32 flags) =
+	(void *) BPF_FUNC_fib_lookup;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions

Signature · 1 line

2.11.0

Daniel Borkmann

maintainer Applied

2018-05-08 23:53 UTC #21 lore raw

In reply to David Ahern

Quoted 28 lines · David Ahern

Provide a helper for doing a FIB and neighbor lookup in the kernel tables from an XDP program. The helper provides a fastpath for forwarding packets. If the packet is a local delivery or for any reason is not a simple lookup and forward, the packet is expected to continue up the stack for full processing.
The response from a FIB and neighbor lookup is either the egress index with the bpf_fib_lookup struct filled in with dmac and gateway or 0 meaning the packet should continue up the stack. In time we can revisit this to return the FIB lookup result errno if it is one of the special RTN_'s such as RTN_BLACKHOLE (-EINVAL) so that the XDP programs can do an early drop if desired.
Patches 1-6 do some more refactoring to IPv6 with the end goal of extracting a FIB lookup function that aligns with fib_lookup for IPv4, basically returning a fib6_info without creating a dst based entry.
Patch 7 adds lookup functions to the ipv6 stub. These are needed since bpf is built into the kernel and ipv6 may not be built or loaded.
Patch 8 adds the bpf helper and 9 adds a sample program.
v2
- removed pkt_access from bpf_func_proto as noticed by Daniel
- added check in that IPv6 forwarding is enabled
- added DaveM's ack on patches 1-7 and 9 based on v1 response and fact that no changes were made to them in v2

Quoted 8 lines

v1
- updated commit messages and cover letter
- added comment to sample program noting lack of verification on egress device supporting XDP
RFC v2
- fixed use of foward helper from cls_act as noted by Daniel
- in patch 1 rename fib6_lookup_1 as well for consistency

Applied to bpf-next, thanks David.