Re: [PATCH bpf-next v2 12/13] selftests/bpf: migrate bpf flow dissectors tests to test_progs

On 11/14, Alexis Lothoré (eBPF Foundation) wrote:

test_flow_dissector.sh loads flow_dissector program and subprograms, creates and configured relevant tunnels and interfaces, and ensure that the bpf dissection is actually performed correctly. Similar tests exist in test_progs (thanks to flow_dissector.c) and run the same programs, but those are only executed with BPF_PROG_RUN: those tests are then missing some coverage (eg: coverage for flow keys manipulated when the configured flower uses a port range, which has a dedicated test in test_flow_dissector.sh)

Convert test_flow_dissector.sh into test_progs so that the corresponding tests are also run in CI.

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com

Changes in v2:

• use new helpers added to network_helpers

The content of this new test is heavily based on the initial test_flow_dissector.c. I have kept most of the packet build function (even if not all packet types are used for the test) to allow extending the test later if needed.

The new test has been executed in a local x86 qemu environment as well as in CI:

# ./test_progs -a flow_dissector_classification #102/1 flow_dissector_classification/ipv4:OK #102/2 flow_dissector_classification/ipv4_continue_dissect:OK #102/3 flow_dissector_classification/ipip:OK #102/4 flow_dissector_classification/gre:OK #102/5 flow_dissector_classification/port_range:OK #102/6 flow_dissector_classification/ipv6:OK

#102 flow_dissector_classification:OK

.../bpf/prog_tests/flow_dissector_classification.c | 807 +++++++++++++++++++++ 1 file changed, 807 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c new file mode 100644 index 0000000000000000000000000000000000000000..e9bd145ee8f0cbee3487bc1e241f3c6b5f25fecb --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c @@ -0,0 +1,807 @@ +// SPDX-License-Identifier: GPL-2.0

+#define _GNU_SOURCE +#include <stdbool.h> +#include <stdlib.h> +#include <stdio.h> +#include <bpf/bpf.h> +#include <linux/bpf.h> +#include <bpf/libbpf.h> +#include <arpa/inet.h> +#include <asm/byteorder.h> +#include <netinet/udp.h> +#include <poll.h> +#include <string.h> +#include <sys/ioctl.h> +#include <sys/socket.h> +#include <sys/time.h> +#include <unistd.h> +#include "test_progs.h" +#include "network_helpers.h" +#include "bpf_util.h" +#include "bpf_flow.skel.h"

+#define CFG_PORT_INNER 8000 +#define CFG_PORT_GUE 6080 +#define SUBTEST_NAME_MAX_LEN 32 +#define TEST_NAME_MAX_LEN (32 + SUBTEST_NAME_MAX_LEN) +#define MAX_SOURCE_PORTS 3 +#define TEST_PACKETS_COUNT 10 +#define TEST_PACKET_LEN 100 +#define TEST_PACKET_PATTERN 'a' +#define TEST_IPV4 "192.168.0.1/32" +#define TEST_IPV6 "100::a/128" +#define TEST_TUNNEL_REMOTE "127.0.0.2" +#define TEST_TUNNEL_LOCAL "127.0.0.1"

+#define INIT_ADDR4(addr4, port) \

• { \

• .sin_family = AF_INET,				\
  

• .sin_port = __constant_htons(port),		\
  

• .sin_addr.s_addr = __constant_htonl(addr4),	\
  

• }

+#define INIT_ADDR6(addr6, port) \

• { \

• .sin6_family = AF_INET6,		\
  

• .sin6_port = __constant_htons(port),	\
  

• .sin6_addr = addr6,			\
  

• }

+#define TEST_IN4_SRC_ADDR_DEFAULT INIT_ADDR4(INADDR_LOOPBACK + 2, 0) +#define TEST_IN4_DST_ADDR_DEFAULT INIT_ADDR4(INADDR_LOOPBACK, CFG_PORT_INNER) +#define TEST_OUT4_SRC_ADDR_DEFAULT INIT_ADDR4(INADDR_LOOPBACK + 1, 0) +#define TEST_OUT4_DST_ADDR_DEFAULT INIT_ADDR4(INADDR_LOOPBACK, 0)

