On 27/04/18 19:08, Petr Machata wrote: > Nikolay Aleksandrov <niko...@cumulusnetworks.com> writes: > >> On 27/04/18 18:11, Ido Schimmel wrote: >>> From: Petr Machata <pe...@mellanox.com> >>> >>> Add a couple new functions to allow querying FDB and vlan settings of a >>> bridge. >>> >>> Signed-off-by: Petr Machata <pe...@mellanox.com> >>> Signed-off-by: Ido Schimmel <ido...@mellanox.com> >>> --- >>> include/linux/if_bridge.h | 28 ++++++++++++++++++++++++++++ >>> net/bridge/br_fdb.c | 22 ++++++++++++++++++++++ >>> net/bridge/br_private.h | 11 +++++++++++ >>> net/bridge/br_vlan.c | 39 +++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 100 insertions(+) >>> >> >> Thanks! This looks good to me although the new exported helpers could've >> taken both bridge or port and return the result. Usually when adding a >> port-only functions we name them with nbp_ prefix instead of br_. >> >> Anyway this can be done later since the API is internal, > > The idea is that the API would accept both ports and bridges, but > currently there's no user for the parts that I didn't code up--it would > be a dead code. It's super simple to extend these if/when there are > clients, e.g.: > > modified net/bridge/br_vlan.c > @@ -1155,7 +1155,10 @@ int br_vlan_pvid_rtnl(const struct net_device *dev, > u16 *p_pvid) > struct net_bridge_vlan_group *vg; > > ASSERT_RTNL(); > - if (netif_is_bridge_master(dev)) > + p = br_port_get_check_rtnl(dev); > + if (p) > + vg = nbp_vlan_group(p); > + else if (netif_is_bridge_master(dev)) > vg = br_vlan_group(netdev_priv(dev)); > else > return -EINVAL; > > I can post a follow-up with the renames if you prefer that. > > Thanks, > Petr >
I know, that's why I said it can be done later. :-) No need to do it now, it was only a comment based on other accessors in the bridge code and for completeness. I'm very happy with this version and have acked/reviewed it. Cheers, Nik
