RE: Re: [PATCH net-next 2/2] net: enetc: count the tc-taprio window drops

Hi Jakub, Vladimir,

-----Original Message-----
From: Vladimir Oltean <olteanv@gmail.com>
Sent: 2021年6月3日 2:00
To: Jakub Kicinski <kuba@kernel.org>
Cc: David S. Miller <davem@davemloft.net>; netdev@vger.kernel.org; Claudiu
Manoil [off-list ref]; Michael Walle [off-list ref]; Po
Liu [off-list ref]; Vinicius Costa Gomes [off-list ref];
Vladimir Oltean [off-list ref]
Subject: Re: [PATCH net-next 2/2] net: enetc: count the tc-taprio window
drops

Caution: EXT Email

On Wed, Jun 02, 2021 at 10:19:20AM -0700, Jakub Kicinski wrote:

quoted

On Wed,  2 Jun 2021 15:21:14 +0300 Vladimir Oltean wrote:

quoted

From: Po Liu <redacted>

The enetc scheduler for IEEE 802.1Qbv has 2 options (depending on
PTGCR[TG_DROP_DISABLE]) when we attempt to send an oversized packet
which will never fit in its allotted time slot for its traffic class:
either block the entire port due to head-of-line blocking, or drop
the

the entire port or the entire queue?

I don't remember, I need to re-test.

Although, this patch is focus on the drop counters for larger frames than any time slot for that TC. But for case in the blocking mode, the frame should only blocking the TC which setting all time slots shorter than the frame size IF the queue and TC are one to one paired.
I copy the TG_DROP_DISABLE bit description:
"Time gate drop disable
This field disables the TCs from dropping frames that are too large for any open window for
the current cycle.
0 Enabled
1 Disabled
"

quoted

quoted

packet and set a bit in the writeback format of the transmit buffer
descriptor, allowing other packets to be sent.

We obviously choose the second option in the driver, but we do not
detect the drop condition, so from the perspective of the network
stack, the packet is sent and no error counter is incremented.

This change checks the writeback of the TX BD when tc-taprio is
enabled, and increments a specific ethtool statistics counter and a
generic "tx_dropped" counter in ndo_get_stats64.

Any chance we should also report that back to the qdisc to have a
standard way of querying from user space? Qdisc offload supports stats
in general, it shouldn't be an issue, and the stat seems generic
enough, no?

You're thinking of something along the lines of tc_codel_xstats?
How do you propose I pass this on to the taprio qdisc? Just call a function in
enetc that is exported by net/sched/sch_taprio.c?
If the skb is bound to a socket, I'm thinking it might be more useful to report a
struct sock_extended_err similar to the SO_EE_TXTIME_MISSED stuff for tc-etf,
what do you think?

Yes, these counters lead out for each TC, ethtool could check each TC dropping counter due to the out of size issue. 
I remember Qdisc drop counters would include all the drop frames in one number and not explain any source.

Br,
Po Liu