[iproute2/net-next,v2] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics

Message ID 20180810115846.15762.3693.stgit@wsfd-netdev20.ntdv.lab.eng.bos.redhat.com
State Awaiting Upstream
Delegated to: David Ahern
Headers show
Series
  • [iproute2/net-next,v2] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics
Related show

Commit Message

Eelco Chaudron Aug. 10, 2018, 11:59 a.m.
Add support for showing hardware specific counters to easy
troubleshooting 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>
---
v2:
 * Removed unnecessary initialization
 * Made not displaying of missing TCA_STATS_BASIC_HW more obvious
 * Use _SL_ macro for single line output

 include/uapi/linux/gen_stats.h |    1 +
 tc/tc_util.c                   |   41 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

Comments

Stephen Hemminger Aug. 10, 2018, 2:44 p.m. | #1
On Fri, 10 Aug 2018 07:59:30 -0400
Eelco Chaudron <echaudro@redhat.com> wrote:

> +		if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
> +			print_string(PRINT_FP, NULL, "%s", _SL_);
> +			print_string(PRINT_FP, NULL, "%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, "%s", _SL_);
> +	print_string(PRINT_FP, NULL, "%s", prefix);
> +	print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
> +		     bs_hw.bytes);

What does the output look like?
Eelco Chaudron Aug. 10, 2018, 2:48 p.m. | #2
On 10 Aug 2018, at 16:44, Stephen Hemminger wrote:

> On Fri, 10 Aug 2018 07:59:30 -0400
> Eelco Chaudron <echaudro@redhat.com> wrote:
>
>> +		if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
>> +			print_string(PRINT_FP, NULL, "%s", _SL_);
>> +			print_string(PRINT_FP, NULL, "%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, "%s", _SL_);
>> +	print_string(PRINT_FP, NULL, "%s", prefix);
>> +	print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
>> +		     bs_hw.bytes);
>
> What does the output look like?

See the two +’es below:

$ 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
Eelco Chaudron Oct. 1, 2018, 7:08 a.m. | #3
On 10 Aug 2018, at 16:48, Eelco Chaudron wrote:

> On 10 Aug 2018, at 16:44, Stephen Hemminger wrote:
>
>> On Fri, 10 Aug 2018 07:59:30 -0400
>> Eelco Chaudron <echaudro@redhat.com> wrote:
>>
>>> +		if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
>>> +			print_string(PRINT_FP, NULL, "%s", _SL_);
>>> +			print_string(PRINT_FP, NULL, "%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, "%s", _SL_);
>>> +	print_string(PRINT_FP, NULL, "%s", prefix);
>>> +	print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
>>> +		     bs_hw.bytes);
>>
>> What does the output look like?
>
> See the two +’es below:
>
> $ 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

Hi Stephen, anything else required for this patch to be accepted?

FYI the kernel side of this patch has been excepted on net-next.

Cheers,

Eelco
Stephen Hemminger Oct. 1, 2018, 9:10 a.m. | #4
On Mon, 01 Oct 2018 09:08:32 +0200
"Eelco Chaudron" <echaudro@redhat.com> wrote:

> On 10 Aug 2018, at 16:48, Eelco Chaudron wrote:
> 
> > On 10 Aug 2018, at 16:44, Stephen Hemminger wrote:
> >  
> >> On Fri, 10 Aug 2018 07:59:30 -0400
> >> Eelco Chaudron <echaudro@redhat.com> wrote:
> >>  
> >>> +		if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
> >>> +			print_string(PRINT_FP, NULL, "%s", _SL_);
> >>> +			print_string(PRINT_FP, NULL, "%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, "%s", _SL_);
> >>> +	print_string(PRINT_FP, NULL, "%s", prefix);
> >>> +	print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
> >>> +		     bs_hw.bytes);  
> >>
> >> What does the output look like?  
> >
> > See the two +’es below:
> >
> > $ 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  
> 
> Hi Stephen, anything else required for this patch to be accepted?
> 
> FYI the kernel side of this patch has been excepted on net-next.
> 
> Cheers,
> 
> Eelco

David Ahern handles net-next see patchwork
  https://patchwork.ozlabs.org/patch/956225/

I think he was just waiting for the kernel part to merge.
Eelco Chaudron Oct. 1, 2018, 10:29 a.m. | #5
On 1 Oct 2018, at 11:10, Stephen Hemminger wrote:

> On Mon, 01 Oct 2018 09:08:32 +0200
> "Eelco Chaudron" <echaudro@redhat.com> wrote:
>
>> On 10 Aug 2018, at 16:48, Eelco Chaudron wrote:
>>
>>> On 10 Aug 2018, at 16:44, Stephen Hemminger wrote:
>>>
>>>> On Fri, 10 Aug 2018 07:59:30 -0400
>>>> Eelco Chaudron <echaudro@redhat.com> wrote:
>>>>
>>>>> +		if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
>>>>> +			print_string(PRINT_FP, NULL, "%s", _SL_);
>>>>> +			print_string(PRINT_FP, NULL, "%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, "%s", _SL_);
>>>>> +	print_string(PRINT_FP, NULL, "%s", prefix);
>>>>> +	print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
>>>>> +		     bs_hw.bytes);
>>>>
>>>> What does the output look like?
>>>
>>> See the two +’es below:
>>>
>>> $ 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
>>
>> Hi Stephen, anything else required for this patch to be accepted?
>>
>> FYI the kernel side of this patch has been excepted on net-next.
>>
>> Cheers,
>>
>> Eelco
>
> David Ahern handles net-next see patchwork
>   https://patchwork.ozlabs.org/patch/956225/
>
> I think he was just waiting for the kernel part to merge.

Thanks for making me aware of the patchwork for iproute.
David Ahern Oct. 1, 2018, 3:12 p.m. | #6
On 10/1/18 4:29 AM, Eelco Chaudron wrote:
>>> Hi Stephen, anything else required for this patch to be accepted?
>>>
>>> FYI the kernel side of this patch has been excepted on net-next.
>>>
>>> Cheers,
>>>
>>> Eelco
>>
>> David Ahern handles net-next see patchwork
>>   https://patchwork.ozlabs.org/patch/956225/
>>
>> I think he was just waiting for the kernel part to merge.
> 
> Thanks for making me aware of the patchwork for iproute.
> 

Now that the kernel side is in, please re-send the iproute2 patches.
Eelco Chaudron Oct. 2, 2018, 7:28 a.m. | #7
On 1 Oct 2018, at 17:12, David Ahern wrote:

> On 10/1/18 4:29 AM, Eelco Chaudron wrote:
>>>> Hi Stephen, anything else required for this patch to be accepted?
>>>>
>>>> FYI the kernel side of this patch has been excepted on net-next.
>>>>
>>>> Cheers,
>>>>
>>>> Eelco
>>>
>>> David Ahern handles net-next see patchwork
>>>   https://patchwork.ozlabs.org/patch/956225/
>>>
>>> I think he was just waiting for the kernel part to merge.
>>
>> Thanks for making me aware of the patchwork for iproute.
>>
>
> Now that the kernel side is in, please re-send the iproute2 patches.

Rebased the patch on the latest iproute2-next and sent a v3.

//Eelco

Patch

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..5a1bbf2 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -800,6 +800,44 @@  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_hw;
+
+	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]) {
+		struct gnet_stats_basic bs;
+
+		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, "%s", _SL_);
+			print_string(PRINT_FP, NULL, "%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, "%s", _SL_);
+	print_string(PRINT_FP, NULL, "%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 +864,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};