[PATCH 0/4] Teach trinity to not use some sockets family

[PATCH 0/4] Teach trinity to not use some sockets family

[Cyrill Gorcunov]

When I've been running trinity on testing machine admins were
not that happy about traffic generated by trinity (especially
ones created with packet socket). So here is an attempt to teach
trinity to use say only some set of sockets if needed with help
of --fds option.

Please take a look, thanks.

[PATCH 1/4] make: Add cscope target

[Cyrill Gorcunov]

Just another source indexing system which might
be preferred over tags.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile b/Makefile
index b0bdb36..76f078b 100644
--- a/Makefile
+++ b/Makefile
@@ -125,3 +125,6 @@ coverity:
 	cov-build --dir cov-int make
 	tar czvf trinity-coverity.tgz cov-int
 
+cscope:
+	@find . -name '*.[hcS]' ! -path './.*' -print > cscope.files
+	@cscope -bkqu
-- 
1.8.3.1

[PATCH 2/4] log: Add BUG_ON helper

[Cyrill Gorcunov]

We will use it in sockets creating routine.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 include/log.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/log.h b/include/log.h
index 500353d..22ad80a 100644
--- a/include/log.h
+++ b/include/log.h
@@ -59,4 +59,6 @@ void debugf(const char *fmt, ...);
 	}\
 }
 
+#define BUG_ON(condition)	do { if ((condition)) BUG(BUGTXT); } while (0)
+
 #endif	/* _LOG_H */
-- 
1.8.3.1

[PATCH 3/4] params: Add parsing of -f|--fds option

[Cyrill Gorcunov]

Here we introduce a trivial tokenizer to control socket
domains assignment. It might be needed to disabled some
sockets (or even all of them, if you're not interested
in testing sockets). For this sake -f|--fds option introduced.

The option takes argument in form of

	--fds sockets:[(all,)|(none,)|(PF_X,^PF_X)]

where PF_X is appropriate domain like PF_INET and such.
The prefix ^ before name means to exclude some particular
family from being tested.

Also note that all|none do overwrite previous option, this
is done in a sake of code simplicity. IOW, if

	--fds sockets:^PF_INET,all

is passed, in result all socket families are allowed.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 include/net.h   |  6 +++-
 net/protocols.c | 11 +++++++
 params.c        | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/include/net.h b/include/net.h
index 4e43546..c5baaf4 100644
--- a/include/net.h
+++ b/include/net.h
@@ -1,9 +1,12 @@
 #ifndef _NET_H
 #define _NET_H 1
 
+#include <sys/types.h>
+#include <sys/socket.h>
 #include <netinet/in.h>
 
 extern unsigned int nr_sockets;
+extern char disabled_protos[PF_MAX];
 void open_sockets(void);
 void generate_sockaddr(unsigned long *addr, unsigned long *addrlen, int pf);
 
@@ -11,7 +14,8 @@ void generate_sockaddr(unsigned long *addr, unsigned long *addrlen, int pf);
 extern unsigned int specific_proto;
 const char * get_proto_name(unsigned int proto);
 void find_specific_proto(const char *protoarg);
-
+unsigned int find_proto_by_name(const char *name);
+int disable_socket(const char *domain);
 
 /* glibc headers might be older than the kernel, so chances are we know
  * about more protocols than glibc does. So we define our own PF_MAX */
diff --git a/net/protocols.c b/net/protocols.c
index 891acfe..6aedd29 100644
--- a/net/protocols.c
+++ b/net/protocols.c
@@ -58,6 +58,17 @@ static const struct protocol protocols[] = {
 	{ "PF_VSOCK",        40 },
 };
 