+#define TEST_IN6_SRC_ADDR_DEFAULT INIT_ADDR6(IN6ADDR_LOOPBACK_INIT, 0) +#define TEST_IN6_DST_ADDR_DEFAULT \

• INIT_ADDR6(IN6ADDR_LOOPBACK_INIT, CFG_PORT_INNER)

+#define TEST_OUT6_SRC_ADDR_DEFAULT INIT_ADDR6(IN6ADDR_LOOPBACK_INIT, 0) +#define TEST_OUT6_DST_ADDR_DEFAULT INIT_ADDR6(IN6ADDR_LOOPBACK_INIT, 0)

+#define TEST_IN4_SRC_ADDR_DISSECT_CONTINUE INIT_ADDR4(INADDR_LOOPBACK + 126, 0) +#define TEST_IN4_SRC_ADDR_IPIP INIT_ADDR4((in_addr_t)0x01010101, 0) +#define TEST_IN4_DST_ADDR_IPIP INIT_ADDR4((in_addr_t)0xC0A80001, CFG_PORT_INNER)

+struct grehdr {

• uint16_t unused;
• uint16_t protocol;

+} __packed;

+struct guehdr {

• union {

• struct {
  

+#if defined(__LITTLE_ENDIAN_BITFIELD)

• 	__u8 hlen : 5, control : 1, version : 2;
  

+#elif defined(__BIG_ENDIAN_BITFIELD)

• 	__u8 version : 2, control : 1, hlen : 5;
  

+#else +#error "Please fix <asm/byteorder.h>" +#endif

• 	__u8 proto_ctype;
  

• 	__be16 flags;
  

• };
  

• __be32 word;
  

• };

+};

+static char buf[ETH_DATA_LEN];

+struct test_configuration {

• char name[SUBTEST_NAME_MAX_LEN];
• int (*test_setup)(void);
• void (*test_teardown)(void);
• int source_ports[MAX_SOURCE_PORTS];
• int cfg_l3_inner;
• struct sockaddr_in in_saddr4;
• struct sockaddr_in in_daddr4;
• struct sockaddr_in6 in_saddr6;
• struct sockaddr_in6 in_daddr6;
• int cfg_l3_outer;
• struct sockaddr_in out_saddr4;
• struct sockaddr_in out_daddr4;
• struct sockaddr_in6 out_saddr6;
• struct sockaddr_in6 out_daddr6;
• int cfg_encap_proto;
• uint8_t cfg_dsfield_inner;
• uint8_t cfg_dsfield_outer;
• int cfg_l3_extra;
• struct sockaddr_in extra_saddr4;
• struct sockaddr_in extra_daddr4;
• struct sockaddr_in6 extra_saddr6;
• struct sockaddr_in6 extra_daddr6;

+};

+static unsigned long util_gettime(void) +{

• struct timeval tv;
• gettimeofday(&tv, NULL);
• return (tv.tv_sec * 1000) + (tv.tv_usec / 1000);

+}

+static void build_ipv4_header(void *header, uint8_t proto, uint32_t src,

• 	      uint32_t dst, int payload_len, uint8_t tos)
  

+{

• struct iphdr *iph = header;
• iph->ihl = 5;
• iph->version = 4;
• iph->tos = tos;
• iph->ttl = 8;
• iph->tot_len = htons(sizeof(*iph) + payload_len);
• iph->id = htons(1337);
• iph->protocol = proto;
• iph->saddr = src;
• iph->daddr = dst;
• iph->check = build_ip_csum((void *)iph);

+}

+static void ipv6_set_dsfield(struct ipv6hdr *ip6h, uint8_t dsfield) +{

• uint16_t val, *ptr = (uint16_t *)ip6h;
• val = ntohs(*ptr);
• val &= 0xF00F;
• val |= ((uint16_t)dsfield) << 4;
• *ptr = htons(val);

+}

