Re: [PATCH net-next 4/6] net: dsa: add port_fdb_prepare

[Vivien Didelot <vivien.didelot@savoirfairelinux.com>]

Hi Andrew,

On Oct. Thursday 08 (41) 02:25 AM, Andrew Lunn wrote:
> On Wed, Oct 07, 2015 at 07:48:29PM -0400, Vivien Didelot wrote:
> > Push the prepare phase for FDB operations down to the DSA drivers, with
> > a new port_fdb_prepare function. Currently only mv88e6xxx is affected.
> > 
> > Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> > ---
> >  drivers/net/dsa/mv88e6171.c |  1 +
> >  drivers/net/dsa/mv88e6352.c |  1 +
> >  drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++
> >  drivers/net/dsa/mv88e6xxx.h |  3 +++
> >  include/net/dsa.h           |  4 ++++
> >  net/dsa/slave.c             |  7 +++++--
> >  6 files changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
> > index c95cfab..ca3330a 100644
> > --- a/drivers/net/dsa/mv88e6171.c
> > +++ b/drivers/net/dsa/mv88e6171.c
> > @@ -121,6 +121,7 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
> >  	.port_vlan_add		= mv88e6xxx_port_vlan_add,
> >  	.port_vlan_del		= mv88e6xxx_port_vlan_del,
> >  	.vlan_getnext		= mv88e6xxx_vlan_getnext,
> > +	.port_fdb_prepare	= mv88e6xxx_port_fdb_prepare,
> 
> Hi Vivien
> 
> Bike shedding a bit, but i would call this
> mv88e6xxx_port_fdb_prepare_add.

I think port_fdb_prepare is fine because it is the only step that
actually needs the 2-phase model. del and dump are safe and don't need
pre-check.

> >  	.port_fdb_add		= mv88e6xxx_port_fdb_add,
> >  	.port_fdb_del		= mv88e6xxx_port_fdb_del,
> >  	.port_fdb_getnext	= mv88e6xxx_port_fdb_getnext,
> 
> Taking a theoretical example, say mv88e6xxx_port_fdb_getnext needed a
> prepare call to allocate memory to put the returned ATU into. What
> would you call that?
> 
> mv88e6xxx_port_fdb_prepare_add and mv88e6xxx_port_fdb_prepare_getnext
> just seems unambiguous and future proof.

the switchdev dump operation is called just once, so no preparation is
implied (from the switchdev point of view). It is the responsability of
the driver to call the switchdev dump callback itself.

Thanks,
-v