Skip to contentSkip to conversation

bpf: A fix and a change to bpf_fib_lookup

PATCH In review Martin KaFai Lau 3 participants 9 messages Feb 17, 2023

Subsystems (6)
Networking Network drivers Kselftest BPF +2 more subsystems
lore
Share & tools
lore

Martin KaFai Lau

author reviewer
#0 lore raw

cover bpf: A fix and a change to bpf_fib_lookup

From: Martin KaFai Lau <[email protected]>

This set fixes the bpf_fib_lookup such that it won't return a NUD_FAILED neigh which may have invalid dmac. It also adds a SKIP_NEIGH lookup flag to have bpf_fib_lookup skipping the neigh lookup.

Please see individual patch for details.

Martin KaFai Lau (4):
bpf: Disable bh in bpf_test_run for xdp and tc prog
bpf: bpf_fib_lookup should not return neigh in NUD_FAILED state
bpf: Add BPF_FIB_LOOKUP_SKIP_NEIGH for bpf_fib_lookup selftests/bpf: Add bpf_fib_lookup test

 include/uapi/linux/bpf.h                      |   1 +
 net/bpf/test_run.c                            |   2 +
 net/core/filter.c                             |  37 ++--
 tools/include/uapi/linux/bpf.h                |   1 +
 .../selftests/bpf/prog_tests/fib_lookup.c     | 187 ++++++++++++++++++
 .../testing/selftests/bpf/progs/fib_lookup.c  |  22 +++
 6 files changed, 238 insertions(+), 12 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/fib_lookup.c
 create mode 100644 tools/testing/selftests/bpf/progs/fib_lookup.c
Signature · 1 line
2.30.2
author reviewer
#1 lore raw

PATCH 1/4 bpf: Disable bh in bpf_test_run for xdp and tc prog

From: Martin KaFai Lau <[email protected]>

Some of the bpf helpers require bh disabled. eg. The bpf_fib_lookup helper that will be used in a latter selftest. In particular, it calls ___neigh_lookup_noref that expects the bh disabled.

This patch disables bh before calling bpf_prog_run[_xdp], so the testing prog can also use those helpers.

Signed-off-by Martin KaFai Lau <[email protected]>
---
 net/bpf/test_run.c | 2 ++
 1 file changed, 2 insertions(+)
net/bpf/test_run.c +2 -0
index 1ab396a2b87f..982e81bba6cf 100644
@@ -413,10 +413,12 @@ static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
 	old_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
 	do {
 		run_ctx.prog_item = &item;
+		local_bh_disable();
 		if (xdp)
 			*retval = bpf_prog_run_xdp(prog, ctx);
 		else
 			*retval = bpf_prog_run(prog, ctx);
+		local_bh_enable();
 	} while (bpf_test_timer_continue(&t, 1, repeat, &ret, time));
 	bpf_reset_run_ctx(old_ctx);
 	bpf_test_timer_leave(&t);
Signature · 1 line
2.30.2
author reviewer
#2 lore raw

PATCH 2/4 bpf: bpf_fib_lookup should not return neigh in NUD_FAILED state

In reply to Martin KaFai Lau

From: Martin KaFai Lau <[email protected]>

The bpf_fib_lookup() helper does not only look up the fib (ie. route) but it also looks up the neigh. Before returning the neigh, the helper does not check for NUD_VALID. When a neigh state (neigh->nud_state) is in NUD_FAILED, its dmac (neigh->ha) could be all zeros. The helper still returns SUCCESS instead of NO_NEIGH in this case. Because of the SUCCESS return value, the bpf prog directly uses the returned dmac and ends up filling all zero in the eth header.

This patch checks for NUD_VALID and returns NO_NEIGH if the neigh is not valid.

Signed-off-by Martin KaFai Lau <[email protected]>
---
 net/core/filter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
net/core/filter.c +2 -2
index 2ce06a72a5ba..8daaaf76ab15 100644
@@ -5849,7 +5849,7 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 		neigh = __ipv6_neigh_lookup_noref_stub(dev, dst);
 	}
 