+static void build_ipv6_header(void *header, uint8_t proto,

• 	      struct sockaddr_in6 *src,
  

• 	      struct sockaddr_in6 *dst, int payload_len,
  

• 	      uint8_t dsfield)
  

+{

• struct ipv6hdr *ip6h = header;
• ip6h->version = 6;
• ip6h->payload_len = htons(payload_len);
• ip6h->nexthdr = proto;
• ip6h->hop_limit = 8;
• ipv6_set_dsfield(ip6h, dsfield);
• memcpy(&ip6h->saddr, &src->sin6_addr, sizeof(ip6h->saddr));
• memcpy(&ip6h->daddr, &dst->sin6_addr, sizeof(ip6h->daddr));

+}

+static void build_udp_header(void *header, int payload_len, uint16_t sport,

• 	     uint16_t dport, int family)
  

+{

• struct udphdr *udph = header;
• int len = sizeof(*udph) + payload_len;
• udph->source = htons(sport);
• udph->dest = htons(dport);
• udph->len = htons(len);
• udph->check = 0;
• if (family == AF_INET)

• udph->check = build_udp_v4_csum(header - sizeof(struct iphdr),
  

• 				IPPROTO_UDP, udph->len,
  

• 				udph, len >> 1);
  

• else

• udph->check = build_udp_v6_csum(header - sizeof(struct ipv6hdr),
  

• 				udph, len >> 1);
  

+}

+static void build_gue_header(void *header, uint8_t proto) +{

• struct guehdr *gueh = header;
• gueh->proto_ctype = proto;

+}

+static void build_gre_header(void *header, uint16_t proto) +{

• struct grehdr *greh = header;
• greh->protocol = htons(proto);

+}

+static int l3_length(int family) +{

• if (family == AF_INET)

• return sizeof(struct iphdr);
  

• else

• return sizeof(struct ipv6hdr);
  

+}

