[bpf,v2,4/4] selftests/bpf: Add netkit test for pkt_type

Commit Message

Daniel Borkmann May 24, 2024, 4:36 p.m. UTC

Add a test case to assert that the skb->pkt_type which was set from the BPF
program is retained from the netkit xmit side to the peer's device at tcx
ingress location.

  # ./vmtest.sh -- ./test_progs -t netkit
  [...]
  ./test_progs -t netkit
  [    1.140780] bpf_testmod: loading out-of-tree module taints kernel.
  [    1.141127] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel
  [    1.284601] tsc: Refined TSC clocksource calibration: 3408.006 MHz
  [    1.286672] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x311fd9b189d, max_idle_ns: 440795225691 ns
  [    1.290384] clocksource: Switched to clocksource tsc
  #345     tc_netkit_basic:OK
  #346     tc_netkit_device:OK
  #347     tc_netkit_multi_links:OK
  #348     tc_netkit_multi_opts:OK
  #349     tc_netkit_neigh_links:OK
  #350     tc_netkit_pkt_type:OK
  Summary: 6/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <[e-mail]>
---
 .../selftests/bpf/prog_tests/tc_netkit.c      | 84 +++++++++++++++++++
 .../selftests/bpf/progs/test_tc_link.c        | 33 ++++++++
 2 files changed, 117 insertions(+)

Comments

Martin KaFai Lau May 24, 2024, 10:05 p.m. UTC | #1

On 5/24/24 9:36 AM, Daniel Borkmann wrote:
> diff --git a/tools/testing/selftests/bpf/progs/test_tc_link.c b/tools/testing/selftests/bpf/progs/test_tc_link.c
> index 992400acb957..b64fcb70ef2f 100644
> --- a/tools/testing/selftests/bpf/progs/test_tc_link.c
> +++ b/tools/testing/selftests/bpf/progs/test_tc_link.c
> @@ -4,6 +4,7 @@
>   
>   #include <linux/bpf.h>
>   #include <linux/if_ether.h>
> +#include <linux/if_packet.h>

The set looks good.

A minor thing is that I am hitting this compilation issue in my environment:

In file included from progs/test_tc_link.c:7:
In file included from /usr/include/linux/if_packet.h:5:
In file included from /usr/include/asm/byteorder.h:5:
In file included from /usr/include/linux/byteorder/little_endian.h:13:
/usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline'
   136 | static __always_inline unsigned long __swab(const unsigned long y)
       |        ^

Adding '#include <linux/stddef.h>' solved it. If the addition is fine, this can 
be adjusted before landing.

Alexei Starovoitov May 25, 2024, 5:55 p.m. UTC | #2

On Fri, May 24, 2024 at 3:05 PM Martin KaFai Lau <[e-mail]> wrote:
>
> On 5/24/24 9:36 AM, Daniel Borkmann wrote:
> > diff --git a/tools/testing/selftests/bpf/progs/test_tc_link.c b/tools/testing/selftests/bpf/progs/test_tc_link.c
> > index 992400acb957..b64fcb70ef2f 100644
> > --- a/tools/testing/selftests/bpf/progs/test_tc_link.c
> > +++ b/tools/testing/selftests/bpf/progs/test_tc_link.c
> > @@ -4,6 +4,7 @@
> >
> >   #include <linux/bpf.h>
> >   #include <linux/if_ether.h>
> > +#include <linux/if_packet.h>
>
> The set looks good.
>
> A minor thing is that I am hitting this compilation issue in my environment:
>
> In file included from progs/test_tc_link.c:7:
> In file included from /usr/include/linux/if_packet.h:5:
> In file included from /usr/include/asm/byteorder.h:5:
> In file included from /usr/include/linux/byteorder/little_endian.h:13:
> /usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline'
>    136 | static __always_inline unsigned long __swab(const unsigned long y)
>        |        ^
>
> Adding '#include <linux/stddef.h>' solved it. If the addition is fine, this can
> be adjusted before landing.

I hit the same build issue. So added stddef.h before if_packet.h as
Martin suggested before applying.

Daniel Borkmann May 26, 2024, 10:12 a.m. UTC | #3

