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

Kirill Tkhai ktkhai at odin.com
Wed Jun 10 08:05:53 PDT 2015


В Ср, 10/06/2015 в 17:37 +0300, 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.

Ok, thanks for the explanation.

> 
> >> --
> >> 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
> >>>
> > 
> > 
> > 





More information about the Devel mailing list