[bpf-next PATCH v2 0/7] sockmap sample update

[bpf-next PATCH v2 0/7] sockmap sample update

[John Fastabend]

The sockmap sample is pretty simple at the moment. All it does is open
a few sockets attach BPF programs/sockmaps and sends a few packets.

However, for testing and debugging I wanted to have more control over
the sendmsg format and data than provided by tools like iperf3/netperf,
etc. The reason is for testing BPF programs and stream parser it is
helpful to be able submit multiple sendmsg calls with different msg
layouts. For example lots of 1B iovs or a single large MB of data, etc.

Additionally, my current test setup requires an entire orchestration
layer (cilium) to run. As well as lighttpd and http traffic generators
or for kafka testing brokers and clients. This makes it a bit more
difficult when doing performance optimizations to incrementally test
small changes and come up with performance delta's and perf numbers.

By adding a few more options and an additional few tests the sockmap
sample program can show a more complete example and do some of the
above. Because the sample program is self contained it doesn't require
additional infrastructure to run either.

This series, although still fairly crude, does provide some nice
additions. They are

  - a new sendmsg tests with a sender and recv threads
  - a new base tests so we can get metrics/data without BPF
  - multiple GBps of throughput on base and sendmsg tests
  - automatically set rlimit and common variables

That said the UI is still primitive, more features could be added,
more tests might be useful, the reporting is bare bones, etc. But,
IMO lets push this now rather than sit on it for weeks until I get
time to do the above improvements. Additional patches can address
the other limitations/issues.

v2: removed bogus file added by patch 3/7
---

John Fastabend (7):
      bpf: refactor sockmap sample program update for arg parsing
      bpf: add sendmsg option for testing BPF programs
      bpf: sockmap sample, use fork() for send and recv
      bpf: sockmap sample, report bytes/sec
      bpf: sockmap sample add base test without any BPF for comparison
      bpf: sockmap put client sockets in blocking mode
      bpf: sockmap set rlimit


 samples/sockmap/Makefile       |    2 
 samples/sockmap/sockmap_user.c |  357 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 318 insertions(+), 41 deletions(-)

[bpf-next PATCH v2 1/7] bpf: refactor sockmap sample program update for arg parsing

[John Fastabend]

sockmap sample program takes arguments from cmd line but it reads them
in using offsets into the array. Because we want to add more arguments
in the future lets do proper argument handling.

Also refactor code to pull apart sock init and ping/pong test. This
allows us to add new tests in the future.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 samples/sockmap/sockmap_user.c |  142 +++++++++++++++++++++++++++++-----------
 1 file changed, 103 insertions(+), 39 deletions(-)

diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index 7cc9d22..5cbe7a5 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -35,6 +35,8 @@
 #include <assert.h>
 #include <libgen.h>
 
+#include <getopt.h>
+
 #include "../bpf/bpf_load.h"
 #include "../bpf/bpf_util.h"
 #include "../bpf/libbpf.h"
@@ -46,15 +48,39 @@
 #define S1_PORT 10000
 #define S2_PORT 10001
 
-static int sockmap_test_sockets(int rate, int dot)
+/* global sockets */
+int s1, s2, c1, c2, p1, p2;
+
+static const struct option long_options[] = {
+	{"help",	no_argument,		NULL, 'h' },
+	{"cgroup",	required_argument,	NULL, 'c' },
+	{"rate",	required_argument,	NULL, 'r' },
+	{"verbose",	no_argument,		NULL, 'v' },
+	{0, 0, NULL, 0 }
+};
+
+static void usage(char *argv[])
+{
+	int i;
+
+	printf(" Usage: %s --cgroup <cgroup_path>\n", argv[0]);
+	printf(" options:\n");
+	for (i = 0; long_options[i].name != 0; i++) {
+		printf(" --%-12s", long_options[i].name);
+		if (long_options[i].flag != NULL)
+			printf(" flag (internal value:%d)\n",
+				*long_options[i].flag);
+		else
+			printf(" -%c\n", long_options[i].val);
+	}
+	printf("\n");
+}
+
+static int sockmap_init_sockets(void)
 {
-	int i, sc, err, max_fd, one = 1;
-	int s1, s2, c1, c2, p1, p2;
+	int i, err, one = 1;
 	struct sockaddr_in addr;
-	struct timeval timeout;
-	char buf[1024] = {0};
 	int *fds[4] = {&s1, &s2, &c1, &c2};
-	fd_set w;
 
 	s1 = s2 = p1 = p2 = c1 = c2 = 0;
 
@@ -134,6 +160,8 @@ static int sockmap_test_sockets(int rate, int dot)
 	if (err < 0 && errno != EINPROGRESS) {
 		perror("connect c2 failed()\n");
 		goto out;
+	} else if (err < 0) {
+		err = 0;
 	}
 
 	/* Accept Connecrtions */
@@ -149,23 +177,32 @@ static int sockmap_test_sockets(int rate, int dot)
 		goto out;
 	}
 
-	max_fd = p2;
-	timeout.tv_sec = 10;
-	timeout.tv_usec = 0;
-
 	printf("connected sockets: c1 <-> p1, c2 <-> p2\n");
 	printf("cgroups binding: c1(%i) <-> s1(%i) - - - c2(%i) <-> s2(%i)\n",
 		c1, s1, c2, s2);
+out:
+	return err;
+}
+
+static int forever_ping_pong(int rate, int verbose)
+{
+	struct timeval timeout;
+	char buf[1024] = {0};
+	int sc;
+
+	timeout.tv_sec = 10;
+	timeout.tv_usec = 0;
 
 	/* Ping/Pong data from client to server */
 	sc = send(c1, buf, sizeof(buf), 0);
 	if (sc < 0) {
 		perror("send failed()\n");
-		goto out;
+		return sc;
 	}
 
 	do {
-		int s, rc, i;
+		int s, rc, i, max_fd = p2;
+		fd_set w;
 
 		/* FD sets */
 		FD_ZERO(&w);
@@ -193,7 +230,7 @@ static int sockmap_test_sockets(int rate, int dot)
 			if (rc < 0) {
 				if (errno != EWOULDBLOCK) {
 					perror("recv failed()\n");
-					break;
+					return rc;
 				}
 			}
 
@@ -205,35 +242,60 @@ static int sockmap_test_sockets(int rate, int dot)
 			sc = send(i, buf, rc, 0);
 			if (sc < 0) {
 				perror("send failed()\n");
-				break;
+				return sc;
 			}
 		}
-		sleep(rate);
-		if (dot) {
+
+		if (rate)
+			sleep(rate);
+
+		if (verbose) {
 			printf(".");
 			fflush(stdout);
 
 		}
 	} while (running);
 
-out:
-	close(s1);
-	close(s2);
-	close(p1);
-	close(p2);
-	close(c1);
-	close(c2);
-	return err;
+	return 0;
 }
 
 int main(int argc, char **argv)
 {
-	int rate = 1, dot = 1;
+	int rate = 1, verbose = 0;
+	int opt, longindex, err, cg_fd = 0;
 	char filename[256];
-	int err, cg_fd;
-	char *cg_path;
 
-	cg_path = argv[argc - 1];
+	while ((opt = getopt_long(argc, argv, "hvc:r:",
+				  long_options, &longindex)) != -1) {
+		switch (opt) {
+		/* Cgroup configuration */
+		case 'c':
+			cg_fd = open(optarg, O_DIRECTORY, O_RDONLY);
+			if (cg_fd < 0) {
+				fprintf(stderr, "ERROR: (%i) open cg path failed: %s\n",
+					cg_fd, optarg);
+				return cg_fd;
+			}
+			break;
+		case 'r':
+			rate = atoi(optarg);
+			break;
+		case 'v':
+			verbose = 1;
+			break;
+		case 'h':
+		default:
+			usage(argv);
+			return -1;
+		}
+	}
+
+	if (!cg_fd) {
+		fprintf(stderr, "%s requires cgroup option: --cgroup <path>\n",
+			argv[0]);
+		return -1;
+	}
+
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 
 	running = 1;
@@ -247,14 +309,6 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	/* Cgroup configuration */
-	cg_fd = open(cg_path, O_DIRECTORY, O_RDONLY);
-	if (cg_fd < 0) {
-		fprintf(stderr, "ERROR: (%i) open cg path failed: %s\n",
-			cg_fd, cg_path);
-		return cg_fd;
-	}
-
 	/* Attach programs to sockmap */
 	err = bpf_prog_attach(prog_fd[0], map_fd[0],
 				BPF_SK_SKB_STREAM_PARSER, 0);
@@ -280,15 +334,25 @@ int main(int argc, char **argv)
 		return err;
 	}
 
-	err = sockmap_test_sockets(rate, dot);
+	err = sockmap_init_sockets();
 	if (err) {
 		fprintf(stderr, "ERROR: test socket failed: %d\n", err);
-		return err;
+		goto out;
 	}
-	return 0;
+
+	err = forever_ping_pong(rate, verbose);
+out:
+	close(s1);
+	close(s2);
+	close(p1);
+	close(p2);
+	close(c1);
+	close(c2);
+	return err;
 }
 
 void running_handler(int a)
 {
 	running = 0;
+	printf("\n");
 }