+static int build_packet(struct test_configuration *test, uint16_t sport) +{

• int ol3_len = 0, ol4_len = 0, il3_len = 0, il4_len = 0;
• int el3_len = 0, packet_len;
• memset(buf, 0, ETH_DATA_LEN);
• if (test->cfg_l3_extra)

• el3_len = l3_length(test->cfg_l3_extra);
  

• /* calculate header offsets */
• if (test->cfg_encap_proto) {

• ol3_len = l3_length(test->cfg_l3_outer);
  

• if (test->cfg_encap_proto == IPPROTO_GRE)
  

• 	ol4_len = sizeof(struct grehdr);
  

• else if (test->cfg_encap_proto == IPPROTO_UDP)
  

• 	ol4_len = sizeof(struct udphdr) + sizeof(struct guehdr);
  

• }
• il3_len = l3_length(test->cfg_l3_inner);
• il4_len = sizeof(struct udphdr);
• packet_len = el3_len + ol3_len + ol4_len + il3_len + il4_len +

•      TEST_PACKET_LEN;
  

• if (!ASSERT_LE(packet_len, sizeof(buf), "check packet size"))

• return -1;
  

• /*

• * Fill packet from inside out, to calculate correct checksums.
  

• * But create ip before udp headers, as udp uses ip for pseudo-sum.
  

• */
  

• memset(buf + el3_len + ol3_len + ol4_len + il3_len + il4_len,

•       TEST_PACKET_PATTERN, TEST_PACKET_LEN);
  

• /* add zero byte for udp csum padding */
• buf[el3_len + ol3_len + ol4_len + il3_len + il4_len + TEST_PACKET_LEN] =

• 0;
  

• switch (test->cfg_l3_inner) {
• case PF_INET:

• build_ipv4_header(buf + el3_len + ol3_len + ol4_len,
  

• 		  IPPROTO_UDP, test->in_saddr4.sin_addr.s_addr,
  

• 		  test->in_daddr4.sin_addr.s_addr,
  

• 		  il4_len + TEST_PACKET_LEN,
  

• 		  test->cfg_dsfield_inner);
  

• break;
  

• case PF_INET6:

• build_ipv6_header(buf + el3_len + ol3_len + ol4_len,
  

• 		  IPPROTO_UDP, &test->in_saddr6,
  

• 		  &test->in_daddr6, il4_len + TEST_PACKET_LEN,
  

• 		  test->cfg_dsfield_inner);
  

• break;
  

• }
• build_udp_header(buf + el3_len + ol3_len + ol4_len + il3_len,

• 	 TEST_PACKET_LEN, sport, CFG_PORT_INNER,
  

• 	 test->cfg_l3_inner);
  

• if (!test->cfg_encap_proto)

• return il3_len + il4_len + TEST_PACKET_LEN;
  

• switch (test->cfg_l3_outer) {
• case PF_INET:

• build_ipv4_header(buf + el3_len, test->cfg_encap_proto,
  

• 		  test->out_saddr4.sin_addr.s_addr,
  

• 		  test->out_daddr4.sin_addr.s_addr,
  

• 		  ol4_len + il3_len + il4_len + TEST_PACKET_LEN,
  

• 		  test->cfg_dsfield_outer);
  

• break;
  

• case PF_INET6:

• build_ipv6_header(buf + el3_len, test->cfg_encap_proto,
  

• 		  &test->out_saddr6, &test->out_daddr6,
  

• 		  ol4_len + il3_len + il4_len + TEST_PACKET_LEN,
  

• 		  test->cfg_dsfield_outer);
  

• break;
  

• }
• switch (test->cfg_encap_proto) {
• case IPPROTO_UDP:

• build_gue_header(buf + el3_len + ol3_len + ol4_len -
  

• 			 sizeof(struct guehdr),
  

• 		 test->cfg_l3_inner == PF_INET ? IPPROTO_IPIP :
  

• 						 IPPROTO_IPV6);
  

• build_udp_header(buf + el3_len + ol3_len,
  

• 		 sizeof(struct guehdr) + il3_len + il4_len +
  

• 			 TEST_PACKET_LEN,
  

• 		 sport, CFG_PORT_GUE, test->cfg_l3_outer);
  

• break;
  

• case IPPROTO_GRE:

• build_gre_header(buf + el3_len + ol3_len,
  

• 		 test->cfg_l3_inner == PF_INET ? ETH_P_IP :
  

• 						 ETH_P_IPV6);
  

• break;
  

• }
• switch (test->cfg_l3_extra) {
• case PF_INET:

• build_ipv4_header(buf,
  

• 		  test->cfg_l3_outer == PF_INET ? IPPROTO_IPIP :
  

• 						  IPPROTO_IPV6,
  

• 		  test->extra_saddr4.sin_addr.s_addr,
  

• 		  test->extra_daddr4.sin_addr.s_addr,
  

• 		  ol3_len + ol4_len + il3_len + il4_len +
  

• 			  TEST_PACKET_LEN,
  

• 		  0);
  

• break;
  

• case PF_INET6:

• build_ipv6_header(buf,
  

• 		  test->cfg_l3_outer == PF_INET ? IPPROTO_IPIP :
  

• 						  IPPROTO_IPV6,
  

• 		  &test->extra_saddr6, &test->extra_daddr6,
  

• 		  ol3_len + ol4_len + il3_len + il4_len +
  

• 			  TEST_PACKET_LEN,
  

• 		  0);
  

• break;
  

• }
• return el3_len + ol3_len + ol4_len + il3_len + il4_len +

•       TEST_PACKET_LEN;
  

+}

