[Devel] [PATCH 04/14] Subject: ve/net/bridge: don't set NULL in skb->dev

[Konstantin Khorenko]

On 06/10/2015 11:42 AM, Kirill Tkhai wrote:
> В Ср, 10/06/2015 в 11:35 +0300, Konstantin Khorenko пишет:
>> JFYI: this patch is related to via_phys_dev feature,
>> we've dropped it => no need for this patch.
> 
> When you did that? My git tree is about to be fresh updated,
> but I'm still seeing via_phys_dev there.

Those places are left from the initial global rebase commit and are to be removed.
Nowadays libvirt manager handles the problem with bridge MAC flipping and thus we don't need via_phys_dev anymore.

>> --
>> Best regards,
>>
>> Konstantin Khorenko,
>> Virtuozzo Linux Kernel Team
>>
>> On 06/08/2015 05:20 PM, Kirill Tkhai wrote:
>>> Porting patches diff-ve-net-bridge-dont-set-NULL-in-skb-dev
>>> and diff-ve-net-bridge-dont-forget-to-init-master_dev from 2.6.32:
>>>
>>> skb->dev can't be NULL, because it is accessed from netfilters
>>> without testing.
>>>
>>> In the upstream code the bridge devices is used. We have
>>> the via_phys_dev feature, but the master_dev is accesses
>>> without locks, so here is a race.
>>>
>>> This patch allows safely assess to master_dev in rcu context.
>>>
>>> https://jira.sw.ru/browse/PSBM-24056
>>>
>>> Signed-off-by: Andrey Vagin <avagin at openvz.org>
>>> Acked-by: Stanislav Kinsbursky <skinsbursky at parallels.com>
>>> Signed-off-by: Kirill Tkhai <ktkhai at odin.com>
>>> ---
>>>  net/bridge/br_if.c        |    8 ++++----
>>>  net/bridge/br_input.c     |   14 ++++++++++----
>>>  net/bridge/br_netfilter.c |    4 +++-
>>>  3 files changed, 17 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>>> index a357ce8..21d815b 100644
>>> --- a/net/bridge/br_if.c
>>> +++ b/net/bridge/br_if.c
>>> @@ -171,7 +171,7 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
>>>  
>>>  	if (br->master_dev) {
>>>  		dev_put(br->master_dev);
>>> -		br->master_dev = NULL;
>>> +		rcu_assign_pointer(br->master_dev, NULL);
>>>  	}
>>>  
>>>  	list_for_each_entry_safe(p, n, &br->port_list, list) {
>>> @@ -399,7 +399,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
>>>  		br_stp_enable_port(p);
>>>  	if (!(dev->features & NETIF_F_VIRTUAL) && !br->master_dev) {
>>>  		dev_hold(dev);
>>> -		br->master_dev = dev;
>>> +		rcu_assign_pointer(br->master_dev, dev);
>>>  	}
>>>  	spin_unlock_bh(&br->lock);
>>>  
>>> @@ -453,12 +453,12 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
>>>  	spin_lock_bh(&br->lock);
>>>  	changed_addr = br_stp_recalculate_bridge_id(br);
>>>  	if (br->master_dev == dev) {
>>> -		br->master_dev = NULL;
>>> +		rcu_assign_pointer(br->master_dev, NULL);
>>>  		dev_put(dev);
>>>  		list_for_each_entry(p, &br->port_list, list)
>>>  			if (!(p->dev->features & NETIF_F_VIRTUAL)) {
>>>  				dev_hold(p->dev);
>>> -				br->master_dev = p->dev;
>>> +				rcu_assign_pointer(br->master_dev, p->dev);
>>>  				break;
>>>  			}
>>>  	}
>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>> index 15f99fe..82f020a 100644
>>> --- a/net/bridge/br_input.c
>>> +++ b/net/bridge/br_input.c
>>> @@ -28,6 +28,7 @@ static int br_pass_frame_up(struct net_bridge *br, struct sk_buff *skb)
>>>  {
>>>  	struct net_device *indev, *brdev = BR_INPUT_SKB_CB(skb)->brdev;
>>>  	struct br_cpu_netstats *brstats = this_cpu_ptr(br->stats);
>>> +	struct net_device *master_dev = NULL;
>>>  
>>>  	u64_stats_update_begin(&brstats->syncp);
>>>  	brstats->rx_packets++;
>>> @@ -49,12 +50,14 @@ static int br_pass_frame_up(struct net_bridge *br, struct sk_buff *skb)
>>>  		return NET_RX_DROP;
>>>  
>>>  	indev = skb->dev;
>>> -	if (!br->via_phys_dev)
>>> +
>>> +	if (br->via_phys_dev)
>>> +		master_dev = rcu_dereference(br->master_dev);
>>> +	if (!master_dev)
>>>  		skb->dev = brdev;
>>>  	else {
>>>  		skb->brmark = BR_ALREADY_SEEN;
>>> -		if (br->master_dev)
>>> -			skb->dev = br->master_dev;
>>> +		skb->dev = master_dev;
>>>  	}
>>>  
>>>  	return NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, indev, NULL,
>>> @@ -239,7 +242,10 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
>>>  		if (skb->brmark == BR_ALREADY_SEEN)
>>>  			return RX_HANDLER_PASS;
>>>  
>>> -		out = p->br->via_phys_dev ? p->br->master_dev : p->br->dev;
>>> +		if (p->br->via_phys_dev)
>>> +			out = rcu_dereference(p->br->master_dev);
>>> +		else
>>> +			out = p->br->dev;
>>>  
>>>  		if (out && ether_addr_equal(out->dev_addr, dest))
>>>  			skb->pkt_type = PACKET_HOST;
>>> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
>>> index 0afdf3e..7ddb9ad 100644
>>> --- a/net/bridge/br_netfilter.c
>>> +++ b/net/bridge/br_netfilter.c
>>> @@ -181,6 +181,7 @@ static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)
>>>  
>>>  static inline struct net_device *bridge_parent(const struct net_device *dev)
>>>  {
>>> +	struct net_device *master_dev;
>>>  	struct net_bridge_port *port;
>>>  	struct net_bridge *br;
>>>  
>>> @@ -189,7 +190,8 @@ static inline struct net_device *bridge_parent(const struct net_device *dev)
>>>  		return NULL;
>>>  
>>>  	br = port->br;
>>> -	if (br->via_phys_dev && br->master_dev)
>>> +	master_dev = rcu_dereference(br->master_dev);
>>> +	if (br->via_phys_dev && master_dev)
>>>  		return br->master_dev;
>>>  	else
>>>  		return br->dev;
>>>
>>> _______________________________________________
>>> Devel mailing list
>>> Devel at openvz.org
>>> https://lists.openvz.org/mailman/listinfo/devel
>>>
> 
> 
>