[PATCH 0/3 RFC v2] selftests: Basic framework for tests

[PATCH 0/3 RFC v2] selftests: Basic framework for tests

[Alexandru Copot]

This series adds a generic test abstraction that can make
writing testcases easier. A generic_test structure is
used to define a test and its methods: prepare, run, cleanup.

This is a generic implementation so it was placed in selftests/lib.

The second patch updates the socket tests to use the
new framework and the third patch creates new tests
for [set/get]sockopt with some IPV6_* options.

Signed-of-by Alexandru Copot <alex.mihai.c@gmail.com>
Cc: Daniel Baluta <dbaluta@ixiacom.com>

Changes since v1:
 - moved the implementation to selftests/lib
 - use goto instead of directly returning

Alexandru Copot (3):
  selftests: introduce testing abstractions
  selftests/net: update socket test to use new testing framework
  selftests/net: add socket options test with IPv6 testcases

 tools/testing/selftests/Makefile             |   3 +-
 tools/testing/selftests/lib/Makefile         |  14 ++
 tools/testing/selftests/lib/selftests.c      |  57 +++++++++
 tools/testing/selftests/lib/selftests.h      |  67 ++++++++++
 tools/testing/selftests/net/Makefile         |  17 ++-
 tools/testing/selftests/net/run_netsocktests |  10 ++
 tools/testing/selftests/net/socket.c         | 108 +++++++++++-----
 tools/testing/selftests/net/sockopt.c        | 185 +++++++++++++++++++++++++++
 8 files changed, 425 insertions(+), 36 deletions(-)
 create mode 100644 tools/testing/selftests/lib/Makefile
 create mode 100644 tools/testing/selftests/lib/selftests.c
 create mode 100644 tools/testing/selftests/lib/selftests.h
 create mode 100644 tools/testing/selftests/net/sockopt.c

-- 
1.8.2.1

[PATCH 1/3 RFC v2] selftests: introduce testing abstractions

[Alexandru Copot]

Signed-of-by Alexandru Copot <alex.mihai.c@gmail.com>
Cc: Daniel Baluta <dbaluta@ixiacom.com>
---
 tools/testing/selftests/Makefile        |  3 +-
 tools/testing/selftests/lib/Makefile    | 14 +++++++
 tools/testing/selftests/lib/selftests.c | 57 ++++++++++++++++++++++++++++
 tools/testing/selftests/lib/selftests.h | 67 +++++++++++++++++++++++++++++++++
 4 files changed, 140 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/lib/Makefile
 create mode 100644 tools/testing/selftests/lib/selftests.c
 create mode 100644 tools/testing/selftests/lib/selftests.h

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index a480593..e0fccd9 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -1,4 +1,5 @@
-TARGETS = breakpoints
+TARGETS = lib
+TARGETS += breakpoints
 TARGETS += kcmp
 TARGETS += mqueue
 TARGETS += vm
diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
new file mode 100644
index 0000000..9c81d0c
--- /dev/null
+++ b/tools/testing/selftests/lib/Makefile
@@ -0,0 +1,14 @@
+
+CFLAGS = -Wall -O2 -c -g
+
+selftests.a: selftests.o
+	ar qc $@ $^
+
+%.c: %.h
+
+%.o: %.c
+	$(CC) $(CFLAGS) -o $@ $^
+
+clean:
+	rm -rf *.o *.a
+
diff --git a/tools/testing/selftests/lib/selftests.c b/tools/testing/selftests/lib/selftests.c
new file mode 100644
index 0000000..1a65785
--- /dev/null
+++ b/tools/testing/selftests/lib/selftests.c
@@ -0,0 +1,57 @@
+#include <stdio.h>
+#include <stdarg.h>
+
+#include "selftests.h"
+
+test_result_t __assert(int expr, const char *filename, int line, const char *fmt, ...)
+{
+	va_list vl;
+	const char *m;
+	char msg[BUFSIZ];
+
+	if (expr)
+		return TEST_PASS;
+
+	fprintf(stderr, "\n(%s:%d) ", filename, line);
+
+	va_start(vl, fmt);
+	m = va_arg(vl, char *);
+	if (!m)
+		m = fmt;
+	else
+		fprintf(stderr, "%s ", fmt);
+
+	vsnprintf(msg, sizeof msg, m, vl);
+	va_end(vl);
+
+	fprintf(stderr, "%s\n", msg);
+
+	return TEST_FAIL;
+}
+
+test_result_t run_all_tests(struct generic_test *test, void *param)
+{
+	int i;
+	char *ptr = test->testcases;
+	test_result_t rc = TEST_PASS;
+
+	rc = test->prepare ? test->prepare(param) : 0;
+	if (rc == TEST_FAIL)
+		return rc;
+
+	fprintf(stderr, "Testing: %s ", test->name);
+	for (i = 0; i < test->testcase_count; i++) {
+
+		rc |= test->run(ptr);
+		ptr += test->testcase_size;
+		
+		if (rc == TEST_FAIL && test->abort_on_fail) {
+			fprintf(stderr, "Test aborted\n");
+			break;
+		}
+	}
+
+	fprintf(stderr, "\t\t%s\n", rc ? "[FAIL]" : "[PASS]");
+	return rc;
+}
+
diff --git a/tools/testing/selftests/lib/selftests.h b/tools/testing/selftests/lib/selftests.h
new file mode 100644
index 0000000..6210199
--- /dev/null
+++ b/tools/testing/selftests/lib/selftests.h
@@ -0,0 +1,67 @@
+#ifndef SELFTESTS_H
+#define SELFTESTS_H
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+
+typedef enum test_result {
+	TEST_PASS = 0,
+	TEST_FAIL,
+} test_result_t;
+
+test_result_t __assert(int expr, const char *filename, int line, const char *fmt, ...);
+
+#define abort(cond) do {							\
+	if (!(cond)) {								\
+		fprintf(stderr, "%s:%d %s\n", __FILE__, __LINE__, #cond);	\
+		perror("");							\
+		exit(EXIT_FAILURE);						\
+	}									\
+} while (0)
+
+#define pass_if(expr, label, ret)				\
+	do {                                                    \
+		if (expr) {					\
+			ret = TEST_PASS;			\
+			goto label;				\
+		}						\
+	} while (0)
+
+/* @expr:   a boolean expression
+ * @label:  jump here to free resources if it fails
+ * @ret:    a test_result_t variable that will hold the result of the test
+ *          This can be later returned from the test function.
+ */
+#define check(expr, label, ret, ...)					     \
+	do {                                                                 \
+		ret = __assert(expr, __FILE__, __LINE__,       		     \
+			       "Assertion '" #expr "' failed", __VA_ARGS__); \
+		if (!(expr)) {						     \
+			perror("");					     \
+			goto label;                                          \
+		}							     \
+	} while (0)
+
+struct generic_test {
+	const char *name;
+	void *private;
+
+
+	void *testcases;
+	int testcase_size;
+	int testcase_count;
+
+	/* Ends all tests if one fails */
+	int abort_on_fail;
+
+	test_result_t (*prepare)(void *);
+	test_result_t (*run)(void *);
+	test_result_t (*cleanup)(void *);
+};
+
+test_result_t run_all_tests(struct generic_test *test, void *param);
+
+#endif
+
+
-- 
1.8.2.1

Re: [PATCH 1/3 RFC v2] selftests: introduce testing abstractions

[Daniel Borkmann]

On 04/25/2013 01:04 PM, Alexandru Copot wrote:
> Signed-of-by Alexandru Copot <alex.mihai.c@gmail.com>
> Cc: Daniel Baluta <dbaluta@ixiacom.com>
> ---
>   tools/testing/selftests/Makefile        |  3 +-
>   tools/testing/selftests/lib/Makefile    | 14 +++++++
>   tools/testing/selftests/lib/selftests.c | 57 ++++++++++++++++++++++++++++
>   tools/testing/selftests/lib/selftests.h | 67 +++++++++++++++++++++++++++++++++
>   4 files changed, 140 insertions(+), 1 deletion(-)
>   create mode 100644 tools/testing/selftests/lib/Makefile
>   create mode 100644 tools/testing/selftests/lib/selftests.c
>   create mode 100644 tools/testing/selftests/lib/selftests.h
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index a480593..e0fccd9 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -1,4 +1,5 @@
> -TARGETS = breakpoints
> +TARGETS = lib
> +TARGETS += breakpoints
>   TARGETS += kcmp
>   TARGETS += mqueue
>   TARGETS += vm
> diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
> new file mode 100644
> index 0000000..9c81d0c
> --- /dev/null
> +++ b/tools/testing/selftests/lib/Makefile
> @@ -0,0 +1,14 @@
> +
> +CFLAGS = -Wall -O2 -c -g
> +
> +selftests.a: selftests.o
> +	ar qc $@ $^
> +
> +%.c: %.h
> +
> +%.o: %.c
> +	$(CC) $(CFLAGS) -o $@ $^
> +
> +clean:
> +	rm -rf *.o *.a
> +
> diff --git a/tools/testing/selftests/lib/selftests.c b/tools/testing/selftests/lib/selftests.c
> new file mode 100644
> index 0000000..1a65785
> --- /dev/null
> +++ b/tools/testing/selftests/lib/selftests.c
> @@ -0,0 +1,57 @@
> +#include <stdio.h>
> +#include <stdarg.h>
> +
> +#include "selftests.h"
> +
> +test_result_t __assert(int expr, const char *filename, int line, const char *fmt, ...)
> +{

Usually I associate sth named assert() that it does an ungraceful exit. Maybe __test() or __check()?

> +	va_list vl;
> +	const char *m;
> +	char msg[BUFSIZ];
> +
> +	if (expr)
> +		return TEST_PASS;
> +
> +	fprintf(stderr, "\n(%s:%d) ", filename, line);
> +
> +	va_start(vl, fmt);
> +	m = va_arg(vl, char *);
> +	if (!m)
> +		m = fmt;
> +	else
> +		fprintf(stderr, "%s ", fmt);
> +
> +	vsnprintf(msg, sizeof msg, m, vl);

Nitpick: sizeof(msg)

> +	va_end(vl);
> +
> +	fprintf(stderr, "%s\n", msg);

Not sure what exactly you are trying to accomplish here in this function?
Why don't you use vfprintf() and leave this hackery aside?

va_list vl;

va_start(vl, fmt);
fprintf(stderr, "(%s:%d) ", filename, line);
vfprintf(stderr, fmt, vl);
va_end(vl);

> +	return TEST_FAIL;
> +}
> +
> +test_result_t run_all_tests(struct generic_test *test, void *param)
> +{
> +	int i;
> +	char *ptr = test->testcases;
> +	test_result_t rc = TEST_PASS;

Here you don't need to inint rc, since you overwrite immediately.

> +	rc = test->prepare ? test->prepare(param) : 0;

Also replace 0 with whatever you have defined for TEST_PASS or TEST_FAIL,
otherwise misleading.

> +	if (rc == TEST_FAIL)
> +		return rc;
> +
> +	fprintf(stderr, "Testing: %s ", test->name);
> +	for (i = 0; i < test->testcase_count; i++) {
> +

No whitespace here.

> +		rc |= test->run(ptr);
> +		ptr += test->testcase_size;
> +		
> +		if (rc == TEST_FAIL && test->abort_on_fail) {
> +			fprintf(stderr, "Test aborted\n");
> +			break;
> +		}
> +	}
> +
> +	fprintf(stderr, "\t\t%s\n", rc ? "[FAIL]" : "[PASS]");
> +	return rc;
> +}
> +
> diff --git a/tools/testing/selftests/lib/selftests.h b/tools/testing/selftests/lib/selftests.h
> new file mode 100644
> index 0000000..6210199
> --- /dev/null
> +++ b/tools/testing/selftests/lib/selftests.h
> @@ -0,0 +1,67 @@
> +#ifndef SELFTESTS_H
> +#define SELFTESTS_H
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +
> +typedef enum test_result {
> +	TEST_PASS = 0,
> +	TEST_FAIL,
> +} test_result_t;

I have no strong opinion here, but do we need to make this as an enum and then do
bit-operations on it?

> +test_result_t __assert(int expr, const char *filename, int line, const char *fmt, ...);

Nitpick:

If would be nice if you move run_all_tests() either here below this one or the other way
around, so that function decls are not distributed over the file, but together.

> +#define abort(cond) do {							\
> +	if (!(cond)) {								\
> +		fprintf(stderr, "%s:%d %s\n", __FILE__, __LINE__, #cond);	\
> +		perror("");							\

perror() could be removed from here and put into fprintf() by using f.e. %s + strerror(errno).

> +		exit(EXIT_FAILURE);						\
> +	}									\
> +} while (0)
> +
> +#define pass_if(expr, label, ret)				\
> +	do {                                                    \
> +		if (expr) {					\
> +			ret = TEST_PASS;			\
> +			goto label;				\
> +		}						\
> +	} while (0)

In cases where you make use of this macro, it's always like ...

<function>
{
	pass_if(<cond>, out, ret);
[...]
out:
	return ret;
}

Not sure if this simplifies or rather obfuscates things. Maybe other people
like it, not sure ...

> +/* @expr:   a boolean expression
> + * @label:  jump here to free resources if it fails
> + * @ret:    a test_result_t variable that will hold the result of the test
> + *          This can be later returned from the test function.
> + */
> +#define check(expr, label, ret, ...)					     \
> +	do {                                                                 \
> +		ret = __assert(expr, __FILE__, __LINE__,       		     \
> +			       "Assertion '" #expr "' failed", __VA_ARGS__); \
> +		if (!(expr)) {						     \
> +			perror("");					     \
> +			goto label;                                          \
> +		}							     \
> +	} while (0)

Ditto.

> +struct generic_test {
> +	const char *name;
> +	void *private;
> +
> +

Why do you have two blank lines here?

> +	void *testcases;

It seems you like having void* pointers, right? This is very unintuitive here (and
weird that you iterate with a char* pointer over it in function run_all_tests())
and at least needs some comments. Otherwise people will have to look into your
example to get to know what you mean.

Also, what is the difference between testcases and private? I have not seen that
you use private anywhere.

> +	int testcase_size;

Nitpick (but that depends if you keep void*): size_t len;

> +	int testcase_count;

Nitpick (ditto): unsigned int num_cases;

> +	/* Ends all tests if one fails */
> +	int abort_on_fail;

bool abort_on_fail;

> +	test_result_t (*prepare)(void *);
> +	test_result_t (*run)(void *);
> +	test_result_t (*cleanup)(void *);
> +};
> +
> +test_result_t run_all_tests(struct generic_test *test, void *param);
> +
> +#endif
> +
> +

Also no blank lines at the end of a file!

Ok, for now I leave the other patches aside, maybe you have some comments to the
review, a new version, or wait a while for others to give feedback as well.

Thanks and cheers,

Daniel

Re: [PATCH 1/3 RFC v2] selftests: introduce testing abstractions

[Alexandru Copot]

On Thu, Apr 25, 2013 at 9:19 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
> Usually I associate sth named assert() that it does an ungraceful exit.
> Maybe __test() or __check()?

This looks like a good idea, will do.

>> +       va_list vl;
>> +       const char *m;
>> +       char msg[BUFSIZ];
>> +
>> +       if (expr)
>> +               return TEST_PASS;
>> +
>> +       fprintf(stderr, "\n(%s:%d) ", filename, line);
>> +
>> +       va_start(vl, fmt);
>> +       m = va_arg(vl, char *);
>> +       if (!m)
>> +               m = fmt;
>> +       else
>> +               fprintf(stderr, "%s ", fmt);
>> +
>> +       vsnprintf(msg, sizeof msg, m, vl);
>
>
> Nitpick: sizeof(msg)

sizeof is actually an operator. I could change it especially to
keep an uniform coding style with the rest of the kernel and tools.

>
>> +       va_end(vl);
>> +
>> +       fprintf(stderr, "%s\n", msg);
>
>
> Not sure what exactly you are trying to accomplish here in this function?
> Why don't you use vfprintf() and leave this hackery aside?
>
> va_list vl;
>
> va_start(vl, fmt);
> fprintf(stderr, "(%s:%d) ", filename, line);
> vfprintf(stderr, fmt, vl);
> va_end(vl);
>

Here I am trying to format both using the fmt string and the one that
the user might write as the next argument. It's not really safe, but
this way we can print whatever data is needed to diagnose a failure.

Example: check(r == 0, out_label, ret, "option %d", 5);

But I think you are right, I will keep it simple for the moment.

>> +typedef enum test_result {
>> +       TEST_PASS = 0,
>> +       TEST_FAIL,
>> +} test_result_t;
>
>
> I have no strong opinion here, but do we need to make this as an enum and
> then do bit-operations on it?

I like enums because of type safety, but I will change these to defines.

>> +
>> +#define pass_if(expr, label, ret)                              \
>> +       do {                                                    \
>> +               if (expr) {                                     \
>> +                       ret = TEST_PASS;                        \
>> +                       goto label;                             \
>> +               }                                               \
>> +       } while (0)
>
>
> In cases where you make use of this macro, it's always like ...
>
> <function>
> {
>         pass_if(<cond>, out, ret);
> [...]
> out:
>         return ret;
> }
>
> Not sure if this simplifies or rather obfuscates things. Maybe other people
> like it, not sure ...

This looks like the cleanest solution to me. We have found some other
implementations
which used weird things like setjmp/longjmp. We are open to any
suggestions on this.

>
>> +struct generic_test {
>> +       const char *name;
>> +       void *private;
>> +
>> +
>
>
> Why do you have two blank lines here?
>

Correct, will remove.

>> +       void *testcases;
>
>
> It seems you like having void* pointers, right? This is very unintuitive
> here (and weird that you iterate with a char* pointer over it in function
> run_all_tests())
> and at least needs some comments. Otherwise people will have to look into
> your example to get to know what you mean.

Having void pointers is simpler because it doesn't force you to write casts. It
is also generic. I iterate with char* because I can't do it with a void*.
Yes, you are right about the comments.

> Also, what is the difference between testcases and private? I have not seen
> that you use private anywhere.

The private field is mostly for future use, will remove it for now until
we find some real use case.

>> +       int testcase_size;
>
>
> Nitpick (but that depends if you keep void*): size_t len;
>
>> +       int testcase_count;
>
>
> Nitpick (ditto): unsigned int num_cases;
>
>
>> +       /* Ends all tests if one fails */
>> +       int abort_on_fail;
>
>
> bool abort_on_fail;
>
> Also no blank lines at the end of a file!

I agree with all of these.

> Ok, for now I leave the other patches aside, maybe you have some comments to
> the review, a new version, or wait a while for others to give feedback as well.
>
> Thanks and cheers,
>
> Daniel

Thanks for being so thorough!

Alex

[PATCH 2/3 RFC v2] selftests/net: update socket test to use new testing framework

[Alexandru Copot]

Signed-of-by Alexandru Copot <alex.mihai.c@gmail.com>
Cc: Daniel Baluta <dbaluta@ixiacom.com>
---
 tools/testing/selftests/net/Makefile |  14 +++--
 tools/testing/selftests/net/socket.c | 108 +++++++++++++++++++++++++----------
 2 files changed, 88 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 750512b..7984f80 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -3,17 +3,23 @@
 CC = $(CROSS_COMPILE)gcc
 CFLAGS = -Wall -O2 -g
 
-CFLAGS += -I../../../../usr/include/
+CFLAGS += -I../../../../usr/include/ -I../lib
 
 NET_PROGS = socket psock_fanout psock_tpacket
 
+LIB = ../lib/selftests.a
+
 all: $(NET_PROGS)
-%: %.c
-	$(CC) $(CFLAGS) -o $@ $^
+
+socket: socket.o $(LIB)
+
+%.o: %.c
+	$(CC) $(CFLAGS) -c -o $@ $^
+
 
 run_tests: all
 	@/bin/sh ./run_netsocktests || echo "sockettests: [FAIL]"
 	@/bin/sh ./run_afpackettests || echo "afpackettests: [FAIL]"
 
 clean:
-	$(RM) $(NET_PROGS)
+	$(RM) $(NET_PROGS) *.o
diff --git a/tools/testing/selftests/net/socket.c b/tools/testing/selftests/net/socket.c
index 0f227f2..198a57b 100644
--- a/tools/testing/selftests/net/socket.c
+++ b/tools/testing/selftests/net/socket.c
@@ -6,6 +6,8 @@
 #include <sys/socket.h>
 #include <netinet/in.h>
 
+#include "selftests.h"
+
 struct socket_testcase {
 	int	domain;
 	int	type;
@@ -22,7 +24,7 @@ struct socket_testcase {
 	int	nosupport_ok;
 };
 
-static struct socket_testcase tests[] = {
+static struct socket_testcase tests_inet[] = {
 	{ AF_MAX,  0,           0,           -EAFNOSUPPORT,    0 },
 	{ AF_INET, SOCK_STREAM, IPPROTO_TCP, 0,                1  },
 	{ AF_INET, SOCK_DGRAM,  IPPROTO_TCP, -EPROTONOSUPPORT, 1  },
@@ -30,58 +32,104 @@ static struct socket_testcase tests[] = {
 	{ AF_INET, SOCK_STREAM, IPPROTO_UDP, -EPROTONOSUPPORT, 1  },
 };
 
+static struct socket_testcase tests_inet6[] = {
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP, 0,                1  },
+	{ AF_INET6, SOCK_DGRAM,  IPPROTO_TCP, -EPROTONOSUPPORT, 1  },
+	{ AF_INET6, SOCK_DGRAM,  IPPROTO_UDP, 0,                1  },
+	{ AF_INET6, SOCK_STREAM, IPPROTO_UDP, -EPROTONOSUPPORT, 1  },
+};
+
+static struct socket_testcase tests_unix[] = {
+	{ AF_UNIX, SOCK_STREAM,    0,           0,                1  },
+	{ AF_UNIX, SOCK_DGRAM,     0,           0,                1  },
+	{ AF_UNIX, SOCK_SEQPACKET, 0,           0,                1  },
+	{ AF_UNIX, SOCK_STREAM,    PF_UNIX,     0,                1  },
+	{ AF_UNIX, SOCK_DGRAM,     PF_UNIX,     0,                1  },
+	{ AF_UNIX, SOCK_SEQPACKET, PF_UNIX,     0,                1  },
+	{ AF_UNIX, SOCK_STREAM,    IPPROTO_TCP, -EPROTONOSUPPORT, 1  },
+	{ AF_UNIX, SOCK_STREAM,    IPPROTO_UDP, -EPROTONOSUPPORT, 1  },
+	{ AF_UNIX, SOCK_DCCP,      0,           -ESOCKTNOSUPPORT, 1  },
+};
+
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 #define ERR_STRING_SZ	64
 
-static int run_tests(void)
+static test_result_t my_run_testcase(void *arg)
 {
+	struct socket_testcase *s = (struct socket_testcase*)arg;
+	int fd;
 	char err_string1[ERR_STRING_SZ];
 	char err_string2[ERR_STRING_SZ];
-	int i, err;
-
-	err = 0;
-	for (i = 0; i < ARRAY_SIZE(tests); i++) {
-		struct socket_testcase *s = &tests[i];
-		int fd;
+	test_result_t ret;
 
-		fd = socket(s->domain, s->type, s->protocol);
-		if (fd < 0) {
-			if (s->nosupport_ok &&
-			    errno == EAFNOSUPPORT)
-				continue;
+	ret = TEST_PASS;
+	fd = socket(s->domain, s->type, s->protocol);
+	if (fd < 0) {
+		pass_if(s->nosupport_ok && errno == EAFNOSUPPORT, out, ret);
 
-			if (s->expect < 0 &&
-			    errno == -s->expect)
-				continue;
+		pass_if(s->expect < 0 && errno == -s->expect, out, ret);
 
-			strerror_r(-s->expect, err_string1, ERR_STRING_SZ);
-			strerror_r(errno, err_string2, ERR_STRING_SZ);
+		strerror_r(-s->expect, err_string1, ERR_STRING_SZ);
+		strerror_r(errno, err_string2, ERR_STRING_SZ);
 
-			fprintf(stderr, "socket(%d, %d, %d) expected "
+		fprintf(stderr, "socket(%d, %d, %d) expected "
 				"err (%s) got (%s)\n",
 				s->domain, s->type, s->protocol,
 				err_string1, err_string2);
 
-			err = -1;
-			break;
-		} else {
-			close(fd);
+		return TEST_FAIL;
+	} else {
+		close(fd);
 
-			if (s->expect < 0) {
-				strerror_r(errno, err_string1, ERR_STRING_SZ);
+		if (s->expect < 0) {
+			strerror_r(errno, err_string1, ERR_STRING_SZ);
 
-				fprintf(stderr, "socket(%d, %d, %d) expected "
+			fprintf(stderr, "socket(%d, %d, %d) expected "
 					"success got err (%s)\n",
 					s->domain, s->type, s->protocol,
 					err_string1);
 
-				err = -1;
-				break;
-			}
+			return TEST_FAIL;
 		}
 	}
+out:
+	return ret;
+}
 
-	return err;
+static int run_tests(void)
+{
+	int rc;
+	struct generic_test test1 = {
+		.name = "socket AF_INET",
+		.prepare = NULL,
+		.run = my_run_testcase,
+		.testcases = tests_inet,
+		.testcase_size = sizeof(struct socket_testcase),
+		.testcase_count = ARRAY_SIZE(tests_inet),
+	};
+	struct generic_test test2 = {
+		.name = "socket AF_INET6",
+		.prepare = NULL,
+		.run = my_run_testcase,
+		.testcases = tests_inet6,
+		.testcase_size = sizeof(struct socket_testcase),
+		.testcase_count = ARRAY_SIZE(tests_inet6),
+	};
+	struct generic_test test3 = {
+		.name = "socket AF_UNIX",
+		.prepare = NULL,
+		.run = my_run_testcase,
+		.testcases = tests_unix,
+		.testcase_size = sizeof(struct socket_testcase),
+		.testcase_count = ARRAY_SIZE(tests_unix),
+	};
+
+	rc = 0;
+	rc |= run_all_tests(&test1, NULL);
+	rc |= run_all_tests(&test2, NULL);
+	rc |= run_all_tests(&test3, NULL);
+
+	return rc;
 }
 
 int main(void)
-- 
1.8.2.1

[PATCH 3/3 RFC v2] selftests/net: add socket options test with IPv6 testcases

[Alexandru Copot]

Only a part of the boolean socket options for IPv6 are tested.

Signed-of-by Alexandru Copot <alex.mihai.c@gmail.com>
Cc: Daniel Baluta <dbaluta@ixiacom.com>
---
 tools/testing/selftests/net/Makefile         |   3 +-
 tools/testing/selftests/net/run_netsocktests |  10 ++
 tools/testing/selftests/net/sockopt.c        | 185 +++++++++++++++++++++++++++
 3 files changed, 197 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/sockopt.c

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 7984f80..087e9a0 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -5,13 +5,14 @@ CFLAGS = -Wall -O2 -g
 
 CFLAGS += -I../../../../usr/include/ -I../lib
 
-NET_PROGS = socket psock_fanout psock_tpacket
+NET_PROGS = socket sockopt psock_fanout psock_tpacket
 
 LIB = ../lib/selftests.a
 
 all: $(NET_PROGS)
 
 socket: socket.o $(LIB)
+sockopt: sockopt.o $(LIB)
 
 %.o: %.c
 	$(CC) $(CFLAGS) -c -o $@ $^
diff --git a/tools/testing/selftests/net/run_netsocktests b/tools/testing/selftests/net/run_netsocktests
index c09a682..9f21498 100644
--- a/tools/testing/selftests/net/run_netsocktests
+++ b/tools/testing/selftests/net/run_netsocktests
@@ -10,3 +10,13 @@ else
 	echo "[PASS]"
 fi
 
+echo "--------------------"
+echo "running setsockopt test"
+echo "--------------------"
+./sockopt
+if [ $? -ne 0 ]; then
+	echo "[FAIL]"
+else
+	echo "[PASS]"
+fi
+
diff --git a/tools/testing/selftests/net/sockopt.c b/tools/testing/selftests/net/sockopt.c
new file mode 100644
index 0000000..56311bc
--- /dev/null
+++ b/tools/testing/selftests/net/sockopt.c
@@ -0,0 +1,185 @@
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <linux/ipv6.h>
+#include <linux/in.h>
+
+#include "selftests.h"
+
+struct sockopt_testcase {
+	/* socket */
+	int	domain;
+	int	type;
+	int	protocol;
+
+	/* option */
+	int	level;
+	int	optname;
+	void	*value;
+	socklen_t size;
+
+	#define TYPE_INT   1
+	#define TYPE_DATA  2
+	int data_type;
+	/* 0    = valid file descriptor
+	 * -foo = error foo
+	 */
+	int	expect_set;
+	int	expect_get;
+
+	/* If non-zero, accept EAFNOSUPPORT to handle the case
+	 * of the protocol not being configured into the kernel.
+	 */
+	int	nosupport_ok;
+};
+
+static struct sockopt_testcase tests_inet6[] = {
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_V6ONLY,                  (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_V6ONLY,                  (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVPKTINFO,             (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVPKTINFO,             (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292PKTINFO,             (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292PKTINFO,             (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+	
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVHOPLIMIT,            (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVHOPLIMIT,            (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292HOPLIMIT,            (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292HOPLIMIT,            (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+ 	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVRTHDR,               (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVRTHDR,               (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292RTHDR,               (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292RTHDR,               (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+	
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVHOPOPTS,             (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVHOPOPTS,             (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292HOPOPTS,             (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292HOPOPTS,             (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVDSTOPTS,             (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVDSTOPTS,             (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292DSTOPTS,             (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_2292DSTOPTS,             (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_TCLASS,                  (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_TCLASS,                  (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVTCLASS,              (void*)1, sizeof(int),  TYPE_INT, 0,  0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVTCLASS,              (void*)0, sizeof(int),  TYPE_INT, 0,  0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_FLOWINFO,                (void*)1, sizeof(int),  TYPE_INT, 0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_FLOWINFO,                (void*)0, sizeof(int),  TYPE_INT, 0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVPATHMTU,             (void*)1, sizeof(int),  TYPE_INT, 0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVPATHMTU,             (void*)0, sizeof(int),  TYPE_INT, 0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVORIGDSTADDR,         (void*)1, sizeof(int),  TYPE_INT, 0,  0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_RECVORIGDSTADDR,         (void*)0, sizeof(int),  TYPE_INT, 0,  0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_MTU,                     (void*)512,          sizeof(int),  TYPE_INT, -EINVAL, -ENOTCONN, 0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_MTU,                     (void*)IPV6_MIN_MTU, sizeof(int),  TYPE_INT,       0, -ENOTCONN, 0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_DONTFRAG,                (void*)1,            sizeof(int),  TYPE_INT,       0,         0, 0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_DONTFRAG,                (void*)0,            sizeof(int),  TYPE_INT,       0,         0, 0},
+
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_MTU_DISCOVER,             (void*)IP_PMTUDISC_DONT, sizeof(int),  TYPE_INT,       0,     0, 0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_MTU_DISCOVER,             (void*)IP_PMTUDISC_WANT, sizeof(int),  TYPE_INT,       0,     0, 0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_MTU_DISCOVER,             (void*)IP_PMTUDISC_DO,   sizeof(int),  TYPE_INT,       0,     0, 0},
+	{ AF_INET6, SOCK_STREAM, IPPROTO_TCP,   IPPROTO_IPV6, IPV6_MTU_DISCOVER,             (void*)IP_PMTUDISC_PROBE,sizeof(int),  TYPE_INT,       0,     0, 0},
+
+};
+
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+#define ERR_STRING_SZ	64
+
+static test_result_t my_run_testcase(void *arg)
+{
+	struct sockopt_testcase *s = (struct sockopt_testcase*)arg;
+	int fd, rc, val;
+	void *optdata;
+	socklen_t readsize;
+	test_result_t ret;
+
+	ret = 0;
+	fd = socket(s->domain, s->type, s->protocol);
+	abort(fd > 0);
+
+	if (s->data_type == TYPE_INT) {
+		val = (long)s->value;
+		optdata = &val;
+	} else {
+		optdata = s->value;
+	}
+
+	rc = setsockopt(fd, s->level, s->optname, optdata, s->size);
+	check(rc == 0 || errno == -s->expect_set, clean_socket, ret,
+	      "setsockopt option %d", s->optname);
+
+	if (s->data_type == TYPE_INT) {
+		optdata = &val;
+		val = -1;
+	} else {
+		optdata = malloc(s->size);
+		abort(optdata != NULL);
+	}
+
+	readsize = s->size;
+	rc = getsockopt(fd, s->level, s->optname, optdata, &readsize);
+	check(rc == 0 || errno == -s->expect_get, clean_data, ret,
+	      "getsockopt option %d", s->optname);
+	abort(readsize == s->size);
+
+	if (!rc) {
+		if (s->data_type == TYPE_INT) {
+			check(val == (long)s->value, clean_socket, ret,
+			      "option %d: Read value different from written value\n", s->optname);
+		} else {
+			check(!memcmp(optdata, s->value, s->size), clean_data, ret,
+			      "option %d: Read value different from written value\n", s->optname);
+		}
+	}
+
+clean_data:
+	if (s->data_type != TYPE_INT)
+		free(optdata);
+clean_socket:
+	abort(close(fd) == 0);
+
+	return ret;
+}
+
+static int run_tests(void)
+{
+	int rc;
+	struct generic_test test1 = {
+		.name = "sockopt AF_INET6",
+		.prepare = NULL,
+		.run = my_run_testcase,
+		.testcases = tests_inet6,
+		.testcase_size = sizeof(struct sockopt_testcase),
+		.testcase_count = ARRAY_SIZE(tests_inet6),
+		.abort_on_fail = 1,
+	};
+
+	rc = 0;
+	rc |= run_all_tests(&test1, NULL);
+
+	return rc;
+}
+
+int main(void)
+{
+	int err = run_tests();
+
+	return err;
+}
-- 
1.8.2.1

Re: [PATCH 0/3 RFC v2] selftests: Basic framework for tests

[Daniel Borkmann]

On 04/25/2013 01:04 PM, Alexandru Copot wrote:
> This series adds a generic test abstraction that can make
> writing testcases easier. A generic_test structure is
> used to define a test and its methods: prepare, run, cleanup.
>
> This is a generic implementation so it was placed in selftests/lib.
>
> The second patch updates the socket tests to use the
> new framework and the third patch creates new tests
> for [set/get]sockopt with some IPV6_* options.

This already looks better than the previous one. I will probably go
a bit more in depth through the code in the evening. A couple of minor
general items I can already tell you:

> Signed-of-by Alexandru Copot <alex.mihai.c@gmail.com>

It's: Signed-off-by: Alexandru Copot <alex.mihai.c@gmail.com>

You have this wrong in all your patches.

Also, in patches 1-3 a proper commit message would be nice, i.e. in
patch 1 when you add the library that everyone should use. It could be
that your cover letter will not go into the Git history, thus people
will only see you empty commit bodies.

> Cc: Daniel Baluta <dbaluta@ixiacom.com>
>
> Changes since v1:
>   - moved the implementation to selftests/lib
>   - use goto instead of directly returning
>
> Alexandru Copot (3):
>    selftests: introduce testing abstractions
>    selftests/net: update socket test to use new testing framework
>    selftests/net: add socket options test with IPv6 testcases
>
>   tools/testing/selftests/Makefile             |   3 +-
>   tools/testing/selftests/lib/Makefile         |  14 ++
>   tools/testing/selftests/lib/selftests.c      |  57 +++++++++
>   tools/testing/selftests/lib/selftests.h      |  67 ++++++++++
>   tools/testing/selftests/net/Makefile         |  17 ++-
>   tools/testing/selftests/net/run_netsocktests |  10 ++
>   tools/testing/selftests/net/socket.c         | 108 +++++++++++-----
>   tools/testing/selftests/net/sockopt.c        | 185 +++++++++++++++++++++++++++
>   8 files changed, 425 insertions(+), 36 deletions(-)
>   create mode 100644 tools/testing/selftests/lib/Makefile
>   create mode 100644 tools/testing/selftests/lib/selftests.c
>   create mode 100644 tools/testing/selftests/lib/selftests.h
>   create mode 100644 tools/testing/selftests/net/sockopt.c

Re: [PATCH 0/3 RFC v2] selftests: Basic framework for tests

[Alexandru Copot]

On Thu, Apr 25, 2013 at 2:27 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> Signed-of-by Alexandru Copot <alex.mihai.c@gmail.com>
>
>
> It's: Signed-off-by: Alexandru Copot <alex.mihai.c@gmail.com>
>
> You have this wrong in all your patches.

I know, it was copy-pasted and I saw it just after sending. But for now
it's fine, these are just RFCs.

> Also, in patches 1-3 a proper commit message would be nice, i.e. in
> patch 1 when you add the library that everyone should use. It could be
> that your cover letter will not go into the Git history, thus people
> will only see you empty commit bodies.

You are right, I just thought the code was small enough to not
require a description.

Thanks!
Alex

On Thu, Apr 25, 2013 at 2:27 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 04/25/2013 01:04 PM, Alexandru Copot wrote:
>>
>> This series adds a generic test abstraction that can make
>> writing testcases easier. A generic_test structure is
>> used to define a test and its methods: prepare, run, cleanup.
>>
>> This is a generic implementation so it was placed in selftests/lib.
>>
>> The second patch updates the socket tests to use the
>> new framework and the third patch creates new tests
>> for [set/get]sockopt with some IPV6_* options.
>
>
> This already looks better than the previous one. I will probably go
> a bit more in depth through the code in the evening. A couple of minor
> general items I can already tell you:
>
>> Signed-of-by Alexandru Copot <alex.mihai.c@gmail.com>
>
>
> It's: Signed-off-by: Alexandru Copot <alex.mihai.c@gmail.com>
>
> You have this wrong in all your patches.
>
> Also, in patches 1-3 a proper commit message would be nice, i.e. in
> patch 1 when you add the library that everyone should use. It could be
> that your cover letter will not go into the Git history, thus people
> will only see you empty commit bodies.
>
>
>> Cc: Daniel Baluta <dbaluta@ixiacom.com>
>>
>> Changes since v1:
>>   - moved the implementation to selftests/lib
>>   - use goto instead of directly returning
>>
>> Alexandru Copot (3):
>>    selftests: introduce testing abstractions
>>    selftests/net: update socket test to use new testing framework
>>    selftests/net: add socket options test with IPv6 testcases
>>
>>   tools/testing/selftests/Makefile             |   3 +-
>>   tools/testing/selftests/lib/Makefile         |  14 ++
>>   tools/testing/selftests/lib/selftests.c      |  57 +++++++++
>>   tools/testing/selftests/lib/selftests.h      |  67 ++++++++++
>>   tools/testing/selftests/net/Makefile         |  17 ++-
>>   tools/testing/selftests/net/run_netsocktests |  10 ++
>>   tools/testing/selftests/net/socket.c         | 108 +++++++++++-----
>>   tools/testing/selftests/net/sockopt.c        | 185
>> +++++++++++++++++++++++++++
>>   8 files changed, 425 insertions(+), 36 deletions(-)
>>   create mode 100644 tools/testing/selftests/lib/Makefile
>>   create mode 100644 tools/testing/selftests/lib/selftests.c
>>   create mode 100644 tools/testing/selftests/lib/selftests.h
>>   create mode 100644 tools/testing/selftests/net/sockopt.c