+/* sender transmits encapsulated over RAW or unencap'd over UDP */ +static int setup_tx(struct test_configuration *test) +{

• int family, fd, ret;
• if (test->cfg_l3_extra)

• family = test->cfg_l3_extra;
  

• else if (test->cfg_l3_outer)

• family = test->cfg_l3_outer;
  

• else

• family = test->cfg_l3_inner;
  

• fd = socket(family, SOCK_RAW, IPPROTO_RAW);
• if (!ASSERT_OK_FD(fd, "setup tx socket"))

• return fd;
  

• if (test->cfg_l3_extra) {

• if (test->cfg_l3_extra == PF_INET)
  

• 	ret = connect(fd, (void *)&test->extra_daddr4,
  

• 		      sizeof(test->extra_daddr4));
  

• else
  

• 	ret = connect(fd, (void *)&test->extra_daddr6,
  

• 		      sizeof(test->extra_daddr6));
  

• if (!ASSERT_OK(ret, "connect")) {
  

• 	close(fd);
  

• 	return ret;
  

• }
  

• } else if (test->cfg_l3_outer) {

• /* connect to destination if not encapsulated */
  

• if (test->cfg_l3_outer == PF_INET)
  

• 	ret = connect(fd, (void *)&test->out_daddr4,
  

• 		      sizeof(test->out_daddr4));
  

• else
  

• 	ret = connect(fd, (void *)&test->out_daddr6,
  

• 		      sizeof(test->out_daddr6));
  

• if (!ASSERT_OK(ret, "connect")) {
  

• 	close(fd);
  

• 	return ret;
  

• }
  

• } else {

• /* otherwise using loopback */
  

• if (test->cfg_l3_inner == PF_INET)
  

• 	ret = connect(fd, (void *)&test->in_daddr4,
  

• 		      sizeof(test->in_daddr4));
  

• else
  

• 	ret = connect(fd, (void *)&test->in_daddr6,
  

• 		      sizeof(test->in_daddr6));
  

• if (!ASSERT_OK(ret, "connect")) {
  

• 	close(fd);
  

• 	return ret;
  

• }
  

• }
• return fd;

+}

+/* receiver reads unencapsulated UDP */ +static int setup_rx(struct test_configuration *test) +{

• int fd, ret;
• fd = socket(test->cfg_l3_inner, SOCK_DGRAM, 0);
• if (!ASSERT_OK_FD(fd, "socket rx"))

• return fd;
  

• if (test->cfg_l3_inner == PF_INET)

• ret = bind(fd, (void *)&test->in_daddr4,
  

• 	   sizeof(test->in_daddr4));
  

• else

• ret = bind(fd, (void *)&test->in_daddr6,
  

• 	   sizeof(test->in_daddr6));
  

• if (!ASSERT_OK(ret, "bind rx")) {

• close(fd);
  

• return ret;
  

• }
• return fd;

+}

+static int do_tx(int fd, const char *pkt, int len) +{

• int ret;
• ret = write(fd, pkt, len);
• return ret != len;

+}

+static int do_poll(int fd, short events, int timeout) +{

• struct pollfd pfd;
• int ret;
• pfd.fd = fd;
• pfd.events = events;
• ret = poll(&pfd, 1, timeout);
• return ret;

+}

+static int do_rx(int fd) +{

• char rbuf;
• int ret, num = 0;
• while (1) {

• ret = recv(fd, &rbuf, 1, MSG_DONTWAIT);
  

• if (ret == -1 && errno == EAGAIN)
  

• 	break;
  

• if (!ASSERT_GE(ret, 0, "check recv return code"))
  

• 	return -1;
  

• if (!ASSERT_EQ(rbuf, TEST_PACKET_PATTERN, "check pkt pattern"))
  

• 	return -1;
  

• num++;
  

• }
• return num;

+}

+static int run_test(struct test_configuration *test, int source_port_index) +{

• int fdt = -1, fdr = -1, len, tx = 0, rx = 0, err;
• unsigned long tstop, tcur;
• fdr = setup_rx(test);
• fdt = setup_tx(test);
• if (!ASSERT_OK_FD(fdr, "setup rx") || !ASSERT_OK_FD(fdt, "setup tx")) {

• err = -1;
  

• goto out_close_sockets;
  

• }
• len = build_packet(test,

• 	   (uint16_t)test->source_ports[source_port_index]);
  

• if (!ASSERT_GT(len, 0, "build test packet"))

• return -1;
  

• tcur = util_gettime();
• tstop = tcur;
• while (tx < TEST_PACKETS_COUNT) {

• if (!ASSERT_OK(do_tx(fdt, buf, len), "do_tx"))
  

• 	break;
  

• tx++;
  

• err = do_rx(fdr);
  

[..]

• if (!ASSERT_GE(err, 0, "do_rx"))
  

• 	break;
  

You seem to be already doing similar ASSERT_GE inside the do_rx, maybe drop one?

• rx += err;
  

• }
• /* read straggler packets, if any */
• if (rx < tx) {

• tstop = util_gettime() + 100;
  

• while (rx < tx) {
  

• 	tcur = util_gettime();
  

• 	if (tcur >= tstop)
  

• 		break;
  

• 	err = do_poll(fdr, POLLIN, tstop - tcur);
  

• 	if (err < 0)
  

• 		break;
  

• 	err = do_rx(fdr);
  

• 	if (err >= 0)
  

• 		rx += err;
  

• }
  

• }

+out_close_sockets:

• close(fdt);
• close(fdr);
• return rx;

+}

