| Message ID | 20200911232853.1072362-1-kuba@kernel.org |
|---|---|
| Headers | show |
| Series | ethtool: add pause frame stats | expand |
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 > > Jakub Kicinski (8): > ethtool: add standard pause stats > docs: net: include the new ethtool pause stats in the stats doc > netdevsim: add pause frame stats > selftests: add a test for ethtool pause stats > bnxt: add pause frame stats > ixgbe: add pause frame stats > mlx5: add pause frame stats > mlx4: add pause frame stats > > Documentation/networking/ethtool-netlink.rst | 11 ++ > Documentation/networking/statistics.rst | 57 ++++++++- > .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 17 +++ > .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 11 ++ > .../net/ethernet/mellanox/mlx4/en_ethtool.c | 19 +++ > .../net/ethernet/mellanox/mlx4/mlx4_stats.h | 12 ++ > .../ethernet/mellanox/mlx5/core/en_ethtool.c | 9 ++ > .../net/ethernet/mellanox/mlx5/core/en_rep.c | 9 ++ > .../ethernet/mellanox/mlx5/core/en_stats.c | 29 +++++ > .../ethernet/mellanox/mlx5/core/en_stats.h | 3 + > drivers/net/netdevsim/Makefile | 2 +- > drivers/net/netdevsim/ethtool.c | 64 +++++++++++ > drivers/net/netdevsim/netdev.c | 1 + > drivers/net/netdevsim/netdevsim.h | 11 ++ > include/linux/ethtool.h | 26 +++++ > include/uapi/linux/ethtool_netlink.h | 18 ++- > net/ethtool/pause.c | 57 ++++++++- > .../drivers/net/netdevsim/ethtool-pause.sh | 108 ++++++++++++++++++ > 18 files changed, 456 insertions(+), 8 deletions(-) > create mode 100644 drivers/net/netdevsim/ethtool.c > create mode 100755 tools/testing/selftests/drivers/net/netdevsim/ethtool-pause.sh > > -- > 2.26.2 > 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? Thanks, -Vladimir
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.
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. 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.
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.
On 9/11/2020 5:42 PM, Jakub Kicinski wrote: > 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. It would be a lot easier to append the stats if there was not an additional ndo introduce to fetch the pause statistics because DSA overlay ndo on a function by function basis. Alternatively we should consider ethtool netlink operations over a devlink port at some point so we can get rid of the ugly ndo and ethtool_ops overlay that DSA does. Can we consider using get_ethtool_stats and ETH_SS_PAUSE_STATS as a stringset identifier? That way there is a single point within driver to fetch stats. -- Florian
On Fri, Sep 11, 2020 at 05:42:46PM -0700, Jakub Kicinski wrote: > > 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? No, it appears to me that you're trying to standardize some things out of ethtool -S. I just want to make sure that, while doing so, you are aware of some of the more subtle uses of that interface. > 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. If things work, yes. If they don't, no. > 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. Well, with a fixed-link, there's not a lot you can change with "ethtool --pause" (the link is managed with phylink). Up until now, I have mostly only heard of links between a DSA master and the CPU port that are not fixed-link. In theory (and in limited practice), phylink can even drive a PHY on the CPU port, so in that case, it may make sense for DSA to try to automatically apply the mirror pause frame configuration of the master. However, that wasn't supported before, either, and was not what I was talking about. > You can still append your custom CPU port stats to ethtool -S or > debugfs or whatnot, I don't understand, so you're saying that DSA can keep pause stats reporting in "ethtool -S", but the rest of devices should move to "ethtool -a"? You know that a typical switching chip will report the same statistics counters on all ports, including the ones that do have a net_device, right? So DSA gets a waiver from implementing .get_pause_stats()? > 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. A day-to-day user shouldn't need to look at ethtool -S or any other statistics for that matter, either. If they need to look at flow control on the CPU port they'd better get the full story rather than half of it. Sorry for the non-constructive answer. Like Florian said, it would be nice to have some built-in mechanism for this new ndo that DSA could use to keep annotating its own stats. Thanks, -Vladimir
> DSA used to override the "ethtool -S" callback of the host port, and > append its own CPU port counters to that. That was always a hack. It was bound to break sooner or later. Ido planned to add statistics to devlink. I hope we can make use of that to replace the CPU port statistics, and also add DSA port statistics, since these interfaces do exist in devlink. Andrew
On Fri, 11 Sep 2020 19:54:11 -0700 Florian Fainelli wrote: > > 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. > > It would be a lot easier to append the stats if there was not an > additional ndo introduce to fetch the pause statistics because DSA > overlay ndo on a function by function basis. Alternatively we should > consider ethtool netlink operations over a devlink port at some point so > we can get rid of the ugly ndo and ethtool_ops overlay that DSA does. I'm trying to target the 99.9% of users here, so in all honesty I'm concerned that if we try to cater to strange corner cases too much the entire interface will suffer. Hence I decided not to go with devlink, but extend the API people already know and use. It's the most logical and obvious place to me. > Can we consider using get_ethtool_stats and ETH_SS_PAUSE_STATS as a > stringset identifier? That way there is a single point within driver to > fetch stats. Can you say more? There are no strings reported in this patch set.
On Sat, 12 Sep 2020 10:16:12 +0300 Vladimir Oltean wrote: > > You can still append your custom CPU port stats to ethtool -S or > > debugfs or whatnot, > > I don't understand, so you're saying that DSA can keep pause stats > reporting in "ethtool -S", but the rest of devices should move to > "ethtool -a"? I'm saying report pause stats via the new interface on normal ports which have a netdev (external switch ports, CPU MAC). Deal with the weird CPU port in other, correspondingly weird, ways, like ethtool -S :) > You know that a typical switching chip will report the > same statistics counters on all ports, including the ones that do have a > net_device, right? I never used a DSA device. But I was under the impression they were supposed to be modeled like separate NICs.. > So DSA gets a waiver from implementing .get_pause_stats()? > > > 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. > > A day-to-day user shouldn't need to look at ethtool -S or any other > statistics for that matter, either. If they need to look at flow control > on the CPU port they'd better get the full story rather than half of it. Flow control stats are a really important piece of data on how the network operates. And they are part of normal operation of the network. Stats on the "CPU port" should be symmetrical with the CPU MAC. > Sorry for the non-constructive answer. Like Florian said, it would be > nice to have some built-in mechanism for this new ndo that DSA could use > to keep annotating its own stats. I do sympathize with the challenges of DSA. I never had any good ideas on how to help with it, TBH. I feel like this is a larger challenge, adding stats to the already existing (and problematic from DSA perspective) interface to configure pause frames is not changing the situation all that much.
On 9/14/2020 8:53 AM, Jakub Kicinski wrote: > On Fri, 11 Sep 2020 19:54:11 -0700 Florian Fainelli wrote: >>> 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. >> >> It would be a lot easier to append the stats if there was not an >> additional ndo introduce to fetch the pause statistics because DSA >> overlay ndo on a function by function basis. Alternatively we should >> consider ethtool netlink operations over a devlink port at some point so >> we can get rid of the ugly ndo and ethtool_ops overlay that DSA does. > > I'm trying to target the 99.9% of users here, so in all honesty I'm > concerned that if we try to cater to strange corner cases too much > the entire interface will suffer. Hence I decided not to go with > devlink, but extend the API people already know and use. It's the most > logical and obvious place to me. > >> Can we consider using get_ethtool_stats and ETH_SS_PAUSE_STATS as a >> stringset identifier? That way there is a single point within driver to >> fetch stats. > > Can you say more? There are no strings reported in this patch set. What I am suggesting is that we have a central and unique method for drivers to be called for all ethtool statisitcs to be obtained, and not create another ethtool operation specifically for pause stats. Today we have get_ethtool_stats and a stringset argument that tells you which type of statistic to return. I am not suggesting that we return strings or that it should be necessary for fetching pause stats.
On Mon, 14 Sep 2020 04:08:14 +0200 Andrew Lunn wrote: > > DSA used to override the "ethtool -S" callback of the host port, and > > append its own CPU port counters to that. > > That was always a hack. It was bound to break sooner or later. > > Ido planned to add statistics to devlink. I hope we can make use of > that to replace the CPU port statistics, and also add DSA port > statistics, since these interfaces do exist in devlink. I considered devlink but it really doesn't make much sense to me to configure something via ethtool and have its stats in devlink. If devlink was the way to go then the config interface should have been added there, too. And it wasn't (we just merged ethtool-nl for pause a couple of releases ago). Besides, doesn't it go against our "Linux is in control policy" to facilitate ports that don't have netdevs? Especially making a precedent like this for completely symmetrical pause frame config and stats does not seem like the right trade off to me.
On Mon, 14 Sep 2020 09:25:44 -0700 Florian Fainelli wrote: > >> Can we consider using get_ethtool_stats and ETH_SS_PAUSE_STATS as a > >> stringset identifier? That way there is a single point within driver to > >> fetch stats. > > > > Can you say more? There are no strings reported in this patch set. > > What I am suggesting is that we have a central and unique method for > drivers to be called for all ethtool statisitcs to be obtained, and not > create another ethtool operation specifically for pause stats. That won't work for statistics which correspond to a non-singleton object, like queue stats. > Today we have get_ethtool_stats and a stringset argument that tells you > which type of statistic to return. I am not suggesting that we return > strings or that it should be necessary for fetching pause stats. A multiplexer call or a call that dumps everything and then core picks out what it needs?
On Mon, Sep 14, 2020 at 09:15:18AM -0700, Jakub Kicinski wrote: > On Sat, 12 Sep 2020 10:16:12 +0300 Vladimir Oltean wrote: > I never used a DSA device. But I was under the impression they were > supposed to be modeled like separate NICs.. The front panel ports are. However there are other types of ports as well. You have at least one port of the switch connected to the SoC, so the SoC can send/receive frames. This is the so called CPU port of the switch. And Marvell switches support connecting switch ports together to form a cluster of switches. These are the so called DSA ports of the switch. Neither CPU nor DSA ports have a netdev, since they are internal plumbing. > Stats on the "CPU port" should be symmetrical with the CPU MAC. If things are working as expected. But pause is configurable per MAC. It could be one end has been configured to asym pause, and the other to pause. It could be one end is configured to asym pause, and the other end is failing to autoneg, etc. Just seeing that the stats are significantly different is a good clue something is up. Andrew
> > Can we consider using get_ethtool_stats and ETH_SS_PAUSE_STATS as a > > stringset identifier? That way there is a single point within driver to > > fetch stats. > > Can you say more? There are no strings reported in this patch set. Let me ask another question. Is pause stats the end of the story? Or do you plan to add more use case specific statistics? ethtool -T|--show-time-stamping could show statistics for PTP frames sent/received? ethtool --show-eee could show statistics for sleep/wake cycles? ethtool --show-rxfh-indir could show RSS statistics? Would you add a new ethtool op for each of these? Or maybe we should duplex them all through get_ethtool_stats()? Andrew
On Mon, 14 Sep 2020 19:36:50 +0200 Andrew Lunn wrote: > > > Can we consider using get_ethtool_stats and ETH_SS_PAUSE_STATS as a > > > stringset identifier? That way there is a single point within driver to > > > fetch stats. > > > > Can you say more? There are no strings reported in this patch set. > > Let me ask another question. Is pause stats the end of the story? Or > do you plan to add more use case specific statistics? > > ethtool -T|--show-time-stamping could show statistics for PTP frames > sent/received? > > ethtool --show-eee could show statistics for sleep/wake cycles? > > ethtool --show-rxfh-indir could show RSS statistics? I don't have a need for any of these. But they may make sense. I'll add FEC stats next: 30.5.1.1.17 aFECCorrectedBlocks 30.5.1.1.18 aFECUncorrectableBlocks I was tempted to add RMON stats, cause a lot of MACs expose those, but I don't actually have a use for them personally, so it's lower prio. > Would you add a new ethtool op for each of these? Or maybe we should > duplex them all through get_ethtool_stats()? I don't see a problem with an op for each of those. It makes for natural querying granularity. Works quite well for drivers converted here, IMHO.
On Mon, 14 Sep 2020 19:28:29 +0200 Andrew Lunn wrote: > On Mon, Sep 14, 2020 at 09:15:18AM -0700, Jakub Kicinski wrote: > > On Sat, 12 Sep 2020 10:16:12 +0300 Vladimir Oltean wrote: > > I never used a DSA device. But I was under the impression they were > > supposed to be modeled like separate NICs.. > > The front panel ports are. However there are other types of ports as > well. You have at least one port of the switch connected to the SoC, > so the SoC can send/receive frames. This is the so called CPU port of > the switch. And Marvell switches support connecting switch ports > together to form a cluster of switches. These are the so called DSA > ports of the switch. Neither CPU nor DSA ports have a netdev, since > they are internal plumbing. > > > Stats on the "CPU port" should be symmetrical with the CPU MAC. > > If things are working as expected. But pause is configurable per > MAC. It could be one end has been configured to asym pause, and the > other to pause. It could be one end is configured to asym pause, and > the other end is failing to autoneg, etc. Just seeing that the stats > are significantly different is a good clue something is up. Andrew, I appreciate DSA's complexities, but those are inherent to the lack of netdevs. Nobody raised an eyelid when pause config was converted to ethtool-nl, why are the statistics a problem? I'm adding an interface for monitoring daemons to use. sigar, zabbix, whatever. For those being able to query pause frames or FEC errors of real ports is important. Pauses on internal DSA ports are a different beast. If the monitoring dashboard starts listing internal DSA ports alongside real netdevs users will see them as ports, and wonder where the netdevs are.
On Fri, 2020-09-11 at 16:28 -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 > > LGTM, Reviewed-by: Saeed Mahameed <saeedm@nvidia.com> API for stats only reporting comment is not critical.