[bpf-next PATCH v2 2/7] bpf: add sendmsg option for testing BPF programs

[John Fastabend]

When testing BPF programs using sockmap I often want to have more
control over how sendmsg is exercised. This becomes even more useful
as new sockmap program types are added.

This adds a test type option to select type of test to run. Currently,
only "ping" and "sendmsg" are supported, but more can be added as
needed.

The new help argument gives the following,

 Usage: ./sockmap --cgroup <cgroup_path>
 options:
 --help         -h
 --cgroup       -c
 --rate         -r
 --verbose      -v
 --iov_count    -i
 --length       -l
 --test         -t

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 samples/sockmap/sockmap_user.c |  143 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 140 insertions(+), 3 deletions(-)

diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index 5cbe7a5..2d51672 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -56,6 +56,9 @@
 	{"cgroup",	required_argument,	NULL, 'c' },
 	{"rate",	required_argument,	NULL, 'r' },
 	{"verbose",	no_argument,		NULL, 'v' },
+	{"iov_count",	required_argument,	NULL, 'i' },
+	{"length",	required_argument,	NULL, 'l' },
+	{"test",	required_argument,	NULL, 't' },
 	{0, 0, NULL, 0 }
 };
 
@@ -184,6 +187,113 @@ static int sockmap_init_sockets(void)
 	return err;
 }
 
+struct msg_stats {
+	size_t bytes_sent;
+	size_t bytes_recvd;
+};
+
+static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
+		    struct msg_stats *s, bool tx)
+{
+	struct msghdr msg = {0};
+	struct iovec *iov;
+	int i, flags = 0;
+
+	iov = calloc(iov_count, sizeof(struct iovec));
+	if (!iov)
+		return -ENOMEM;
+
+	for (i = 0; i < iov_count; i++) {
+		char *d = calloc(iov_length, sizeof(char));
+
+		if (!d) {
+			fprintf(stderr, "iov_count %i/%i OOM\n", i, iov_count);
+			free(iov);
+			return -ENOMEM;
+		}
+		iov[i].iov_base = d;
+		iov[i].iov_len = iov_length;
+	}
+
+	msg.msg_iov = iov;
+	msg.msg_iovlen = iov_count;
+
+	if (tx) {
+		for (i = 0; i < cnt; i++) {
+			int sent = sendmsg(fd, &msg, flags);
+
+			if (sent < 0) {
+				perror("send loop error:");
+				return sent;
+			}
+			s->bytes_sent += sent;
+		}
+	} else {
+		int slct, recv, max_fd = fd;
+		struct timeval timeout;
+		float total_bytes;
+		fd_set w;
+
+		total_bytes = (float)iov_count * (float)iov_length * (float)cnt;
+		while (s->bytes_recvd < total_bytes) {
+			timeout.tv_sec = 1;
+			timeout.tv_usec = 0;
+
+			/* FD sets */
+			FD_ZERO(&w);
+			FD_SET(fd, &w);
+
+			slct = select(max_fd + 1, &w, NULL, NULL, &timeout);
+			if (slct == -1) {
+				perror("select()");
+				return slct;
+			} else if (!slct) {
+				fprintf(stderr, "unexpected timeout\n");
+				return slct;
+			}
+
+			recv = recvmsg(fd, &msg, flags);
+			if (recv < 0) {
+				if (errno != EWOULDBLOCK) {
+					perror("recv failed()\n");
+					return errno;
+				}
+			}
+
+			s->bytes_recvd += recv;
+		}
+	}
+
+	for (i = 0; i < iov_count; i++)
+		free(iov[i].iov_base);
+	free(iov);
+	return 0;
+}
+
+static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
+{
+	struct msg_stats s = {0};
+	int err;
+
+	err = msg_loop(c1, iov_count, iov_buf, cnt, &s, true);
+	if (err) {
+		fprintf(stderr,
+			"msg_loop_tx: iov_count %i iov_buf %i cnt %i err %i\n",
+			iov_count, iov_buf, cnt, err);
+		return err;
+	}
+
+	msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
+	if (err)
+		fprintf(stderr,
+			"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
+			iov_count, iov_buf, cnt, err);
+
+	fprintf(stdout, "sendmsg: TX_bytes %zu RX_bytes %zu\n",
+		s.bytes_sent, s.bytes_recvd);
+	return err;
+}
+
 static int forever_ping_pong(int rate, int verbose)
 {
 	struct timeval timeout;
@@ -259,13 +369,19 @@ static int forever_ping_pong(int rate, int verbose)
 	return 0;
 }
 
