| Message ID | 2cf5c0b12a53f37fee1f7af9ccc3761cbda6c030.1620297647.git.aclaudi@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next] tc: htb: improve burst error messages | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 5/6/21 4:42 AM, Andrea Claudi wrote: > When a wrong value is provided for "burst" or "cburst" parameters, the > resulting error message is unclear and can be misleading: > > $ tc class add dev dummy0 parent 1: classid 1:1 htb rate 100KBps burst errtrigger > Illegal "buffer" > > The message claims an illegal "buffer" is provided, but neither the > inline help nor the man page list "buffer" among the htb parameters, and > the only way to know that "burst", "maxburst" and "buffer" are synonyms > is to look into tc/q_htb.c. > > This commit tries to improve this simply changing the error string to > the parameter name provided in the user-given command, clearly pointing > out where the wrong value is. > > $ tc class add dev dummy0 parent 1: classid 1:1 htb rate 100KBps burst errtrigger > Illegal "burst" > > $ tc class add dev dummy0 parent 1: classid 1:1 htb rate 100Kbps maxburst errtrigger > Illegal "maxburst" > > Reported-by: Sebastian Mitterle <smitterl@redhat.com> > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> > --- > tc/q_htb.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > applied. thanks,
diff --git a/tc/q_htb.c b/tc/q_htb.c index 42566355..b5f95f67 100644 --- a/tc/q_htb.c +++ b/tc/q_htb.c @@ -125,6 +125,7 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str unsigned int linklayer = LINKLAYER_ETHERNET; /* Assume ethernet */ struct rtattr *tail; __u64 ceil64 = 0, rate64 = 0; + char *param; while (argc > 0) { if (matches(*argv, "prio") == 0) { @@ -160,17 +161,19 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str } else if (matches(*argv, "burst") == 0 || strcmp(*argv, "buffer") == 0 || strcmp(*argv, "maxburst") == 0) { + param = *argv; NEXT_ARG(); if (get_size_and_cell(&buffer, &cell_log, *argv) < 0) { - explain1("buffer"); + explain1(param); return -1; } } else if (matches(*argv, "cburst") == 0 || strcmp(*argv, "cbuffer") == 0 || strcmp(*argv, "cmaxburst") == 0) { + param = *argv; NEXT_ARG(); if (get_size_and_cell(&cbuffer, &ccell_log, *argv) < 0) { - explain1("cbuffer"); + explain1(param); return -1; } } else if (strcmp(*argv, "ceil") == 0) {
When a wrong value is provided for "burst" or "cburst" parameters, the resulting error message is unclear and can be misleading: $ tc class add dev dummy0 parent 1: classid 1:1 htb rate 100KBps burst errtrigger Illegal "buffer" The message claims an illegal "buffer" is provided, but neither the inline help nor the man page list "buffer" among the htb parameters, and the only way to know that "burst", "maxburst" and "buffer" are synonyms is to look into tc/q_htb.c. This commit tries to improve this simply changing the error string to the parameter name provided in the user-given command, clearly pointing out where the wrong value is. $ tc class add dev dummy0 parent 1: classid 1:1 htb rate 100KBps burst errtrigger Illegal "burst" $ tc class add dev dummy0 parent 1: classid 1:1 htb rate 100Kbps maxburst errtrigger Illegal "maxburst" Reported-by: Sebastian Mitterle <smitterl@redhat.com> Signed-off-by: Andrea Claudi <aclaudi@redhat.com> --- tc/q_htb.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)