Messages in this thread | | | Date | Wed, 2 Nov 2022 06:49:15 +0100 | From | Felix Fietkau <> | Subject | Re: [PATCH v3] net: ethernet: mediatek: ppe: add support for flow accounting |
| |
On 02.11.22 02:26, Andrew Lunn wrote: > On Wed, Nov 02, 2022 at 01:03:08AM +0000, Daniel Golle wrote: >> Hi Andrew, >> >> On Wed, Nov 02, 2022 at 01:51:21AM +0100, Andrew Lunn wrote: >> > On Wed, Nov 02, 2022 at 12:42:40AM +0000, Daniel Golle wrote: >> > > The PPE units found in MT7622 and newer support packet and byte >> > > accounting of hw-offloaded flows. Add support for reading those >> > > counters as found in MediaTek's SDK[1]. >> > > >> > > [1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/bc6a6a375c800dc2b80e1a325a2c732d1737df92 >> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> >> > >> > Sorry, but NACK. >> > >> > You have not explained why doing this correctly via ethtool -S cannot >> > be done. debugfs is a vendor crap way of doing this. >> >> The debugfs interface is pre-existing and **in addition** to the >> standard Linux interfaces which are also provided. It is true that >> the debugfs interface in this case doesn't provide much additional >> value apart from having the counter listed next to the hardware- >> specific hashtable keys. As the debugfs interface for now aims to >> be as complete as possible, naturally there is some redundance of >> things which can also be accessed using other (standard) interfaces. > > debugfs is by definition unstable. It is not ABI. Anything using it is > expected to break in the near future when it changes its layout. It is > also totally option, you cannot expect it to be mounted. > > I hope you don't have any user space code using it. > > Maybe i should submit a patch which just for the fun of it rearranged > the order in debugfs and change the file name?
I believe that OpenWrt is still the main user of the PPE offloading code, since most vendors of devices with these SoC still use different out-of-tree implementations. OpenWrt does not ship or contain any user space code that relies on these debugfs files.
Whenever I'm debugging PPE related issues, I rely heavily on the debugfs API, so keeping it as complete as possible is important to me as well.
Aside from that, exposing per-flow statistics (which is what this patch does) via ethtool -S API makes no sense to me at all. I'm not aware of any other offload capable driver that does this.
- Felix
| |