On 5/25/24 7:55 PM, Alexei Starovoitov wrote:
> On Fri, May 24, 2024 at 3:05 PM Martin KaFai Lau <[e-mail]> wrote:
>>
>> On 5/24/24 9:36 AM, Daniel Borkmann wrote:
>>> diff --git a/tools/testing/selftests/bpf/progs/test_tc_link.c b/tools/testing/selftests/bpf/progs/test_tc_link.c
>>> index 992400acb957..b64fcb70ef2f 100644
>>> --- a/tools/testing/selftests/bpf/progs/test_tc_link.c
>>> +++ b/tools/testing/selftests/bpf/progs/test_tc_link.c
>>> @@ -4,6 +4,7 @@
>>>
>>>    #include <linux/bpf.h>
>>>    #include <linux/if_ether.h>
>>> +#include <linux/if_packet.h>
>>
>> The set looks good.
>>
>> A minor thing is that I am hitting this compilation issue in my environment:
>>
>> In file included from progs/test_tc_link.c:7:
>> In file included from /usr/include/linux/if_packet.h:5:
>> In file included from /usr/include/asm/byteorder.h:5:
>> In file included from /usr/include/linux/byteorder/little_endian.h:13:
>> /usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline'
>>     136 | static __always_inline unsigned long __swab(const unsigned long y)
>>         |        ^
>>
>> Adding '#include <linux/stddef.h>' solved it. If the addition is fine, this can
>> be adjusted before landing.
> 
> I hit the same build issue. So added stddef.h before if_packet.h as
> Martin suggested before applying.

Awesome, thanks all!

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/tc_netkit.c b/tools/testing/selftests/bpf/prog_tests/tc_netkit.c
index 18b2e969a456..b9135720024c 100644
--- a/tools/testing/selftests/bpf/prog_tests/tc_netkit.c
+++ b/tools/testing/selftests/bpf/prog_tests/tc_netkit.c
@@ -99,6 +99,16 @@  static int create_netkit(int mode, int policy, int peer_policy, int *ifindex,
 	return err;
 }
 
+static void move_netkit(void)
+{
+	ASSERT_OK(system("ip link set " netkit_peer " netns foo"),
+			 "move peer");
+	ASSERT_OK(system("ip netns exec foo ip link set dev "
+			 netkit_peer " up"), "up peer");
+	ASSERT_OK(system("ip netns exec foo ip addr add dev "
+			 netkit_peer " 10.0.0.2/24"), "addr peer");
+}
+
 static void destroy_netkit(void)
 {
 	ASSERT_OK(system("ip link del dev " netkit_name), "del primary");
@@ -695,3 +705,77 @@  void serial_test_tc_netkit_neigh_links(void)
 	serial_test_tc_netkit_neigh_links_target(NETKIT_L2, BPF_NETKIT_PRIMARY);
 	serial_test_tc_netkit_neigh_links_target(NETKIT_L3, BPF_NETKIT_PRIMARY);
 }
