Thread (12 messages) flat view 12 messages, 4 authors, 2018-01-12

Re: [patch iproute2 v8 2/2] tc: Add batchsize feature for filter and actions

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: 2018-01-10 11:42:08

On Wed, Jan 10, 2018 at 12:27:42PM +0900, Chris Mi wrote:
quoted hunk ↗ jump to hunk
Currently in tc batch mode, only one command is read from the batch
file and sent to kernel to process. With this support, at most 128
commands can be accumulated before sending to kernel.

Now it only works for the following successive commands:
filter and actions add/delete/change/replace.

Signed-off-by: Chris Mi <redacted>
---
 tc/m_action.c  |  60 +++++++++++++--------
 tc/tc.c        | 165 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 tc/tc_common.h |   5 +-
 tc/tc_filter.c |  97 +++++++++++++++++++--------------
 4 files changed, 249 insertions(+), 78 deletions(-)
diff --git a/tc/m_action.c b/tc/m_action.c
index fc422364..e5c53a80 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -546,40 +546,56 @@ bad_val:
 	return ret;
 }
 
+struct tc_action_req {
+	struct nlmsghdr		n;
+	struct tcamsg		t;
+	char			buf[MAX_MSG];
+};
+
 static int tc_action_modify(int cmd, unsigned int flags,
-			    int *argc_p, char ***argv_p)
+			    int *argc_p, char ***argv_p,
+			    void *buf)
 {
-	int argc = *argc_p;
+	struct tc_action_req *req, action_req;
 	char **argv = *argv_p;
+	struct rtattr *tail;
+	int argc = *argc_p;
+	struct iovec iov;
 	int ret = 0;
-	struct {
-		struct nlmsghdr         n;
-		struct tcamsg           t;
-		char                    buf[MAX_MSG];
-	} req = {
-		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg)),
-		.n.nlmsg_flags = NLM_F_REQUEST | flags,
-		.n.nlmsg_type = cmd,
-		.t.tca_family = AF_UNSPEC,
-	};
-	struct rtattr *tail = NLMSG_TAIL(&req.n);
+
+	if (buf)
+		req = buf;
+	else
+		req = &action_req;
+
+	req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg));
+	req->n.nlmsg_flags = NLM_F_REQUEST | flags;
+	req->n.nlmsg_type = cmd;
+	req->t.tca_family = AF_UNSPEC;
+	tail = NLMSG_TAIL(&req->n);
 
 	argc -= 1;
 	argv += 1;
-	if (parse_action(&argc, &argv, TCA_ACT_TAB, &req.n)) {
+	if (parse_action(&argc, &argv, TCA_ACT_TAB, &req->n)) {
 		fprintf(stderr, "Illegal \"action\"\n");
 		return -1;
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail;
+	tail->rta_len = (void *) NLMSG_TAIL(&req->n) - (void *) tail;
+
+	*argc_p = argc;
+	*argv_p = argv;
 
-	if (rtnl_talk(&rth, &req.n, NULL) < 0) {
+	iov.iov_base = &req->n;
+	iov.iov_len = req->n.nlmsg_len;
+
+	if (buf)
+		return 0;
+
+	if (rtnl_talk_iov(&rth, &iov, 1, NULL) < 0) {
 		fprintf(stderr, "We have an error talking to the kernel\n");
 		ret = -1;
 	}
 
-	*argc_p = argc;
-	*argv_p = argv;
-
 	return ret;
 }
 
@@ -679,7 +695,7 @@ bad_val:
 	return ret;
 }
 
-int do_action(int argc, char **argv)
+int do_action(int argc, char **argv, void *buf)
 {
 
 	int ret = 0;
@@ -689,12 +705,12 @@ int do_action(int argc, char **argv)
 		if (matches(*argv, "add") == 0) {
 			ret =  tc_action_modify(RTM_NEWACTION,
 						NLM_F_EXCL | NLM_F_CREATE,
-						&argc, &argv);
+						&argc, &argv, buf);
 		} else if (matches(*argv, "change") == 0 ||
 			  matches(*argv, "replace") == 0) {
 			ret = tc_action_modify(RTM_NEWACTION,
 					       NLM_F_CREATE | NLM_F_REPLACE,
-					       &argc, &argv);
+					       &argc, &argv, buf);
 		} else if (matches(*argv, "delete") == 0) {
 			argc -= 1;
 			argv += 1;
diff --git a/tc/tc.c b/tc/tc.c
index ad9f07e9..44277405 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -193,16 +193,16 @@ static void usage(void)
 			"                    -nm | -nam[es] | { -cf | -conf } path } | -j[son]\n");
 }
 
