linux-kernel - RE: [PATCH net-next v20 10/13] rtase: Implement ethtool function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 18 Jun 2024 07:28:13 +0000
From: Justin Lai <justinlai0215@...ltek.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com"
	<edumazet@...gle.com>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "andrew@...n.ch"
	<andrew@...n.ch>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "horms@...nel.org"
	<horms@...nel.org>,
        "rkannoth@...vell.com" <rkannoth@...vell.com>,
        "Ping-Ke
 Shih" <pkshih@...ltek.com>,
        Larry Chiu <larry.chiu@...ltek.com>
Subject: RE: [PATCH net-next v20 10/13] rtase: Implement ethtool function

> On Mon, 17 Jun 2024 06:54:59 +0000 Justin Lai wrote:
> > > Are you sure this is the correct statistic to report as?
> > > What's the definition of rx_missed in the datasheet?
> >
> > What we refer to as rx miss is the packets that can't be received because
> > the fifo in the MAC is full. We consider this a type of MAC error, identical
> > to the definition of FramesLostDueToIntMACRcvError.
> 
> Is this a FIFO which silicon designers say can't overflow?
> Or it will overflow if the host is busy and doesn't pick up packets?
> If it's the latter I recommend using stats64::rx_missed_errors.
> That's the start we try to steer all NIC drivers towards for "host
> is too slow".

Our fifo falls under the second situation you mentioned, and we are
currently already using stats64::rx_missed_errors. Therefore, we will
remove the parts that use FramesLostDueToIntMACRcvError.

> 
> > > Also is 16 bits enough for a packet counter at 5Gbps?
> > > Don't you have to periodically accumulate this counter so that it doesn't
> wrap
> > > around?
> >
> > Indeed, this counter may wrap, but we don't need to accumulate it, because
> > an increase in the number of rx_miss largely indicates that the system
> > processing speed is not fast enough. Therefore, the size of this counter
> > doesn't need to be too large.
> 
> Are you basically saying that since its an error it only matters if
> its zero or not? It's not going to be a great experience for anyone
> trying to use this driver. You can read this counter periodically from
> a timer and accumulate a fuller value in the driver. There's even
> struct ethtool_coalesce::stats_block_coalesce_usecs if you want to let
> user configure the period.

As we've discussed, as long as this counter has a value, it can inform
the user that the host speed is too slow, and it will not affect other
transmission functions. Can we add this periodic reading function
after this patch version is merged?