| Message ID | 20180809151602.5951.21421.stgit@wsfd-netdev20.ntdv.lab.eng.bos.redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2/net-next] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics | expand |
On Thu, 9 Aug 2018 11:16:02 -0400 Eelco Chaudron <echaudro@redhat.com> wrote: > > +static void print_tcstats_basic_hw(struct rtattr **tbs, char *prefix) > +{ > + struct gnet_stats_basic bs = {0}; If not present don't print it rather than printing zero. > + struct gnet_stats_basic bs_hw = {0}; This initialization is unnecessary since you always overwrite it. > + > + if (!tbs[TCA_STATS_BASIC_HW]) > + return; > + > + memcpy(&bs_hw, RTA_DATA(tbs[TCA_STATS_BASIC_HW]), > + MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC_HW]), sizeof(bs_hw))); > + > + if (bs_hw.bytes == 0 && bs_hw.packets == 0) > + return; > + > + if (tbs[TCA_STATS_BASIC]) { > + memcpy(&bs, RTA_DATA(tbs[TCA_STATS_BASIC]), > + MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC]), > + sizeof(bs))); > + } > + > + if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) { > + print_string(PRINT_FP, NULL, "\n%s", prefix); Please use the magic string _SL_ to allow supporting single line output mode. > + print_lluint(PRINT_ANY, "sw_bytes", > + "Sent software %llu bytes", > + bs.bytes - bs_hw.bytes); > + print_uint(PRINT_ANY, "sw_packets", " %u pkt", > + bs.packets - bs_hw.packets); > + } > + > + print_string(PRINT_FP, NULL, "\n%s", prefix); > + print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes", > + bs_hw.bytes); > + print_uint(PRINT_ANY, "hw_packets", " %u pkt", bs_hw.packets); > +}
Thanks for the quick reply, see inline responses. On 9 Aug 2018, at 18:07, Stephen Hemminger wrote: > On Thu, 9 Aug 2018 11:16:02 -0400 > Eelco Chaudron <echaudro@redhat.com> wrote: > >> >> +static void print_tcstats_basic_hw(struct rtattr **tbs, char >> *prefix) >> +{ >> + struct gnet_stats_basic bs = {0}; > > If not present don't print it rather than printing zero. > This is used to print separate SW counters below, which is not displayed if 0, i.e. not present. However I will move it under the “if (tbs[TCA_STATS_BASIC])” statement, so it’s more explicit. >> + struct gnet_stats_basic bs_hw = {0}; > > This initialization is unnecessary since you always overwrite it. > Thanks will remove it in the v2 >> + >> + if (!tbs[TCA_STATS_BASIC_HW]) >> + return; >> + >> + memcpy(&bs_hw, RTA_DATA(tbs[TCA_STATS_BASIC_HW]), >> + MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC_HW]), sizeof(bs_hw))); >> + >> + if (bs_hw.bytes == 0 && bs_hw.packets == 0) >> + return; >> + >> + if (tbs[TCA_STATS_BASIC]) { >> + memcpy(&bs, RTA_DATA(tbs[TCA_STATS_BASIC]), >> + MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC]), >> + sizeof(bs))); >> + } >> + >> + if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) { >> + print_string(PRINT_FP, NULL, "\n%s", prefix); > > Please use the magic string _SL_ to allow supporting single line > output mode. > Will do in the V2. >> + print_lluint(PRINT_ANY, "sw_bytes", >> + "Sent software %llu bytes", >> + bs.bytes - bs_hw.bytes); >> + print_uint(PRINT_ANY, "sw_packets", " %u pkt", >> + bs.packets - bs_hw.packets); >> + } >> + >> + print_string(PRINT_FP, NULL, "\n%s", prefix); >> + print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes", >> + bs_hw.bytes); >> + print_uint(PRINT_ANY, "hw_packets", " %u pkt", bs_hw.packets); >> +}
diff --git a/include/uapi/linux/gen_stats.h b/include/uapi/linux/gen_stats.h index 24a861c..065408e 100644 --- a/include/uapi/linux/gen_stats.h +++ b/include/uapi/linux/gen_stats.h @@ -12,6 +12,7 @@ enum { TCA_STATS_APP, TCA_STATS_RATE_EST64, TCA_STATS_PAD, + TCA_STATS_BASIC_HW, __TCA_STATS_MAX, }; #define TCA_STATS_MAX (__TCA_STATS_MAX - 1) diff --git a/tc/tc_util.c b/tc/tc_util.c index d757852..43a2013 100644 --- a/tc/tc_util.c +++ b/tc/tc_util.c @@ -800,6 +800,41 @@ void print_tm(FILE *f, const struct tcf_t *tm) } } +static void print_tcstats_basic_hw(struct rtattr **tbs, char *prefix) +{ + struct gnet_stats_basic bs = {0}; + struct gnet_stats_basic bs_hw = {0}; + + if (!tbs[TCA_STATS_BASIC_HW]) + return; + + memcpy(&bs_hw, RTA_DATA(tbs[TCA_STATS_BASIC_HW]), + MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC_HW]), sizeof(bs_hw))); + + if (bs_hw.bytes == 0 && bs_hw.packets == 0) + return; + + if (tbs[TCA_STATS_BASIC]) { + memcpy(&bs, RTA_DATA(tbs[TCA_STATS_BASIC]), + MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC]), + sizeof(bs))); + } + + if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) { + print_string(PRINT_FP, NULL, "\n%s", prefix); + print_lluint(PRINT_ANY, "sw_bytes", + "Sent software %llu bytes", + bs.bytes - bs_hw.bytes); + print_uint(PRINT_ANY, "sw_packets", " %u pkt", + bs.packets - bs_hw.packets); + } + + print_string(PRINT_FP, NULL, "\n%s", prefix); + print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes", + bs_hw.bytes); + print_uint(PRINT_ANY, "hw_packets", " %u pkt", bs_hw.packets); +} + void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct rtattr **xstats) { SPRINT_BUF(b1); @@ -826,6 +861,9 @@ void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct rtat print_uint(PRINT_ANY, "requeues", " requeues %u) ", q.requeues); } + if (tbs[TCA_STATS_BASIC_HW]) + print_tcstats_basic_hw(tbs, prefix); + if (tbs[TCA_STATS_RATE_EST64]) { struct gnet_stats_rate_est64 re = {0};
Add support for showing hardware specific counters to easily troubleshoot hardware offload. $ tc -s filter show dev enp3s0np0 parent ffff: filter protocol ip pref 1 flower chain 0 filter protocol ip pref 1 flower chain 0 handle 0x1 eth_type ipv4 dst_ip 2.0.0.0 src_ip 1.0.0.0 ip_flags nofrag in_hw action order 1: mirred (Egress Redirect to device eth1) stolen index 1 ref 1 bind 1 installed 0 sec used 0 sec Action statistics: Sent 534884742 bytes 8915697 pkt (dropped 0, overlimits 0 requeues 0) Sent software 187542 bytes 4077 pkt Sent hardware 534697200 bytes 8911620 pkt backlog 0b 0p requeues 0 cookie 89173e6a44447001becfd486bda17e29 Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- include/uapi/linux/gen_stats.h | 1 + tc/tc_util.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)