[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