+static int attach_and_configure_program(struct bpf_flow *skel) +{

• struct bpf_map *prog_array = skel->maps.jmp_table;
• int main_prog_fd, sub_prog_fd, map_fd, i, err;
• struct bpf_program *prog;
• char prog_name[32];
• main_prog_fd = bpf_program__fd(skel->progs._dissect);
• if (main_prog_fd < 0)

• return main_prog_fd;
  

• err = bpf_prog_attach(main_prog_fd, 0, BPF_FLOW_DISSECTOR, 0);
• if (err)

• return err;
  

• map_fd = bpf_map__fd(prog_array);
• if (map_fd < 0)

• return map_fd;
  

• for (i = 0; i < bpf_map__max_entries(prog_array); i++) {

• snprintf(prog_name, sizeof(prog_name), "flow_dissector_%d", i);
  

• prog = bpf_object__find_program_by_name(skel->obj, prog_name);
  

• if (!prog)
  

• 	return -1;
  

• sub_prog_fd = bpf_program__fd(prog);
  

• if (sub_prog_fd < 0)
  

• 	return -1;
  

• err = bpf_map_update_elem(map_fd, &i, &sub_prog_fd, BPF_ANY);
  

• if (err)
  

• 	return -1;
  

• }
• return main_prog_fd;

+}

+static void detach_program(struct bpf_flow *skel, int prog_fd) +{

• bpf_prog_detach2(prog_fd, 0, BPF_FLOW_DISSECTOR);

+}

+static int set_port_drop(int pf, bool multi_port) +{

• SYS(fail, "tc qdisc add dev lo ingress");
• SYS(fail_delete_qdisc, "tc filter add %s %s %s %s %s %s %s %s %s %s",

•    "dev lo",
  

•    "parent FFFF:",
  

•    "protocol", pf == PF_INET6 ? "ipv6" : "ip",
  

•    "pref 1337",
  

•    "flower",
  

•    "ip_proto udp",
  

•    "src_port", multi_port ? "8-10" : "9",
  

•    "action drop");
  

• return 0;

+fail_delete_qdisc:

• SYS_NOFAIL("tc qdisc del dev lo ingress");

+fail:

• return 1;

+}

+static void remove_filter(void) +{

• SYS_NOFAIL("tc filter del dev lo ingress");
• SYS_NOFAIL("tc qdisc del dev lo ingress");

+}

+static int ipv4_setup(void) +{

• return set_port_drop(PF_INET, false);

+}

+static int ipv6_setup(void) +{

• return set_port_drop(PF_INET6, false);

+}

+static int port_range_setup(void) +{

• return set_port_drop(PF_INET, true);

+}

[..]

+static void ipv4_shutdown(void) +{

• remove_filter();

+}

+static void ipv6_shutdown(void) +{

• remove_filter();

+}

+static void port_range_shutdown(void) +{

• remove_filter();

+}

nit: Maybe use remove_filter directly as .test_teardown? These extra wrappers are not adding anything (imho).

+static int set_addresses(void) +{

• SYS(out, "ip -4 addr add %s dev lo", TEST_IPV4);
• SYS(out_remove_ipv4, "ip -6 addr add %s dev lo", TEST_IPV6);
• return 0;

+out_remove_ipv4:

• SYS_NOFAIL("ip -4 addr del %s dev lo", TEST_IPV4);

+out:

• return -1;

+}

