Message-ID: <cover.1704449760.git.ante.knezic@helmholz.de>
Date: Fri, 5 Jan 2024 11:46:13 +0100
From: Ante Knezic <ante.knezic@...mholz.de>
To: <netdev@...r.kernel.org>
CC: <andrew@...n.ch>, <f.fainelli@...il.com>, <olteanv@...il.com>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <ante.knezic@...mholz.de>
Subject: [RFC PATCH net-next 0/6] add dsa cross-chip port mirroring

This patch is an attempt to add cross-chip port mirroring to dsa core and
mv88e6xxx switches but should apply similarly to other devices as well.
It is a work in progress so I am posting as RFC hopefully to get some
feedback on the general idea behind he problem rather than on the quality
of the code itself, though provided you have the time I would appreciate
any suggestions to code as well.

Cross-chip mirroring requires creating mirroring segments on each switch
that is acting as a part of the mirroring route, from source to destination
mirroring port. For example, configuration:

  SW0            SW1           SW2           SW3
+-------+     +-------+     +-------+     +--------+
|     P9|<--->|P10  P9|<--->|P10    |<--->|P10     |
|       |     |       |     |       |     |        |
+-------+     +-------+     +----+--+     +--------+
    ^                            |
    |                            v
   P2                           P8

for which dsa tree devices need to be configured as follows:
  ----------------------------------------
  |    |     source         destination  |
  | SW |  mirror port       mirror port  |
  ----------------------------------------
  |  0 |   P2          ->      P9        |
  |  1 |   P10         ->      P9        |
  |  2 |   P10         ->      P8        |
  |  3 |               ->                |
  ----------------------------------------

This basically means that request for adding port mirroring needs to be
propagated to the entire dsa switch tree as we can have a mirroring path
in which all switches need to be setup. This can be achieved through use
of switch event notifiers. First step is to create a mirroring route
which will contain source and destination ports for each switch in the
route. Then, this route is passed on to each switch where it is evaluated
for mirror settings for this particular switch. If a switch is contained
inside the route, its source and destination ports are passed on to
ds->ops->port_mirror_add().
Things get a bit more complicated when we need to remove port mirroring.
Again, we make use of the switch event notifiers to be able to inform
all switches that make a part of the route. Additionally, we must also
check if the source/destination mirror ports are being used by another
route and if so they should be kept active as we would interfere with
operation of another mirror action.

The critical part of the patch is getting the actual routing figured out,
which is done inside the dsa_route_create routine. It is a recursive
function that makes use of dsa_link and dst->rtable which is apparently
planned to be removed at some point (indicated by the TODO for dsa_link
struct in include/net/dsa.h). I'm not sure if this is a good way of getting
the routing done and what is the general opinion on using recursive functions
for this kind of work? In lack of real hardware, I isolated and tested
dsa_route_create routine on dummy switch topologies with up to three dsa
links. Even a sort of circular topology seems to work provided that the
requested to/from ports can actually be found in the dsa tree. Are there
any other variants that should be considered?

I decided to drop passing struct dsa_mall_mirror_tc_entry *mirror to
dsa drivers in favor of simpler from/to port and constraint the
dsa_mall_mirror_tc_entry to dsa core only. Member u8 to_local_port is
replaced with struct dsa_port *to_port as we are may no longer be addressing
the same switch for source/dest mirror. In this sense, passing the struct
dsa_port *to_port as a member of the dsa_mall_mirror_tc_entry to individual
switch devices seemed a bit off. This is addressed in patch no. 1

Patch no.5 sets destination mirroring ports to 0x1f when trying to disable
destination mirroring as suggested by the Switch Functional Specification
for 88E6390X/88E6390/88E6290/88E6190X/88E6190 and Switch Functional
Functional Specification for 88E6393X/88E6193X/88E6191X. If I see correctly
we have 3 variants of mv88e6xxx_ops->set_egress_port and they are:
- mv88e6390_g1_set_egress_port
- mv88e6393x_set_egress_port
- mv88e6095_g1_set_egress_port
I guess that we can assume that above mentioned documents apply to the first
two variants, and if someone can confirm the same for mv88e6095 than
MV88E6XXX_EGRESS_DEST_DISABLE should apply for the complete mv88e6xxx family.

Finally, patch is tested on a board consisting of two cascaded mv88e6390x
switches and seems to work fine, but this obviously is not sufficient and
would be great if someone can do a bit more testing on proper hardware
assuming there are no objections to the patch itself.

Any feedback is appreciated.
Thanks,
	Ante

Ante Knezic (6):
  net:dsa: drop mirror struct from port mirror functions
  net: dsa: add groundwork for cross chip mirroring
  net: dsa: implement cross chip port mirroring
  net: dsa: check for busy mirror ports when removing mirroring action
  net: dsa: mv88e6xxx: properly disable mirror destination
  net: dsa: mv88e6xxx add cross-chip mirroring

 drivers/net/dsa/b53/b53_common.c       |  21 +++--
 drivers/net/dsa/b53/b53_priv.h         |   8 +-
 drivers/net/dsa/microchip/ksz8.h       |  10 +-
 drivers/net/dsa/microchip/ksz8795.c    |  36 +++----
 drivers/net/dsa/microchip/ksz9477.c    |  26 ++---
 drivers/net/dsa/microchip/ksz9477.h    |  10 +-
 drivers/net/dsa/microchip/ksz_common.c |  15 +--
 drivers/net/dsa/microchip/ksz_common.h |   7 +-
 drivers/net/dsa/mt7530.c               |  35 +++----
 drivers/net/dsa/mv88e6xxx/chip.c       |  91 +++++++++++-------
 drivers/net/dsa/mv88e6xxx/chip.h       |   5 +-
 drivers/net/dsa/mv88e6xxx/port.c       |   5 -
 drivers/net/dsa/ocelot/felix.c         |  15 +--
 drivers/net/dsa/qca/qca8k-common.c     |  39 ++++----
 drivers/net/dsa/qca/qca8k.h            |  11 ++-
 drivers/net/dsa/sja1105/sja1105_main.c |  14 +--
 include/net/dsa.h                      |  38 ++++++--
 net/dsa/dsa.c                          |  14 +++
 net/dsa/switch.c                       |  84 +++++++++++++++++
 net/dsa/switch.h                       |   8 ++
 net/dsa/user.c                         | 167 +++++++++++++++++++++++++++++++--
 21 files changed, 480 insertions(+), 179 deletions(-)

-- 
2.11.0

Powered by blists - more mailing lists