diff mbox series

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

Message ID 20180108180507.13647.62367.stgit@john-Precision-Tower-5810
State Changes Requested, archived
Delegated to: BPF Maintainers
Headers show
Series sockmap sample updates | expand

Commit Message

John Fastabend Jan. 8, 2018, 6:05 p.m. UTC
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(-)

Comments

Jesper Dangaard Brouer Jan. 9, 2018, 1:30 p.m. UTC | #1
On Mon, 08 Jan 2018 10:05:07 -0800
John Fastabend <john.fastabend@gmail.com> 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");
> +}
> +

I love that you are using --long-options :-)

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
John Fastabend Jan. 9, 2018, 4:10 p.m. UTC | #2
On 01/09/2018 05:30 AM, Jesper Dangaard Brouer wrote:
> On Mon, 08 Jan 2018 10:05:07 -0800
> John Fastabend <john.fastabend@gmail.com> 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");
>> +}
>> +
> 
> I love that you are using --long-options :-)

Yep, saw it being used in xdp_rxq_info_user.c

> 
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
diff mbox series

Patch

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");
 }