Re: [patch net-next 01/18] bridge: Export VLAN filtering state

[Ido Schimmel <idosch@mellanox.com>]

Hi Nik,

On Fri, May 26, 2017 at 11:55:18AM +0300, Nikolay Aleksandrov wrote:
> On 5/26/17 9:37 AM, Jiri Pirko wrote:
> > From: Ido Schimmel <idosch@mellanox.com>
> > 
> > It's useful for drivers supporting bridge offload to be able to query
> > the bridge's VLAN filtering state.
> > 
> > Currently, upon enslavement to a bridge master, the offloading driver
> > will only learn about the bridge's VLAN filtering state after the bridge
> > device was already linked with its slave.
> > 
> > Being able to query the bridge's VLAN filtering state allows such
> > drivers to forbid enslavement in case resource couldn't be allocated for
> > a VLAN-aware bridge and also choose the correct initialization routine
> > for the enslaved port, which is dependent on the bridge type.
> > 
> > Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> > Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> > ---
> >   include/linux/if_bridge.h | 9 +++++++++
> >   net/bridge/br_if.c        | 2 +-
> >   net/bridge/br_mdb.c       | 4 ++--
> >   net/bridge/br_netlink.c   | 2 +-
> >   net/bridge/br_private.h   | 9 ---------
> >   net/bridge/br_vlan.c      | 8 ++++++++
> >   6 files changed, 21 insertions(+), 13 deletions(-)
> > 
> 
> I must say this bridge -> dev -> bridge looks weird from the bridge POV.
> Since exports like this seem to be increasing I think it'd be nice to
> make some API that can be queried instead of exporting symbols for each
> bridge option or attribute. In this case maybe a simpler solution would've
> been only a new exported symbol for external users.

It seemed more logical to me to export the existing function, but I can
instead leave it as-is and introduce a new symbol. I'll wait for more
comments and send a v2 if deemed necessary.

> The patch itself looks good to me.
> 
> Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

Thanks for reviewing!