netdev - Re: [PATCH net-next v2 0/8] ethtool: add pause frame stats

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]

Message-ID: <20200911174246.76466eec@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 11 Sep 2020 17:42:46 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, mkubecek@...e.cz,
        michael.chan@...adcom.com, tariqt@...dia.com, saeedm@...dia.com,
        alexander.duyck@...il.com, andrew@...n.ch
Subject: Re: [PATCH net-next v2 0/8] ethtool: add pause frame stats

On Sat, 12 Sep 2020 03:15:42 +0300 Vladimir Oltean wrote:
> On Fri, Sep 11, 2020 at 05:07:24PM -0700, Jakub Kicinski wrote:
> > On Sat, 12 Sep 2020 02:49:32 +0300 Vladimir Oltean wrote:  
> > > On Fri, Sep 11, 2020 at 04:28:45PM -0700, Jakub Kicinski wrote:  
> > > > Hi!
> > > >
> > > > This is the first (small) series which exposes some stats via
> > > > the corresponding ethtool interface. Here (thanks to the
> > > > excitability of netlink) we expose pause frame stats via
> > > > the same interfaces as ethtool -a / -A.
> > > >
> > > > In particular the following stats from the standard:
> > > >  - 30.3.4.2 aPAUSEMACCtrlFramesTransmitted
> > > >  - 30.3.4.3 aPAUSEMACCtrlFramesReceived
> > > >
> > > > 4 real drivers are converted, hopefully the semantics match
> > > > the standard.
> > > >
> > > > v2:
> > > >  - netdevsim: add missing static
> > > >  - bnxt: fix sparse warning
> > > >  - mlx5: address Saeed's comments  
> > >
> > > DSA used to override the "ethtool -S" callback of the host port, and
> > > append its own CPU port counters to that.
> > >
> > > So you could actually see pause frames transmitted by the host port and
> > > received by the switch's CPU port:
> > >
> > > # ethtool -S eno2 | grep pause
> > > MAC rx valid pause frames: 1339603152
> > > MAC tx valid pause frames: 0
> > > p04_rx_pause: 0
> > > p04_tx_pause: 1339603152
> > >
> > > With this new command what's the plan?  
> >
> > Sounds like something for DSA folks to decide :)
> >
> > What does ethtool -A $cpu_port control?
> > The stats should match what the interface controls.  
> 
> Error: $cpu_port: undefined variable.
> With DSA switches, the CPU port is a physical Ethernet port mostly like
> any other, except that its orientation is inwards towards the system
> rather than outwards. So there is no network interface registered for
> it, since I/O from the network stack would have to literally loop back
> into the system to fulfill the request of sending a packet to that
> interface.

Sorry, perhaps I should have said $MAC, but that kind of in itself
answers the question.

> The ethtool -S framework was nice because you could append to the
> counters of the master interface while not losing them.
> As for "ethtool -A", those parameters are fixed as part of the
> fixed-link device tree node corresponding to the CPU port.

I think I'm missing the problem you're trying to describe.
Are you making a general comment / argument on ethtool stats?

Pause stats are symmetrical - as can be seen in your quote
what's RX for the CPU is TX for the switch, and vice versa.

Since ethtool -A $cpu_mac controls whether CPU netdev generates
and accepts pause frames, correspondingly the direction and meaning
of pause statistics on that interface is well defined.

You can still append your custom CPU port stats to ethtool -S or
debugfs or whatnot, but those are only useful for validating that 
the configuration of the CPU port is not completely broken. Otherwise
the counters are symmetrical. A day-to-day user of the device doesn't
need to see both of them.