Nikolay Aleksandrov <[email protected]> writes: > On 26/04/18 12:06, Ido Schimmel wrote: >> From: Petr Machata <[email protected]> >> >> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c >> b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c >> index 65a77708ff61..92fb81839852 100644 >> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c >> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c >> @@ -167,6 +169,79 @@ mlxsw_sp_span_entry_unoffloadable(struct >> mlxsw_sp_span_parms *sparmsp) >> return 0; >> } >> +static struct net_device * >> +mlxsw_sp_span_entry_bridge_8021q(const struct net_device *br_dev, >> + unsigned char *dmac, >> + u16 *p_vid) >> +{ >> + struct net_bridge_vlan_group *vg = br_vlan_group_rtnl(br_dev); >> + u16 pvid = br_vlan_group_pvid(vg); >> + struct net_device *edev = NULL; >> + struct net_bridge_vlan *v; >> + > > Hi, > These structures are not really exported anywhere outside of br_private.h > Instead of passing them around and risking someone else actually trying to > dereference an incomplete type, why don't you add just 2 new helpers - > br_vlan_pvid(netdevice) and br_vlan_info(netdevice, vid, *bridge_vlan_info) > > br_vlan_info can return the exported and already available type > "bridge_vlan_info" > (defined in uapi/linux/if_bridge.h) into the bridge_vlan_info arg > and br_vlan_pvid is obvious - return the current dev pvid if available. > > Another bonus you'll avoid dealing with the bridge's vlan internals.
All right, I'll do it like that. > >> + if (pvid) >> + edev = br_fdb_find_port_hold(br_dev, dmac, pvid); >> + if (!edev) >> + return NULL; >> + >> + /* RTNL prevents edev from being removed. */ >> + dev_put(edev); > > Minor point here - since this is always called in rtnl, the dev_hold/put > dance isn't needed, > unless you plan to use the fdb_find_port_hold outside of rtnl of course. In > case that is > not planned why don't you do br_fdb_find_port_rtnl() instead with RCU inside > (thus not blocking > learning) and ASSERT_RTNL() to avoid some complexity ? OK, sounds good. I'll spin a v2. Thanks, Petr