-static int do_cmd(int argc, char **argv)
+static int do_cmd(int argc, char **argv, void *buf)
 {
 	if (matches(*argv, "qdisc") == 0)
 		return do_qdisc(argc-1, argv+1);
 	if (matches(*argv, "class") == 0)
 		return do_class(argc-1, argv+1);
 	if (matches(*argv, "filter") == 0)
-		return do_filter(argc-1, argv+1);
+		return do_filter(argc-1, argv+1, buf);
 	if (matches(*argv, "actions") == 0)
-		return do_action(argc-1, argv+1);
+		return do_action(argc-1, argv+1, buf);
 	if (matches(*argv, "monitor") == 0)
 		return do_tcmonitor(argc-1, argv+1);
 	if (matches(*argv, "exec") == 0)
@@ -217,11 +217,79 @@ static int do_cmd(int argc, char **argv)
 	return -1;
 }
 
+static bool batchsize_enabled(int argc, char *argv[])
+{
+	if (argc < 2)
+		return false;
+	if ((matches(argv[0], "filter") && matches(argv[0], "actions"))
+	    || (matches(argv[1], "add") && matches(argv[1], "delete")
+	    && matches(argv[1], "change") && matches(argv[1], "replace")))
+		return false;
+	return true;
+}
+
+struct batch_buf {
+	struct batch_buf	*next;
+	char			buf[16420];	/* sizeof (struct nlmsghdr) +
+						   sizeof (struct tcmsg) +
+						   MAX_MSG */
+};
+
+static struct batch_buf *get_batch_buf(struct batch_buf **pool)
+{
+	struct batch_buf *buf;
+
+	if (*pool == NULL)
+		buf = calloc(1, sizeof (struct batch_buf));
+	else {
+		buf = *pool;
+		*pool = (*pool)->next;
+		memset(buf, 0, sizeof (struct batch_buf));
+	}
+	return buf;
+}
+
+static void put_batch_bufs(struct batch_buf **pool,
+			   struct batch_buf **head, struct batch_buf **tail)
+{
+	if (*head == NULL || *tail == NULL)
+		return;
+
+	if (*pool == NULL)
+		*pool = *head;
+	else {
+		(*tail)->next = *pool;
+		*pool = *head;
+	}
+	*head = NULL;
+	*tail = NULL;
+}
+
+static void free_batch_bufs(struct batch_buf **pool)
+{
+	struct batch_buf *buf;
+
+	for (buf = *pool; buf != NULL; buf = *pool) {
+		*pool = buf->next;
+		free(buf);
+	}
+	*pool = NULL;
+}
+
 static int batch(const char *name)
 {
-	char *line = NULL;
+	struct batch_buf *head = NULL, *tail = NULL, *buf_pool = NULL;
+	char *largv[100], *largv_next[100];
+	char *line, *line_next = NULL;
+	bool bs_enabled_next = false;
+	bool bs_enabled = false;
+	bool lastline = false;
+	int largc, largc_next;
+	bool bs_enabled_saved;
+	int batchsize = 0;
 	size_t len = 0;
 	int ret = 0;
+	bool send;
 
 	batch_mode = 1;
 	if (name && strcmp(name, "-") != 0) {
@@ -240,25 +308,94 @@ static int batch(const char *name)
 	}
 
 	cmdlineno = 0;
-	while (getcmdline(&line, &len, stdin) != -1) {
-		char *largv[100];
-		int largc;
+	if (getcmdline(&line, &len, stdin) == -1)
+		goto Exit;
+	largc = makeargs(line, largv, 100);
+	bs_enabled = batchsize_enabled(largc, largv);
+	bs_enabled_saved = bs_enabled;
+	do {
+		if (getcmdline(&line_next, &len, stdin) == -1)
+			lastline = true;
+
+		largc_next = makeargs(line_next, largv_next, 100);
+		bs_enabled_next = batchsize_enabled(largc_next, largv_next);
+		if (bs_enabled) {
+			struct batch_buf *buf;
+			buf = get_batch_buf(&buf_pool);
+			if (!buf) {
+				fprintf(stderr, "failed to allocate batch_buf\n");
+				return -1;
+			}
+			if (head == NULL)
+				head = tail = buf;
+			else {
+				tail->next = buf;
+				tail = buf;
+			}
What about moving this list handling to get_batch_buf(), like you did
for put_batch_buf() ? Just for the sake of consistency.

Otherwise LGTM!

  Marcelo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help