Re: [PATCH bpf-next 09/10] selftests/bpf: migrate bpf flow dissectors tests to test_progs

[Stanislav Fomichev]

On 11/13, 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@xxxxxxxxxxx>
> ---
> 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 | 851 +++++++++++++++++++++
>  1 file changed, 851 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..de90c3e7b6a4b1890c380e384a255b030014a21d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c
> @@ -0,0 +1,851 @@
> +// 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 "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 unsigned long add_csum_hword(const uint16_t *start, int num_u16)
> +{
> +	unsigned long sum = 0;
> +	int i;
> +
> +	for (i = 0; i < num_u16; i++)
> +		sum += start[i];
> +
> +	return sum;
> +}
> +
> +static uint16_t build_ip_csum(const uint16_t *start, int num_u16,
> +			      unsigned long sum)
> +{
> +	sum += add_csum_hword(start, num_u16);
> +
> +	while (sum >> 16)
> +		sum = (sum & 0xffff) + (sum >> 16);
> +
> +	return ~sum;
> +}
> +
> +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, iph->ihl << 1, 0);
> +}
> +
> +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 uint16_t build_udp_v4_csum(const struct iphdr *iph,
> +				  const struct udphdr *udph, int num_words)
> +{
> +	unsigned long pseudo_sum;
> +	int num_u16 = sizeof(iph->saddr); /* halfwords: twice byte len */
> +
> +	pseudo_sum = add_csum_hword((void *)&iph->saddr, num_u16);
> +	pseudo_sum += htons(IPPROTO_UDP);
> +	pseudo_sum += udph->len;
> +	return build_ip_csum((void *)udph, num_words, pseudo_sum);
> +}
> +
> +static uint16_t build_udp_v6_csum(const struct ipv6hdr *ip6h,
> +				  const struct udphdr *udph, int num_words)
> +{
> +	unsigned long pseudo_sum;
> +	int num_u16 = sizeof(ip6h->saddr); /* halfwords: twice byte len */
> +
> +	pseudo_sum = add_csum_hword((void *)&ip6h->saddr, num_u16);
> +	pseudo_sum += htons(ip6h->nexthdr);
> +	pseudo_sum += ip6h->payload_len;
> +	return build_ip_csum((void *)udph, num_words, pseudo_sum);
> +}

I remember adding a bunch of similar code to tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
and tools/testing/selftests/bpf/network_helpers.h. The csum handling in
particular (csum_tcpudp_magic/etc for pseudo headers).
Can you see if something can be reused? Maybe something we
can move into network_helpers.h? For example build_ip_csum/ip_csum.