[Bridge] [PATCH net-next 6/6] mlxsw: spectrum_span: Allow bridge for gretap mirror
Petr Machata
petrm at mellanox.com
Thu Apr 26 13:03:03 UTC 2018
Nikolay Aleksandrov <nikolay at cumulusnetworks.com> writes:
> On 26/04/18 12:06, Ido Schimmel wrote:
>> From: Petr Machata <petrm at mellanox.com>
>>
>> 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
More information about the Bridge
mailing list