* [RFC iproute2 0/4] Revert tc batchsize feature
@ 2019-08-01 0:45 Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 1/4] Revert "tc: Remove pointless assignments in batch()" Stephen Hemminger
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Stephen Hemminger @ 2019-08-01 0:45 UTC (permalink / raw)
To: jiri, chrism; +Cc: netdev, Stephen Hemminger
The batchsize feature of tc might save a few cycles but it
is a maintaince nightmare, it has uninitialized variables and
poor error handling.
This patch set reverts back to the original state.
Please don't resubmit original code. Go back to the drawing
board and do something generic. For example, the routing
daemons have figured out that by using multiple threads and
turning off the netlink ACK they can update millions of routes
quickly.
Stephen Hemminger (4):
Revert "tc: Remove pointless assignments in batch()"
Revert "tc: flush after each command in batch mode"
Revert "tc: fix batch force option"
Revert "tc: Add batchsize feature for filter and actions"
tc/m_action.c | 65 ++++++----------
tc/tc.c | 201 ++++---------------------------------------------
tc/tc_common.h | 7 +-
tc/tc_filter.c | 129 ++++++++++++-------------------
4 files changed, 87 insertions(+), 315 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC iproute2 1/4] Revert "tc: Remove pointless assignments in batch()"
2019-08-01 0:45 [RFC iproute2 0/4] Revert tc batchsize feature Stephen Hemminger
@ 2019-08-01 0:45 ` Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 2/4] Revert "tc: flush after each command in batch mode" Stephen Hemminger
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2019-08-01 0:45 UTC (permalink / raw)
To: jiri, chrism; +Cc: netdev, Stephen Hemminger
This reverts commit 6358bbc381c6e38465838370bcbbdeb77ec3565a.
---
tc/tc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tc/tc.c b/tc/tc.c
index 64e342dd85bf..1f23971ae4b9 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -326,11 +326,11 @@ static int batch(const char *name)
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;
- bool bs_enabled_next;
int batchsize = 0;
size_t len = 0;
int ret = 0;
@@ -359,6 +359,7 @@ static int batch(const char *name)
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;
@@ -394,6 +395,7 @@ static int batch(const char *name)
len = 0;
bs_enabled_saved = bs_enabled;
bs_enabled = bs_enabled_next;
+ bs_enabled_next = false;
if (largc == 0) {
largc = largc_next;
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC iproute2 2/4] Revert "tc: flush after each command in batch mode"
2019-08-01 0:45 [RFC iproute2 0/4] Revert tc batchsize feature Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 1/4] Revert "tc: Remove pointless assignments in batch()" Stephen Hemminger
@ 2019-08-01 0:45 ` Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 3/4] Revert "tc: fix batch force option" Stephen Hemminger
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2019-08-01 0:45 UTC (permalink / raw)
To: jiri, chrism; +Cc: netdev, Stephen Hemminger
This reverts commit d66fdfda71e4a30c1ca0ddb7b1a048bef30fe79e.
---
tc/tc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tc/tc.c b/tc/tc.c
index 1f23971ae4b9..c115155b2234 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -405,7 +405,6 @@ static int batch(const char *name)
err = do_cmd(largc, largv, tail == NULL ? NULL : tail->buf,
tail == NULL ? 0 : sizeof(tail->buf));
- fflush(stdout);
if (err != 0) {
fprintf(stderr, "Command failed %s:%d\n", name,
cmdlineno - 1);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC iproute2 3/4] Revert "tc: fix batch force option"
2019-08-01 0:45 [RFC iproute2 0/4] Revert tc batchsize feature Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 1/4] Revert "tc: Remove pointless assignments in batch()" Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 2/4] Revert "tc: flush after each command in batch mode" Stephen Hemminger
@ 2019-08-01 0:45 ` Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 4/4] Revert "tc: Add batchsize feature for filter and actions" Stephen Hemminger
2019-08-01 1:11 ` [RFC iproute2 0/4] Revert tc batchsize feature Dave Taht
4 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2019-08-01 0:45 UTC (permalink / raw)
To: jiri, chrism; +Cc: netdev, Stephen Hemminger
This reverts commit b133392468d1f404077a8f3554d1f63d48bb45e8.
---
tc/tc.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/tc/tc.c b/tc/tc.c
index c115155b2234..b7b6bd288897 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -334,7 +334,6 @@ static int batch(const char *name)
int batchsize = 0;
size_t len = 0;
int ret = 0;
- int err;
bool send;
batch_mode = 1;
@@ -403,9 +402,9 @@ static int batch(const char *name)
continue; /* blank line */
}
- err = do_cmd(largc, largv, tail == NULL ? NULL : tail->buf,
+ ret = do_cmd(largc, largv, tail == NULL ? NULL : tail->buf,
tail == NULL ? 0 : sizeof(tail->buf));
- if (err != 0) {
+ if (ret != 0) {
fprintf(stderr, "Command failed %s:%d\n", name,
cmdlineno - 1);
ret = 1;
@@ -427,17 +426,15 @@ static int batch(const char *name)
iov->iov_len = n->nlmsg_len;
}
- err = rtnl_talk_iov(&rth, iovs, batchsize, NULL);
- put_batch_bufs(&buf_pool, &head, &tail);
- free(iovs);
- if (err < 0) {
+ ret = rtnl_talk_iov(&rth, iovs, batchsize, NULL);
+ if (ret < 0) {
fprintf(stderr, "Command failed %s:%d\n", name,
- cmdlineno - (batchsize + err) - 1);
- ret = 1;
- if (!force)
- break;
+ cmdlineno - (batchsize + ret) - 1);
+ return 2;
}
+ put_batch_bufs(&buf_pool, &head, &tail);
batchsize = 0;
+ free(iovs);
}
} while (!lastline);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC iproute2 4/4] Revert "tc: Add batchsize feature for filter and actions"
2019-08-01 0:45 [RFC iproute2 0/4] Revert tc batchsize feature Stephen Hemminger
` (2 preceding siblings ...)
2019-08-01 0:45 ` [RFC iproute2 3/4] Revert "tc: fix batch force option" Stephen Hemminger
@ 2019-08-01 0:45 ` Stephen Hemminger
2019-08-01 1:11 ` [RFC iproute2 0/4] Revert tc batchsize feature Dave Taht
4 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2019-08-01 0:45 UTC (permalink / raw)
To: jiri, chrism; +Cc: netdev, Stephen Hemminger
This reverts commit 485d0c6001c4aa134b99c86913d6a7089b7b2ab0.
---
tc/m_action.c | 65 ++++++----------
tc/tc.c | 199 ++++---------------------------------------------
tc/tc_common.h | 7 +-
tc/tc_filter.c | 129 ++++++++++++--------------------
4 files changed, 87 insertions(+), 313 deletions(-)
diff --git a/tc/m_action.c b/tc/m_action.c
index ab6bc0ad28ff..bdc62720879c 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -556,61 +556,40 @@ 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,
- void *buf, size_t buflen)
+ int *argc_p, char ***argv_p)
{
- struct tc_action_req *req, action_req;
- char **argv = *argv_p;
- struct rtattr *tail;
int argc = *argc_p;
- struct iovec iov;
+ char **argv = *argv_p;
int ret = 0;
-
- if (buf) {
- req = buf;
- if (buflen < sizeof (struct tc_action_req)) {
- fprintf(stderr, "buffer is too small: %zu\n", buflen);
- return -1;
- }
- } else {
- memset(&action_req, 0, sizeof (struct tc_action_req));
- 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);
+ 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);
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 (buf)
- return 0;
-
- iov.iov_base = &req->n;
- iov.iov_len = req->n.nlmsg_len;
- if (rtnl_talk_iov(&rth, &iov, 1, NULL) < 0) {
+ if (rtnl_talk(&rth, &req.n, NULL) < 0) {
fprintf(stderr, "We have an error talking to the kernel\n");
ret = -1;
}
+ *argc_p = argc;
+ *argv_p = argv;
+
return ret;
}
@@ -711,7 +690,7 @@ bad_val:
return ret;
}
-int do_action(int argc, char **argv, void *buf, size_t buflen)
+int do_action(int argc, char **argv)
{
int ret = 0;
@@ -721,12 +700,12 @@ int do_action(int argc, char **argv, void *buf, size_t buflen)
if (matches(*argv, "add") == 0) {
ret = tc_action_modify(RTM_NEWACTION,
NLM_F_EXCL | NLM_F_CREATE,
- &argc, &argv, buf, buflen);
+ &argc, &argv);
} else if (matches(*argv, "change") == 0 ||
matches(*argv, "replace") == 0) {
ret = tc_action_modify(RTM_NEWACTION,
NLM_F_CREATE | NLM_F_REPLACE,
- &argc, &argv, buf, buflen);
+ &argc, &argv);
} else if (matches(*argv, "delete") == 0) {
argc -= 1;
argv += 1;
diff --git a/tc/tc.c b/tc/tc.c
index b7b6bd288897..a0a18f380b46 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -205,18 +205,18 @@ static void usage(void)
" -nm | -nam[es] | { -cf | -conf } path }\n");
}
-static int do_cmd(int argc, char **argv, void *buf, size_t buflen)
+static int do_cmd(int argc, char **argv)
{
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, buf, buflen);
+ return do_filter(argc-1, argv+1);
if (matches(*argv, "chain") == 0)
- return do_chain(argc-1, argv+1, buf, buflen);
+ return do_chain(argc-1, argv+1);
if (matches(*argv, "actions") == 0)
- return do_action(argc-1, argv+1, buf, buflen);
+ return do_action(argc-1, argv+1);
if (matches(*argv, "monitor") == 0)
return do_tcmonitor(argc-1, argv+1);
if (matches(*argv, "exec") == 0)
@@ -231,110 +231,11 @@ static int do_cmd(int argc, char **argv, void *buf, size_t buflen)
return -1;
}
-#define TC_MAX_SUBC 10
-static bool batchsize_enabled(int argc, char *argv[])
-{
- struct {
- char *c;
- char *subc[TC_MAX_SUBC];
- } table[] = {
- { "filter", { "add", "delete", "change", "replace", NULL} },
- { "actions", { "add", "change", "replace", NULL} },
- { NULL },
- }, *iter;
- char *s;
- int i;
-
- if (argc < 2)
- return false;
-
- for (iter = table; iter->c; iter++) {
- if (matches(argv[0], iter->c))
- continue;
- for (i = 0; i < TC_MAX_SUBC; i++) {
- s = iter->subc[i];
- if (s && matches(argv[1], s) == 0)
- return true;
- }
- }
-
- return false;
-}
-
-struct batch_buf {
- struct batch_buf *next;
- char buf[16420]; /* sizeof (struct nlmsghdr) +
- max(sizeof (struct tcmsg) +
- sizeof (struct tcamsg)) +
- MAX_MSG */
-};
-
-static struct batch_buf *get_batch_buf(struct batch_buf **pool,
- struct batch_buf **head,
- struct batch_buf **tail)
-{
- 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));
- }
-
- if (*head == NULL)
- *head = *tail = buf;
- else {
- (*tail)->next = buf;
- (*tail) = 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)
{
- 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;
+ char *line = NULL;
size_t len = 0;
int ret = 0;
- bool send;
batch_mode = 1;
if (name && strcmp(name, "-") != 0) {
@@ -354,95 +255,25 @@ static int batch(const char *name)
}
cmdlineno = 0;
- 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, &head, &tail);
- if (!buf) {
- fprintf(stderr,
- "failed to allocate batch_buf\n");
- return -1;
- }
- ++batchsize;
- }
+ while (getcmdline(&line, &len, stdin) != -1) {
+ char *largv[100];
+ int largc;
- /*
- * In batch mode, if we haven't accumulated enough commands
- * and this is not the last command and this command & next
- * command both support the batchsize feature, don't send the
- * message immediately.
- */
- if (!lastline && bs_enabled && bs_enabled_next
- && batchsize != MSG_IOV_MAX)
- send = false;
- else
- send = true;
-
- line = line_next;
- line_next = NULL;
- len = 0;
- bs_enabled_saved = bs_enabled;
- bs_enabled = bs_enabled_next;
- bs_enabled_next = false;
-
- if (largc == 0) {
- largc = largc_next;
- memcpy(largv, largv_next, largc * sizeof(char *));
+ largc = makeargs(line, largv, 100);
+ if (largc == 0)
continue; /* blank line */
- }
- ret = do_cmd(largc, largv, tail == NULL ? NULL : tail->buf,
- tail == NULL ? 0 : sizeof(tail->buf));
- if (ret != 0) {
- fprintf(stderr, "Command failed %s:%d\n", name,
- cmdlineno - 1);
+ if (do_cmd(largc, largv)) {
+ fprintf(stderr, "Command failed %s:%d\n",
+ name, cmdlineno);
ret = 1;
if (!force)
break;
}
- largc = largc_next;
- memcpy(largv, largv_next, largc * sizeof(char *));
-
- if (send && bs_enabled_saved) {
- struct iovec *iov, *iovs;
- struct batch_buf *buf;
- struct nlmsghdr *n;
-
- iov = iovs = malloc(batchsize * sizeof(struct iovec));
- for (buf = head; buf != NULL; buf = buf->next, ++iov) {
- n = (struct nlmsghdr *)&buf->buf;
- iov->iov_base = n;
- iov->iov_len = n->nlmsg_len;
- }
-
- ret = rtnl_talk_iov(&rth, iovs, batchsize, NULL);
- if (ret < 0) {
- fprintf(stderr, "Command failed %s:%d\n", name,
- cmdlineno - (batchsize + ret) - 1);
- return 2;
- }
- put_batch_bufs(&buf_pool, &head, &tail);
- batchsize = 0;
- free(iovs);
- }
- } while (!lastline);
+ }
- free_batch_bufs(&buf_pool);
-Exit:
free(line);
rtnl_close(&rth);
-
return ret;
}
@@ -536,7 +367,7 @@ int main(int argc, char **argv)
goto Exit;
}
- ret = do_cmd(argc-1, argv+1, NULL, 0);
+ ret = do_cmd(argc-1, argv+1);
Exit:
rtnl_close(&rth);
diff --git a/tc/tc_common.h b/tc/tc_common.h
index d8a6dfdeabd4..802fb7f01fe4 100644
--- a/tc/tc_common.h
+++ b/tc/tc_common.h
@@ -1,15 +1,14 @@
/* SPDX-License-Identifier: GPL-2.0 */
#define TCA_BUF_MAX (64*1024)
-#define MSG_IOV_MAX 128
extern struct rtnl_handle rth;
int do_qdisc(int argc, char **argv);
int do_class(int argc, char **argv);
-int do_filter(int argc, char **argv, void *buf, size_t buflen);
-int do_chain(int argc, char **argv, void *buf, size_t buflen);
-int do_action(int argc, char **argv, void *buf, size_t buflen);
+int do_filter(int argc, char **argv);
+int do_chain(int argc, char **argv);
+int do_action(int argc, char **argv);
int do_tcmonitor(int argc, char **argv);
int do_exec(int argc, char **argv);
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index cd78c2441efa..53759a7a8876 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -58,42 +58,30 @@ struct tc_filter_req {
char buf[MAX_MSG];
};
-static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
- void *buf, size_t buflen)
+static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv)
{
- struct tc_filter_req *req, filter_req;
+ struct {
+ struct nlmsghdr n;
+ struct tcmsg t;
+ char buf[MAX_MSG];
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
+ .n.nlmsg_flags = NLM_F_REQUEST | flags,
+ .n.nlmsg_type = cmd,
+ .t.tcm_family = AF_UNSPEC,
+ };
struct filter_util *q = NULL;
- struct tc_estimator est = {};
- char k[FILTER_NAMESZ] = {};
- int chain_index_set = 0;
- char d[IFNAMSIZ] = {};
- int protocol_set = 0;
- __u32 block_index = 0;
- char *fhandle = NULL;
+ __u32 prio = 0;
__u32 protocol = 0;
+ int protocol_set = 0;
__u32 chain_index;
- struct iovec iov;
- __u32 prio = 0;
- int ret;
-
- if (buf) {
- req = buf;
- if (buflen < sizeof (struct tc_filter_req)) {
- fprintf(stderr, "buffer is too small: %zu\n", buflen);
- return -1;
- }
- } else {
- memset(&filter_req, 0, sizeof (struct tc_filter_req));
- req = &filter_req;
- }
-
- req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg));
- req->n.nlmsg_flags = NLM_F_REQUEST | flags;
- req->n.nlmsg_type = cmd;
- req->t.tcm_family = AF_UNSPEC;
+ int chain_index_set = 0;
+ char *fhandle = NULL;
+ char d[IFNAMSIZ] = {};
+ char k[FILTER_NAMESZ] = {};
+ struct tc_estimator est = {};
- if ((cmd == RTM_NEWTFILTER || cmd == RTM_NEWCHAIN) &&
- flags & NLM_F_CREATE)
+ if (cmd == RTM_NEWTFILTER && flags & NLM_F_CREATE)
protocol = htons(ETH_P_ALL);
while (argc > 0) {
@@ -101,53 +89,39 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
NEXT_ARG();
if (d[0])
duparg("dev", *argv);
- if (block_index) {
- fprintf(stderr, "Error: \"dev\" and \"block\" are mutually exlusive\n");
- return -1;
- }
strncpy(d, *argv, sizeof(d)-1);
- } else if (matches(*argv, "block") == 0) {
- NEXT_ARG();
- if (block_index)
- duparg("block", *argv);
- if (d[0]) {
- fprintf(stderr, "Error: \"dev\" and \"block\" are mutually exlusive\n");
- return -1;
- }
- if (get_u32(&block_index, *argv, 0) || !block_index)
- invarg("invalid block index value", *argv);
} else if (strcmp(*argv, "root") == 0) {
- if (req->t.tcm_parent) {
+ if (req.t.tcm_parent) {
fprintf(stderr,
"Error: \"root\" is duplicate parent ID\n");
return -1;
}
- req->t.tcm_parent = TC_H_ROOT;
+ req.t.tcm_parent = TC_H_ROOT;
} else if (strcmp(*argv, "ingress") == 0) {
- if (req->t.tcm_parent) {
+ if (req.t.tcm_parent) {
fprintf(stderr,
"Error: \"ingress\" is duplicate parent ID\n");
return -1;
}
- req->t.tcm_parent = TC_H_MAKE(TC_H_CLSACT,
+ req.t.tcm_parent = TC_H_MAKE(TC_H_CLSACT,
TC_H_MIN_INGRESS);
} else if (strcmp(*argv, "egress") == 0) {
- if (req->t.tcm_parent) {
+ if (req.t.tcm_parent) {
fprintf(stderr,
"Error: \"egress\" is duplicate parent ID\n");
return -1;
}
- req->t.tcm_parent = TC_H_MAKE(TC_H_CLSACT,
+ req.t.tcm_parent = TC_H_MAKE(TC_H_CLSACT,
TC_H_MIN_EGRESS);
} else if (strcmp(*argv, "parent") == 0) {
__u32 handle;
NEXT_ARG();
- if (req->t.tcm_parent)
+ if (req.t.tcm_parent)
duparg("parent", *argv);
if (get_tc_classid(&handle, *argv))
invarg("Invalid parent ID", *argv);
- req->t.tcm_parent = handle;
+ req.t.tcm_parent = handle;
} else if (strcmp(*argv, "handle") == 0) {
NEXT_ARG();
if (fhandle)
@@ -194,27 +168,26 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
argc--; argv++;
}
- req->t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+ req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
if (chain_index_set)
- addattr32(&req->n, sizeof(*req), TCA_CHAIN, chain_index);
+ addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
if (k[0])
- addattr_l(&req->n, sizeof(*req), TCA_KIND, k, strlen(k)+1);
+ addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
if (d[0]) {
ll_init_map(&rth);
- req->t.tcm_ifindex = ll_name_to_index(d);
- if (!req->t.tcm_ifindex)
- return -nodev(d);
- } else if (block_index) {
- req->t.tcm_ifindex = TCM_IFINDEX_MAGIC_BLOCK;
- req->t.tcm_block_index = block_index;
+ req.t.tcm_ifindex = ll_name_to_index(d);
+ if (req.t.tcm_ifindex == 0) {
+ fprintf(stderr, "Cannot find device \"%s\"\n", d);
+ return 1;
+ }
}
if (q) {
- if (q->parse_fopt(q, fhandle, argc, argv, &req->n))
+ if (q->parse_fopt(q, fhandle, argc, argv, &req.n))
return 1;
} else {
if (fhandle) {
@@ -233,16 +206,10 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
}
if (est.ewma_log)
- addattr_l(&req->n, sizeof(*req), TCA_RATE, &est, sizeof(est));
+ addattr_l(&req.n, sizeof(req), TCA_RATE, &est, sizeof(est));
- if (buf)
- return 0;
-
- iov.iov_base = &req->n;
- iov.iov_len = req->n.nlmsg_len;
- ret = rtnl_talk_iov(&rth, &iov, 1, NULL);
- if (ret < 0) {
- fprintf(stderr, "We have an error talking to the kernel, %d\n", ret);
+ if (rtnl_talk(&rth, &req.n, NULL) < 0) {
+ fprintf(stderr, "We have an error talking to the kernel\n");
return 2;
}
@@ -751,22 +718,20 @@ static int tc_filter_list(int cmd, int argc, char **argv)
return 0;
}
-int do_filter(int argc, char **argv, void *buf, size_t buflen)
+int do_filter(int argc, char **argv)
{
if (argc < 1)
return tc_filter_list(RTM_GETTFILTER, 0, NULL);
if (matches(*argv, "add") == 0)
return tc_filter_modify(RTM_NEWTFILTER, NLM_F_EXCL|NLM_F_CREATE,
- argc-1, argv+1, buf, buflen);
+ argc-1, argv+1);
if (matches(*argv, "change") == 0)
- return tc_filter_modify(RTM_NEWTFILTER, 0, argc-1, argv+1,
- buf, buflen);
+ return tc_filter_modify(RTM_NEWTFILTER, 0, argc-1, argv+1);
if (matches(*argv, "replace") == 0)
return tc_filter_modify(RTM_NEWTFILTER, NLM_F_CREATE, argc-1,
- argv+1, buf, buflen);
+ argv+1);
if (matches(*argv, "delete") == 0)
- return tc_filter_modify(RTM_DELTFILTER, 0, argc-1, argv+1,
- buf, buflen);
+ return tc_filter_modify(RTM_DELTFILTER, 0, argc-1, argv+1);
if (matches(*argv, "get") == 0)
return tc_filter_get(RTM_GETTFILTER, 0, argc-1, argv+1);
if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0
@@ -781,16 +746,16 @@ int do_filter(int argc, char **argv, void *buf, size_t buflen)
return -1;
}
-int do_chain(int argc, char **argv, void *buf, size_t buflen)
+int do_chain(int argc, char **argv)
{
if (argc < 1)
return tc_filter_list(RTM_GETCHAIN, 0, NULL);
if (matches(*argv, "add") == 0) {
return tc_filter_modify(RTM_NEWCHAIN, NLM_F_EXCL | NLM_F_CREATE,
- argc - 1, argv + 1, buf, buflen);
+ argc - 1, argv + 1);
} else if (matches(*argv, "delete") == 0) {
return tc_filter_modify(RTM_DELCHAIN, 0,
- argc - 1, argv + 1, buf, buflen);
+ argc - 1, argv + 1);
} else if (matches(*argv, "get") == 0) {
return tc_filter_get(RTM_GETCHAIN, 0,
argc - 1, argv + 1);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC iproute2 0/4] Revert tc batchsize feature
2019-08-01 0:45 [RFC iproute2 0/4] Revert tc batchsize feature Stephen Hemminger
` (3 preceding siblings ...)
2019-08-01 0:45 ` [RFC iproute2 4/4] Revert "tc: Add batchsize feature for filter and actions" Stephen Hemminger
@ 2019-08-01 1:11 ` Dave Taht
4 siblings, 0 replies; 6+ messages in thread
From: Dave Taht @ 2019-08-01 1:11 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jiří Pírko, chrism,
Linux Kernel Network Developers
Does this fix my longstanding issue with piping commands into it? :P
( https://github.com/tohojo/flent/issues/146 )
On Wed, Jul 31, 2019 at 5:46 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The batchsize feature of tc might save a few cycles but it
> is a maintaince nightmare, it has uninitialized variables and
> poor error handling.
>
> This patch set reverts back to the original state.
> Please don't resubmit original code. Go back to the drawing
> board and do something generic. For example, the routing
> daemons have figured out that by using multiple threads and
> turning off the netlink ACK they can update millions of routes
> quickly.
>
> Stephen Hemminger (4):
> Revert "tc: Remove pointless assignments in batch()"
> Revert "tc: flush after each command in batch mode"
> Revert "tc: fix batch force option"
> Revert "tc: Add batchsize feature for filter and actions"
>
> tc/m_action.c | 65 ++++++----------
> tc/tc.c | 201 ++++---------------------------------------------
> tc/tc_common.h | 7 +-
> tc/tc_filter.c | 129 ++++++++++++-------------------
> 4 files changed, 87 insertions(+), 315 deletions(-)
>
--
Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-01 1:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-01 0:45 [RFC iproute2 0/4] Revert tc batchsize feature Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 1/4] Revert "tc: Remove pointless assignments in batch()" Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 2/4] Revert "tc: flush after each command in batch mode" Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 3/4] Revert "tc: fix batch force option" Stephen Hemminger
2019-08-01 0:45 ` [RFC iproute2 4/4] Revert "tc: Add batchsize feature for filter and actions" Stephen Hemminger
2019-08-01 1:11 ` [RFC iproute2 0/4] Revert tc batchsize feature Dave Taht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).