+static void unset_addresses(void) +{

• SYS_NOFAIL("ip -4 addr del %s dev lo", TEST_IPV4);
• SYS_NOFAIL("ip -6 addr del %s dev lo", TEST_IPV6);

+}

+static int ipip_setup(void) +{

• if (!ASSERT_OK(set_addresses(), "configure addresses"))

• return -1;
  

• if (!ASSERT_OK(set_port_drop(PF_INET, false), "set filter"))

• goto out_unset_addresses;
  

• SYS(out_remove_filter,

•    "ip link add ipip_test type ipip remote %s local %s dev lo",
  

•    TEST_TUNNEL_REMOTE, TEST_TUNNEL_LOCAL);
  

• SYS(out_clean_netif, "ip link set ipip_test up");
• return 0;

+out_clean_netif:

• SYS_NOFAIL("ip link del ipip_test");

+out_remove_filter:

• remove_filter();

+out_unset_addresses:

• unset_addresses();
• return -1;

+}

+static void ipip_shutdown(void) +{

• SYS_NOFAIL("ip link del ipip_test");
• remove_filter();
• unset_addresses();

+}

+static int gre_setup(void) +{

• if (!ASSERT_OK(set_addresses(), "configure addresses"))

• return -1;
  

• if (!ASSERT_OK(set_port_drop(PF_INET, false), "set filter"))

• goto out_unset_addresses;
  

• SYS(out_remove_filter,

•    "ip link add gre_test type gre remote %s local %s dev lo",
  

•    TEST_TUNNEL_REMOTE, TEST_TUNNEL_LOCAL);
  

• SYS(out_clean_netif, "ip link set gre_test up");
• return 0;

+out_clean_netif:

• SYS_NOFAIL("ip link del ipip_test");

+out_remove_filter:

• remove_filter();

+out_unset_addresses:

• unset_addresses();
• return -1;

+}

+static void gre_shutdown(void) +{

• SYS_NOFAIL("ip link del gre_test");
• remove_filter();
• unset_addresses();

+}

+static const struct test_configuration tests_input[] = {

• {

• .name = "ipv4",
  

• .test_setup = ipv4_setup,
  

• .test_teardown = ipv4_shutdown,
  

• .source_ports = { 8, 9, 10 },
  

• .cfg_l3_inner = PF_INET,
  

• .in_saddr4 = TEST_IN4_SRC_ADDR_DEFAULT,
  

• .in_daddr4 = TEST_IN4_DST_ADDR_DEFAULT
  

• },
• {

• .name = "ipv4_continue_dissect",
  

• .test_setup = ipv4_setup,
  

• .test_teardown = ipv4_shutdown,
  

• .source_ports = { 8, 9, 10 },
  

• .cfg_l3_inner = PF_INET,
  

• .in_saddr4 = TEST_IN4_SRC_ADDR_DISSECT_CONTINUE,
  

• .in_daddr4 = TEST_IN4_DST_ADDR_DEFAULT },
  

• {

• .name = "ipip",
  

• .test_setup = ipip_setup,
  

• .test_teardown = ipip_shutdown,
  

• .source_ports = { 8, 9, 10 },
  

• .cfg_l3_inner = PF_INET,
  

• .in_saddr4 = TEST_IN4_SRC_ADDR_IPIP,
  

• .in_daddr4 = TEST_IN4_DST_ADDR_IPIP,
  

• .out_saddr4 = TEST_OUT4_SRC_ADDR_DEFAULT,
  

• .out_daddr4 = TEST_OUT4_DST_ADDR_DEFAULT,
  

• .cfg_l3_outer = PF_INET,
  

• .cfg_encap_proto = IPPROTO_IPIP,
  

• },
• {

• .name = "gre",
  

• .test_setup = gre_setup,
  

• .test_teardown = gre_shutdown,
  

• .source_ports = { 8, 9, 10 },
  

• .cfg_l3_inner = PF_INET,
  

• .in_saddr4 = TEST_IN4_SRC_ADDR_IPIP,
  

• .in_daddr4 = TEST_IN4_DST_ADDR_IPIP,
  

• .out_saddr4 = TEST_OUT4_SRC_ADDR_DEFAULT,
  

• .out_daddr4 = TEST_OUT4_DST_ADDR_DEFAULT,
  

• .cfg_l3_outer = PF_INET,
  

• .cfg_encap_proto = IPPROTO_GRE,
  

• },
• {

• .name = "port_range",
  

• .test_setup = port_range_setup,
  

• .test_teardown = port_range_shutdown,
  

• .source_ports = { 7, 9, 11 },
  

• .cfg_l3_inner = PF_INET,
  

• .in_saddr4 = TEST_IN4_SRC_ADDR_DEFAULT,
  

• .in_daddr4 = TEST_IN4_DST_ADDR_DEFAULT },
  

• {

• .name = "ipv6",
  

• .test_setup = ipv6_setup,
  

• .test_teardown = ipv6_shutdown,
  

• .source_ports = { 8, 9, 10 },
  

• .cfg_l3_inner = PF_INET6,
  

• .in_saddr6 = TEST_IN6_SRC_ADDR_DEFAULT,
  

• .in_daddr6 = TEST_IN6_DST_ADDR_DEFAULT
  

• },

+};

