Re: [net-next 07/15] net/mlx5: Bridge, expose FDB state via debugfs

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 19 Jun 2023 11:37:30 +0300 Vlad Buslov wrote:
> On Sat 17 Jun 2023 at 00:48, Jakub Kicinski <kuba@kernel.org> wrote:
> > On Fri, 16 Jun 2023 13:11:05 -0700 Saeed Mahameed wrote:  
> >> $ cat mlx5/0000\:08\:00.0/esw/bridge/bridge1/fdb
> >> DEV              MAC               VLAN              PACKETS                BYTES              LASTUSE FLAGS
> >> enp8s0f0_1       e4:0a:05:08:00:06    2                    2                  204           4295567112   0x0
> >> enp8s0f0_0       e4:0a:05:08:00:03    2                    3                  278           4295567112   0x0  
> >
> > The flags here are the only thing that's mlx5 specific?  
> 
> Not exactly. This debugfs exposes the state of our bridge offload layer.
> For example, when VF representors from different eswitches are added to
> the same bridge every FDB entry on such bridge will have multiple
> underlying offloaded steering rules (one per eswitch instance connected
> to the bridge). User will observe the entries in all connected 'fdb'
> debugfs' (all except the 'main' entry will have flag
> MLX5_ESW_BRIDGE_FLAG_PEER set) and their corresponding counters will
> increment only on the eswitch instance that is actually processing the
> packets, which depends on the mode (when bonding device is added to the
> bridge in single FDB LAG mode all traffic appears on eswitch 0, without
> it the the traffic is on the eswitch of parent uplink of the VF). I
> understand that this is rather convoluted but this is exactly why we are
> going with debugfs.
> 
> > Why not add an API for dumping this kind of stats that other drivers
> > can reuse?  
> 
> As explained in previous paragraph we would like to expose internal mlx5
> bridge layer for debug purposes, not to design generic bridge FDB
> counter interface. Also, the debugging needs of our implementation may
> not correspond to other drivers because we don't have a 'hardware
> switch' on our NIC, so we do things like learning and ageing in
> software, and have to deal with multiple possible mode of operations
> (single FDB vs merged eswitch from previous example, etc.).

Looks like my pw-bot shenanigans backfired / crashed, patches didn't
get marked as Changes Requested and Dave applied the series :S

I understand the motivation but the information is easy enough to
understand to potentially tempt a user to start depending on it for
production needs. Then another vendor may get asked to implement
similar but not exactly the same set of stats etc. etc.

Do you have customer who will need this?

At the very least please follow up to make the files readable to only
root. Normal users should never look at debugfs IMO.