-	if (!neigh)
+	if (!neigh || !(neigh->nud_state & NUD_VALID))
 		return BPF_FIB_LKUP_RET_NO_NEIGH;
 
 	return bpf_fib_set_fwd_params(params, neigh, dev, mtu);

@@ -5964,7 +5964,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	 * not needed here.
 	 */
 	neigh = __ipv6_neigh_lookup_noref_stub(dev, dst);
-	if (!neigh)
+	if (!neigh || !(neigh->nud_state & NUD_VALID))
 		return BPF_FIB_LKUP_RET_NO_NEIGH;
 
 	return bpf_fib_set_fwd_params(params, neigh, dev, mtu);
Signature · 1 line
2.30.2
author reviewer
#3 lore raw

PATCH 3/4 bpf: Add BPF_FIB_LOOKUP_SKIP_NEIGH for bpf_fib_lookup

In reply to Martin KaFai Lau

From: Martin KaFai Lau <[email protected]>

The bpf_fib_lookup() also looks up the neigh table. This was done before bpf_redirect_neigh() was added.

In the use case that does not manage the neigh table and requires bpf_fib_lookup() to lookup a fib to decide if it needs to redirect or not, the bpf prog can depend only on using bpf_redirect_neigh() to lookup the neigh. It also keeps the neigh entries fresh and connected.

This patch adds a bpf_fib_lookup flag, SKIP_NEIGH, to avoid the double neigh lookup when the bpf prog always call bpf_redirect_neigh() to do the neigh lookup.

Signed-off-by Martin KaFai Lau <[email protected]>
---
 include/uapi/linux/bpf.h       |  1 +
 net/core/filter.c              | 33 +++++++++++++++++++++++----------
 tools/include/uapi/linux/bpf.h |  1 +
 3 files changed, 25 insertions(+), 10 deletions(-)