+struct test_ctx {

• struct bpf_flow *skel;
• struct netns_obj *ns;
• int prog_fd;

+};

+static int test_global_init(struct test_ctx *ctx) +{

• int err;
• ctx->skel = bpf_flow__open_and_load();
• if (!ASSERT_OK_PTR(ctx->skel, "open and load flow_dissector"))

• return -1;
  

• ctx->ns = netns_new("flow_dissector_classification", true);
• if (!ASSERT_OK_PTR(ctx->ns, "switch ns"))

• goto out_destroy_skel;
  

• err = write_sysctl("/proc/sys/net/ipv4/conf/default/rp_filter", "0");
• err |= write_sysctl("/proc/sys/net/ipv4/conf/all/rp_filter", "0");
• err |= write_sysctl("/proc/sys/net/ipv4/conf/lo/rp_filter", "0");
• if (!ASSERT_OK(err, "configure net tunables"))

• goto out_clean_ns;
  

• ctx->prog_fd = attach_and_configure_program(ctx->skel);
• if (!ASSERT_OK_FD(ctx->prog_fd, "attach and configure program"))

• goto out_clean_ns;
  

• return 0;

+out_clean_ns:

• netns_free(ctx->ns);

+out_destroy_skel:

• bpf_flow__destroy(ctx->skel);
• return -1;

+}

+static void test_global_shutdown(struct test_ctx *ctx) +{

• detach_program(ctx->skel, ctx->prog_fd);
• netns_free(ctx->ns);
• bpf_flow__destroy(ctx->skel);

+}

+void test_flow_dissector_classification(void) +{

• struct test_ctx ctx;
• struct test_configuration *test;
• int i;
• if (test_global_init(&ctx))

• return;
  

• for (i = 0; i < ARRAY_SIZE(tests_input); i++) {

• if (!test__start_subtest(tests_input[i].name))
  

• 	continue;
  

[..]

• test = (struct test_configuration *)&tests_input[i];
  

nit: What's the purpose of the cast? Is it to de-constify? Can we change run_test arguments to accept const struct test_configuration ptr instead?

• /* All tests are expected to have one rx-ok port first,
  

•  * then a non-working rx port, and finally a rx-ok port
  

•  */
  

• if (test->test_setup &&
  

•     !ASSERT_OK(test->test_setup(), "init filter"))
  

• 	continue;
  

• ASSERT_EQ(run_test(test, 0), TEST_PACKETS_COUNT,
  

• 	  "test first port");
  

• ASSERT_EQ(run_test(test, 1), 0, "test second port");
  

• ASSERT_EQ(run_test(test, 2), TEST_PACKETS_COUNT,
  

• 	  "test third port");
  

• if (test->test_teardown)
  

• 	test->test_teardown();
  

• }
• test_global_shutdown(&ctx);

+}

-- 2.47.0