[net-next,00/15] Accomodate DSA front-end into Ocelot
mbox series

Message ID 20191109130301.13716-1-olteanv@gmail.com
Headers show
Series
  • Accomodate DSA front-end into Ocelot
Related show

Message

Vladimir Oltean Nov. 9, 2019, 1:02 p.m. UTC
After the nice "change-my-mind" discussion about Ocelot, Felix and
LS1028A (which can be read here: https://lkml.org/lkml/2019/6/21/630),
we have decided to take the route of reworking the Ocelot implementation
in a way that is DSA-compatible.

This is a large series, but hopefully is easy enough to digest, since it
contains mostly code refactoring. What needs to be changed:
- The struct net_device, phy_device needs to be isolated from Ocelot
  private structures (struct ocelot, struct ocelot_port). These will
  live as 1-to-1 equivalents to struct dsa_switch and struct dsa_port.
- The function prototypes need to be compatible with DSA (of course,
  struct dsa_switch will become struct ocelot).
- The CPU port needs to be assigned via a higher-level API, not
  hardcoded in the driver.

What is going to be interesting is that the new DSA front-end of Ocelot
will need to have features in lockstep with the DSA core itself. At the
moment, some more advanced tc offloading features of Ocelot (tc-flower,
etc) are not available in the DSA front-end due to lack of API in the
DSA core. It also means that Ocelot practically re-implements large
parts of DSA (although it is not a DSA switch per se) - see the FDB API
for example.

The code has been only compile-tested on Ocelot, since I don't have
access to any VSC7514 hardware. It was proven to work on NXP LS1028A,
which instantiates a DSA derivative of Ocelot. So I would like to ask
Alex Belloni if you could confirm this series causes no regression on
the Ocelot MIPS SoC.

The goal is to get this rework upstream as quickly as possible,
precisely because it is a large volume of code that risks gaining merge
conflicts if we keep it for too long.

This is but the first chunk of the LS1028A Felix DSA driver upstreaming.
For those who are interested, the concept can be seen on my private
Github repo, the user of this reworked Ocelot driver living under
drivers/net/dsa/vitesse/:
https://github.com/vladimiroltean/ls1028ardb-linux

Claudiu Manoil (1):
  net: mscc: ocelot: initialize list of multicast addresses in common
    code

Vladimir Oltean (14):
  net: mscc: ocelot: break apart ocelot_vlan_port_apply
  net: mscc: ocelot: break apart vlan operations into
    ocelot_vlan_{add,del}
  net: mscc: ocelot: break out fdb operations into abstract
    implementations
  net: mscc: ocelot: change prototypes of hwtstamping ioctls
  net: mscc: ocelot: change prototypes of switchdev port attribute
    handlers
  net: mscc: ocelot: refactor struct ocelot_port out of function
    prototypes
  net: mscc: ocelot: separate net_device related items out of
    ocelot_port
  net: mscc: ocelot: refactor ethtool callbacks
  net: mscc: ocelot: limit vlan ingress filtering to actual number of
    ports
  net: mscc: ocelot: move port initialization into separate function
  net: mscc: ocelot: separate the common implementation of ndo_open and
    ndo_stop
  net: mscc: ocelot: refactor adjust_link into a netdev-independent
    function
  net: mscc: ocelot: split assignment of the cpu port into a separate
    function
  net: mscc: ocelot: don't hardcode the number of the CPU port

 drivers/net/ethernet/mscc/ocelot.c        | 948 +++++++++++++---------
 drivers/net/ethernet/mscc/ocelot.h        |  33 +-
 drivers/net/ethernet/mscc/ocelot_ace.h    |   4 +-
 drivers/net/ethernet/mscc/ocelot_board.c  |  24 +-
 drivers/net/ethernet/mscc/ocelot_flower.c |  32 +-
 drivers/net/ethernet/mscc/ocelot_police.c |  36 +-
 drivers/net/ethernet/mscc/ocelot_police.h |   4 +-
 drivers/net/ethernet/mscc/ocelot_tc.c     |  56 +-
 8 files changed, 680 insertions(+), 457 deletions(-)

Comments

Andrew Lunn Nov. 10, 2019, 5:16 p.m. UTC | #1
On Sat, Nov 09, 2019 at 03:02:46PM +0200, Vladimir Oltean wrote:
> After the nice "change-my-mind" discussion about Ocelot, Felix and
> LS1028A (which can be read here: https://lkml.org/lkml/2019/6/21/630),
> we have decided to take the route of reworking the Ocelot implementation
> in a way that is DSA-compatible.
> 
> This is a large series, but hopefully is easy enough to digest, since it
> contains mostly code refactoring.

I just skimmed over the patches. Apart from the naming confusion at
the end, it all looks O.K.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

> It also means that Ocelot practically re-implements large parts of
> DSA (although it is not a DSA switch per se)

Would it make sense to refactor parts of the DSA core and export them
as helper function?

   Andrew
Vladimir Oltean Nov. 10, 2019, 5:22 p.m. UTC | #2
On Sun, 10 Nov 2019 at 19:16, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sat, Nov 09, 2019 at 03:02:46PM +0200, Vladimir Oltean wrote:
> > After the nice "change-my-mind" discussion about Ocelot, Felix and
> > LS1028A (which can be read here: https://lkml.org/lkml/2019/6/21/630),
> > we have decided to take the route of reworking the Ocelot implementation
> > in a way that is DSA-compatible.
> >
> > This is a large series, but hopefully is easy enough to digest, since it
> > contains mostly code refactoring.
>
> I just skimmed over the patches. Apart from the naming confusion at
> the end, it all looks O.K.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>

Thanks a lot! Would it be too early if I also posted the Felix DSA
driver as well?

> > It also means that Ocelot practically re-implements large parts of
> > DSA (although it is not a DSA switch per se)
>
> Would it make sense to refactor parts of the DSA core and export them
> as helper function?

Where it helps, I'll sure consider doing that. We'll anyway need to
add support for tc-flower in DSA, filter blocks and all of that. At
the moment, only the FDB dump code was slightly duplicated, but then
again, that's because some boilerplate is needed, and it was there
anyway. So far it's manageable.

>
>    Andrew
Horatiu Vultur Nov. 11, 2019, 12:10 p.m. UTC | #3
The 11/09/2019 15:02, Vladimir Oltean wrote:
> External E-Mail
> 
> 
> After the nice "change-my-mind" discussion about Ocelot, Felix and
> LS1028A (which can be read here: https://lkml.org/lkml/2019/6/21/630),
> we have decided to take the route of reworking the Ocelot implementation
> in a way that is DSA-compatible.
> 
> This is a large series, but hopefully is easy enough to digest, since it
> contains mostly code refactoring. What needs to be changed:
> - The struct net_device, phy_device needs to be isolated from Ocelot
>   private structures (struct ocelot, struct ocelot_port). These will
>   live as 1-to-1 equivalents to struct dsa_switch and struct dsa_port.
> - The function prototypes need to be compatible with DSA (of course,
>   struct dsa_switch will become struct ocelot).
> - The CPU port needs to be assigned via a higher-level API, not
>   hardcoded in the driver.
> 
> What is going to be interesting is that the new DSA front-end of Ocelot
> will need to have features in lockstep with the DSA core itself. At the
> moment, some more advanced tc offloading features of Ocelot (tc-flower,
> etc) are not available in the DSA front-end due to lack of API in the
> DSA core. It also means that Ocelot practically re-implements large
> parts of DSA (although it is not a DSA switch per se) - see the FDB API
> for example.
> 
> The code has been only compile-tested on Ocelot, since I don't have
> access to any VSC7514 hardware. It was proven to work on NXP LS1028A,
> which instantiates a DSA derivative of Ocelot. So I would like to ask
> Alex Belloni if you could confirm this series causes no regression on
> the Ocelot MIPS SoC.
> 
> The goal is to get this rework upstream as quickly as possible,
> precisely because it is a large volume of code that risks gaining merge
> conflicts if we keep it for too long.
> 
> This is but the first chunk of the LS1028A Felix DSA driver upstreaming.
> For those who are interested, the concept can be seen on my private
> Github repo, the user of this reworked Ocelot driver living under
> drivers/net/dsa/vitesse/:
> https://github.com/vladimiroltean/ls1028ardb-linux

I have done some tests on Ocelot hardware and it seems to work fine.

Acked-by: Horatiu Vultur <horatiu.vultur@microchip.com>

> 
> Claudiu Manoil (1):
>   net: mscc: ocelot: initialize list of multicast addresses in common
>     code
> 
> Vladimir Oltean (14):
>   net: mscc: ocelot: break apart ocelot_vlan_port_apply
>   net: mscc: ocelot: break apart vlan operations into
>     ocelot_vlan_{add,del}
>   net: mscc: ocelot: break out fdb operations into abstract
>     implementations
>   net: mscc: ocelot: change prototypes of hwtstamping ioctls
>   net: mscc: ocelot: change prototypes of switchdev port attribute
>     handlers
>   net: mscc: ocelot: refactor struct ocelot_port out of function
>     prototypes
>   net: mscc: ocelot: separate net_device related items out of
>     ocelot_port
>   net: mscc: ocelot: refactor ethtool callbacks
>   net: mscc: ocelot: limit vlan ingress filtering to actual number of
>     ports
>   net: mscc: ocelot: move port initialization into separate function
>   net: mscc: ocelot: separate the common implementation of ndo_open and
>     ndo_stop
>   net: mscc: ocelot: refactor adjust_link into a netdev-independent
>     function
>   net: mscc: ocelot: split assignment of the cpu port into a separate
>     function
>   net: mscc: ocelot: don't hardcode the number of the CPU port
> 
>  drivers/net/ethernet/mscc/ocelot.c        | 948 +++++++++++++---------
>  drivers/net/ethernet/mscc/ocelot.h        |  33 +-
>  drivers/net/ethernet/mscc/ocelot_ace.h    |   4 +-
>  drivers/net/ethernet/mscc/ocelot_board.c  |  24 +-
>  drivers/net/ethernet/mscc/ocelot_flower.c |  32 +-
>  drivers/net/ethernet/mscc/ocelot_police.c |  36 +-
>  drivers/net/ethernet/mscc/ocelot_police.h |   4 +-
>  drivers/net/ethernet/mscc/ocelot_tc.c     |  56 +-
>  8 files changed, 680 insertions(+), 457 deletions(-)
> 
> -- 
> 2.17.1
> 
>
Vladimir Oltean Nov. 11, 2019, 12:17 p.m. UTC | #4
On Mon, 11 Nov 2019 at 14:10, Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> The 11/09/2019 15:02, Vladimir Oltean wrote:
> > External E-Mail
> >
> >
> > After the nice "change-my-mind" discussion about Ocelot, Felix and
> > LS1028A (which can be read here: https://lkml.org/lkml/2019/6/21/630),
> > we have decided to take the route of reworking the Ocelot implementation
> > in a way that is DSA-compatible.
> >
> > This is a large series, but hopefully is easy enough to digest, since it
> > contains mostly code refactoring. What needs to be changed:
> > - The struct net_device, phy_device needs to be isolated from Ocelot
> >   private structures (struct ocelot, struct ocelot_port). These will
> >   live as 1-to-1 equivalents to struct dsa_switch and struct dsa_port.
> > - The function prototypes need to be compatible with DSA (of course,
> >   struct dsa_switch will become struct ocelot).
> > - The CPU port needs to be assigned via a higher-level API, not
> >   hardcoded in the driver.
> >
> > What is going to be interesting is that the new DSA front-end of Ocelot
> > will need to have features in lockstep with the DSA core itself. At the
> > moment, some more advanced tc offloading features of Ocelot (tc-flower,
> > etc) are not available in the DSA front-end due to lack of API in the
> > DSA core. It also means that Ocelot practically re-implements large
> > parts of DSA (although it is not a DSA switch per se) - see the FDB API
> > for example.
> >
> > The code has been only compile-tested on Ocelot, since I don't have
> > access to any VSC7514 hardware. It was proven to work on NXP LS1028A,
> > which instantiates a DSA derivative of Ocelot. So I would like to ask
> > Alex Belloni if you could confirm this series causes no regression on
> > the Ocelot MIPS SoC.
> >
> > The goal is to get this rework upstream as quickly as possible,
> > precisely because it is a large volume of code that risks gaining merge
> > conflicts if we keep it for too long.
> >
> > This is but the first chunk of the LS1028A Felix DSA driver upstreaming.
> > For those who are interested, the concept can be seen on my private
> > Github repo, the user of this reworked Ocelot driver living under
> > drivers/net/dsa/vitesse/:
> > https://github.com/vladimiroltean/ls1028ardb-linux
>
> I have done some tests on Ocelot hardware and it seems to work fine.
>
> Acked-by: Horatiu Vultur <horatiu.vultur@microchip.com>
>

Thanks, Horatiu!

> --
> /Horatiu

-Vladimir
David Miller Nov. 11, 2019, 8:59 p.m. UTC | #5
From: Vladimir Oltean <olteanv@gmail.com>
Date: Sat,  9 Nov 2019 15:02:46 +0200

> After the nice "change-my-mind" discussion about Ocelot, Felix and
> LS1028A (which can be read here: https://lkml.org/lkml/2019/6/21/630),
> we have decided to take the route of reworking the Ocelot implementation
> in a way that is DSA-compatible.
 ...

I'm going to apply this series as-is.

But please address Andrew's feedback about port naming and such.

Thank you.