include/uapi/linux/bpf.h +1 -0
index 1503f61336b6..6c1956e36c97 100644
@@ -6750,6 +6750,7 @@ struct bpf_raw_tracepoint_args {
 enum {
 	BPF_FIB_LOOKUP_DIRECT  = (1U << 0),
 	BPF_FIB_LOOKUP_OUTPUT  = (1U << 1),
+	BPF_FIB_LOOKUP_SKIP_NEIGH = (1U << 2),
 };
 
 enum {
net/core/filter.c +23 -10
index 8daaaf76ab15..08f0f21863bc 100644
@@ -5723,10 +5723,8 @@ static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = {
 
 #if IS_ENABLED(CONFIG_INET) || IS_ENABLED(CONFIG_IPV6)
 static int bpf_fib_set_fwd_params(struct bpf_fib_lookup *params,
-				  const struct neighbour *neigh,
 				  const struct net_device *dev, u32 mtu)
 {
-	memcpy(params->dmac, neigh->ha, ETH_ALEN);
 	memcpy(params->smac, dev->dev_addr, ETH_ALEN);
 	params->h_vlan_TCI = 0;
 	params->h_vlan_proto = 0;

@@ -5838,21 +5836,28 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	if (likely(nhc->nhc_gw_family != AF_INET6)) {
 		if (nhc->nhc_gw_family)
 			params->ipv4_dst = nhc->nhc_gw.ipv4;
-
-		neigh = __ipv4_neigh_lookup_noref(dev,
-						 (__force u32)params->ipv4_dst);
 	} else {
 		struct in6_addr *dst = (struct in6_addr *)params->ipv6_dst;
 
 		params->family = AF_INET6;
 		*dst = nhc->nhc_gw.ipv6;
-		neigh = __ipv6_neigh_lookup_noref_stub(dev, dst);
 	}
 
+	if (flags & BPF_FIB_LOOKUP_SKIP_NEIGH)
+		goto set_fwd_params;
+
+	if (params->family == AF_INET6)
+		neigh = __ipv6_neigh_lookup_noref_stub(dev, params->ipv6_dst);
+	else
+		neigh = __ipv4_neigh_lookup_noref(dev,
+						  (__force u32)params->ipv4_dst);
+
 	if (!neigh || !(neigh->nud_state & NUD_VALID))
 		return BPF_FIB_LKUP_RET_NO_NEIGH;
+	memcpy(params->dmac, neigh->ha, ETH_ALEN);
 
-	return bpf_fib_set_fwd_params(params, neigh, dev, mtu);
+set_fwd_params:
+	return bpf_fib_set_fwd_params(params, dev, mtu);
 }
 #endif
 

@@ -5960,24 +5965,32 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	params->rt_metric = res.f6i->fib6_metric;
 	params->ifindex = dev->ifindex;
 
+	if (flags & BPF_FIB_LOOKUP_SKIP_NEIGH)
+		goto set_fwd_params;
+
 	/* xdp and cls_bpf programs are run in RCU-bh so rcu_read_lock_bh is
 	 * not needed here.
 	 */
 	neigh = __ipv6_neigh_lookup_noref_stub(dev, dst);
 	if (!neigh || !(neigh->nud_state & NUD_VALID))
 		return BPF_FIB_LKUP_RET_NO_NEIGH;
+	memcpy(params->dmac, neigh->ha, ETH_ALEN);
 
-	return bpf_fib_set_fwd_params(params, neigh, dev, mtu);
+set_fwd_params:
+	return bpf_fib_set_fwd_params(params, dev, mtu);
 }
 #endif
 
+#define BPF_FIB_LOOKUP_MASK (BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_OUTPUT | \
+			     BPF_FIB_LOOKUP_SKIP_NEIGH)
+
 BPF_CALL_4(bpf_xdp_fib_lookup, struct xdp_buff *, ctx,
 	   struct bpf_fib_lookup *, params, int, plen, u32, flags)
 {
 	if (plen < sizeof(*params))
 		return -EINVAL;
 
-	if (flags & ~(BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_OUTPUT))
+	if (flags & ~BPF_FIB_LOOKUP_MASK)
 		return -EINVAL;
 
 	switch (params->family) {

@@ -6015,7 +6028,7 @@ BPF_CALL_4(bpf_skb_fib_lookup, struct sk_buff *, skb,
 	if (plen < sizeof(*params))
 		return -EINVAL;
 
-	if (flags & ~(BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_OUTPUT))
+	if (flags & ~BPF_FIB_LOOKUP_MASK)
 		return -EINVAL;
 
 	if (params->tot_len)
tools/include/uapi/linux/bpf.h +1 -0
index 1503f61336b6..6c1956e36c97 100644
@@ -6750,6 +6750,7 @@ struct bpf_raw_tracepoint_args {
 enum {
 	BPF_FIB_LOOKUP_DIRECT  = (1U << 0),
 	BPF_FIB_LOOKUP_OUTPUT  = (1U << 1),
+	BPF_FIB_LOOKUP_SKIP_NEIGH = (1U << 2),
 };
 
 enum {
Signature · 1 line
2.30.2
maintainer
#4 lore raw

Re: [PATCH bpf-next 3/4] bpf: Add BPF_FIB_LOOKUP_SKIP_NEIGH for bpf_fib_lookup

Quoted 26 lines · Martin KaFai Lau

From: Martin KaFai Lau <[email protected]>

The bpf_fib_lookup() also looks up the neigh table. This was done before bpf_redirect_neigh() was added.

In the use case that does not manage the neigh table and requires bpf_fib_lookup() to lookup a fib to decide if it needs to redirect or not, the bpf prog can depend only on using bpf_redirect_neigh() to lookup the neigh. It also keeps the neigh entries fresh and connected.

This patch adds a bpf_fib_lookup flag, SKIP_NEIGH, to avoid the double neigh lookup when the bpf prog always call bpf_redirect_neigh() to do the neigh lookup.

Signed-off-by Martin KaFai Lau <[email protected]>
---
  include/uapi/linux/bpf.h       |  1 +
  net/core/filter.c              | 33 +++++++++++++++++++++++----------
  tools/include/uapi/linux/bpf.h |  1 +
  3 files changed, 25 insertions(+), 10 deletions(-)
include/uapi/linux/bpf.h +0 -0
index 1503f61336b6..6c1956e36c97 100644

[...]

Quoted 9 lines
include/uapi/linux/bpf.h +0 -3
@@ -5838,21 +5836,28 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
  	if (likely(nhc->nhc_gw_family != AF_INET6)) {
  		if (nhc->nhc_gw_family)
  			params->ipv4_dst = nhc->nhc_gw.ipv4;
-
-		neigh = __ipv4_neigh_lookup_noref(dev,
-						 (__force u32)params->ipv4_dst);
  	} else {
  		struct in6_addr *dst = (struct in6_addr *)params->ipv6_dst;

include/uapi/linux/bpf.h static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params, · line 5849

Quoted 10 lines
include/uapi/linux/bpf.h +4 -1
 
  		params->family = AF_INET6;
  		*dst = nhc->nhc_gw.ipv6;
-		neigh = __ipv6_neigh_lookup_noref_stub(dev, dst);
  	}
 
+	if (flags & BPF_FIB_LOOKUP_SKIP_NEIGH)
+		goto set_fwd_params;
+
+	if (params->family == AF_INET6)

Nit, would have probably more intuitive to keep the same test also here (nhc->nhc_gw_family != AF_INET6), but either way, lgtm.

Are you still required to fill the params->smac in bpf_fib_set_fwd_params() in that case, meaning, shouldn't bpf_redirect_neigh() take care of it as well from neigh_output()? Looks unnecessary and could be moved out too.

(Took in first 2 in the meantime which look good.)

Quoted 12 lines
include/uapi/linux/bpf.h +8 -1
+		neigh = __ipv6_neigh_lookup_noref_stub(dev, params->ipv6_dst);
+	else
+		neigh = __ipv4_neigh_lookup_noref(dev,
+						  (__force u32)params->ipv4_dst);
+
  	if (!neigh || !(neigh->nud_state & NUD_VALID))
  		return BPF_FIB_LKUP_RET_NO_NEIGH;
+	memcpy(params->dmac, neigh->ha, ETH_ALEN);
 
-	return bpf_fib_set_fwd_params(params, neigh, dev, mtu);
+set_fwd_params:
+	return bpf_fib_set_fwd_params(params, dev, mtu);
author reviewer
#5 lore raw

Re: [PATCH bpf-next 3/4] bpf: Add BPF_FIB_LOOKUP_SKIP_NEIGH for bpf_fib_lookup

Quoted 18 lines · Daniel Borkmann
Quoted 4 lines
include/uapi/linux/bpf.h +0 -0
index 1503f61336b6..6c1956e36c97 100644

[...]

Quoted 13 lines
include/uapi/linux/bpf.h +0 -4
@@ -5838,21 +5836,28 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct 
 bpf_fib_lookup *params,
       if (likely(nhc->nhc_gw_family != AF_INET6)) {
           if (nhc->nhc_gw_family)
               params->ipv4_dst = nhc->nhc_gw.ipv4;
-
-        neigh = __ipv4_neigh_lookup_noref(dev,
-                         (__force u32)params->ipv4_dst);
       } else {
           struct in6_addr *dst = (struct in6_addr *)params->ipv6_dst;
           params->family = AF_INET6;
           *dst = nhc->nhc_gw.ipv6;
-        neigh = __ipv6_neigh_lookup_noref_stub(dev, dst);
Quoted 8 lines

include/uapi/linux/bpf.h static int bpf_ipv4_fib_lookup(struct net *net, struct · line 5848

Quoted 5 lines
include/uapi/linux/bpf.h +4 -0
       }
+    if (flags & BPF_FIB_LOOKUP_SKIP_NEIGH)
+        goto set_fwd_params;
+
+    if (params->family == AF_INET6)

Nit, would have probably more intuitive to keep the same test also here (nhc->nhc_gw_family != AF_INET6), but either way, lgtm.

Ack.

Quoted 4 lines

Are you still required to fill the params->smac in bpf_fib_set_fwd_params() in that case, meaning, shouldn't bpf_redirect_neigh() take care of it as well from neigh_output()? Looks unnecessary and could be moved out too.

Good point. will move it out from bpf_fib_set_fwd_params also. Thanks for the review.

author reviewer
#6 lore raw

PATCH 4/4 selftests/bpf: Add bpf_fib_lookup test

In reply to Martin KaFai Lau

From: Martin KaFai Lau <[email protected]>

This patch tests the bpf_fib_lookup helper when looking up a neigh in NUD_FAILED and NUD_STALE state. It also adds test for the new BPF_FIB_LOOKUP_SKIP_NEIGH flag.

Signed-off-by Martin KaFai Lau <[email protected]>
---
 .../selftests/bpf/prog_tests/fib_lookup.c     | 187 ++++++++++++++++++
 .../testing/selftests/bpf/progs/fib_lookup.c  |  22 +++
 2 files changed, 209 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/fib_lookup.c
 create mode 100644 tools/testing/selftests/bpf/progs/fib_lookup.c
tools/testing/selftests/bpf/prog_tests/fib_lookup.c new file +187 -0
new file mode 100644
index 000000000000..61ccddccf485
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include <sys/types.h>
+#include <net/if.h>
+
+#include "test_progs.h"
+#include "network_helpers.h"
+#include "fib_lookup.skel.h"
+
+#define SYS(fmt, ...)						\
+	({							\
+		char cmd[1024];					\
+		snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__);	\
+		if (!ASSERT_OK(system(cmd), cmd))		\
+			goto fail;				\
+	})
+
+#define NS_TEST			"fib_lookup_ns"
+#define IPV6_IFACE_ADDR		"face::face"
+#define IPV6_NUD_FAILED_ADDR	"face::1"
+#define IPV6_NUD_STALE_ADDR	"face::2"
+#define IPV4_IFACE_ADDR		"10.0.0.254"
+#define IPV4_NUD_FAILED_ADDR	"10.0.0.1"
+#define IPV4_NUD_STALE_ADDR	"10.0.0.2"
+#define DMAC			"11:11:11:11:11:11"
+#define DMAC_INIT { 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, }
+
+struct fib_lookup_test {
+	const char *desc;
+	const char *daddr;
+	int expected_ret;
+	int lookup_flags;
+	__u8 dmac[6];
+};
+
+static const struct fib_lookup_test tests[] = {
+	{ .desc = "IPv6 failed neigh",
+	  .daddr = IPV6_NUD_FAILED_ADDR, .expected_ret = BPF_FIB_LKUP_RET_NO_NEIGH, },
+	{ .desc = "IPv6 stale neigh",
+	  .daddr = IPV6_NUD_STALE_ADDR, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS,
+	  .dmac = DMAC_INIT, },
+	{ .desc = "IPv6 skip neigh",
+	  .daddr = IPV6_NUD_FAILED_ADDR, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS,
+	  .lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH, },
+	{ .desc = "IPv4 failed neigh",
+	  .daddr = IPV4_NUD_FAILED_ADDR, .expected_ret = BPF_FIB_LKUP_RET_NO_NEIGH, },
+	{ .desc = "IPv4 stale neigh",
+	  .daddr = IPV4_NUD_STALE_ADDR, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS,
+	  .dmac = DMAC_INIT, },
+	{ .desc = "IPv4 skip neigh",
+	  .daddr = IPV4_NUD_FAILED_ADDR, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS,
+	  .lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH, },
+};
+
+static int ifindex;
+
+static int setup_netns(void)
+{
+	int err;
+
+	SYS("ip link add veth1 type veth peer name veth2");
+	SYS("ip link set dev veth1 up");
+
+	SYS("ip addr add %s/64 dev veth1 nodad", IPV6_IFACE_ADDR);
+	SYS("ip neigh add %s dev veth1 nud failed", IPV6_NUD_FAILED_ADDR);
+	SYS("ip neigh add %s dev veth1 lladdr %s nud stale", IPV6_NUD_STALE_ADDR, DMAC);
+
+	SYS("ip addr add %s/24 dev veth1 nodad", IPV4_IFACE_ADDR);
+	SYS("ip neigh add %s dev veth1 nud failed", IPV4_NUD_FAILED_ADDR);
+	SYS("ip neigh add %s dev veth1 lladdr %s nud stale", IPV4_NUD_STALE_ADDR, DMAC);
+
+	err = write_sysctl("/proc/sys/net/ipv4/conf/veth1/forwarding", "1");
+	if (!ASSERT_OK(err, "write_sysctl(net.ipv4.conf.veth1.forwarding)"))
+		goto fail;
+
+	err = write_sysctl("/proc/sys/net/ipv6/conf/veth1/forwarding", "1");
+	if (!ASSERT_OK(err, "write_sysctl(net.ipv6.conf.veth1.forwarding)"))
+		goto fail;
+
+	return 0;
+fail:
+	return -1;
+}
+
+static int set_lookup_params(struct bpf_fib_lookup *params, const char *daddr)
+{
+	int ret;
+
+	memset(params, 0, sizeof(*params));
+
+	params->l4_protocol = IPPROTO_TCP;
+	params->ifindex = ifindex;
+
+	if (inet_pton(AF_INET6, daddr, params->ipv6_dst) == 1) {
+		params->family = AF_INET6;
+		ret = inet_pton(AF_INET6, IPV6_IFACE_ADDR, params->ipv6_src);
+		if (!ASSERT_EQ(ret, 1, "inet_pton(IPV6_IFACE_ADDR)"))
+			return -1;
+		return 0;
+	}
+
+	ret = inet_pton(AF_INET, daddr, &params->ipv4_dst);
+	if (!ASSERT_EQ(ret, 1, "convert IP[46] address"))
+		return -1;
+	params->family = AF_INET;
+	ret = inet_pton(AF_INET, IPV4_IFACE_ADDR, &params->ipv4_src);
+	if (!ASSERT_EQ(ret, 1, "inet_pton(IPV4_IFACE_ADDR)"))
+		return -1;
+
+	return 0;
+}
+
+static void mac_str(char *b, const __u8 *mac)
+{
+	sprintf(b, "%02X:%02X:%02X:%02X:%02X:%02X",
+		mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
+}
+
+void test_fib_lookup(void)
+{
+	struct bpf_fib_lookup *fib_params;
+	struct nstoken *nstoken = NULL;
+	struct __sk_buff skb = { };
+	struct fib_lookup *skel;
+	int prog_fd, err, ret, i;
+
+	/* The test does not use the skb->data, so
+	 * use pkt_v6 for both v6 and v4 test.
+	 */
+	LIBBPF_OPTS(bpf_test_run_opts, run_opts,
+		    .data_in = &pkt_v6,
+		    .data_size_in = sizeof(pkt_v6),
+		    .ctx_in = &skb,
+		    .ctx_size_in = sizeof(skb),
+	);
+
+	skel = fib_lookup__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel open_and_load"))
+		return;
+	prog_fd = bpf_program__fd(skel->progs.fib_lookup);
+
+	SYS("ip netns add %s", NS_TEST);
+
+	nstoken = open_netns(NS_TEST);
+	if (!ASSERT_OK_PTR(nstoken, "open_netns"))
+		goto fail;
+
+	if (setup_netns())
+		goto fail;
+
+	ifindex = if_nametoindex("veth1");
+	skb.ifindex = ifindex;
+	fib_params = &skel->bss->fib_params;
+
+	for (i = 0; i < ARRAY_SIZE(tests); i++) {
+		printf("Testing %s\n", tests[i].desc);
+
+		if (set_lookup_params(fib_params, tests[i].daddr))
+			continue;
+		skel->bss->fib_lookup_ret = -1;
+		skel->bss->lookup_flags = BPF_FIB_LOOKUP_OUTPUT |
+			tests[i].lookup_flags;
+
+		err = bpf_prog_test_run_opts(prog_fd, &run_opts);
+		if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
+			continue;
+
+		ASSERT_EQ(tests[i].expected_ret, skel->bss->fib_lookup_ret,
+			  "fib_lookup_ret");
+
+		ret = memcmp(tests[i].dmac, fib_params->dmac, sizeof(tests[i].dmac));
+		if (!ASSERT_EQ(ret, 0, "dmac not match")) {
+			char expected[18], actual[18];
+
+			mac_str(expected, tests[i].dmac);
+			mac_str(actual, fib_params->dmac);
+			printf("dmac expected %s actual %s\n", expected, actual);
+		}
+	}
+
+fail:
+	if (nstoken)
+		close_netns(nstoken);
+	system("ip netns del " NS_TEST " &> /dev/null");
+	fib_lookup__destroy(skel);
+}
tools/testing/selftests/bpf/progs/fib_lookup.c new file +22 -0
new file mode 100644
index 000000000000..c4514dd58c62
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include <linux/types.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_tracing_net.h"
+
+struct bpf_fib_lookup fib_params = {};
+int fib_lookup_ret = 0;
+int lookup_flags = 0;
+
+SEC("tc")
+int fib_lookup(struct __sk_buff *skb)
+{
+	fib_lookup_ret = bpf_fib_lookup(skb, &fib_params, sizeof(fib_params),
+					lookup_flags);
+
+	return TC_ACT_SHOT;
+}
+
+char _license[] SEC("license") = "GPL";
Signature · 1 line
2.30.2
#7 lore raw

Re: [PATCH bpf-next 4/4] selftests/bpf: Add bpf_fib_lookup test

Quoted 28 lines · Martin KaFai Lau

From: Martin KaFai Lau <[email protected]>

This patch tests the bpf_fib_lookup helper when looking up a neigh in NUD_FAILED and NUD_STALE state. It also adds test for the new BPF_FIB_LOOKUP_SKIP_NEIGH flag.

Signed-off-by Martin KaFai Lau <[email protected]>
---
 .../selftests/bpf/prog_tests/fib_lookup.c     | 187 ++++++++++++++++++
 .../testing/selftests/bpf/progs/fib_lookup.c  |  22 +++
 2 files changed, 209 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/fib_lookup.c
 create mode 100644 tools/testing/selftests/bpf/progs/fib_lookup.c
tools/testing/selftests/bpf/prog_tests/fib_lookup.c new file +7 -0
new file mode 100644
index 000000000000..61ccddccf485
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include <sys/types.h>
+#include <net/if.h>
+
+#include "test_progs.h"

tools/testing/selftests/bpf/prog_tests/fib_lookup.c line 17

Quoted 10 lines
tools/testing/selftests/bpf/prog_tests/fib_lookup.c new file +10 -0
+#include "network_helpers.h"
+#include "fib_lookup.skel.h"
+
+#define SYS(fmt, ...)                                          \
+       ({                                                      \
+               char cmd[1024];                                 \
+               snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__); \
+               if (!ASSERT_OK(system(cmd), cmd))               \
+                       goto fail;                              \
+       })

it's probably a high time to move this SYS() macro into test_progs.h and stop copy/pasting it across many test?

[...]

author reviewer
#8 lore raw

Re: [PATCH bpf-next 4/4] selftests/bpf: Add bpf_fib_lookup test

Quoted 10 lines · Andrii Nakryiko

tools/testing/selftests/bpf/prog_tests/fib_lookup.c line 17

Quoted 7 lines
tools/testing/selftests/bpf/prog_tests/fib_lookup.c new file +7 -0
+#define SYS(fmt, ...)                                          \
+       ({                                                      \
+               char cmd[1024];                                 \
+               snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__); \
+               if (!ASSERT_OK(system(cmd), cmd))               \
+                       goto fail;                              \
+       })

it's probably a high time to move this SYS() macro into test_progs.h and stop copy/pasting it across many test?

Make sense. will follow up.