* [PATCH bpf-next v3 1/6] selftests/bpf: Add traffic monitor functions.
2024-07-30 0:27 [PATCH bpf-next v3 0/6] monitor network traffic for flaky test cases Kui-Feng Lee
@ 2024-07-30 0:27 ` Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 2/6] selftests/bpf: Add the traffic monitor option to test_progs Kui-Feng Lee
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Kui-Feng Lee @ 2024-07-30 0:27 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii, sdf, geliang
Cc: sinquersw, kuifeng, Kui-Feng Lee
Add functions that capture packets and print log in the background. They
are supposed to be used for debugging flaky network test cases. A monitored
test case should call traffic_monitor_start() to start a thread to capture
packets in the background for a given namespace and call
traffic_monitor_stop() to stop capturing. (Or, option '-m' implemented by
the later patches.)
IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 68, ifindex 1, SYN
IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 60, ifindex 1, SYN, ACK
IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 60, ifindex 1, ACK
IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 52, ifindex 1, ACK
IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 52, ifindex 1, FIN, ACK
IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 52, ifindex 1, RST, ACK
Packet file: packets-2172-86-select_reuseport:sockhash-test.log
#280/87 select_reuseport/sockhash IPv4/TCP LOOPBACK test_detach_bpf:OK
The above is the output of an example. It shows the packets of a connection
and the name of the file that contains captured packets in the directory
/tmp/tmon_pcap. The file can be loaded by tcpdump or wireshark.
This feature only works if TRAFFIC_MONITOR variable has been passed to
build BPF selftests. For example,
make TRAFFIC_MONITOR=1 -C tools/testing/selftests/bpf
This command will build BPF selftests with this feature enabled.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
tools/testing/selftests/bpf/Makefile | 5 +
tools/testing/selftests/bpf/test_progs.c | 432 +++++++++++++++++++++++
tools/testing/selftests/bpf/test_progs.h | 16 +
3 files changed, 453 insertions(+)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index dd49c1d23a60..9dfe17588689 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -41,6 +41,11 @@ CFLAGS += -g $(OPT_FLAGS) -rdynamic \
LDFLAGS += $(SAN_LDFLAGS)
LDLIBS += $(LIBELF_LIBS) -lz -lrt -lpthread
+ifneq ($(TRAFFIC_MONITOR),)
+LDLIBS += -lpcap
+CFLAGS += -DTRAFFIC_MONITOR=1
+endif
+
# The following tests perform type punning and they may break strict
# aliasing rules, which are exploited by both GCC and clang by default
# while optimizing. This can lead to broken programs.
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 89ff704e9dad..62303eca11f4 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -15,10 +15,25 @@
#include <netinet/in.h>
#include <sys/select.h>
#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/types.h>
#include <sys/un.h>
#include <bpf/btf.h>
#include "json_writer.h"
+#include <linux/ip.h>
+#include <linux/udp.h>
+#include <netinet/tcp.h>
+#include <net/if.h>
+#include "network_helpers.h"
+
+#ifdef TRAFFIC_MONITOR
+/* Prevent pcap.h from including pcap/bpf.h and causing conflicts */
+#define PCAP_DONT_INCLUDE_PCAP_BPF_H 1
+#include <pcap/pcap.h>
+#include <pcap/dlt.h>
+#endif
+
static bool verbose(void)
{
return env.verbosity > VERBOSE_NONE;
@@ -402,6 +417,423 @@ static void restore_netns(void)
}
}
+#ifdef TRAFFIC_MONITOR
+struct tmonitor_ctx {
+ pcap_t *pcap;
+ pcap_dumper_t *dumper;
+ pthread_t thread;
+ int wake_fd_r;
+ int wake_fd_w;
+
+ bool done;
+ char pkt_fname[PATH_MAX];
+ int pcap_fd;
+};
+
+/* Is this packet captured with a Ethernet protocol type? */
+static bool is_ethernet(const u_char *packet)
+{
+ u16 arphdr_type;
+
+ memcpy(&arphdr_type, packet + 8, 2);
+ arphdr_type = ntohs(arphdr_type);
+
+ /* Except the following cases, the protocol type contains the
+ * Ethernet protocol type for the packet.
+ *
+ * https://www.tcpdump.org/linktypes/LINKTYPE_LINUX_SLL2.html
+ */
+ switch (arphdr_type) {
+ case 770: /* ARPHRD_FRAD */
+ case 778: /* ARPHDR_IPGRE */
+ case 803: /* ARPHRD_IEEE80211_RADIOTAP */
+ return false;
+ }
+ return true;
+}
+
+/* Show the information of the transport layer in the packet */
+static void show_transport(const u_char *packet, u16 len, u32 ifindex,
+ const char *src_addr, const char *dst_addr,
+ u16 proto, bool ipv6)
+{
+ struct udphdr *udp;
+ struct tcphdr *tcp;
+ u16 src_port, dst_port;
+ const char *transport_str;
+ char *ifname, _ifname[IF_NAMESIZE];
+
+ ifname = if_indextoname(ifindex, _ifname);
+ if (!ifname) {
+ snprintf(_ifname, sizeof(_ifname), "unknown(%d)", ifindex);
+ ifname = _ifname;
+ }
+
+ if (proto == IPPROTO_UDP) {
+ udp = (struct udphdr *)packet;
+ src_port = ntohs(udp->source);
+ dst_port = ntohs(udp->dest);
+ transport_str = "UDP";
+ } else if (proto == IPPROTO_TCP) {
+ tcp = (struct tcphdr *)packet;
+ src_port = ntohs(tcp->source);
+ dst_port = ntohs(tcp->dest);
+ transport_str = "TCP"
+;
+ } else if (proto == IPPROTO_ICMP) {
+ printf("IPv4 ICMP packet: %s -> %s, len %d, type %d, code %d, ifname %s\n",
+ src_addr, dst_addr, len, packet[0], packet[1], ifname);
+ return;
+ } else if (proto == IPPROTO_ICMPV6) {
+ printf("IPv6 ICMPv6 packet: %s -> %s, len %d, type %d, code %d, ifname %s\n",
+ src_addr, dst_addr, len, packet[0], packet[1], ifname);
+ return;
+ } else {
+ printf("%s (proto %d): %s -> %s, ifname %s\n",
+ ipv6 ? "IPv6" : "IPv4", proto, src_addr, dst_addr, ifname);
+ return;
+ }
+
+ /* TCP */
+
+ if (ipv6)
+ printf("IPv6 %s packet: [%s]:%d -> [%s]:%d, len %d, ifname %s",
+ transport_str, src_addr, src_port,
+ dst_addr, dst_port, len, ifname);
+ else
+ printf("IPv4 %s packet: %s:%d -> %s:%d, len %d, ifname %s",
+ transport_str, src_addr, src_port,
+ dst_addr, dst_port, len, ifname);
+
+ if (proto == IPPROTO_TCP) {
+ if (tcp->fin)
+ printf(", FIN");
+ if (tcp->syn)
+ printf(", SYN");
+ if (tcp->rst)
+ printf(", RST");
+ if (tcp->ack)
+ printf(", ACK");
+ }
+
+ printf("\n");
+}
+
+static void show_ipv6_packet(const u_char *packet, u32 ifindex)
+{
+ struct ipv6hdr *pkt = (struct ipv6hdr *)packet;
+ struct in6_addr src;
+ struct in6_addr dst;
+ char src_str[INET6_ADDRSTRLEN], dst_str[INET6_ADDRSTRLEN];
+ u_char proto;
+
+ memcpy(&src, &pkt->saddr, sizeof(src));
+ memcpy(&dst, &pkt->daddr, sizeof(dst));
+ inet_ntop(AF_INET6, &src, src_str, sizeof(src_str));
+ inet_ntop(AF_INET6, &dst, dst_str, sizeof(dst_str));
+ proto = pkt->nexthdr;
+ show_transport(packet + sizeof(struct ipv6hdr),
+ ntohs(pkt->payload_len),
+ ifindex, src_str, dst_str, proto, true);
+}
+
+static void show_ipv4_packet(const u_char *packet, u32 ifindex)
+{
+ struct iphdr *pkt = (struct iphdr *)packet;
+ struct in_addr src;
+ struct in_addr dst;
+ u_char proto;
+ char src_str[INET_ADDRSTRLEN], dst_str[INET_ADDRSTRLEN];
+
+ memcpy(&src, &pkt->saddr, sizeof(src));
+ memcpy(&dst, &pkt->daddr, sizeof(dst));
+ inet_ntop(AF_INET, &src, src_str, sizeof(src_str));
+ inet_ntop(AF_INET, &dst, dst_str, sizeof(dst_str));
+ proto = pkt->protocol;
+ show_transport(packet + sizeof(struct iphdr),
+ ntohs(pkt->tot_len),
+ ifindex, src_str, dst_str, proto, false);
+}
+
+static void *traffic_monitor_thread(void *arg)
+{
+ char *ifname, _ifname[IF_NAMESIZE];
+ const u_char *packet, *payload;
+ struct tmonitor_ctx *ctx = arg;
+ struct pcap_pkthdr header;
+ pcap_t *pcap = ctx->pcap;
+ pcap_dumper_t *dumper = ctx->dumper;
+ int fd = ctx->pcap_fd;
+ int wake_fd = ctx->wake_fd_r;
+ u16 proto;
+ u32 ifindex;
+ fd_set fds;
+ int nfds, r;
+
+ nfds = (fd > wake_fd ? fd : wake_fd) + 1;
+ FD_ZERO(&fds);
+
+ while (!ctx->done) {
+ FD_SET(fd, &fds);
+ FD_SET(wake_fd, &fds);
+ r = select(nfds, &fds, NULL, NULL, NULL);
+ if (!r)
+ continue;
+ if (r < 0) {
+ if (errno == EINTR)
+ continue;
+ log_err("Fail to select on pcap fd and wake fd: %s", strerror(errno));
+ break;
+ }
+
+ packet = pcap_next(pcap, &header);
+ if (!packet)
+ continue;
+
+ /* According to the man page of pcap_dump(), first argument
+ * is the pcap_dumper_t pointer even it's argument type is
+ * u_char *.
+ */
+ pcap_dump((u_char *)dumper, &header, packet);
+
+ /* Not sure what other types of packets look like. Here, we
+ * parse only Ethernet and compatible packets.
+ */
+ if (!is_ethernet(packet)) {
+ printf("Packet captured\n");
+ continue;
+ }
+
+ /* Skip SLL2 header
+ * https://www.tcpdump.org/linktypes/LINKTYPE_LINUX_SLL2.html
+ *
+ * Although the document doesn't mention that, the payload
+ * doesn't include the Ethernet header. The payload starts
+ * from the first byte of the network layer header.
+ */
+ payload = packet + 20;
+
+ memcpy(&proto, packet, 2);
+ proto = ntohs(proto);
+ memcpy(&ifindex, packet + 4, 4);
+ ifindex = ntohl(ifindex);
+
+ if (proto == ETH_P_IPV6) {
+ show_ipv6_packet(payload, ifindex);
+ } else if (proto == ETH_P_IP) {
+ show_ipv4_packet(payload, ifindex);
+ } else {
+ ifname = if_indextoname(ifindex, _ifname);
+ if (!ifname) {
+ snprintf(_ifname, sizeof(_ifname), "unknown(%d)", ifindex);
+ ifname = _ifname;
+ }
+
+ printf("Unknown network protocol type %x, ifname %s\n", proto, ifname);
+ }
+ }
+
+ return NULL;
+}
+
+/* Prepare the pcap handle to capture packets.
+ *
+ * This pcap is non-blocking and immediate mode is enabled to receive
+ * captured packets as soon as possible. The snaplen is set to 1024 bytes
+ * to limit the size of captured content. The format of the link-layer
+ * header is set to DLT_LINUX_SLL2 to enable handling various link-layer
+ * technologies.
+ */
+static pcap_t *traffic_monitor_prepare_pcap(void)
+{
+ char errbuf[PCAP_ERRBUF_SIZE];
+ pcap_t *pcap;
+ int r;
+
+ /* Listen on all NICs in the namespace */
+ pcap = pcap_create("any", errbuf);
+ if (!pcap) {
+ log_err("Failed to open pcap: %s", errbuf);
+ return NULL;
+ }
+ /* Limit the size of the packet (first N bytes) */
+ r = pcap_set_snaplen(pcap, 1024);
+ if (r) {
+ log_err("Failed to set snaplen: %s", pcap_geterr(pcap));
+ goto error;
+ }
+ /* To receive packets as fast as possible */
+ r = pcap_set_immediate_mode(pcap, 1);
+ if (r) {
+ log_err("Failed to set immediate mode: %s", pcap_geterr(pcap));
+ goto error;
+ }
+ r = pcap_setnonblock(pcap, 1, errbuf);
+ if (r) {
+ log_err("Failed to set nonblock: %s", errbuf);
+ goto error;
+ }
+ r = pcap_activate(pcap);
+ if (r) {
+ log_err("Failed to activate pcap: %s", pcap_geterr(pcap));
+ goto error;
+ }
+ /* Determine the format of the link-layer header */
+ r = pcap_set_datalink(pcap, DLT_LINUX_SLL2);
+ if (r) {
+ log_err("Failed to set datalink: %s", pcap_geterr(pcap));
+ goto error;
+ }
+
+ return pcap;
+error:
+ pcap_close(pcap);
+ return NULL;
+}
+
+static void encode_test_name(char *buf, size_t len)
+{
+ struct prog_test_def *test = env.test;
+ struct subtest_state *subtest_state = env.subtest_state;
+ char *p;
+
+ if (subtest_state)
+ snprintf(buf, len, "%s:%s", test->test_name, subtest_state->name);
+ else
+ snprintf(buf, len, "%s", test->test_name);
+ while ((p = strchr(buf, '/')))
+ *p = '_';
+ while ((p = strchr(buf, ' ')))
+ *p = '_';
+}
+
+#define PCAP_DIR "/tmp/tmon_pcap"
+
+/* Start to monitor the network traffic in the given network namespace.
+ *
+ * netns: the name of the network namespace to monitor. If NULL, the
+ * current network namespace is monitored.
+ *
+ * This function will start a thread to capture packets going through NICs
+ * in the give network namespace.
+ */
+struct tmonitor_ctx *traffic_monitor_start(const char *netns)
+{
+ struct tmonitor_ctx *ctx = NULL;
+ struct nstoken *nstoken = NULL;
+ int pipefd[2] = {-1, -1};
+ static int tmon_seq;
+ char test_name[64];
+ int r;
+
+ if (netns) {
+ nstoken = open_netns(netns);
+ if (!nstoken)
+ return NULL;
+ }
+ ctx = malloc(sizeof(*ctx));
+ if (!ctx) {
+ log_err("Failed to malloc ctx");
+ goto fail_ctx;
+ }
+ memset(ctx, 0, sizeof(*ctx));
+
+ encode_test_name(test_name, sizeof(test_name));
+ snprintf(ctx->pkt_fname, sizeof(ctx->pkt_fname),
+ PCAP_DIR "/packets-%d-%d-%s-%s.log", getpid(), tmon_seq++,
+ test_name, netns ? netns : "unknown");
+
+ r = mkdir(PCAP_DIR, 0755);
+ if (r && errno != EEXIST) {
+ log_err("Failed to create " PCAP_DIR);
+ goto fail_pcap;
+ }
+
+ ctx->pcap = traffic_monitor_prepare_pcap();
+ if (!ctx->pcap)
+ goto fail_pcap;
+ ctx->pcap_fd = pcap_get_selectable_fd(ctx->pcap);
+ if (ctx->pcap_fd < 0) {
+ log_err("Failed to get pcap fd");
+ goto fail_dumper;
+ }
+
+ /* Create a packet file */
+ ctx->dumper = pcap_dump_open(ctx->pcap, ctx->pkt_fname);
+ if (!ctx->dumper) {
+ log_err("Failed to open pcap dump: %s", ctx->pkt_fname);
+ goto fail_dumper;
+ }
+
+ /* Create a pipe to wake up the monitor thread */
+ r = pipe(pipefd);
+ if (r) {
+ log_err("Failed to create pipe: %s", strerror(errno));
+ goto fail;
+ }
+ ctx->wake_fd_r = pipefd[0];
+ ctx->wake_fd_w = pipefd[1];
+
+ r = pthread_create(&ctx->thread, NULL, traffic_monitor_thread, ctx);
+ if (r) {
+ log_err("Failed to create thread: %s", strerror(r));
+ goto fail;
+ }
+
+ close_netns(nstoken);
+
+ return ctx;
+
+fail:
+ close(pipefd[0]);
+ close(pipefd[1]);
+
+ pcap_dump_close(ctx->dumper);
+ unlink(ctx->pkt_fname);
+
+fail_dumper:
+ pcap_close(ctx->pcap);
+
+fail_pcap:
+ free(ctx);
+
+fail_ctx:
+ close_netns(nstoken);
+
+ return NULL;
+}
+
+static void traffic_monitor_release(struct tmonitor_ctx *ctx)
+{
+ pcap_close(ctx->pcap);
+ pcap_dump_close(ctx->dumper);
+
+ close(ctx->wake_fd_r);
+ close(ctx->wake_fd_w);
+
+ free(ctx);
+}
+
+/* Stop the network traffic monitor.
+ *
+ * ctx: the context returned by traffic_monitor_start()
+ */
+void traffic_monitor_stop(struct tmonitor_ctx *ctx)
+{
+ if (!ctx)
+ return;
+
+ /* Stop the monitor thread */
+ ctx->done = true;
+ write(ctx->wake_fd_w, "x", 1);
+ pthread_join(ctx->thread, NULL);
+
+ printf("Packet file: %s\n", strrchr(ctx->pkt_fname, '/') + 1);
+
+ traffic_monitor_release(ctx);
+}
+#endif /* TRAFFIC_MONITOR */
+
void test__end_subtest(void)
{
struct prog_test_def *test = env.test;
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 51341d50213b..5a9191f69707 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -474,4 +474,20 @@ extern void test_loader_fini(struct test_loader *tester);
test_loader_fini(&tester); \
})
+struct tmonitor_ctx;
+
+#ifdef TRAFFIC_MONITOR
+struct tmonitor_ctx *traffic_monitor_start(const char *netns);
+void traffic_monitor_stop(struct tmonitor_ctx *ctx);
+#else
+static inline struct tmonitor_ctx *traffic_monitor_start(const char *netns)
+{
+ return (struct tmonitor_ctx *)-1;
+}
+
+static inline void traffic_monitor_stop(struct tmonitor_ctx *ctx)
+{
+}
+#endif
+
#endif /* __TEST_PROGS_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 2/6] selftests/bpf: Add the traffic monitor option to test_progs.
2024-07-30 0:27 [PATCH bpf-next v3 0/6] monitor network traffic for flaky test cases Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 1/6] selftests/bpf: Add traffic monitor functions Kui-Feng Lee
@ 2024-07-30 0:27 ` Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 3/6] selftests/bpf: netns_new() and netns_free() helpers Kui-Feng Lee
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Kui-Feng Lee @ 2024-07-30 0:27 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii, sdf, geliang
Cc: sinquersw, kuifeng, Kui-Feng Lee
Add option '-m' to test_progs to accept names and patterns of test cases.
This option will be used later to enable traffic monitor that capture
network packets generated by test cases.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
tools/testing/selftests/bpf/test_progs.c | 85 +++++++++++++++++-------
tools/testing/selftests/bpf/test_progs.h | 2 +
2 files changed, 63 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 62303eca11f4..2fb375ecc095 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -156,6 +156,7 @@ struct prog_test_def {
void (*run_serial_test)(void);
bool should_run;
bool need_cgroup_cleanup;
+ bool should_tmon;
};
/* Override C runtime library's usleep() implementation to ensure nanosleep()
@@ -193,46 +194,59 @@ static bool should_run(struct test_selector *sel, int num, const char *name)
return num < sel->num_set_len && sel->num_set[num];
}
-static bool should_run_subtest(struct test_selector *sel,
- struct test_selector *subtest_sel,
- int subtest_num,
- const char *test_name,
- const char *subtest_name)
+static bool match_subtest(struct test_filter_set *filter,
+ const char *test_name,
+ const char *subtest_name)
{
int i, j;
- for (i = 0; i < sel->blacklist.cnt; i++) {
- if (glob_match(test_name, sel->blacklist.tests[i].name)) {
- if (!sel->blacklist.tests[i].subtest_cnt)
- return false;
-
- for (j = 0; j < sel->blacklist.tests[i].subtest_cnt; j++) {
- if (glob_match(subtest_name,
- sel->blacklist.tests[i].subtests[j]))
- return false;
- }
- }
- }
-
- for (i = 0; i < sel->whitelist.cnt; i++) {
- if (glob_match(test_name, sel->whitelist.tests[i].name)) {
- if (!sel->whitelist.tests[i].subtest_cnt)
+ for (i = 0; i < filter->cnt; i++) {
+ if (glob_match(test_name, filter->tests[i].name)) {
+ if (!filter->tests[i].subtest_cnt)
return true;
- for (j = 0; j < sel->whitelist.tests[i].subtest_cnt; j++) {
+ for (j = 0; j < filter->tests[i].subtest_cnt; j++) {
if (glob_match(subtest_name,
- sel->whitelist.tests[i].subtests[j]))
+ filter->tests[i].subtests[j]))
return true;
}
}
}
+ return false;
+}
+
+static bool should_run_subtest(struct test_selector *sel,
+ struct test_selector *subtest_sel,
+ int subtest_num,
+ const char *test_name,
+ const char *subtest_name)
+{
+ if (match_subtest(&sel->blacklist, test_name, subtest_name))
+ return false;
+
+ if (match_subtest(&sel->whitelist, test_name, subtest_name))
+ return true;
+
if (!sel->whitelist.cnt && !subtest_sel->num_set)
return true;
return subtest_num < subtest_sel->num_set_len && subtest_sel->num_set[subtest_num];
}
+static bool should_tmon(struct test_selector *sel, int num, const char *name)
+{
+ int i;
+
+ for (i = 0; i < sel->whitelist.cnt; i++) {
+ if (glob_match(name, sel->whitelist.tests[i].name) &&
+ !sel->whitelist.tests[i].subtest_cnt)
+ return true;
+ }
+
+ return false;
+}
+
static char *test_result(bool failed, bool skipped)
{
return failed ? "FAIL" : (skipped ? "SKIP" : "OK");
@@ -906,6 +920,10 @@ bool test__start_subtest(const char *subtest_name)
return false;
}
+ subtest_state->should_tmon = match_subtest(&env.tmon_selector.whitelist,
+ test->test_name,
+ subtest_name);
+
env.subtest_state = subtest_state;
stdio_hijack_init(&subtest_state->log_buf, &subtest_state->log_cnt);
@@ -1085,7 +1103,8 @@ enum ARG_KEYS {
ARG_TEST_NAME_GLOB_DENYLIST = 'd',
ARG_NUM_WORKERS = 'j',
ARG_DEBUG = -1,
- ARG_JSON_SUMMARY = 'J'
+ ARG_JSON_SUMMARY = 'J',
+ ARG_TRAFFIC_MONITOR = 'm',
};
static const struct argp_option opts[] = {
@@ -1112,6 +1131,8 @@ static const struct argp_option opts[] = {
{ "debug", ARG_DEBUG, NULL, 0,
"print extra debug information for test_progs." },
{ "json-summary", ARG_JSON_SUMMARY, "FILE", 0, "Write report in json format to this file."},
+ { "traffic-monitor", ARG_TRAFFIC_MONITOR, "NAMES", 0,
+ "Monitor network traffic of tests with name matching the pattern (supports '*' wildcard)." },
{},
};
@@ -1323,6 +1344,18 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
break;
case ARGP_KEY_END:
break;
+ case ARG_TRAFFIC_MONITOR: {
+ if (arg[0] == '@')
+ err = parse_test_list_file(arg + 1,
+ &env->tmon_selector.whitelist,
+ true);
+ else
+ err = parse_test_list(arg,
+ &env->tmon_selector.whitelist,
+ true);
+
+ break;
+ }
default:
return ARGP_ERR_UNKNOWN;
}
@@ -2154,6 +2187,9 @@ int main(int argc, char **argv)
test->test_num, test->test_name, test->test_name, test->test_name);
exit(EXIT_ERR_SETUP_INFRA);
}
+ if (test->should_run)
+ test->should_tmon = should_tmon(&env.tmon_selector,
+ test->test_num, test->test_name);
}
/* ignore workers if we are just listing */
@@ -2238,6 +2274,7 @@ int main(int argc, char **argv)
free_test_selector(&env.test_selector);
free_test_selector(&env.subtest_selector);
+ free_test_selector(&env.tmon_selector);
free_test_states();
if (env.succ_cnt + env.fail_cnt + env.skip_cnt == 0)
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 5a9191f69707..6c88e15564d6 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -74,6 +74,7 @@ struct subtest_state {
int error_cnt;
bool skipped;
bool filtered;
+ bool should_tmon;
FILE *stdout;
};
@@ -98,6 +99,7 @@ struct test_state {
struct test_env {
struct test_selector test_selector;
struct test_selector subtest_selector;
+ struct test_selector tmon_selector;
bool verifier_stats;
bool debug;
enum verbosity verbosity;
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 3/6] selftests/bpf: netns_new() and netns_free() helpers.
2024-07-30 0:27 [PATCH bpf-next v3 0/6] monitor network traffic for flaky test cases Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 1/6] selftests/bpf: Add traffic monitor functions Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 2/6] selftests/bpf: Add the traffic monitor option to test_progs Kui-Feng Lee
@ 2024-07-30 0:27 ` Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect Kui-Feng Lee
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Kui-Feng Lee @ 2024-07-30 0:27 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii, sdf, geliang
Cc: sinquersw, kuifeng, Kui-Feng Lee
netns_new()/netns_free() create/delete network namespaces. They support the
option '-m' of test_progs to start/stop traffic monitor for the network
namespace being created for matched tests.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
tools/testing/selftests/bpf/network_helpers.c | 26 ++++++
tools/testing/selftests/bpf/network_helpers.h | 2 +
tools/testing/selftests/bpf/test_progs.c | 80 +++++++++++++++++++
tools/testing/selftests/bpf/test_progs.h | 4 +
4 files changed, 112 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index e0cba4178e41..99c67020c824 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -448,6 +448,32 @@ char *ping_command(int family)
return "ping";
}
+int make_netns(const char *name)
+{
+ char cmd[128];
+ int r;
+
+ snprintf(cmd, sizeof(cmd), "ip netns add %s", name);
+ r = system(cmd);
+ if (r > 0)
+ /* exit code */
+ return -r;
+ return r;
+}
+
+int remove_netns(const char *name)
+{
+ char cmd[128];
+ int r;
+
+ snprintf(cmd, sizeof(cmd), "ip netns del %s >/dev/null 2>&1", name);
+ r = system(cmd);
+ if (r > 0)
+ /* exit code */
+ return -r;
+ return r;
+}
+
struct nstoken {
int orig_netns_fd;
};
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index aac5b94d6379..91763fbe58d2 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -92,6 +92,8 @@ struct nstoken;
struct nstoken *open_netns(const char *name);
void close_netns(struct nstoken *token);
int send_recv_data(int lfd, int fd, uint32_t total_bytes);
+int make_netns(const char *name);
+int remove_netns(const char *name);
static __u16 csum_fold(__u32 csum)
{
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 2fb375ecc095..a5036f3359e8 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1060,6 +1060,86 @@ int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len)
return err;
}
+struct netns_obj {
+ char nsname[128];
+ struct tmonitor_ctx *tmon;
+ struct nstoken *nstoken;
+};
+
+/* Create a new network namespace with the given name.
+ *
+ * Create a new network namespace and set the network namespace of the
+ * current process to the new network namespace if the argument "open" is
+ * true. This function should be paired with netns_free() to release the
+ * resource and delete the network namespace.
+ *
+ * It also implements the functionality of the option "-m" by starting
+ * traffic monitor on the background to capture the packets in this network
+ * namespace if the current test or subtest matching the pattern.
+ *
+ * name: the name of the network namespace to create.
+ * open: open the network namespace if true.
+ *
+ * Return: the network namespace object on success, NULL on failure.
+ */
+struct netns_obj *netns_new(const char *name, bool open)
+{
+ struct netns_obj *netns_obj = malloc(sizeof(*netns_obj));
+ int r;
+
+ if (!netns_obj)
+ return NULL;
+ memset(netns_obj, 0, sizeof(*netns_obj));
+
+ strncpy(netns_obj->nsname, name, sizeof(netns_obj->nsname));
+ netns_obj->nsname[sizeof(netns_obj->nsname) - 1] = '\0';
+
+ /* Create the network namespace */
+ r = make_netns(name);
+ if (r)
+ goto fail;
+
+ /* Set the network namespace of the current process */
+ if (open) {
+ netns_obj->nstoken = open_netns(name);
+ if (!netns_obj->nstoken)
+ goto fail;
+ }
+
+ /* Start traffic monitor */
+ if (env.test->should_tmon ||
+ (env.subtest_state && env.subtest_state->should_tmon)) {
+ netns_obj->tmon = traffic_monitor_start(name);
+ if (!netns_obj->tmon)
+ goto fail;
+ } else {
+ netns_obj->tmon = NULL;
+ }
+
+ return netns_obj;
+fail:
+ close_netns(netns_obj->nstoken);
+ remove_netns(name);
+ free(netns_obj);
+ return NULL;
+}
+
+/* Delete the network namespace.
+ *
+ * This function should be paired with netns_new() to delete the namespace
+ * created by netns_new().
+ */
+void netns_free(struct netns_obj *netns_obj)
+{
+ if (!netns_obj)
+ return;
+ if (netns_obj->tmon)
+ traffic_monitor_stop(netns_obj->tmon);
+ close_netns(netns_obj->nstoken);
+ remove_netns(netns_obj->nsname);
+ free(netns_obj);
+}
+
/* extern declarations for test funcs */
#define DEFINE_TEST(name) \
extern void test_##name(void) __weak; \
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 6c88e15564d6..b1419589d8c1 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -430,6 +430,10 @@ int write_sysctl(const char *sysctl, const char *value);
int get_bpf_max_tramp_links_from(struct btf *btf);
int get_bpf_max_tramp_links(void);
+struct netns_obj;
+struct netns_obj *netns_new(const char *name, bool open);
+void netns_free(struct netns_obj *netns);
+
#ifdef __x86_64__
#define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep"
#elif defined(__s390x__)
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect.
2024-07-30 0:27 [PATCH bpf-next v3 0/6] monitor network traffic for flaky test cases Kui-Feng Lee
` (2 preceding siblings ...)
2024-07-30 0:27 ` [PATCH bpf-next v3 3/6] selftests/bpf: netns_new() and netns_free() helpers Kui-Feng Lee
@ 2024-07-30 0:27 ` Kui-Feng Lee
2024-07-30 9:43 ` Geliang Tang
2024-07-30 0:27 ` [PATCH bpf-next v3 5/6] selftests/bpf: Monitor traffic for sockmap_listen Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 6/6] selftests/bpf: Monitor traffic for select_reuseport Kui-Feng Lee
5 siblings, 1 reply; 10+ messages in thread
From: Kui-Feng Lee @ 2024-07-30 0:27 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii, sdf, geliang
Cc: sinquersw, kuifeng, Kui-Feng Lee
Enable traffic monitoring for the test case tc_redirect.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
.../selftests/bpf/prog_tests/tc_redirect.c | 48 ++++++++++++++-----
1 file changed, 36 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
index 327d51f59142..46d397c5c79a 100644
--- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
+++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
@@ -68,6 +68,7 @@
__FILE__, __LINE__, strerror(errno), ##__VA_ARGS__)
static const char * const namespaces[] = {NS_SRC, NS_FWD, NS_DST, NULL};
+static struct netns_obj *netns_objs[3];
static int write_file(const char *path, const char *newval)
{
@@ -85,29 +86,52 @@ static int write_file(const char *path, const char *newval)
return 0;
}
-static int netns_setup_namespaces(const char *verb)
+enum NETNS_VERB {
+ NETNS_ADD,
+ NETNS_DEL,
+};
+
+static int netns_setup_namespaces(enum NETNS_VERB verb)
{
const char * const *ns = namespaces;
- char cmd[128];
+ struct netns_obj **ns_obj = netns_objs;
while (*ns) {
- snprintf(cmd, sizeof(cmd), "ip netns %s %s", verb, *ns);
- if (!ASSERT_OK(system(cmd), cmd))
- return -1;
+ if (verb == NETNS_ADD) {
+ *ns_obj = netns_new(*ns, false);
+ if (!*ns_obj) {
+ log_err("netns_new failed");
+ return -1;
+ }
+ } else {
+ if (!*ns_obj) {
+ log_err("netns_obj is NULL");
+ return -1;
+ }
+ netns_free(*ns_obj);
+ *ns_obj = NULL;
+ }
ns++;
+ ns_obj++;
}
return 0;
}
-static void netns_setup_namespaces_nofail(const char *verb)
+static void netns_setup_namespaces_nofail(enum NETNS_VERB verb)
{
const char * const *ns = namespaces;
- char cmd[128];
+ struct netns_obj **ns_obj = netns_objs;
while (*ns) {
- snprintf(cmd, sizeof(cmd), "ip netns %s %s > /dev/null 2>&1", verb, *ns);
- system(cmd);
+ if (verb == NETNS_ADD) {
+ *ns_obj = netns_new(*ns, false);
+ } else {
+ if (*ns_obj)
+ netns_free(*ns_obj);
+ *ns_obj = NULL;
+ }
ns++;
+ ns_obj++;
}
}
@@ -1250,17 +1274,17 @@ static void test_tc_redirect_peer_l3(struct netns_setup_result *setup_result)
({ \
struct netns_setup_result setup_result = { .dev_mode = mode, }; \
if (test__start_subtest(#name)) \
- if (ASSERT_OK(netns_setup_namespaces("add"), "setup namespaces")) { \
+ if (ASSERT_OK(netns_setup_namespaces(NETNS_ADD), "setup namespaces")) { \
if (ASSERT_OK(netns_setup_links_and_routes(&setup_result), \
"setup links and routes")) \
test_ ## name(&setup_result); \
- netns_setup_namespaces("delete"); \
+ netns_setup_namespaces(NETNS_DEL); \
} \
})
static void *test_tc_redirect_run_tests(void *arg)
{
- netns_setup_namespaces_nofail("delete");
+ netns_setup_namespaces_nofail(NETNS_DEL);
RUN_TEST(tc_redirect_peer, MODE_VETH);
RUN_TEST(tc_redirect_peer, MODE_NETKIT);
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect.
2024-07-30 0:27 ` [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect Kui-Feng Lee
@ 2024-07-30 9:43 ` Geliang Tang
2024-07-30 16:33 ` Kui-Feng Lee
0 siblings, 1 reply; 10+ messages in thread
From: Geliang Tang @ 2024-07-30 9:43 UTC (permalink / raw)
To: Kui-Feng Lee, bpf, ast, martin.lau, song, kernel-team, andrii,
sdf
Cc: sinquersw, kuifeng
On Mon, 2024-07-29 at 17:27 -0700, Kui-Feng Lee wrote:
> Enable traffic monitoring for the test case tc_redirect.
>
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
> .../selftests/bpf/prog_tests/tc_redirect.c | 48 ++++++++++++++---
> --
> 1 file changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> index 327d51f59142..46d397c5c79a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> @@ -68,6 +68,7 @@
> __FILE__, __LINE__, strerror(errno), ##__VA_ARGS__)
>
> static const char * const namespaces[] = {NS_SRC, NS_FWD, NS_DST,
> NULL};
> +static struct netns_obj *netns_objs[3];
>
> static int write_file(const char *path, const char *newval)
> {
> @@ -85,29 +86,52 @@ static int write_file(const char *path, const
> char *newval)
> return 0;
> }
>
> -static int netns_setup_namespaces(const char *verb)
> +enum NETNS_VERB {
> + NETNS_ADD,
> + NETNS_DEL,
> +};
> +
> +static int netns_setup_namespaces(enum NETNS_VERB verb)
> {
> const char * const *ns = namespaces;
> - char cmd[128];
> + struct netns_obj **ns_obj = netns_objs;
>
> while (*ns) {
> - snprintf(cmd, sizeof(cmd), "ip netns %s %s", verb,
> *ns);
> - if (!ASSERT_OK(system(cmd), cmd))
> - return -1;
> + if (verb == NETNS_ADD) {
Maybe better to keep "verb" parameter as "char *", and use
if (!strcmp(verb, "add"))
here instead?
> + *ns_obj = netns_new(*ns, false);
> + if (!*ns_obj) {
> + log_err("netns_new failed");
> + return -1;
> + }
> + } else {
> + if (!*ns_obj) {
> + log_err("netns_obj is NULL");
> + return -1;
> + }
> + netns_free(*ns_obj);
> + *ns_obj = NULL;
> + }
> ns++;
> + ns_obj++;
> }
> return 0;
> }
>
> -static void netns_setup_namespaces_nofail(const char *verb)
> +static void netns_setup_namespaces_nofail(enum NETNS_VERB verb)
> {
> const char * const *ns = namespaces;
> - char cmd[128];
> + struct netns_obj **ns_obj = netns_objs;
>
> while (*ns) {
> - snprintf(cmd, sizeof(cmd), "ip netns %s %s >
> /dev/null 2>&1", verb, *ns);
> - system(cmd);
> + if (verb == NETNS_ADD) {
> + *ns_obj = netns_new(*ns, false);
> + } else {
> + if (*ns_obj)
> + netns_free(*ns_obj);
> + *ns_obj = NULL;
> + }
> ns++;
> + ns_obj++;
> }
> }
>
> @@ -1250,17 +1274,17 @@ static void test_tc_redirect_peer_l3(struct
> netns_setup_result *setup_result)
> ({
> \
> struct netns_setup_result setup_result = { .dev_mode
> = mode, }; \
> if
> (test__start_subtest(#name))
> \
> - if (ASSERT_OK(netns_setup_namespaces("add"),
> "setup namespaces")) { \
> + if
> (ASSERT_OK(netns_setup_namespaces(NETNS_ADD), "setup namespaces")) {
> \
> if
> (ASSERT_OK(netns_setup_links_and_routes(&setup_result), \
> "setup links and
> routes")) \
> test_ ##
> name(&setup_result); \
> -
> netns_setup_namespaces("delete"); \
> + netns_setup_namespaces(NETNS_DEL);
> \
> }
> \
> })
>
> static void *test_tc_redirect_run_tests(void *arg)
> {
> - netns_setup_namespaces_nofail("delete");
> + netns_setup_namespaces_nofail(NETNS_DEL);
>
> RUN_TEST(tc_redirect_peer, MODE_VETH);
> RUN_TEST(tc_redirect_peer, MODE_NETKIT);
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect.
2024-07-30 9:43 ` Geliang Tang
@ 2024-07-30 16:33 ` Kui-Feng Lee
2024-07-31 1:25 ` Geliang Tang
0 siblings, 1 reply; 10+ messages in thread
From: Kui-Feng Lee @ 2024-07-30 16:33 UTC (permalink / raw)
To: Geliang Tang, Kui-Feng Lee, bpf, ast, martin.lau, song,
kernel-team, andrii, sdf
Cc: kuifeng
On 7/30/24 02:43, Geliang Tang wrote:
> On Mon, 2024-07-29 at 17:27 -0700, Kui-Feng Lee wrote:
>> Enable traffic monitoring for the test case tc_redirect.
>>
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>> .../selftests/bpf/prog_tests/tc_redirect.c | 48 ++++++++++++++---
>> --
>> 1 file changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> index 327d51f59142..46d397c5c79a 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> @@ -68,6 +68,7 @@
>> __FILE__, __LINE__, strerror(errno), ##__VA_ARGS__)
>>
>> static const char * const namespaces[] = {NS_SRC, NS_FWD, NS_DST,
>> NULL};
>> +static struct netns_obj *netns_objs[3];
>>
>> static int write_file(const char *path, const char *newval)
>> {
>> @@ -85,29 +86,52 @@ static int write_file(const char *path, const
>> char *newval)
>> return 0;
>> }
>>
>> -static int netns_setup_namespaces(const char *verb)
>> +enum NETNS_VERB {
>> + NETNS_ADD,
>> + NETNS_DEL,
>> +};
>> +
>> +static int netns_setup_namespaces(enum NETNS_VERB verb)
>> {
>> const char * const *ns = namespaces;
>> - char cmd[128];
>> + struct netns_obj **ns_obj = netns_objs;
>>
>> while (*ns) {
>> - snprintf(cmd, sizeof(cmd), "ip netns %s %s", verb,
>> *ns);
>> - if (!ASSERT_OK(system(cmd), cmd))
>> - return -1;
>> + if (verb == NETNS_ADD) {
>
> Maybe better to keep "verb" parameter as "char *", and use
>
> if (!strcmp(verb, "add"))
>
> here instead?
I have no strong opinion here.
May I know why you think string is better here?
>
>> + *ns_obj = netns_new(*ns, false);
>> + if (!*ns_obj) {
>> + log_err("netns_new failed");
>> + return -1;
>> + }
>> + } else {
>> + if (!*ns_obj) {
>> + log_err("netns_obj is NULL");
>> + return -1;
>> + }
>> + netns_free(*ns_obj);
>> + *ns_obj = NULL;
>> + }
>> ns++;
>> + ns_obj++;
>> }
>> return 0;
>> }
>>
>> -static void netns_setup_namespaces_nofail(const char *verb)
>> +static void netns_setup_namespaces_nofail(enum NETNS_VERB verb)
>> {
>> const char * const *ns = namespaces;
>> - char cmd[128];
>> + struct netns_obj **ns_obj = netns_objs;
>>
>> while (*ns) {
>> - snprintf(cmd, sizeof(cmd), "ip netns %s %s >
>> /dev/null 2>&1", verb, *ns);
>> - system(cmd);
>> + if (verb == NETNS_ADD) {
>> + *ns_obj = netns_new(*ns, false);
>> + } else {
>> + if (*ns_obj)
>> + netns_free(*ns_obj);
>> + *ns_obj = NULL;
>> + }
>> ns++;
>> + ns_obj++;
>> }
>> }
>>
>> @@ -1250,17 +1274,17 @@ static void test_tc_redirect_peer_l3(struct
>> netns_setup_result *setup_result)
>> ({
>> \
>> struct netns_setup_result setup_result = { .dev_mode
>> = mode, }; \
>> if
>> (test__start_subtest(#name))
>> \
>> - if (ASSERT_OK(netns_setup_namespaces("add"),
>> "setup namespaces")) { \
>> + if
>> (ASSERT_OK(netns_setup_namespaces(NETNS_ADD), "setup namespaces")) {
>> \
>> if
>> (ASSERT_OK(netns_setup_links_and_routes(&setup_result), \
>> "setup links and
>> routes")) \
>> test_ ##
>> name(&setup_result); \
>> -
>> netns_setup_namespaces("delete"); \
>> + netns_setup_namespaces(NETNS_DEL);
>> \
>> }
>> \
>> })
>>
>> static void *test_tc_redirect_run_tests(void *arg)
>> {
>> - netns_setup_namespaces_nofail("delete");
>> + netns_setup_namespaces_nofail(NETNS_DEL);
>>
>> RUN_TEST(tc_redirect_peer, MODE_VETH);
>> RUN_TEST(tc_redirect_peer, MODE_NETKIT);
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect.
2024-07-30 16:33 ` Kui-Feng Lee
@ 2024-07-31 1:25 ` Geliang Tang
0 siblings, 0 replies; 10+ messages in thread
From: Geliang Tang @ 2024-07-31 1:25 UTC (permalink / raw)
To: Kui-Feng Lee, Kui-Feng Lee, bpf, ast, martin.lau, song,
kernel-team, andrii, sdf
Cc: kuifeng
On Tue, 2024-07-30 at 09:33 -0700, Kui-Feng Lee wrote:
>
>
> On 7/30/24 02:43, Geliang Tang wrote:
> > On Mon, 2024-07-29 at 17:27 -0700, Kui-Feng Lee wrote:
> > > Enable traffic monitoring for the test case tc_redirect.
> > >
> > > Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> > > ---
> > > .../selftests/bpf/prog_tests/tc_redirect.c | 48
> > > ++++++++++++++---
> > > --
> > > 1 file changed, 36 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> > > b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> > > index 327d51f59142..46d397c5c79a 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> > > @@ -68,6 +68,7 @@
> > > __FILE__, __LINE__, strerror(errno),
> > > ##__VA_ARGS__)
> > >
> > > static const char * const namespaces[] = {NS_SRC, NS_FWD,
> > > NS_DST,
> > > NULL};
> > > +static struct netns_obj *netns_objs[3];
> > >
> > > static int write_file(const char *path, const char *newval)
> > > {
> > > @@ -85,29 +86,52 @@ static int write_file(const char *path, const
> > > char *newval)
> > > return 0;
> > > }
> > >
> > > -static int netns_setup_namespaces(const char *verb)
> > > +enum NETNS_VERB {
> > > + NETNS_ADD,
> > > + NETNS_DEL,
> > > +};
> > > +
> > > +static int netns_setup_namespaces(enum NETNS_VERB verb)
> > > {
> > > const char * const *ns = namespaces;
> > > - char cmd[128];
> > > + struct netns_obj **ns_obj = netns_objs;
> > >
> > > while (*ns) {
> > > - snprintf(cmd, sizeof(cmd), "ip netns %s %s",
> > > verb,
> > > *ns);
> > > - if (!ASSERT_OK(system(cmd), cmd))
> > > - return -1;
> > > + if (verb == NETNS_ADD) {
> >
> > Maybe better to keep "verb" parameter as "char *", and use
> >
> > if (!strcmp(verb, "add"))
> >
> > here instead?
>
> I have no strong opinion here.
> May I know why you think string is better here?
I don't mean that string is better, I mean that "keep using strings" is
better. If we continue to use string, test_tc_redirect_peer_l3 and
test_tc_redirect_run_tests can remain unchanged, it at least makes this
patch smaller.
>
> >
> > > + *ns_obj = netns_new(*ns, false);
> > > + if (!*ns_obj) {
> > > + log_err("netns_new failed");
> > > + return -1;
> > > + }
> > > + } else {
> > > + if (!*ns_obj) {
> > > + log_err("netns_obj is NULL");
> > > + return -1;
> > > + }
> > > + netns_free(*ns_obj);
> > > + *ns_obj = NULL;
> > > + }
> > > ns++;
> > > + ns_obj++;
> > > }
> > > return 0;
> > > }
> > >
> > > -static void netns_setup_namespaces_nofail(const char *verb)
> > > +static void netns_setup_namespaces_nofail(enum NETNS_VERB verb)
> > > {
> > > const char * const *ns = namespaces;
> > > - char cmd[128];
> > > + struct netns_obj **ns_obj = netns_objs;
> > >
> > > while (*ns) {
> > > - snprintf(cmd, sizeof(cmd), "ip netns %s %s >
> > > /dev/null 2>&1", verb, *ns);
> > > - system(cmd);
> > > + if (verb == NETNS_ADD) {
> > > + *ns_obj = netns_new(*ns, false);
> > > + } else {
> > > + if (*ns_obj)
> > > + netns_free(*ns_obj);
> > > + *ns_obj = NULL;
> > > + }
> > > ns++;
> > > + ns_obj++;
> > > }
> > > }
> > >
> > > @@ -1250,17 +1274,17 @@ static void
> > > test_tc_redirect_peer_l3(struct
> > > netns_setup_result *setup_result)
> > > ({
> > > \
> > > struct netns_setup_result setup_result = {
> > > .dev_mode
> > > = mode, }; \
> > > if
> > > (test__start_subtest(#name))
> > > \
> > > - if
> > > (ASSERT_OK(netns_setup_namespaces("add"),
> > > "setup namespaces")) { \
> > > + if
> > > (ASSERT_OK(netns_setup_namespaces(NETNS_ADD), "setup
> > > namespaces")) {
> > > \
> > > if
> > > (ASSERT_OK(netns_setup_links_and_routes(&setup_result), \
> > > "setup links and
> > > routes")) \
> > > test_ ##
> > > name(&setup_result); \
> > > -
> > > netns_setup_namespaces("delete")
> > > ; \
> > > + netns_setup_namespaces(NETNS_DEL
> > > );
> > > \
> > > }
> > > \
> > > })
> > >
> > > static void *test_tc_redirect_run_tests(void *arg)
> > > {
> > > - netns_setup_namespaces_nofail("delete");
> > > + netns_setup_namespaces_nofail(NETNS_DEL);
> > >
> > > RUN_TEST(tc_redirect_peer, MODE_VETH);
> > > RUN_TEST(tc_redirect_peer, MODE_NETKIT);
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf-next v3 5/6] selftests/bpf: Monitor traffic for sockmap_listen.
2024-07-30 0:27 [PATCH bpf-next v3 0/6] monitor network traffic for flaky test cases Kui-Feng Lee
` (3 preceding siblings ...)
2024-07-30 0:27 ` [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect Kui-Feng Lee
@ 2024-07-30 0:27 ` Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 6/6] selftests/bpf: Monitor traffic for select_reuseport Kui-Feng Lee
5 siblings, 0 replies; 10+ messages in thread
From: Kui-Feng Lee @ 2024-07-30 0:27 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii, sdf, geliang
Cc: sinquersw, kuifeng, Kui-Feng Lee
Enable traffic monitor for each subtest of sockmap_listen.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
index e91b59366030..2ca091a30a30 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
@@ -1893,14 +1893,23 @@ static void test_udp_unix_redir(struct test_sockmap_listen *skel, struct bpf_map
{
const char *family_name, *map_name;
char s[MAX_TEST_NAME];
+ struct netns_obj *netns;
family_name = family_str(family);
map_name = map_type_str(map);
snprintf(s, sizeof(s), "%s %s %s", map_name, family_name, __func__);
if (!test__start_subtest(s))
return;
+
+ netns = netns_new("test", true);
+ if (!ASSERT_OK_PTR(netns, "netns_new"))
+ return;
+ system("ip link set lo up");
+
inet_unix_skb_redir_to_connected(skel, map, family);
unix_inet_skb_redir_to_connected(skel, map, family);
+
+ netns_free(netns);
}
static void run_tests(struct test_sockmap_listen *skel, struct bpf_map *map,
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH bpf-next v3 6/6] selftests/bpf: Monitor traffic for select_reuseport.
2024-07-30 0:27 [PATCH bpf-next v3 0/6] monitor network traffic for flaky test cases Kui-Feng Lee
` (4 preceding siblings ...)
2024-07-30 0:27 ` [PATCH bpf-next v3 5/6] selftests/bpf: Monitor traffic for sockmap_listen Kui-Feng Lee
@ 2024-07-30 0:27 ` Kui-Feng Lee
5 siblings, 0 replies; 10+ messages in thread
From: Kui-Feng Lee @ 2024-07-30 0:27 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii, sdf, geliang
Cc: sinquersw, kuifeng, Kui-Feng Lee
Enable traffic monitoring for the subtests of select_reuseport.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
.../bpf/prog_tests/select_reuseport.c | 39 +++++++------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
index 64c5f5eb2994..1bb29137cd0a 100644
--- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
+++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
@@ -37,9 +37,7 @@ static int sk_fds[REUSEPORT_ARRAY_SIZE];
static int reuseport_array = -1, outer_map = -1;
static enum bpf_map_type inner_map_type;
static int select_by_skb_data_prog;
-static int saved_tcp_syncookie = -1;
static struct bpf_object *obj;
-static int saved_tcp_fo = -1;
static __u32 index_zero;
static int epfd;
@@ -193,14 +191,6 @@ static int write_int_sysctl(const char *sysctl, int v)
return 0;
}
-static void restore_sysctls(void)
-{
- if (saved_tcp_fo != -1)
- write_int_sysctl(TCP_FO_SYSCTL, saved_tcp_fo);
- if (saved_tcp_syncookie != -1)
- write_int_sysctl(TCP_SYNCOOKIE_SYSCTL, saved_tcp_syncookie);
-}
-
static int enable_fastopen(void)
{
int fo;
@@ -795,6 +785,7 @@ static void test_config(int sotype, sa_family_t family, bool inany)
};
char s[MAX_TEST_NAME];
const struct test *t;
+ struct netns_obj *netns;
for (t = tests; t < tests + ARRAY_SIZE(tests); t++) {
if (t->need_sotype && t->need_sotype != sotype)
@@ -808,9 +799,23 @@ static void test_config(int sotype, sa_family_t family, bool inany)
if (!test__start_subtest(s))
continue;
+ netns = netns_new("test", true);
+ if (!ASSERT_OK_PTR(netns, "netns_new"))
+ continue;
+
+ system("ip link set dev lo up");
+
+ if (CHECK_FAIL(enable_fastopen()))
+ goto out;
+ if (CHECK_FAIL(disable_syncookie()))
+ goto out;
+
setup_per_test(sotype, family, inany, t->no_inner_map);
t->fn(sotype, family);
cleanup_per_test(t->no_inner_map);
+
+out:
+ netns_free(netns);
}
}
@@ -850,21 +855,7 @@ void test_map_type(enum bpf_map_type mt)
void serial_test_select_reuseport(void)
{
- saved_tcp_fo = read_int_sysctl(TCP_FO_SYSCTL);
- if (saved_tcp_fo < 0)
- goto out;
- saved_tcp_syncookie = read_int_sysctl(TCP_SYNCOOKIE_SYSCTL);
- if (saved_tcp_syncookie < 0)
- goto out;
-
- if (enable_fastopen())
- goto out;
- if (disable_syncookie())
- goto out;
-
test_map_type(BPF_MAP_TYPE_REUSEPORT_SOCKARRAY);
test_map_type(BPF_MAP_TYPE_SOCKMAP);
test_map_type(BPF_MAP_TYPE_SOCKHASH);
-out:
- restore_sysctls();
}
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread