| Message ID | 20190117211855.31030-1-xiyou.wangcong@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2] tc: add hit counter for matchall | expand |
On 1/17/19 2:18 PM, Cong Wang wrote: > Cc: Martin Olsson <martin.olsson+netdev@sentorsecurity.com> > Cc: Jamal Hadi Salim <jhs@mojatatu.com> > Cc: Jiri Pirko <jiri@resnulli.us> > Cc: David Ahern <dsahern@gmail.com> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> > --- > tc/f_matchall.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > applied to iproute2-next. In the future, add some kind of description to the log - preferably including example output from the command.
On Mon, Jan 21, 2019 at 8:35 AM David Ahern <dsahern@gmail.com> wrote: > > On 1/17/19 2:18 PM, Cong Wang wrote: > > Cc: Martin Olsson <martin.olsson+netdev@sentorsecurity.com> > > Cc: Jamal Hadi Salim <jhs@mojatatu.com> > > Cc: Jiri Pirko <jiri@resnulli.us> > > Cc: David Ahern <dsahern@gmail.com> > > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> > > --- > > tc/f_matchall.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > applied to iproute2-next. > > In the future, add some kind of description to the log - preferably > including example output from the command. Will do. I did in the kernel commit, but somehow forgot for iproute2. Thanks.
On Thu, 17 Jan 2019 13:18:55 -0800 Cong Wang <xiyou.wangcong@gmail.com> wrote: > > + if (tb[TCA_MATCHALL_PCNT]) { > + if (RTA_PAYLOAD(tb[TCA_MATCHALL_PCNT]) < sizeof(*pf)) { > + print_string(PRINT_FP, NULL, "Broken perf counters\n", NULL); This the wrong way to print an error message with iproute2 suite. The correct answer is simple. fprintf(stderr, "Broken perf counters\n"); and better yet give a more informative message that says what is incorrect.
On Wed, Jan 23, 2019 at 12:47 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Thu, 17 Jan 2019 13:18:55 -0800 > Cong Wang <xiyou.wangcong@gmail.com> wrote: > > > > > + if (tb[TCA_MATCHALL_PCNT]) { > > + if (RTA_PAYLOAD(tb[TCA_MATCHALL_PCNT]) < sizeof(*pf)) { > > + print_string(PRINT_FP, NULL, "Broken perf counters\n", NULL); > > This the wrong way to print an error message with iproute2 suite. > The correct answer is simple. > fprintf(stderr, "Broken perf counters\n"); > and better yet give a more informative message that says what is incorrect. What's your suggestion? In fact, I am not sure if we really have to check the size here, it should be safe to assume kernel will always dump a non-broken struct here, and this struct will never shrink.
diff --git a/tc/f_matchall.c b/tc/f_matchall.c index 5ebd0415..03dd51de 100644 --- a/tc/f_matchall.c +++ b/tc/f_matchall.c @@ -114,6 +114,7 @@ static int matchall_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, __u32 handle) { struct rtattr *tb[TCA_MATCHALL_MAX+1]; + struct tc_matchall_pcnt *pf = NULL; if (opt == NULL) return 0; @@ -143,6 +144,19 @@ static int matchall_print_opt(struct filter_util *qu, FILE *f, print_bool(PRINT_ANY, "not_in_hw", "\n not_in_hw", true); } + if (tb[TCA_MATCHALL_PCNT]) { + if (RTA_PAYLOAD(tb[TCA_MATCHALL_PCNT]) < sizeof(*pf)) { + print_string(PRINT_FP, NULL, "Broken perf counters\n", NULL); + return -1; + } + pf = RTA_DATA(tb[TCA_MATCHALL_PCNT]); + } + + if (show_stats && NULL != pf) + print_u64(PRINT_ANY, "rule_hit", " (rule hit %llu)", + (unsigned long long) pf->rhit); + + if (tb[TCA_MATCHALL_ACT]) tc_print_action(f, tb[TCA_MATCHALL_ACT], 0);
Cc: Martin Olsson <martin.olsson+netdev@sentorsecurity.com> Cc: Jamal Hadi Salim <jhs@mojatatu.com> Cc: Jiri Pirko <jiri@resnulli.us> Cc: David Ahern <dsahern@gmail.com> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> --- tc/f_matchall.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)