[Intel-wired-lan] [PATCH net-next v1] igc: Add transmission overrun counter

[Zulkifli, Muhammad Husaini]

Hello,

> -----Original Message-----
> From: Brandeburg, Jesse <jesse.brandeburg at intel.com>
> Sent: Wednesday, 22 February, 2023 7:58 AM
> To: Jakub Kicinski <kuba at kernel.org>; Zulkifli, Muhammad Husaini
> <muhammad.husaini.zulkifli at intel.com>
> Cc: intel-wired-lan at osuosl.org; Nguyen, Anthony L
> <anthony.l.nguyen at intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH net-next v1] igc: Add transmission
> overrun counter
> 
> On 2/21/2023 3:11 PM, Jesse Brandeburg wrote:
> > On 2/20/2023 2:20 PM, Jakub Kicinski wrote:
> >> On Sat, 18 Feb 2023 06:19:31 +0000 Zulkifli, Muhammad Husaini wrote:
> >>> Hi Jakub and Vinicius,
> >>>
> >>> I would appreciate any early feedback or thoughts on this patch
> submission.
> >>>
> >>> History of the discussion:
> >>> https://lore.kernel.org/intel-wired-
> lan/SJ1PR11MB6180D0D59EB01AD1E8F
> >>>
> E4991B8A39 at SJ1PR11MB6180.namprd11.prod.outlook.com/T/#r8db595a7b
> 4006
> >>> 7d2315def91d41c9695454d6c9f
> >>
> >> Tony asks very good questions. Driver specific counter always
> >> reported as 0 would be odd for Linux. The counter is of no practical
> importance.
> >
> > Maybe too obvious a question, but it looks like your patch, Muhammad,
> > forgot to add the code that increments the counter?
> >
> > If you add that, then the patch makes sense.
> 
> Oops! it was pointed out to me that your patch had some comment in the
> middle of the code saying that you'll never increment this value.
> 
> But I still don't get it, since your commit message said:
> 
> > Add transmission overrun counter as per defined by IEEE 802.1Q Bridges.
> > The Intel Discrete I225/6 does not have HW counters that can determine
> > whether a packet is still being transmitted after the gate has been closed.
> > But I225/I226 have a mechanism to not transmit a packets if the gate
> > open time is insufficient for the packet transmission by setting the
> > Strict_End bit. Software counters have been created for each queues in
> > response to the IEEE specification. Thus, we can assume that overrun
> > counter is always "0" when setting this bit.
> 
> Generally if a counter isn't reported it's presumed to be zero.
> 
> I think you need to do a better job explaining why userspace needs an "always
> zero" counter, and can't just be told that the counter is not there (so is
> obviously zero)

AVNu TSN Test certification conformance test required this counter to indicate in the
Userapp that whether there is a packets being transmitted on specific queue when the
qbv window size is not sufficient. Then for i226 case, our counter will always be "zero" 
due to the HW mechanism to not transmit the packet if the gate time is not sufficient.

> 
> and if this app requires that every vendor implement a per-queue stat to track
> this item, the code to track the stat may as well be added to core kernel
> instead of for each driver independently.
> 

We are thinking to have a per-queue stat since it will use by other vendor as well
For the certification. I will refactor the code and re submit again for the review.