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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170526140200.GA1037@splinter>
Date:   Fri, 26 May 2017 17:02:00 +0300
From:   Ido Schimmel <idosch@...lanox.com>
To:     Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
CC:     Jiri Pirko <jiri@...nulli.us>, <netdev@...r.kernel.org>,
        <davem@...emloft.net>, <mlxsw@...lanox.com>,
        <stephen@...workplumber.org>
Subject: Re: [patch net-next 01/18] bridge: Export VLAN filtering state

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@...lanox.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@...lanox.com>
> > Signed-off-by: Jiri Pirko <jiri@...lanox.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@...ulusnetworks.com>

Thanks for reviewing!