+
+static void serial_test_tc_netkit_pkt_type_mode(int mode)
+{
+	LIBBPF_OPTS(bpf_netkit_opts, optl_nk);
+	LIBBPF_OPTS(bpf_tcx_opts, optl_tcx);
+	int err, ifindex, ifindex2;
+	struct test_tc_link *skel;
+	struct bpf_link *link;
+
+	err = create_netkit(mode, NETKIT_PASS, NETKIT_PASS,
+			    &ifindex, true);
+	if (err)
+		return;
+
+	ifindex2 = if_nametoindex(netkit_peer);
+	ASSERT_NEQ(ifindex, ifindex2, "ifindex_1_2");
+
+	skel = test_tc_link__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		goto cleanup;
+
+	ASSERT_EQ(bpf_program__set_expected_attach_type(skel->progs.tc1,
+		  BPF_NETKIT_PRIMARY), 0, "tc1_attach_type");
+	ASSERT_EQ(bpf_program__set_expected_attach_type(skel->progs.tc7,
+		  BPF_TCX_INGRESS), 0, "tc7_attach_type");
+
+	err = test_tc_link__load(skel);
+	if (!ASSERT_OK(err, "skel_load"))
+		goto cleanup;
+
+	assert_mprog_count_ifindex(ifindex,  BPF_NETKIT_PRIMARY, 0);
+	assert_mprog_count_ifindex(ifindex2, BPF_TCX_INGRESS, 0);
+
+	link = bpf_program__attach_netkit(skel->progs.tc1, ifindex, &optl_nk);
+	if (!ASSERT_OK_PTR(link, "link_attach"))
+		goto cleanup;
+
+	skel->links.tc1 = link;
+
+	assert_mprog_count_ifindex(ifindex,  BPF_NETKIT_PRIMARY, 1);
+	assert_mprog_count_ifindex(ifindex2, BPF_TCX_INGRESS, 0);
+
+	link = bpf_program__attach_tcx(skel->progs.tc7, ifindex2, &optl_tcx);
+	if (!ASSERT_OK_PTR(link, "link_attach"))
+		goto cleanup;
+
+	skel->links.tc7 = link;
+
+	assert_mprog_count_ifindex(ifindex,  BPF_NETKIT_PRIMARY, 1);
+	assert_mprog_count_ifindex(ifindex2, BPF_TCX_INGRESS, 1);
+
+	move_netkit();
+
+	tc_skel_reset_all_seen(skel);
+	skel->bss->set_type = true;
+	ASSERT_EQ(send_icmp(), 0, "icmp_pkt");
+
+	ASSERT_EQ(skel->bss->seen_tc1, true, "seen_tc1");
+	ASSERT_EQ(skel->bss->seen_tc7, true, "seen_tc7");
+
+	ASSERT_EQ(skel->bss->seen_host,  true, "seen_host");
+	ASSERT_EQ(skel->bss->seen_mcast, true, "seen_mcast");
+cleanup:
+	test_tc_link__destroy(skel);
+
+	assert_mprog_count_ifindex(ifindex,  BPF_NETKIT_PRIMARY, 0);
+	destroy_netkit();
+}
+
+void serial_test_tc_netkit_pkt_type(void)
+{
+	serial_test_tc_netkit_pkt_type_mode(NETKIT_L2);
+	serial_test_tc_netkit_pkt_type_mode(NETKIT_L3);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_tc_link.c b/tools/testing/selftests/bpf/progs/test_tc_link.c
index 992400acb957..b64fcb70ef2f 100644
--- a/tools/testing/selftests/bpf/progs/test_tc_link.c
+++ b/tools/testing/selftests/bpf/progs/test_tc_link.c
@@ -4,6 +4,7 @@ 
 
 #include <linux/bpf.h>
 #include <linux/if_ether.h>
+#include <linux/if_packet.h>
 
 #include <bpf/bpf_endian.h>
 #include <bpf/bpf_helpers.h>
@@ -16,7 +17,13 @@  bool seen_tc3;
 bool seen_tc4;
 bool seen_tc5;
 bool seen_tc6;
+bool seen_tc7;
+
+bool set_type;
+
 bool seen_eth;
+bool seen_host;
+bool seen_mcast;
 
 SEC("tc/ingress")
 int tc1(struct __sk_buff *skb)
@@ -28,8 +35,16 @@  int tc1(struct __sk_buff *skb)
 	if (bpf_skb_load_bytes(skb, 0, &eth, sizeof(eth)))
 		goto out;
 	seen_eth = eth.h_proto == bpf_htons(ETH_P_IP);
+	seen_host = skb->pkt_type == PACKET_HOST;
+	if (seen_host && set_type) {
+		eth.h_dest[0] = 4;
+		if (bpf_skb_store_bytes(skb, 0, &eth, sizeof(eth), 0))
+			goto fail;
+		bpf_skb_change_type(skb, PACKET_MULTICAST);
+	}
 out:
 	seen_tc1 = true;
+fail:
 	return TCX_NEXT;
 }
 
@@ -67,3 +82,21 @@  int tc6(struct __sk_buff *skb)
 	seen_tc6 = true;
 	return TCX_PASS;
 }
+
+SEC("tc/ingress")
+int tc7(struct __sk_buff *skb)
+{
+	struct ethhdr eth = {};
+
+	if (skb->protocol != __bpf_constant_htons(ETH_P_IP))
+		goto out;
+	if (bpf_skb_load_bytes(skb, 0, &eth, sizeof(eth)))
+		goto out;
+	if (eth.h_dest[0] == 4 && set_type) {
+		seen_mcast = skb->pkt_type == PACKET_MULTICAST;
+		bpf_skb_change_type(skb, PACKET_HOST);
+	}
+out:
+	seen_tc7 = true;
+	return TCX_PASS;
+}