Re: [PATCH iproute2/net-next] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics
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 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);
+}
Re: [PATCH iproute2/net-next] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics
On Thu, 9 Aug 2018 11:16:02 -0400
Eelco Chaudron 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);
> +}
[PATCH iproute2/net-next] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics
Add support for showing hardware specific counters to easily
troubleshoot hardware offload.
$ tc -s filter show dev enp3s0np0 parent :
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 89173e6a7001becfd486bda17e29
Signed-off-by: Eelco Chaudron
---
include/uapi/linux/gen_stats.h |1 +
tc/tc_util.c | 38 ++
2 files changed, 39 insertions(+)
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};
