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

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

[Po Liu]

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 <claudiu.manoil@nxp.com>; Michael Walle <michael@walle.cc>; Po
> Liu <po.liu@nxp.com>; Vinicius Costa Gomes <vinicius.gomes@intel.com>;
> Vladimir Oltean <vladimir.oltean@nxp.com>
> 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:
> > On Wed,  2 Jun 2021 15:21:14 +0300 Vladimir Oltean wrote:
> > > From: Po Liu <Po.Liu@nxp.com>
> > >
> > > 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
"

> 
> > > 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