+enum {
+	PING_PONG,
+	SENDMSG,
+};
+
 int main(int argc, char **argv)
 {
-	int rate = 1, verbose = 0;
+	int iov_count = 1, length = 1024, rate = 1, verbose = 0;
 	int opt, longindex, err, cg_fd = 0;
+	int test = PING_PONG;
 	char filename[256];
 
-	while ((opt = getopt_long(argc, argv, "hvc:r:",
+	while ((opt = getopt_long(argc, argv, "hvc:r:i:l:t:",
 				  long_options, &longindex)) != -1) {
 		switch (opt) {
 		/* Cgroup configuration */
@@ -283,6 +399,22 @@ int main(int argc, char **argv)
 		case 'v':
 			verbose = 1;
 			break;
+		case 'i':
+			iov_count = atoi(optarg);
+			break;
+		case 'l':
+			length = atoi(optarg);
+			break;
+		case 't':
+			if (memcmp(optarg, "ping", 4) == 0) {
+				test = PING_PONG;
+			} else if (memcmp(optarg, "sendmsg", 7) == 0) {
+				test = SENDMSG;
+			} else {
+				usage(argv);
+				return -1;
+			}
+			break;
 		case 'h':
 		default:
 			usage(argv);
@@ -340,7 +472,12 @@ int main(int argc, char **argv)
 		goto out;
 	}
 
-	err = forever_ping_pong(rate, verbose);
+	if (test == PING_PONG)
+		err = forever_ping_pong(rate, verbose);
+	else if (test == SENDMSG)
+		err = sendmsg_test(iov_count, length, rate, verbose);
+	else
+		fprintf(stderr, "unknown test\n");
 out:
 	close(s1);
 	close(s2);

[bpf-next PATCH v2 3/7] bpf: sockmap sample, use fork() for send and recv

[John Fastabend]

Currently for SENDMSG tests first send completes then recv runs. This
does not work well for large data sizes and/or many iterations. So
fork the recv and send handler so that we run both send and recv. In
the future we can add a parameter to do more than a single fork of
tx/rx.

With this we can get many GBps of data which helps exercise the
sockmap code.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 samples/sockmap/Makefile       |    2 +
 samples/sockmap/sockmap_user.c |   58 +++++++++++++++++++++++++++++-----------
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/samples/sockmap/Makefile b/samples/sockmap/Makefile
index 73f1da4..4fefd66 100644
--- a/samples/sockmap/Makefile
+++ b/samples/sockmap/Makefile
@@ -8,7 +8,7 @@ HOSTCFLAGS += -I$(objtree)/usr/include
 HOSTCFLAGS += -I$(srctree)/tools/lib/
 HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
 HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
-HOSTCFLAGS += -I$(srctree)/tools/perf
+HOSTCFLAGS += -I$(srctree)/tools/perf -g
 
 sockmap-objs := ../bpf/bpf_load.o $(LIBBPF) sockmap_user.o
 
diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index 2d51672..48fa09a 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -23,6 +23,7 @@
 #include <stdbool.h>
 #include <signal.h>
 #include <fcntl.h>
+#include <sys/wait.h>
 
 #include <sys/time.h>
 #include <sys/types.h>
@@ -197,7 +198,7 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 {
 	struct msghdr msg = {0};
 	struct iovec *iov;
-	int i, flags = 0;
+	int i, flags = MSG_NOSIGNAL;
 
 	iov = calloc(iov_count, sizeof(struct iovec));
 	if (!iov)
@@ -272,25 +273,50 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 
 static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
 {
+	int txpid, rxpid, err = 0;
 	struct msg_stats s = {0};
-	int err;
-
-	err = msg_loop(c1, iov_count, iov_buf, cnt, &s, true);
-	if (err) {
-		fprintf(stderr,
-			"msg_loop_tx: iov_count %i iov_buf %i cnt %i err %i\n",
-			iov_count, iov_buf, cnt, err);
-		return err;
+	int status;
+
+	errno = 0;
+
+	rxpid = fork();
+	if (rxpid == 0) {
+		err = msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
+		if (err)
+			fprintf(stderr,
+				"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
+				iov_count, iov_buf, cnt, err);
+		fprintf(stdout, "rx_sendmsg: TX_bytes %zu RX_bytes %zu\n",
+			s.bytes_sent, s.bytes_recvd);
+		shutdown(p2, SHUT_RDWR);
+		shutdown(p1, SHUT_RDWR);
+		exit(1);
+	} else if (rxpid == -1) {
+		perror("msg_loop_rx: ");
+		err = errno;
 	}
 
-	msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
-	if (err)
-		fprintf(stderr,
-			"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
-			iov_count, iov_buf, cnt, err);
+	txpid = fork();
+	if (txpid == 0) {
+		err = msg_loop(c1, iov_count, iov_buf, cnt, &s, true);
+		if (err)
+			fprintf(stderr,
+				"msg_loop_tx: iov_count %i iov_buf %i cnt %i err %i\n",
+				iov_count, iov_buf, cnt, err);
+		fprintf(stdout, "tx_sendmsg: TX_bytes %zu RX_bytes %zu\n",
+			s.bytes_sent, s.bytes_recvd);
+		shutdown(c1, SHUT_RDWR);
+		exit(1);
+	} else if (txpid == -1) {
+		perror("msg_loop_tx: ");
+		return errno;
+	}
 
-	fprintf(stdout, "sendmsg: TX_bytes %zu RX_bytes %zu\n",
-		s.bytes_sent, s.bytes_recvd);
+	assert(waitpid(rxpid, &status, 0) == rxpid);
+	if (!txpid)
+		goto out;
+	assert(waitpid(txpid, &status, 0) == txpid);
+out:
 	return err;
 }
 

[bpf-next PATCH v2 4/7] bpf: sockmap sample, report bytes/sec

[John Fastabend]

Report bytes/sec sent as well as total bytes. Useful to get rough
idea how different configurations and usage patterns perform with
sockmap.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 samples/sockmap/sockmap_user.c |   37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index 48fa09a..812fc7e 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -24,6 +24,7 @@
 #include <signal.h>
 #include <fcntl.h>
 #include <sys/wait.h>
+#include <time.h>
 
 #include <sys/time.h>
 #include <sys/types.h>
@@ -191,14 +192,16 @@ static int sockmap_init_sockets(void)
 struct msg_stats {
 	size_t bytes_sent;
 	size_t bytes_recvd;
+	struct timespec start;
+	struct timespec end;
 };
 
 static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 		    struct msg_stats *s, bool tx)
 {
 	struct msghdr msg = {0};
+	int err, i, flags = MSG_NOSIGNAL;
 	struct iovec *iov;
-	int i, flags = MSG_NOSIGNAL;
 
 	iov = calloc(iov_count, sizeof(struct iovec));
 	if (!iov)
@@ -220,6 +223,7 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 	msg.msg_iovlen = iov_count;
 
 	if (tx) {
+		clock_gettime(CLOCK_MONOTONIC, &s->start);
 		for (i = 0; i < cnt; i++) {
 			int sent = sendmsg(fd, &msg, flags);
 
@@ -229,6 +233,7 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 			}
 			s->bytes_sent += sent;
 		}
+		clock_gettime(CLOCK_MONOTONIC, &s->end);
 	} else {
 		int slct, recv, max_fd = fd;
 		struct timeval timeout;
@@ -236,6 +241,9 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 		fd_set w;
 
 		total_bytes = (float)iov_count * (float)iov_length * (float)cnt;
+		err = clock_gettime(CLOCK_MONOTONIC, &s->start);
+		if (err < 0)
+			perror("recv start time: ");
 		while (s->bytes_recvd < total_bytes) {
 			timeout.tv_sec = 1;
 			timeout.tv_usec = 0;
@@ -247,15 +255,18 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 			slct = select(max_fd + 1, &w, NULL, NULL, &timeout);
 			if (slct == -1) {
 				perror("select()");
+				clock_gettime(CLOCK_MONOTONIC, &s->end);
 				return slct;
 			} else if (!slct) {
 				fprintf(stderr, "unexpected timeout\n");
+				clock_gettime(CLOCK_MONOTONIC, &s->end);
 				return slct;
 			}
 
 			recv = recvmsg(fd, &msg, flags);
 			if (recv < 0) {
 				if (errno != EWOULDBLOCK) {
+					clock_gettime(CLOCK_MONOTONIC, &s->end);
 					perror("recv failed()\n");
 					return errno;
 				}
@@ -263,6 +274,7 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 
 			s->bytes_recvd += recv;
 		}
+		clock_gettime(CLOCK_MONOTONIC, &s->end);
 	}
 
 	for (i = 0; i < iov_count; i++)
@@ -271,11 +283,14 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 	return 0;
 }
 
+static float giga = 1000000000;
+
 static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
 {
 	int txpid, rxpid, err = 0;
 	struct msg_stats s = {0};
 	int status;
+	float sent_Bps = 0, recvd_Bps = 0;
 
 	errno = 0;
 
@@ -286,10 +301,16 @@ static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
 			fprintf(stderr,
 				"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
 				iov_count, iov_buf, cnt, err);
-		fprintf(stdout, "rx_sendmsg: TX_bytes %zu RX_bytes %zu\n",
-			s.bytes_sent, s.bytes_recvd);
 		shutdown(p2, SHUT_RDWR);
 		shutdown(p1, SHUT_RDWR);
+		if (s.end.tv_sec - s.start.tv_sec) {
+			sent_Bps = s.bytes_sent / (s.end.tv_sec - s.start.tv_sec);
+			recvd_Bps = s.bytes_recvd / (s.end.tv_sec - s.start.tv_sec);
+		}
+		fprintf(stdout,
+			"rx_sendmsg: TX: %zuB %fB/s %fGB/s RX: %zuB %fB/s %fGB/s\n",
+			s.bytes_sent, sent_Bps, sent_Bps/giga,
+			s.bytes_recvd, recvd_Bps, recvd_Bps/giga);
 		exit(1);
 	} else if (rxpid == -1) {
 		perror("msg_loop_rx: ");
@@ -303,9 +324,15 @@ static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
 			fprintf(stderr,
 				"msg_loop_tx: iov_count %i iov_buf %i cnt %i err %i\n",
 				iov_count, iov_buf, cnt, err);
-		fprintf(stdout, "tx_sendmsg: TX_bytes %zu RX_bytes %zu\n",
-			s.bytes_sent, s.bytes_recvd);
 		shutdown(c1, SHUT_RDWR);
+		if (s.end.tv_sec - s.start.tv_sec) {
+			sent_Bps = s.bytes_sent / (s.end.tv_sec - s.start.tv_sec);
+			recvd_Bps = s.bytes_recvd / (s.end.tv_sec - s.start.tv_sec);
+		}
+		fprintf(stdout,
+			"tx_sendmsg: TX: %zuB %fB/s %f GB/s RX: %zuB %fB/s %fGB/s\n",
+			s.bytes_sent, sent_Bps, sent_Bps/giga,
+			s.bytes_recvd, recvd_Bps, recvd_Bps/giga);
 		exit(1);
 	} else if (txpid == -1) {
 		perror("msg_loop_tx: ");

[bpf-next PATCH v2 5/7] bpf: sockmap sample add base test without any BPF for comparison

[John Fastabend]

Add a base test that does not use BPF hooks to test baseline case.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 samples/sockmap/sockmap_user.c |   26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index 812fc7e..eb19d14 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -285,18 +285,24 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 
 static float giga = 1000000000;
 
-static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
+static int sendmsg_test(int iov_count, int iov_buf, int cnt,
+			int verbose, bool base)
 {
-	int txpid, rxpid, err = 0;
+	float sent_Bps = 0, recvd_Bps = 0;
+	int rx_fd, txpid, rxpid, err = 0;
 	struct msg_stats s = {0};
 	int status;
-	float sent_Bps = 0, recvd_Bps = 0;
 
 	errno = 0;
 
+	if (base)
+		rx_fd = p1;
+	else
+		rx_fd = p2;
+
 	rxpid = fork();
 	if (rxpid == 0) {
-		err = msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
+		err = msg_loop(rx_fd, iov_count, iov_buf, cnt, &s, false);
 		if (err)
 			fprintf(stderr,
 				"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
@@ -425,6 +431,7 @@ static int forever_ping_pong(int rate, int verbose)
 enum {
 	PING_PONG,
 	SENDMSG,
+	BASE,
 };
 
 int main(int argc, char **argv)
@@ -463,6 +470,8 @@ int main(int argc, char **argv)
 				test = PING_PONG;
 			} else if (memcmp(optarg, "sendmsg", 7) == 0) {
 				test = SENDMSG;
+			} else if (memcmp(optarg, "base", 4) == 0) {
+				test = BASE;
 			} else {
 				usage(argv);
 				return -1;
@@ -488,6 +497,10 @@ int main(int argc, char **argv)
 	/* catch SIGINT */
 	signal(SIGINT, running_handler);
 
+	/* If base test skip BPF setup */
+	if (test == BASE)
+		goto run;
+
 	if (load_bpf_file(filename)) {
 		fprintf(stderr, "load_bpf_file: (%s) %s\n",
 			filename, strerror(errno));
@@ -519,6 +532,7 @@ int main(int argc, char **argv)
 		return err;
 	}
 
+run:
 	err = sockmap_init_sockets();
 	if (err) {
 		fprintf(stderr, "ERROR: test socket failed: %d\n", err);
@@ -528,7 +542,9 @@ int main(int argc, char **argv)
 	if (test == PING_PONG)
 		err = forever_ping_pong(rate, verbose);
 	else if (test == SENDMSG)
-		err = sendmsg_test(iov_count, length, rate, verbose);
+		err = sendmsg_test(iov_count, length, rate, verbose, false);
+	else if (test == BASE)
+		err = sendmsg_test(iov_count, length, rate, verbose, true);
 	else
 		fprintf(stderr, "unknown test\n");
 out:

[bpf-next PATCH v2 6/7] bpf: sockmap put client sockets in blocking mode

[John Fastabend]

Put client sockets in blocking mode otherwise with sendmsg tests
its easy to overrun the socket buffers which results in the test
being aborted.

The original non-blocking was added to handle listen/accept with
a single thread the client/accepted sockets do not need to be
non-blocking.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 samples/sockmap/sockmap_user.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index eb19d14..9496b2c 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -110,7 +110,7 @@ static int sockmap_init_sockets(void)
 	}
 
 	/* Non-blocking sockets */
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < 2; i++) {
 		err = ioctl(*fds[i], FIONBIO, (char *)&one);
 		if (err < 0) {
 			perror("ioctl s1 failed()");

[bpf-next PATCH v2 7/7] bpf: sockmap set rlimit

[John Fastabend]

Avoid extra step of setting limit from cmdline and do it directly in
the program.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 samples/sockmap/sockmap_user.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index 9496b2c..16c19c5 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -27,6 +27,7 @@
 #include <time.h>
 
 #include <sys/time.h>
+#include <sys/resource.h>
 #include <sys/types.h>
 
 #include <linux/netlink.h>
@@ -437,6 +438,7 @@ enum {
 int main(int argc, char **argv)
 {
 	int iov_count = 1, length = 1024, rate = 1, verbose = 0;
+	struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
 	int opt, longindex, err, cg_fd = 0;
 	int test = PING_PONG;
 	char filename[256];
@@ -490,6 +492,11 @@ int main(int argc, char **argv)
 		return -1;
 	}
 
+	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
+		perror("setrlimit(RLIMIT_MEMLOCK)");
+		return 1;
+	}
+
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 
 	running = 1;

Re: [bpf-next PATCH v2 1/7] bpf: refactor sockmap sample program update for arg parsing

[Daniel Borkmann]

On 01/10/2018 07:39 PM, John Fastabend wrote:
> sockmap sample program takes arguments from cmd line but it reads them
> in using offsets into the array. Because we want to add more arguments
> in the future lets do proper argument handling.
> 
> Also refactor code to pull apart sock init and ping/pong test. This
> allows us to add new tests in the future.
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  samples/sockmap/sockmap_user.c |  142 +++++++++++++++++++++++++++++-----------
>  1 file changed, 103 insertions(+), 39 deletions(-)
[...]
>  
>  	/* Accept Connecrtions */
> @@ -149,23 +177,32 @@ static int sockmap_test_sockets(int rate, int dot)
>  		goto out;
>  	}
>  
> -	max_fd = p2;
> -	timeout.tv_sec = 10;
> -	timeout.tv_usec = 0;
> -
>  	printf("connected sockets: c1 <-> p1, c2 <-> p2\n");
>  	printf("cgroups binding: c1(%i) <-> s1(%i) - - - c2(%i) <-> s2(%i)\n",
>  		c1, s1, c2, s2);
> +out:
> +	return err;

Maybe rather than setting err and goto out where we now just return
err anyway, return from those places directly.

> +}
> +
> +static int forever_ping_pong(int rate, int verbose)
> +{
> +	struct timeval timeout;
> +	char buf[1024] = {0};
> +	int sc;
> +
> +	timeout.tv_sec = 10;
> +	timeout.tv_usec = 0;
>  
>  	/* Ping/Pong data from client to server */
>  	sc = send(c1, buf, sizeof(buf), 0);
>  	if (sc < 0) {
>  		perror("send failed()\n");
> -		goto out;
> +		return sc;
>  	}
>  
>  	do {
> -		int s, rc, i;
> +		int s, rc, i, max_fd = p2;
> +		fd_set w;
>  
>  		/* FD sets */
>  		FD_ZERO(&w);
[...]
> -	err = sockmap_test_sockets(rate, dot);
> +	err = sockmap_init_sockets();
>  	if (err) {
>  		fprintf(stderr, "ERROR: test socket failed: %d\n", err);
> -		return err;
> +		goto out;
>  	}
> -	return 0;
> +
> +	err = forever_ping_pong(rate, verbose);
> +out:
> +	close(s1);
> +	close(s2);
> +	close(p1);
> +	close(p2);
> +	close(c1);
> +	close(c2);
> +	return err;
>  }
>  
>  void running_handler(int a)
>  {
>  	running = 0;
> +	printf("\n");

Do we need this out of the sighandler instead of e.g. main loop when
we break out?

>  }
> 

Re: [bpf-next PATCH v2 2/7] bpf: add sendmsg option for testing BPF programs

[Daniel Borkmann]

On 01/10/2018 07:39 PM, John Fastabend wrote:
> When testing BPF programs using sockmap I often want to have more
> control over how sendmsg is exercised. This becomes even more useful
> as new sockmap program types are added.
> 
> This adds a test type option to select type of test to run. Currently,
> only "ping" and "sendmsg" are supported, but more can be added as
> needed.
> 
> The new help argument gives the following,
> 
>  Usage: ./sockmap --cgroup <cgroup_path>
>  options:
>  --help         -h
>  --cgroup       -c
>  --rate         -r
>  --verbose      -v
>  --iov_count    -i
>  --length       -l
>  --test         -t
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  samples/sockmap/sockmap_user.c |  143 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 140 insertions(+), 3 deletions(-)
> 
> diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
> index 5cbe7a5..2d51672 100644
> --- a/samples/sockmap/sockmap_user.c
> +++ b/samples/sockmap/sockmap_user.c
> @@ -56,6 +56,9 @@
>  	{"cgroup",	required_argument,	NULL, 'c' },
>  	{"rate",	required_argument,	NULL, 'r' },
>  	{"verbose",	no_argument,		NULL, 'v' },
> +	{"iov_count",	required_argument,	NULL, 'i' },
> +	{"length",	required_argument,	NULL, 'l' },
> +	{"test",	required_argument,	NULL, 't' },
>  	{0, 0, NULL, 0 }
>  };
>  
> @@ -184,6 +187,113 @@ static int sockmap_init_sockets(void)
>  	return err;
>  }
>  
> +struct msg_stats {
> +	size_t bytes_sent;
> +	size_t bytes_recvd;
> +};
> +
> +static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
> +		    struct msg_stats *s, bool tx)
> +{
> +	struct msghdr msg = {0};
> +	struct iovec *iov;
> +	int i, flags = 0;
> +
> +	iov = calloc(iov_count, sizeof(struct iovec));
> +	if (!iov)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < iov_count; i++) {
> +		char *d = calloc(iov_length, sizeof(char));
> +
> +		if (!d) {
> +			fprintf(stderr, "iov_count %i/%i OOM\n", i, iov_count);
> +			free(iov);

Here, we free() iov, but not prior allocated iov_base from the loop.

> +			return -ENOMEM;
> +		}
> +		iov[i].iov_base = d;
> +		iov[i].iov_len = iov_length;
> +	}
> +
> +	msg.msg_iov = iov;
> +	msg.msg_iovlen = iov_count;
> +
> +	if (tx) {
> +		for (i = 0; i < cnt; i++) {
> +			int sent = sendmsg(fd, &msg, flags);
> +
> +			if (sent < 0) {
> +				perror("send loop error:");
> +				return sent;

And here as well as other places where we bail out, we don't free anything.

> +			}
> +			s->bytes_sent += sent;
> +		}
> +	} else {
> +		int slct, recv, max_fd = fd;
> +		struct timeval timeout;
> +		float total_bytes;
> +		fd_set w;
> +
> +		total_bytes = (float)iov_count * (float)iov_length * (float)cnt;
> +		while (s->bytes_recvd < total_bytes) {
> +			timeout.tv_sec = 1;
> +			timeout.tv_usec = 0;
> +
> +			/* FD sets */
> +			FD_ZERO(&w);
> +			FD_SET(fd, &w);
> +
> +			slct = select(max_fd + 1, &w, NULL, NULL, &timeout);
> +			if (slct == -1) {
> +				perror("select()");
> +				return slct;
> +			} else if (!slct) {
> +				fprintf(stderr, "unexpected timeout\n");
> +				return slct;
> +			}
> +
> +			recv = recvmsg(fd, &msg, flags);
> +			if (recv < 0) {
> +				if (errno != EWOULDBLOCK) {
> +					perror("recv failed()\n");
> +					return errno;
> +				}
> +			}
> +
> +			s->bytes_recvd += recv;
> +		}
> +	}
> +
> +	for (i = 0; i < iov_count; i++)
> +		free(iov[i].iov_base);
> +	free(iov);
> +	return 0;
> +}

Re: [bpf-next PATCH v2 3/7] bpf: sockmap sample, use fork() for send and recv

[Daniel Borkmann]

On 01/10/2018 07:39 PM, John Fastabend wrote:
> Currently for SENDMSG tests first send completes then recv runs. This
> does not work well for large data sizes and/or many iterations. So
> fork the recv and send handler so that we run both send and recv. In
> the future we can add a parameter to do more than a single fork of
> tx/rx.
> 
> With this we can get many GBps of data which helps exercise the
> sockmap code.
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  samples/sockmap/Makefile       |    2 +
>  samples/sockmap/sockmap_user.c |   58 +++++++++++++++++++++++++++++-----------
>  2 files changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/samples/sockmap/Makefile b/samples/sockmap/Makefile
> index 73f1da4..4fefd66 100644
> --- a/samples/sockmap/Makefile
> +++ b/samples/sockmap/Makefile
> @@ -8,7 +8,7 @@ HOSTCFLAGS += -I$(objtree)/usr/include
>  HOSTCFLAGS += -I$(srctree)/tools/lib/
>  HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
>  HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
> -HOSTCFLAGS += -I$(srctree)/tools/perf
> +HOSTCFLAGS += -I$(srctree)/tools/perf -g

Slipped in here?

>  sockmap-objs := ../bpf/bpf_load.o $(LIBBPF) sockmap_user.o
>  
> diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
> index 2d51672..48fa09a 100644
> --- a/samples/sockmap/sockmap_user.c
> +++ b/samples/sockmap/sockmap_user.c
> @@ -23,6 +23,7 @@
>  #include <stdbool.h>
>  #include <signal.h>
>  #include <fcntl.h>
> +#include <sys/wait.h>
[...]

Re: [bpf-next PATCH v2 1/7] bpf: refactor sockmap sample program update for arg parsing

[Martin KaFai Lau]

On Wed, Jan 10, 2018 at 10:39:04AM -0800, John Fastabend wrote:
> sockmap sample program takes arguments from cmd line but it reads them
> in using offsets into the array. Because we want to add more arguments
> in the future lets do proper argument handling.
> 
> Also refactor code to pull apart sock init and ping/pong test. This
> allows us to add new tests in the future.
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  samples/sockmap/sockmap_user.c |  142 +++++++++++++++++++++++++++++-----------
>  1 file changed, 103 insertions(+), 39 deletions(-)
> 
> diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
> index 7cc9d22..5cbe7a5 100644
> --- a/samples/sockmap/sockmap_user.c
> +++ b/samples/sockmap/sockmap_user.c
> @@ -35,6 +35,8 @@
>  #include <assert.h>
>  #include <libgen.h>
>  
> +#include <getopt.h>
> +
>  #include "../bpf/bpf_load.h"
>  #include "../bpf/bpf_util.h"
>  #include "../bpf/libbpf.h"
> @@ -46,15 +48,39 @@
>  #define S1_PORT 10000
>  #define S2_PORT 10001
>  
> -static int sockmap_test_sockets(int rate, int dot)
> +/* global sockets */
> +int s1, s2, c1, c2, p1, p2;
> +
> +static const struct option long_options[] = {
> +	{"help",	no_argument,		NULL, 'h' },
> +	{"cgroup",	required_argument,	NULL, 'c' },
> +	{"rate",	required_argument,	NULL, 'r' },
> +	{"verbose",	no_argument,		NULL, 'v' },
> +	{0, 0, NULL, 0 }
> +};
> +
> +static void usage(char *argv[])
> +{
> +	int i;
> +
> +	printf(" Usage: %s --cgroup <cgroup_path>\n", argv[0]);
> +	printf(" options:\n");
> +	for (i = 0; long_options[i].name != 0; i++) {
> +		printf(" --%-12s", long_options[i].name);
> +		if (long_options[i].flag != NULL)
> +			printf(" flag (internal value:%d)\n",
> +				*long_options[i].flag);
> +		else
> +			printf(" -%c\n", long_options[i].val);
> +	}
> +	printf("\n");
> +}
> +
> +static int sockmap_init_sockets(void)
>  {
> -	int i, sc, err, max_fd, one = 1;
> -	int s1, s2, c1, c2, p1, p2;
> +	int i, err, one = 1;
>  	struct sockaddr_in addr;
> -	struct timeval timeout;
> -	char buf[1024] = {0};
>  	int *fds[4] = {&s1, &s2, &c1, &c2};
> -	fd_set w;
>  
>  	s1 = s2 = p1 = p2 = c1 = c2 = 0;
>  
> @@ -134,6 +160,8 @@ static int sockmap_test_sockets(int rate, int dot)
>  	if (err < 0 && errno != EINPROGRESS) {
>  		perror("connect c2 failed()\n");
>  		goto out;
> +	} else if (err < 0) {
> +		err = 0;
>  	}
>  
>  	/* Accept Connecrtions */
> @@ -149,23 +177,32 @@ static int sockmap_test_sockets(int rate, int dot)
>  		goto out;
>  	}
>  
> -	max_fd = p2;
> -	timeout.tv_sec = 10;
> -	timeout.tv_usec = 0;
> -
>  	printf("connected sockets: c1 <-> p1, c2 <-> p2\n");
>  	printf("cgroups binding: c1(%i) <-> s1(%i) - - - c2(%i) <-> s2(%i)\n",
>  		c1, s1, c2, s2);
> +out:
> +	return err;
err is not updated with p1 and p2 in the case that accept() errors out.

> +}
> +
> +static int forever_ping_pong(int rate, int verbose)
> +{
> +	struct timeval timeout;
> +	char buf[1024] = {0};
> +	int sc;
> +
> +	timeout.tv_sec = 10;
> +	timeout.tv_usec = 0;
>  
>  	/* Ping/Pong data from client to server */
>  	sc = send(c1, buf, sizeof(buf), 0);
>  	if (sc < 0) {
>  		perror("send failed()\n");
> -		goto out;
> +		return sc;
>  	}
>  
>  	do {
> -		int s, rc, i;
> +		int s, rc, i, max_fd = p2;
> +		fd_set w;
>  
>  		/* FD sets */
>  		FD_ZERO(&w);
> @@ -193,7 +230,7 @@ static int sockmap_test_sockets(int rate, int dot)
>  			if (rc < 0) {
>  				if (errno != EWOULDBLOCK) {
>  					perror("recv failed()\n");
> -					break;
> +					return rc;
>  				}
>  			}
>  
> @@ -205,35 +242,60 @@ static int sockmap_test_sockets(int rate, int dot)
>  			sc = send(i, buf, rc, 0);
>  			if (sc < 0) {
>  				perror("send failed()\n");
> -				break;
> +				return sc;
>  			}
>  		}
> -		sleep(rate);
> -		if (dot) {
> +
> +		if (rate)
> +			sleep(rate);
> +
> +		if (verbose) {
>  			printf(".");
>  			fflush(stdout);
>  
>  		}
>  	} while (running);
>  
> -out:
> -	close(s1);
> -	close(s2);
> -	close(p1);
> -	close(p2);
> -	close(c1);
> -	close(c2);
> -	return err;
> +	return 0;
>  }
>  
>  int main(int argc, char **argv)
>  {
> -	int rate = 1, dot = 1;
> +	int rate = 1, verbose = 0;
> +	int opt, longindex, err, cg_fd = 0;
>  	char filename[256];
> -	int err, cg_fd;
> -	char *cg_path;
>  
> -	cg_path = argv[argc - 1];
> +	while ((opt = getopt_long(argc, argv, "hvc:r:",
> +				  long_options, &longindex)) != -1) {
> +		switch (opt) {
> +		/* Cgroup configuration */
> +		case 'c':
> +			cg_fd = open(optarg, O_DIRECTORY, O_RDONLY);
> +			if (cg_fd < 0) {
> +				fprintf(stderr, "ERROR: (%i) open cg path failed: %s\n",
> +					cg_fd, optarg);
> +				return cg_fd;
> +			}
> +			break;
> +		case 'r':
> +			rate = atoi(optarg);
> +			break;
> +		case 'v':
> +			verbose = 1;
> +			break;
> +		case 'h':
> +		default:
> +			usage(argv);
> +			return -1;
> +		}
> +	}
> +
> +	if (!cg_fd) {
> +		fprintf(stderr, "%s requires cgroup option: --cgroup <path>\n",
> +			argv[0]);
> +		return -1;
> +	}
> +
>  	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
>  
>  	running = 1;
> @@ -247,14 +309,6 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  
> -	/* Cgroup configuration */
> -	cg_fd = open(cg_path, O_DIRECTORY, O_RDONLY);
> -	if (cg_fd < 0) {
> -		fprintf(stderr, "ERROR: (%i) open cg path failed: %s\n",
> -			cg_fd, cg_path);
> -		return cg_fd;
> -	}
> -
>  	/* Attach programs to sockmap */
>  	err = bpf_prog_attach(prog_fd[0], map_fd[0],
>  				BPF_SK_SKB_STREAM_PARSER, 0);
> @@ -280,15 +334,25 @@ int main(int argc, char **argv)
>  		return err;
>  	}
>  
> -	err = sockmap_test_sockets(rate, dot);
> +	err = sockmap_init_sockets();
>  	if (err) {
>  		fprintf(stderr, "ERROR: test socket failed: %d\n", err);
> -		return err;
> +		goto out;
>  	}
> -	return 0;
> +
> +	err = forever_ping_pong(rate, verbose);
> +out:
> +	close(s1);
> +	close(s2);
> +	close(p1);
> +	close(p2);
> +	close(c1);
> +	close(c2);
> +	return err;
>  }
>  
>  void running_handler(int a)
>  {
>  	running = 0;
> +	printf("\n");
>  }
> 

Re: [bpf-next PATCH v2 2/7] bpf: add sendmsg option for testing BPF programs

[Martin KaFai Lau]

On Wed, Jan 10, 2018 at 10:39:21AM -0800, John Fastabend wrote:
> When testing BPF programs using sockmap I often want to have more
> control over how sendmsg is exercised. This becomes even more useful
> as new sockmap program types are added.
> 
> This adds a test type option to select type of test to run. Currently,
> only "ping" and "sendmsg" are supported, but more can be added as
> needed.
> 
> The new help argument gives the following,
> 
>  Usage: ./sockmap --cgroup <cgroup_path>
>  options:
>  --help         -h
>  --cgroup       -c
>  --rate         -r
>  --verbose      -v
>  --iov_count    -i
>  --length       -l
>  --test         -t
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  samples/sockmap/sockmap_user.c |  143 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 140 insertions(+), 3 deletions(-)
> 
> diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
> index 5cbe7a5..2d51672 100644
> --- a/samples/sockmap/sockmap_user.c
> +++ b/samples/sockmap/sockmap_user.c
> @@ -56,6 +56,9 @@
>  	{"cgroup",	required_argument,	NULL, 'c' },
>  	{"rate",	required_argument,	NULL, 'r' },
>  	{"verbose",	no_argument,		NULL, 'v' },
> +	{"iov_count",	required_argument,	NULL, 'i' },
> +	{"length",	required_argument,	NULL, 'l' },
> +	{"test",	required_argument,	NULL, 't' },
>  	{0, 0, NULL, 0 }
>  };
>  
> @@ -184,6 +187,113 @@ static int sockmap_init_sockets(void)
>  	return err;
>  }
>  
> +struct msg_stats {
> +	size_t bytes_sent;
> +	size_t bytes_recvd;
> +};
> +
> +static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
> +		    struct msg_stats *s, bool tx)
> +{
> +	struct msghdr msg = {0};
> +	struct iovec *iov;
> +	int i, flags = 0;
> +
> +	iov = calloc(iov_count, sizeof(struct iovec));
> +	if (!iov)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < iov_count; i++) {
> +		char *d = calloc(iov_length, sizeof(char));
> +
> +		if (!d) {
> +			fprintf(stderr, "iov_count %i/%i OOM\n", i, iov_count);
> +			free(iov);
> +			return -ENOMEM;
> +		}
> +		iov[i].iov_base = d;
> +		iov[i].iov_len = iov_length;
> +	}
> +
> +	msg.msg_iov = iov;
> +	msg.msg_iovlen = iov_count;
> +
> +	if (tx) {
> +		for (i = 0; i < cnt; i++) {
> +			int sent = sendmsg(fd, &msg, flags);
> +
> +			if (sent < 0) {
> +				perror("send loop error:");
> +				return sent;
may use a goto the end of this function so that iov can be freed.
Same for the later error returns of this function.

> +			}
> +			s->bytes_sent += sent;
> +		}
> +	} else {
> +		int slct, recv, max_fd = fd;
max_fd looks unnecessary.

> +		struct timeval timeout;
> +		float total_bytes;
> +		fd_set w;
> +
> +		total_bytes = (float)iov_count * (float)iov_length * (float)cnt;
> +		while (s->bytes_recvd < total_bytes) {
> +			timeout.tv_sec = 1;
> +			timeout.tv_usec = 0;
> +
> +			/* FD sets */
> +			FD_ZERO(&w);
> +			FD_SET(fd, &w);
> +
> +			slct = select(max_fd + 1, &w, NULL, NULL, &timeout);
> +			if (slct == -1) {
> +				perror("select()");
> +				return slct;
> +			} else if (!slct) {
> +				fprintf(stderr, "unexpected timeout\n");
> +				return slct;
slct == 0 here.  return -1 instead?

> +			}
> +
> +			recv = recvmsg(fd, &msg, flags);
> +			if (recv < 0) {
> +				if (errno != EWOULDBLOCK) {
> +					perror("recv failed()\n");
> +					return errno;
Should the above also return errno instead?

> +				}
> +			}
> +
> +			s->bytes_recvd += recv;
> +		}
> +	}
> +
> +	for (i = 0; i < iov_count; i++)
> +		free(iov[i].iov_base);
> +	free(iov);
> +	return 0;
> +}
> +
> +static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
> +{
> +	struct msg_stats s = {0};
> +	int err;
> +
> +	err = msg_loop(c1, iov_count, iov_buf, cnt, &s, true);
> +	if (err) {
> +		fprintf(stderr,
> +			"msg_loop_tx: iov_count %i iov_buf %i cnt %i err %i\n",
> +			iov_count, iov_buf, cnt, err);
> +		return err;
> +	}
> +
> +	msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
err = msg_loop(..., false);

> +	if (err)
> +		fprintf(stderr,
> +			"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
> +			iov_count, iov_buf, cnt, err);
> +
> +	fprintf(stdout, "sendmsg: TX_bytes %zu RX_bytes %zu\n",
> +		s.bytes_sent, s.bytes_recvd);
> +	return err;
> +}
> +
>  static int forever_ping_pong(int rate, int verbose)
>  {
>  	struct timeval timeout;
> @@ -259,13 +369,19 @@ static int forever_ping_pong(int rate, int verbose)
>  	return 0;
>  }
>  
> +enum {
> +	PING_PONG,
> +	SENDMSG,
> +};
> +
>  int main(int argc, char **argv)
>  {
> -	int rate = 1, verbose = 0;
> +	int iov_count = 1, length = 1024, rate = 1, verbose = 0;
>  	int opt, longindex, err, cg_fd = 0;
> +	int test = PING_PONG;
>  	char filename[256];
>  
> -	while ((opt = getopt_long(argc, argv, "hvc:r:",
> +	while ((opt = getopt_long(argc, argv, "hvc:r:i:l:t:",
>  				  long_options, &longindex)) != -1) {
>  		switch (opt) {
>  		/* Cgroup configuration */
> @@ -283,6 +399,22 @@ int main(int argc, char **argv)
>  		case 'v':
>  			verbose = 1;
>  			break;
> +		case 'i':
> +			iov_count = atoi(optarg);
> +			break;
> +		case 'l':
> +			length = atoi(optarg);
> +			break;
> +		case 't':
> +			if (memcmp(optarg, "ping", 4) == 0) {
> +				test = PING_PONG;
> +			} else if (memcmp(optarg, "sendmsg", 7) == 0) {
> +				test = SENDMSG;
> +			} else {
> +				usage(argv);
> +				return -1;
> +			}
> +			break;
>  		case 'h':
>  		default:
>  			usage(argv);
> @@ -340,7 +472,12 @@ int main(int argc, char **argv)
>  		goto out;
>  	}
>  
> -	err = forever_ping_pong(rate, verbose);
> +	if (test == PING_PONG)
> +		err = forever_ping_pong(rate, verbose);
> +	else if (test == SENDMSG)
> +		err = sendmsg_test(iov_count, length, rate, verbose);
> +	else
> +		fprintf(stderr, "unknown test\n");
>  out:
>  	close(s1);
>  	close(s2);
> 

Re: [bpf-next PATCH v2 3/7] bpf: sockmap sample, use fork() for send and recv

[Martin KaFai Lau]

On Wed, Jan 10, 2018 at 10:39:37AM -0800, John Fastabend wrote:
> Currently for SENDMSG tests first send completes then recv runs. This
> does not work well for large data sizes and/or many iterations. So
> fork the recv and send handler so that we run both send and recv. In
> the future we can add a parameter to do more than a single fork of
> tx/rx.
> 
> With this we can get many GBps of data which helps exercise the
> sockmap code.
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  samples/sockmap/Makefile       |    2 +
>  samples/sockmap/sockmap_user.c |   58 +++++++++++++++++++++++++++++-----------
>  2 files changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/samples/sockmap/Makefile b/samples/sockmap/Makefile
> index 73f1da4..4fefd66 100644
> --- a/samples/sockmap/Makefile
> +++ b/samples/sockmap/Makefile
> @@ -8,7 +8,7 @@ HOSTCFLAGS += -I$(objtree)/usr/include
>  HOSTCFLAGS += -I$(srctree)/tools/lib/
>  HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
>  HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
> -HOSTCFLAGS += -I$(srctree)/tools/perf
> +HOSTCFLAGS += -I$(srctree)/tools/perf -g
>  
>  sockmap-objs := ../bpf/bpf_load.o $(LIBBPF) sockmap_user.o
>  
> diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
> index 2d51672..48fa09a 100644
> --- a/samples/sockmap/sockmap_user.c
> +++ b/samples/sockmap/sockmap_user.c
> @@ -23,6 +23,7 @@
>  #include <stdbool.h>
>  #include <signal.h>
>  #include <fcntl.h>
> +#include <sys/wait.h>
>  
>  #include <sys/time.h>
>  #include <sys/types.h>
> @@ -197,7 +198,7 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
>  {
>  	struct msghdr msg = {0};
>  	struct iovec *iov;
> -	int i, flags = 0;
> +	int i, flags = MSG_NOSIGNAL;
>  
>  	iov = calloc(iov_count, sizeof(struct iovec));
>  	if (!iov)
> @@ -272,25 +273,50 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
>  
>  static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
>  {
> +	int txpid, rxpid, err = 0;
>  	struct msg_stats s = {0};
> -	int err;
> -
> -	err = msg_loop(c1, iov_count, iov_buf, cnt, &s, true);
> -	if (err) {
> -		fprintf(stderr,
> -			"msg_loop_tx: iov_count %i iov_buf %i cnt %i err %i\n",
> -			iov_count, iov_buf, cnt, err);
> -		return err;
> +	int status;
> +
> +	errno = 0;
> +
> +	rxpid = fork();
> +	if (rxpid == 0) {
> +		err = msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
> +		if (err)
> +			fprintf(stderr,
> +				"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
> +				iov_count, iov_buf, cnt, err);
> +		fprintf(stdout, "rx_sendmsg: TX_bytes %zu RX_bytes %zu\n",
> +			s.bytes_sent, s.bytes_recvd);
> +		shutdown(p2, SHUT_RDWR);
> +		shutdown(p1, SHUT_RDWR);
> +		exit(1);
> +	} else if (rxpid == -1) {
> +		perror("msg_loop_rx: ");
> +		err = errno;
Bail out here instead of continuing the tx side?

>  	}
>  
> -	msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
> -	if (err)
> -		fprintf(stderr,
> -			"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
> -			iov_count, iov_buf, cnt, err);
> +	txpid = fork();
> +	if (txpid == 0) {
> +		err = msg_loop(c1, iov_count, iov_buf, cnt, &s, true);
> +		if (err)
> +			fprintf(stderr,
> +				"msg_loop_tx: iov_count %i iov_buf %i cnt %i err %i\n",
> +				iov_count, iov_buf, cnt, err);
> +		fprintf(stdout, "tx_sendmsg: TX_bytes %zu RX_bytes %zu\n",
> +			s.bytes_sent, s.bytes_recvd);
> +		shutdown(c1, SHUT_RDWR);
> +		exit(1);
> +	} else if (txpid == -1) {
> +		perror("msg_loop_tx: ");
> +		return errno;
> +	}
>  
> -	fprintf(stdout, "sendmsg: TX_bytes %zu RX_bytes %zu\n",
> -		s.bytes_sent, s.bytes_recvd);
> +	assert(waitpid(rxpid, &status, 0) == rxpid);
> +	if (!txpid)
> +		goto out;
> +	assert(waitpid(txpid, &status, 0) == txpid);
> +out:
>  	return err;
>  }
>  
> 

Re: [bpf-next PATCH v2 5/7] bpf: sockmap sample add base test without any BPF for comparison

[Martin KaFai Lau]

On Wed, Jan 10, 2018 at 10:40:11AM -0800, John Fastabend wrote:
> Add a base test that does not use BPF hooks to test baseline case.
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  samples/sockmap/sockmap_user.c |   26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
> index 812fc7e..eb19d14 100644
> --- a/samples/sockmap/sockmap_user.c
> +++ b/samples/sockmap/sockmap_user.c
> @@ -285,18 +285,24 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
>  
>  static float giga = 1000000000;
>  
> -static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
> +static int sendmsg_test(int iov_count, int iov_buf, int cnt,
> +			int verbose, bool base)
>  {
> -	int txpid, rxpid, err = 0;
> +	float sent_Bps = 0, recvd_Bps = 0;
> +	int rx_fd, txpid, rxpid, err = 0;
>  	struct msg_stats s = {0};
>  	int status;
> -	float sent_Bps = 0, recvd_Bps = 0;
>  
>  	errno = 0;
>  
> +	if (base)
> +		rx_fd = p1;
> +	else
> +		rx_fd = p2;
> +
>  	rxpid = fork();
>  	if (rxpid == 0) {
> -		err = msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
> +		err = msg_loop(rx_fd, iov_count, iov_buf, cnt, &s, false);
I am likely missing something.  After receiving from p1, should the
base-line case also send to c2 which then will be received by p2?

>  		if (err)
>  			fprintf(stderr,
>  				"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
> @@ -425,6 +431,7 @@ static int forever_ping_pong(int rate, int verbose)
>  enum {
>  	PING_PONG,
>  	SENDMSG,
> +	BASE,
>  };
>  
>  int main(int argc, char **argv)
> @@ -463,6 +470,8 @@ int main(int argc, char **argv)
>  				test = PING_PONG;
>  			} else if (memcmp(optarg, "sendmsg", 7) == 0) {
>  				test = SENDMSG;
> +			} else if (memcmp(optarg, "base", 4) == 0) {
> +				test = BASE;
>  			} else {
>  				usage(argv);
>  				return -1;
> @@ -488,6 +497,10 @@ int main(int argc, char **argv)
>  	/* catch SIGINT */
>  	signal(SIGINT, running_handler);
>  
> +	/* If base test skip BPF setup */
> +	if (test == BASE)
> +		goto run;
> +
>  	if (load_bpf_file(filename)) {
>  		fprintf(stderr, "load_bpf_file: (%s) %s\n",
>  			filename, strerror(errno));
> @@ -519,6 +532,7 @@ int main(int argc, char **argv)
>  		return err;
>  	}
>  
> +run:
>  	err = sockmap_init_sockets();
>  	if (err) {
>  		fprintf(stderr, "ERROR: test socket failed: %d\n", err);
> @@ -528,7 +542,9 @@ int main(int argc, char **argv)
>  	if (test == PING_PONG)
>  		err = forever_ping_pong(rate, verbose);
>  	else if (test == SENDMSG)
> -		err = sendmsg_test(iov_count, length, rate, verbose);
> +		err = sendmsg_test(iov_count, length, rate, verbose, false);
> +	else if (test == BASE)
> +		err = sendmsg_test(iov_count, length, rate, verbose, true);
>  	else
>  		fprintf(stderr, "unknown test\n");
>  out:
> 

Re: [bpf-next PATCH v2 1/7] bpf: refactor sockmap sample program update for arg parsing

[John Fastabend]

On 01/11/2018 01:05 PM, Martin KaFai Lau wrote:
> On Wed, Jan 10, 2018 at 10:39:04AM -0800, John Fastabend wrote:
>> sockmap sample program takes arguments from cmd line but it reads them
>> in using offsets into the array. Because we want to add more arguments
>> in the future lets do proper argument handling.
>>
>> Also refactor code to pull apart sock init and ping/pong test. This
>> allows us to add new tests in the future.
>>

[...]

>>  	/* Accept Connecrtions */
>> @@ -149,23 +177,32 @@ static int sockmap_test_sockets(int rate, int dot)
>>  		goto out;
>>  	}
>>  
>> -	max_fd = p2;
>> -	timeout.tv_sec = 10;
>> -	timeout.tv_usec = 0;
>> -
>>  	printf("connected sockets: c1 <-> p1, c2 <-> p2\n");
>>  	printf("cgroups binding: c1(%i) <-> s1(%i) - - - c2(%i) <-> s2(%i)\n",
>>  		c1, s1, c2, s2);
>> +out:
>> +	return err;
> err is not updated with p1 and p2 in the case that accept() errors out.
> 

OK will fix to propagate the error, nice spot. This will avoid
trying to run the test without connected sockets.

Thanks,
John

Re: [bpf-next PATCH v2 3/7] bpf: sockmap sample, use fork() for send and recv

[John Fastabend]

On 01/11/2018 01:08 PM, Martin KaFai Lau wrote:
> On Wed, Jan 10, 2018 at 10:39:37AM -0800, John Fastabend wrote:
>> Currently for SENDMSG tests first send completes then recv runs. This
>> does not work well for large data sizes and/or many iterations. So
>> fork the recv and send handler so that we run both send and recv. In
>> the future we can add a parameter to do more than a single fork of
>> tx/rx.
>>
>> With this we can get many GBps of data which helps exercise the
>> sockmap code.
>>
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> ---

[...]

>>  static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
>>  {
>> +	int txpid, rxpid, err = 0;
>>  	struct msg_stats s = {0};
>> -	int err;
>> -
>> -	err = msg_loop(c1, iov_count, iov_buf, cnt, &s, true);
>> -	if (err) {
>> -		fprintf(stderr,
>> -			"msg_loop_tx: iov_count %i iov_buf %i cnt %i err %i\n",
>> -			iov_count, iov_buf, cnt, err);
>> -		return err;
>> +	int status;
>> +
>> +	errno = 0;
>> +
>> +	rxpid = fork();
>> +	if (rxpid == 0) {
>> +		err = msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
>> +		if (err)
>> +			fprintf(stderr,
>> +				"msg_loop_rx: iov_count %i iov_buf %i cnt %i err %i\n",
>> +				iov_count, iov_buf, cnt, err);
>> +		fprintf(stdout, "rx_sendmsg: TX_bytes %zu RX_bytes %zu\n",
>> +			s.bytes_sent, s.bytes_recvd);
>> +		shutdown(p2, SHUT_RDWR);
>> +		shutdown(p1, SHUT_RDWR);
>> +		exit(1);
>> +	} else if (rxpid == -1) {
>> +		perror("msg_loop_rx: ");
>> +		err = errno;
> Bail out here instead of continuing the tx side?
> 

Sure makes sense. No point in running the TX side here I guess.

Re: [bpf-next PATCH v2 5/7] bpf: sockmap sample add base test without any BPF for comparison

[John Fastabend]

On 01/11/2018 01:10 PM, Martin KaFai Lau wrote:
> On Wed, Jan 10, 2018 at 10:40:11AM -0800, John Fastabend wrote:
>> Add a base test that does not use BPF hooks to test baseline case.
>>
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> ---
>>  samples/sockmap/sockmap_user.c |   26 +++++++++++++++++++++-----
>>  1 file changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
>> index 812fc7e..eb19d14 100644
>> --- a/samples/sockmap/sockmap_user.c
>> +++ b/samples/sockmap/sockmap_user.c
>> @@ -285,18 +285,24 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
>>  
>>  static float giga = 1000000000;
>>  
>> -static int sendmsg_test(int iov_count, int iov_buf, int cnt, int verbose)
>> +static int sendmsg_test(int iov_count, int iov_buf, int cnt,
>> +			int verbose, bool base)
>>  {
>> -	int txpid, rxpid, err = 0;
>> +	float sent_Bps = 0, recvd_Bps = 0;
>> +	int rx_fd, txpid, rxpid, err = 0;
>>  	struct msg_stats s = {0};
>>  	int status;
>> -	float sent_Bps = 0, recvd_Bps = 0;
>>  
>>  	errno = 0;
>>  
>> +	if (base)
>> +		rx_fd = p1;
>> +	else
>> +		rx_fd = p2;
>> +
>>  	rxpid = fork();
>>  	if (rxpid == 0) {
>> -		err = msg_loop(p2, iov_count, iov_buf, cnt, &s, false);
>> +		err = msg_loop(rx_fd, iov_count, iov_buf, cnt, &s, false);
> I am likely missing something.  After receiving from p1, should the
> base-line case also send to c2 which then will be received by p2?
> 

Well I wanted a test to check socket to socket rates and see what
max throughput we could get with this simple tool. It provides a
good reference point for any other 'perf' data, throughput numbers,
etc. The numbers I see here, probably as expected, are very close
to what I get with iperf tests.

Adding another test base_bounce or base_proxy or something along
those lines might be another test we can add. I think you were
expecting this to be a 1:1 comparison with the sendmsg BPF test
but its not. Probably can add it though.

Thanks,
John

Re: [bpf-next PATCH v2 1/7] bpf: refactor sockmap sample program update for arg parsing

[John Fastabend]

On 01/10/2018 05:25 PM, Daniel Borkmann wrote:
> On 01/10/2018 07:39 PM, John Fastabend wrote:
>> sockmap sample program takes arguments from cmd line but it reads them
>> in using offsets into the array. Because we want to add more arguments
>> in the future lets do proper argument handling.
>>
>> Also refactor code to pull apart sock init and ping/pong test. This
>> allows us to add new tests in the future.
>>
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> ---
>>  samples/sockmap/sockmap_user.c |  142 +++++++++++++++++++++++++++++-----------
>>  1 file changed, 103 insertions(+), 39 deletions(-)
> [...]
>>  
>>  	/* Accept Connecrtions */
>> @@ -149,23 +177,32 @@ static int sockmap_test_sockets(int rate, int dot)
>>  		goto out;
>>  	}
>>  
>> -	max_fd = p2;
>> -	timeout.tv_sec = 10;
>> -	timeout.tv_usec = 0;
>> -
>>  	printf("connected sockets: c1 <-> p1, c2 <-> p2\n");
>>  	printf("cgroups binding: c1(%i) <-> s1(%i) - - - c2(%i) <-> s2(%i)\n",
>>  		c1, s1, c2, s2);
>> +out:
>> +	return err;
> 
> Maybe rather than setting err and goto out where we now just return
> err anyway, return from those places directly.
> 

Perhaps but how about doing this in another patch. This
patch is not changing the goto err pattern. I can send
a follow up.

>> +}
>> +
>> +static int forever_ping_pong(int rate, int verbose)
>> +{
>> +	struct timeval timeout;
>> +	char buf[1024] = {0};
>> +	int sc;
>> +
>> +	timeout.tv_sec = 10;
>> +	timeout.tv_usec = 0;
>>  
>>  	/* Ping/Pong data from client to server */
>>  	sc = send(c1, buf, sizeof(buf), 0);
>>  	if (sc < 0) {
>>  		perror("send failed()\n");
>> -		goto out;
>> +		return sc;
>>  	}
>>  
>>  	do {
>> -		int s, rc, i;
>> +		int s, rc, i, max_fd = p2;
>> +		fd_set w;
>>  
>>  		/* FD sets */
>>  		FD_ZERO(&w);
> [...]
>> -	err = sockmap_test_sockets(rate, dot);
>> +	err = sockmap_init_sockets();
>>  	if (err) {
>>  		fprintf(stderr, "ERROR: test socket failed: %d\n", err);
>> -		return err;
>> +		goto out;
>>  	}
>> -	return 0;
>> +
>> +	err = forever_ping_pong(rate, verbose);
>> +out:
>> +	close(s1);
>> +	close(s2);
>> +	close(p1);
>> +	close(p2);
>> +	close(c1);
>> +	close(c2);
>> +	return err;
>>  }
>>  
>>  void running_handler(int a)
>>  {
>>  	running = 0;
>> +	printf("\n");
> 
> Do we need this out of the sighandler instead of e.g. main loop when
> we break out?

Not really let me just remove it. I'll do
another patch with documentation and fixup
error messages so we don't have this issue.

I agree its a bit clumsy.

Re: [bpf-next PATCH v2 3/7] bpf: sockmap sample, use fork() for send and recv

[John Fastabend]

On 01/10/2018 05:31 PM, Daniel Borkmann wrote:
> On 01/10/2018 07:39 PM, John Fastabend wrote:
>> Currently for SENDMSG tests first send completes then recv runs. This
>> does not work well for large data sizes and/or many iterations. So
>> fork the recv and send handler so that we run both send and recv. In
>> the future we can add a parameter to do more than a single fork of
>> tx/rx.
>>
>> With this we can get many GBps of data which helps exercise the
>> sockmap code.
>>
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> ---
>>  samples/sockmap/Makefile       |    2 +
>>  samples/sockmap/sockmap_user.c |   58 +++++++++++++++++++++++++++++-----------
>>  2 files changed, 43 insertions(+), 17 deletions(-)
>>
>> diff --git a/samples/sockmap/Makefile b/samples/sockmap/Makefile
>> index 73f1da4..4fefd66 100644
>> --- a/samples/sockmap/Makefile
>> +++ b/samples/sockmap/Makefile
>> @@ -8,7 +8,7 @@ HOSTCFLAGS += -I$(objtree)/usr/include
>>  HOSTCFLAGS += -I$(srctree)/tools/lib/
>>  HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
>>  HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
>> -HOSTCFLAGS += -I$(srctree)/tools/perf
>> +HOSTCFLAGS += -I$(srctree)/tools/perf -g
> 
> Slipped in here?
> 

Yep, removed in v3. Thanks.

>>  sockmap-objs := ../bpf/bpf_load.o $(LIBBPF) sockmap_user.o
>>  
>> diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
>> index 2d51672..48fa09a 100644
>> --- a/samples/sockmap/sockmap_user.c
>> +++ b/samples/sockmap/sockmap_user.c
>> @@ -23,6 +23,7 @@
>>  #include <stdbool.h>
>>  #include <signal.h>
>>  #include <fcntl.h>
>> +#include <sys/wait.h>
> [...]
> 

Re: [bpf-next PATCH v2 1/7] bpf: refactor sockmap sample program update for arg parsing

[John Fastabend]

On 01/11/2018 08:31 PM, John Fastabend wrote:
> On 01/10/2018 05:25 PM, Daniel Borkmann wrote:
>> On 01/10/2018 07:39 PM, John Fastabend wrote:
>>> sockmap sample program takes arguments from cmd line but it reads them
>>> in using offsets into the array. Because we want to add more arguments
>>> in the future lets do proper argument handling.
>>>
>>> Also refactor code to pull apart sock init and ping/pong test. This
>>> allows us to add new tests in the future.
>>>
>>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>>> ---
>>>  samples/sockmap/sockmap_user.c |  142 +++++++++++++++++++++++++++++-----------
>>>  1 file changed, 103 insertions(+), 39 deletions(-)
>> [...]
>>>  
>>>  	/* Accept Connecrtions */
>>> @@ -149,23 +177,32 @@ static int sockmap_test_sockets(int rate, int dot)
>>>  		goto out;
>>>  	}
>>>  
>>> -	max_fd = p2;
>>> -	timeout.tv_sec = 10;
>>> -	timeout.tv_usec = 0;
>>> -
>>>  	printf("connected sockets: c1 <-> p1, c2 <-> p2\n");
>>>  	printf("cgroups binding: c1(%i) <-> s1(%i) - - - c2(%i) <-> s2(%i)\n",
>>>  		c1, s1, c2, s2);
>>> +out:
>>> +	return err;
>>
>> Maybe rather than setting err and goto out where we now just return
>> err anyway, return from those places directly.
>>
> 
> Perhaps but how about doing this in another patch. This
> patch is not changing the goto err pattern. I can send
> a follow up.

OK I take it back. I went ahead an removed the goto as you
suggested. As Martin noticed the accept err was missing and
also most of those should have been errno instead of err.

v3 in-flight.

.John