On 06/07/2019 16:27, wenxu wrote:
>
> 在 2019/7/6 20:11, Nikolay Aleksandrov 写道:
>> On 06/07/2019 01:09, wenxu@xxxxxxxxx wrote:
>>> From: wenxu <wenxu@xxxxxxxxx>
>>>
>>> This new function allows you to fetch vlan info from packet path.
>>>
>>> Signed-off-by: wenxu <wenxu@xxxxxxxxx>
>>> ---
>>> include/linux/if_bridge.h | 7 +++++++
>>> net/bridge/br_vlan.c | 23 ++++++++++++++++++-----
>>> 2 files changed, 25 insertions(+), 5 deletions(-)
>>>
>> Hi,
>> This patch will need more work, comments below.
>>
>>> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
>>> index 9e57c44..5c85608 100644
>>> --- a/include/linux/if_bridge.h
>>> +++ b/include/linux/if_bridge.h
>>> @@ -92,6 +92,8 @@ static inline bool br_multicast_router(const struct net_device *dev)
>>> int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto);
>>> int br_vlan_get_info(const struct net_device *dev, u16 vid,
>>> struct bridge_vlan_info *p_vinfo);
>>> +int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
>>> + struct bridge_vlan_info *p_vinfo);
>>> #else
>>> static inline bool br_vlan_enabled(const struct net_device *dev)
>>> {
>>> @@ -118,6 +120,11 @@ static inline int br_vlan_get_info(const struct net_device *dev, u16 vid,
>>> {
>>> return -EINVAL;
>>> }
>>> +static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
>>> + struct bridge_vlan_info *p_vinfo)
>>> +{
>>> + return -EINVAL;
>>> +}
>>> #endif
>>>
>>> #if IS_ENABLED(CONFIG_BRIDGE)
>>> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
>>> index 021cc9f66..2799a88 100644
>>> --- a/net/bridge/br_vlan.c
>>> +++ b/net/bridge/br_vlan.c
>>> @@ -1267,15 +1267,13 @@ int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid)
>>> }
>>> EXPORT_SYMBOL_GPL(br_vlan_get_pvid_rcu);
>>>
>>> -int br_vlan_get_info(const struct net_device *dev, u16 vid,
>>> - struct bridge_vlan_info *p_vinfo)
>>> +static int __br_vlan_get_info(const struct net_device *dev, u16 vid,
>>> + struct net_bridge_port *p,
>>> + struct bridge_vlan_info *p_vinfo)
>>> {
>>> struct net_bridge_vlan_group *vg;
>>> struct net_bridge_vlan *v;
>>> - struct net_bridge_port *p;
>>>
>>> - ASSERT_RTNL();
>> Removing the assert here doesn't make the function proper for RCU usage.
>> Also note that for the RCU version you need to check if vg
>> is null.
> Why need check if vg is null? The br_vlan_find already check it.
Ah, right. Fair enough, drop that part.
>>
>>> - p = br_port_get_check_rtnl(dev);
>>> if (p)
>>> vg = nbp_vlan_group(p);
>>> else if (netif_is_bridge_master(dev))
>>> @@ -1291,8 +1289,23 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
>>> p_vinfo->flags = v->flags;
>>> return 0;
>>> }
>>> +
>>> +int br_vlan_get_info(const struct net_device *dev, u16 vid,
>>> + struct bridge_vlan_info *p_vinfo)
>>> +{
>>> + ASSERT_RTNL();
>>> +
>>> + return __br_vlan_get_info(dev, vid, br_port_get_check_rtnl(dev), p_vinfo);
>>> +}
>>> EXPORT_SYMBOL_GPL(br_vlan_get_info);
>>>
>>> +int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
>>> + struct bridge_vlan_info *p_vinfo)
>>> +{
>>> + return __br_vlan_get_info(dev, vid, br_port_get_check_rtnl(dev), p_vinfo);
>>> +}
>>> +EXPORT_SYMBOL_GPL(br_vlan_get_info_rcu);
>>> +
>> This should use br_port_get_check_rcu().
>>
>>> static int br_vlan_is_bind_vlan_dev(const struct net_device *dev)
>>> {
>>> return is_vlan_dev(dev) &&
>>>
>>
