Re: [PATCH net-next v2 2/3] net: dsa: add Arrow SpeedChips XRS700x driver

Replying to George's email 'cause I didn't get Vladimir's email from
the ML.

On Fri, 27 Nov 2020 14:58:29 -0600 George McCollister wrote:

quoted

100 Kbps = 12.5KB/s.
sja1105 has 93 64-bit counters, and during every counter refresh cycle I  

Are these 93 for one port? That sounds like a lot.. There're usually 
~10 stats (per port) that are relevant to the standard netdev stats.

Yeah, that's quite big. The xrs700x counters are only 16 bit. They
need to be polled on an interval anyway or they will roll.

Yup! That's pretty common.

quoted

would need to get some counters from the beginning of that range, some
from the middle and some from the end. With all the back-and-forth
between the sja1105 driver and the SPI controller driver, and the
protocol overhead associated with creating a "SPI read" message, it is
all in all more efficient to just issue a burst read operation for all
the counters, even ones that I'm not going to use. So let's go with
that, 93x8 bytes (and ignore protocol overhead) = 744 bytes of SPI I/O
per second. At a throughput of 12.5KB/s, that takes 59 ms to complete,
and that's just for the raw I/O, that thing which keeps the SPI mutex
locked. You know what else I could do during that time? Anything else!
Like for example perform PTP timestamp reconstruction, which has a hard
deadline at 135 ms after the packet was received, and would appreciate
if the SPI mutex was not locked for 59 ms every second.  

Indeed, however if you need to acquire this data at all it's going to
burden the system at that time so unless you're able to stretch out
the reads over a length of time whether or not you're polling every
second or once a day may not matter if you're never able to miss a
deadline.

Exactly, either way you gotta prepare for users polling those stats.
A design where stats are read synchronously and user (an unprivileged
user, BTW) has the ability to disturb the operation of the system
sounds really flaky.

quoted

And all of that, for what benefit? Honestly any periodic I/O over the
management interface is too much I/O, unless there is any strong reason
to have it.  

True enough.

quoted

Also, even the simple idea of providing out-of-date counters to user
space running in syscall context has me scratching my head. I can only
think of all the drivers in selftests that are checking statistics
counters before, then they send a packet, then they check the counters
after. What do those need to do, put a sleep to make sure the counters
were updated?  

Frankly life sounds simpler on the embedded networking planet than it is
on the one I'm living on ;) High speed systems are often eventually
consistent. Either because stats are gathered from HW periodically by
the FW, or RCU grace period has to expire, or workqueue has to run,
etc. etc. I know it's annoying for writing tests but it's manageable. 

If there is a better alternative I'm all ears but having /proc and
ifconfig return zeros for error counts while ip link doesn't will lead
to too much confusion IMO. While delayed update of stats is a fact of
life for _years_ now (hence it was backed into the ethtool -C API).