Re: [PATCH 0/4] RFC CPSW switchdev mode

From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
To: Ivan Vecera <ivecera@redhat.com>
Cc: Andrew Lunn <andrew@lunn.ch>, Jiri Pirko <jiri@resnulli.us>,
	netdev@vger.kernel.org, grygorii.strashko@ti.com,
	ivan.khoronzhuk@linaro.org, nsekhar@ti.com,
	francois.ozog@linaro.org, yogeshs@ti.com, spatton@ti.com
Subject: Re: [PATCH 0/4] RFC CPSW switchdev mode
Date: Thu, 24 May 2018 17:08:31 +0300	[thread overview]
Message-ID: <20180524140831.GA16793@apalos> (raw)
In-Reply-To: <7437d485-1eac-9619-3827-5af9b32b939e@redhat.com>

On Thu, May 24, 2018 at 03:44:54PM +0200, Ivan Vecera wrote:
> On 24.5.2018 14:54, Andrew Lunn wrote:
> > On Thu, May 24, 2018 at 11:48:31AM +0300, Ilias Apalodimas wrote:
> >> On Thu, May 24, 2018 at 10:05:28AM +0200, Jiri Pirko wrote:
> >>> Thu, May 24, 2018 at 08:56:20AM CEST, ilias.apalodimas@linaro.org wrote:
> >>> Any reason you need cpu port? We don't need it in mlxsw and also in dsa.
> >> Yes i've seen that on mlxsw/rocker drivers and i was reluctant adding one here. 
> >> The reason is that TI wants this configured differently from customer facing 
> >> ports. Apparently there are existing customers already using the "feature". 
> >> So OR'ing and adding the cpu port on every operation (add/del vlans add 
> >> ucast/mcast entries etc) was less favoured. 
> > 
> > Hi Ilias
> > 
> > Nice to see this device moving away from its custom model and towards
> > the switchdev model.
> +1
Thanks. To be honest it opens up so many posibilities for common configuration
from userspace across vendors that doing something new without it doesn't make
any sense (at least to me).

> 
> > Did you consider making a clean break from the existing code and write
> > a new driver. Let the existing customers using the existing
> > driver. Have the new switchdev driver fully conform to switchdev.
> 
> I would also prefer fresh new driver. The existing one can be marked as
> 'bugfix-only' and later pertinently deprecated/removed.
Yes, but given the driver and the platforms it's used at, we ended up patching 
the existing driver. I am not opposed to the idea, but Grygorii is more suited 
to reply on that.

> > 
> > I don't like having this 'cpu' interface. As you say, it breaks the
> > switchhdev model. If we need to extend the switchdev model to support
> > some use case, lets do that. Please can you fully describe the use
> > cases, so we can discuss how to implement them cleanly within the
> > switchdev model.
> +1
There's configuration needs from customers adding or not adding a VLAN to the
CPU port. In my configuration examples for instance, if the cpu port is not
added to the bridge, you cannot get an ip address on it. 
Similar cases exist for customers on adding MDBs as far as i know. So they want
the "customer facing ports" to have the MDBs present but not the cpu port.
In some cases (where the CPE/device that has the switch) participates in the 
traffic they want the cpu port to have the samne MDBs installed.
This is just two simple cases that come in mind, again Grygorii is more suited
to answer and explain existing/more complex use cases better than me.

Adding a cpu port that cannot transmit or receive traffic is a bit "weird", on
the other hand you can access it's configuration using the same userspace tools
and the same commands you do for the "normal" ports. Extending switchdev might
be the proper solution here.

Ilias