Re: [PATCH net-next 00/10] Wire up Ocelot tc-flower to Felix DSA

[Horatiu Vultur <horatiu.vultur@microchip.com>]

The 02/24/2020 15:08, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This series is a proposal on how to wire up the tc-flower callbacks into
> DSA. The example taken is the Microchip Felix switch, whose core
> implementation is actually located in drivers/net/ethernet/mscc/.
> 
> The proposal is largely a compromise solution. The DSA middle layer
> handles just enough to get to the interesting stuff (FLOW_CLS_REPLACE,
> FLOW_CLS_DESTROY, FLOW_CLS_STATS), but also thin enough to let drivers
> decide what filter keys and actions they support without worrying that
> the DSA middle layer will grow exponentially. I am far from being an
> expert, so I am asking reviewers to please voice your opinion if you
> think it can be done differently, with better results.
> 
> The bulk of the work was actually refactoring the ocelot driver enough
> to allow the VCAP (Versatile Content-Aware Processor) code for vsc7514
> and the vsc9959 switch cores to live together.
> 
> Flow block offloads have not been tested yet, only filters attached to a
> single port. It might be as simple as replacing ocelot_ace_rule_create
> with something smarter, it might be more complicated, I haven't tried
> yet.
> 
> I should point out that the tc-matchall filter offload is not
> implemented in the same manner in current mainline. Florian has already
> went all the way down into exposing actual per-action callbacks,
> starting with port mirroring. Because currently only mirred is supported
> by this DSA mid layer, everything else will return -EOPNOTSUPP. So even
> though ocelot supports matchall (aka port-based) policers, we don't have
> a call path to call into them.  Personally I think that this is not
> going to scale for tc-matchall (there may be policers, traps, drops,
> VLAN retagging, etc etc), and that we should consider replacing the port
> mirroring callbacks in DSA with simple accessors to
> TC_CLSMATCHALL_REPLACE and TC_CLSMATCHALL_DESTROY, just like for flower.
> That means that drivers which currently implement the port mirroring
> callbacks will need to have some extra "if" conditions now, in order for
> them to call their port mirroring implementations.
> 
> Vladimir Oltean (9):
>   net: mscc: ocelot: simplify tc-flower offload structures
>   net: mscc: ocelot: replace "rule" and "ocelot_rule" variable names
>     with "ace"
>   net: mscc: ocelot: return directly in
>     ocelot_cls_flower_{replace,destroy}
>   net: mscc: ocelot: don't rely on preprocessor for vcap key/action
>     packing
>   net: mscc: ocelot: remove port_pcs_init indirection for VSC7514
>   net: mscc: ocelot: parameterize the vcap_is2 properties
>   net: dsa: Refactor matchall mirred action to separate function
>   net: dsa: Add bypass operations for the flower classifier-action
>     filter
>   net: dsa: felix: Wire up the ocelot cls_flower methods
> 
> Yangbo Lu (1):
>   net: mscc: ocelot: make ocelot_ace_rule support multiple ports
> 
>  drivers/net/dsa/ocelot/felix.c            |  31 ++
>  drivers/net/dsa/ocelot/felix.h            |   3 +
>  drivers/net/dsa/ocelot/felix_vsc9959.c    | 126 ++++++
>  drivers/net/ethernet/mscc/ocelot.c        |  20 +-
>  drivers/net/ethernet/mscc/ocelot_ace.c    | 472 +++++++++++-----------
>  drivers/net/ethernet/mscc/ocelot_ace.h    |  26 +-
>  drivers/net/ethernet/mscc/ocelot_board.c  | 151 +++++--
>  drivers/net/ethernet/mscc/ocelot_flower.c | 256 ++++--------
>  drivers/net/ethernet/mscc/ocelot_tc.c     |  22 +-
>  drivers/net/ethernet/mscc/ocelot_vcap.h   | 403 ------------------
>  include/net/dsa.h                         |   6 +
>  include/soc/mscc/ocelot.h                 |  20 +-
>  include/soc/mscc/ocelot_vcap.h            | 205 ++++++++++
>  net/dsa/slave.c                           | 128 ++++--
>  14 files changed, 954 insertions(+), 915 deletions(-)
>  delete mode 100644 drivers/net/ethernet/mscc/ocelot_vcap.h
>  create mode 100644 include/soc/mscc/ocelot_vcap.h
> 
> --
> 2.17.1
> 

Hi Vladimir,

From my point, it looks OK the changes to Ocelot.
Also I managed to run some tests and they are passing.

-- 
/Horatiu