+unsigned int find_proto_by_name(const char *name)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(protocols); i++) {
+		if (!strcmp(protocols[i].name, name))
+			return protocols[i].proto;
+	}
+	return -1u;
+}
+
 const char * get_proto_name(unsigned int proto)
 {
 	unsigned int i;
diff --git a/params.c b/params.c
index 77dc7e4..f2b31a3 100644
--- a/params.c
+++ b/params.c
@@ -5,6 +5,7 @@
 #include <getopt.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/socket.h>
 
 #include "trinity.h"	// progname
 #include "random.h"
@@ -12,6 +13,7 @@
 #include "log.h"
 #include "params.h"
 #include "tables.h"
+#include "net.h"
 
 #define TAINT_NAME_LEN 32
 
@@ -53,6 +55,13 @@ int kernel_taint_initial = 0;
 int kernel_taint_mask = 0xFFFFFFFF;
 bool kernel_taint_param_occured = FALSE;
 
+/*
+ * Some socket domains might be disabled in command line,
+ * track the option here, probably worth to convert to
+ * bitmap.
+ */
+char disabled_protos[PF_MAX];
+
 static void usage(void)
 {
 	outputerr("%s\n", progname);
@@ -64,6 +73,7 @@ static void usage(void)
 	outputerr(" --kernel_taint, -T: controls which kernel taint flags should be considered, for more details refer to README file. \n");
 	outputerr(" --list,-L: list all syscalls known on this architecture.\n");
 	outputerr(" --logging,-l: (off=disable logging).\n");
+	outputerr(" --fds,-f: controls which kind of sockets and files to use.\n");
 	outputerr(" --monochrome,-m: don't output ANSI codes\n");
 	outputerr(" --no_files,-n: Only pass sockets as fd's, not files\n");
 	outputerr(" --proto,-P: specify specific network protocol for sockets.\n");
@@ -91,6 +101,7 @@ static const struct option longopts[] = {
 	{ "list", no_argument, NULL, 'L' },
 	{ "ioctls", no_argument, NULL, 'I' },
 	{ "logging", required_argument, NULL, 'l' },
+	{ "fds", required_argument, NULL, 'f' },
 	{ "monochrome", no_argument, NULL, 'm' },
 	{ "no_files", no_argument, NULL, 'n' },
 	{ "proto", required_argument, NULL, 'P' },
@@ -172,11 +183,86 @@ static void process_taint_arg(char *taintarg) {
 	toggle_taint_flag_by_name(beg,end);
 }
 
+static void parse_fds(char *arg)
+{
+	enum {
+		FDS_NONE,
+		FDS_SOCKET,
+	} context = FDS_NONE;
+	char *p = strdup(arg);
+	char *tok;
+
+	if (!p) {
+		outputerr("No memory to parse %s\n", arg);
+		exit(EXIT_FAILURE);
+	}
+
+	/*
+	 * We're expecting here the following format,
+	 * but our parser is a way more greedy and simplier:
+	 * spaces, semicolons and commas are all separators.
+	 *
+	 * 	sockets:[(all,)|(none,)(PF_X,^PF_X)]
+	 *
+	 * note, we're context dependant -- by default all sockets
+	 * are enabled, if you need to disable a particular socket,
+	 * the "^PF_" prefix should be passed. But if you need only
+	 * a few sockets to be enabled then it must be "none,PF_X,PF_X".
+	 *
+	 * It might be not that convenient to type but easier to parse ;-)
+	 */
+	for (tok = strtok(p, " :,"); tok; tok = strtok(NULL, " :,")) {
+		if (strcmp(tok, "sockets") == 0) {
+			context = FDS_SOCKET;
+			continue;
+		}
+
+		switch (context) {
+		case FDS_SOCKET:
+			if (strcmp(tok, "all") == 0) {
+				memset(disabled_protos, 'n', sizeof(disabled_protos));
+			} else if (strcmp(tok, "none") == 0) {
+				memset(disabled_protos, 'y', sizeof(disabled_protos));
+			} else if (strncmp(tok, "PF_", 3) == 0 ||
+				   strncmp(tok, "^PF_", 4) == 0) {
+				int neg = tok[0] == '^';
+				unsigned int proto;
+
+
+				proto = find_proto_by_name(&tok[neg]);
+				if (proto == -1u)
+					goto err;
+				else if (proto > ARRAY_SIZE(disabled_protos))
+					BUG_ON(1);
+				else {
+					if (neg)
+						disabled_protos[proto] = 'y';
+					else
+						disabled_protos[proto] = 'n';
+				}
+			}
+			break;
+		case FDS_NONE:
+		default:
+			outputerr("Unknown context.\n");
+			goto err;
+		}
+	}
+
+	free(p);
+	return;
+
+err:
+	free(p);
+	outputerr("Unrecognizable option in \"%s\".\n", arg);
+	exit(EXIT_FAILURE);
+}
+
 void parse_args(int argc, char *argv[])
 {
 	int opt;
 
-	while ((opt = getopt_long(argc, argv, "a:c:C:dDg:hIl:LN:mnP:pqr:s:T:SV:vx:", longopts, NULL)) != -1) {
+	while ((opt = getopt_long(argc, argv, "a:c:C:dDg:hIlf:LN:mnP:pqr:s:T:SV:vx:", longopts, NULL)) != -1) {
 		switch (opt) {
 		default:
 			if (opt == '?')
@@ -240,6 +326,10 @@ void parse_args(int argc, char *argv[])
 				logging = 0;
 			break;
 
+		case 'f':
+			parse_fds(optarg);
+			break;
+
 		case 'L':
 			show_syscall_list = TRUE;
 			break;
-- 
1.8.3.1

[PATCH 4/4] sockets: Take into account socket families disabled when generating sockets

[Cyrill Gorcunov]

When we generate sockets cache file we should take into account
the option passed with --fds making sure the socket we're about
to create was not disabled.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 sockets.c         | 17 ++++++++++++++---
 syscalls/socket.c | 20 +++++++++++++++++++-
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/sockets.c b/sockets.c
index 590c814..cb9a9a2 100644
--- a/sockets.c
+++ b/sockets.c
@@ -132,6 +132,13 @@ static void generate_sockets(void)
 
 	lock_cachefile(cachefile, F_WRLCK);
 
+	/*
+	 * If we have all sockets disabled, don't create any.
+	 */
+	if (index((const char *)disabled_protos, 'y') &&
+	    !index((const char *)disabled_protos, 'n'))
+		nr_to_create = 0;
+
 	while (nr_to_create > 0) {
 
 		struct socket_triplet st;
@@ -141,6 +148,8 @@ static void generate_sockets(void)
 			return;
 		}
 
+		BUG_ON(ARRAY_SIZE(disabled_protos) >= TRINITY_PF_MAX);
+
 		for (st.family = 0; st.family < TRINITY_PF_MAX; st.family++) {
 
 			if (do_specific_proto == TRUE)
@@ -149,6 +158,9 @@ static void generate_sockets(void)
 			if (get_proto_name(st.family) == NULL)
 				goto skip;
 
+			if (disabled_protos[st.family] == 'y')
+				goto skip;
+
 			if (sanitise_socket_triplet(&st) == -1)
 				rand_proto_type(&st);
 
@@ -236,8 +248,8 @@ void open_sockets(void)
 		type = buffer[1];
 		protocol = buffer[2];
 
-		if (do_specific_proto == TRUE) {
-			if (domain != specific_proto) {
+		if ((do_specific_proto == TRUE && domain != specific_proto) ||
+		    (domain < ARRAY_SIZE(disabled_protos) && disabled_protos[domain] == 'y')) {
 				output(1, "ignoring socket cachefile due to specific protocol request, and stale data in cachefile.\n");
 regenerate:
 				unlock_cachefile(cachefile);	/* drop the reader lock. */
@@ -245,7 +257,6 @@ regenerate:
 				unlink(cachefilename);
 				generate_sockets();
 				return;
-			}
 		}
 
 		fd = open_socket(domain, type, protocol);
diff --git a/syscalls/socket.c b/syscalls/socket.c
index 21dabb1..8d5100c 100644
--- a/syscalls/socket.c
+++ b/syscalls/socket.c
@@ -44,17 +44,35 @@ static const struct socket_ptr socketptrs[] = {
 
 void rand_proto_type(struct socket_triplet *st)
 {
+	int n = 6;
 	st->protocol = rand() % PROTO_MAX;
 
-	switch (rand() % 6) {
+again:
+	switch (rand() % n) {
 	case 0:	st->type = SOCK_DGRAM;	break;
 	case 1:	st->type = SOCK_STREAM;	break;
 	case 2:	st->type = SOCK_SEQPACKET;	break;
 	case 3:	st->type = SOCK_RAW;	break;
 	case 4:	st->type = SOCK_RDM;	break;
+
+	/*
+	 * It must be last.
+	 */
 	case 5:	st->type = SOCK_PACKET;	break;
 	default: break;
 	}
+
+	/*
+	 * One special moment on packet sockets. They
+	 * can be created with SOCK_PACKET, so if
+	 * PF_PACKET is disabled, choose some other type.
+	 */
+	if (st->type == SOCK_PACKET	&&
+	    st->family == PF_INET	&&
+	    disabled_protos[PF_PACKET] == 'y') {
+		n = 5;
+		goto again;
+	}
 }
 
 /* note: also called from generate_sockets() */
-- 
1.8.3.1

Re: [PATCH 0/4] Teach trinity to not use some sockets family

[Cyrill Gorcunov]

On Fri, Oct 25, 2013 at 1:39 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> When I've been running trinity on testing machine admins were
> not that happy about traffic generated by trinity (especially
> ones created with packet socket). So here is an attempt to teach
> trinity to use say only some set of sockets if needed with help
> of --fds option.
>
> Please take a look, thanks.

Drop the serice please, it'll be reworked.

Re: [PATCH 0/4] Teach trinity to not use some sockets family

[Dave Jones]

On Fri, Oct 25, 2013 at 11:48:58AM +0400, Cyrill Gorcunov wrote:
 > On Fri, Oct 25, 2013 at 1:39 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
 > > When I've been running trinity on testing machine admins were
 > > not that happy about traffic generated by trinity (especially
 > > ones created with packet socket). So here is an attempt to teach
 > > trinity to use say only some set of sockets if needed with help
 > > of --fds option.
 > >
 > > Please take a look, thanks.
 > 
 > Drop the serice please, it'll be reworked.

I'm about to apply your later series, but I noticed that that one
instead of the --fds patch, has the -E change.

Do you still plan on submitting the --fds change later?
The reason I ask is that it if we're doing --fds, then it might at some point
mean we can deprecate (and then remove) use of -P in favor of it, so adding an
inverse (-E) seems odd.

	Dave
 

Re: [PATCH 0/4] Teach trinity to not use some sockets family

[Cyrill Gorcunov]

On Thu, Oct 31, 2013 at 03:49:43PM -0400, Dave Jones wrote:
> On Fri, Oct 25, 2013 at 11:48:58AM +0400, Cyrill Gorcunov wrote:
>  > On Fri, Oct 25, 2013 at 1:39 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
>  > > When I've been running trinity on testing machine admins were
>  > > not that happy about traffic generated by trinity (especially
>  > > ones created with packet socket). So here is an attempt to teach
>  > > trinity to use say only some set of sockets if needed with help
>  > > of --fds option.
>  > >
>  > > Please take a look, thanks.
>  > 
>  > Drop the serice please, it'll be reworked.
> 
> I'm about to apply your later series, but I noticed that that one
> instead of the --fds patch, has the -E change.
> 
> Do you still plan on submitting the --fds change later?
> The reason I ask is that it if we're doing --fds, then it might at some point
> mean we can deprecate (and then remove) use of -P in favor of it, so adding an
> inverse (-E) seems odd.

Hi Dave! To be fair, not sure at moment. Initially I wanted to implement a general
--fds option which might take more complex command line like sockets:PF_X,^PF_X and
such, then extend it to file:^pipe,epoll. But this end up in being somehow
more complex rework. So I decided to stick with simplier approach first -- -E option
which would exclude some socket protocols from being generated.

But sure, once time permit I can try to implement --fds option as well. Lets fisrt
summarize what kind of syntax it will carry.

 --fds [sockets:PF_X,^PF_X,N,^N,all,none],[files:pipe,^epoll,all,none]

Sounds good?

Re: [PATCH 0/4] Teach trinity to not use some sockets family

[Dave Jones]

On Fri, Nov 01, 2013 at 12:05:45AM +0400, Cyrill Gorcunov wrote:

 > > I'm about to apply your later series, but I noticed that that one
 > > instead of the --fds patch, has the -E change.
 > > 
 > > Do you still plan on submitting the --fds change later?
 > > The reason I ask is that it if we're doing --fds, then it might at some point
 > > mean we can deprecate (and then remove) use of -P in favor of it, so adding an
 > > inverse (-E) seems odd.
 > 
 > Hi Dave! To be fair, not sure at moment. Initially I wanted to implement a general
 > --fds option which might take more complex command line like sockets:PF_X,^PF_X and
 > such, then extend it to file:^pipe,epoll. But this end up in being somehow
 > more complex rework. So I decided to stick with simplier approach first -- -E option
 > which would exclude some socket protocols from being generated.
 > 
 > But sure, once time permit I can try to implement --fds option as well. Lets fisrt
 > summarize what kind of syntax it will carry.
 > 
 >  --fds [sockets:PF_X,^PF_X,N,^N,all,none],[files:pipe,^epoll,all,none]
 > 
 > Sounds good?

Sure. I just pushed out your 2nd patch set.

thanks,

	Dave