[ovs-dev] [PATCH v2] datapath: rhel: Move RHEL OVS hook registration to netdev_rx_handler_register() backport

Pravin Shelar pshelar at nicira.com
Fri Nov 22 22:47:37 UTC 2013


This patch actually fixes race in case of device is moved from one
bridge to another.

e.g.
ovs-vsctl -- remove Bridge br0 ports <vport-id> -- add Bridge br1
ports <vport-id>

events in this case (This bug only effects RHEL6 kernel).
1. netdev destroy (vport1, dev1) : schedule rcu callback (vport1).
2. netdev create (vport2, dev1) which sets dev->ax25_ptr to vport2
3. free_port_rcu(vport1) this resets vport1->dev->ax25_ptr, but
vport1->dev is dev1 which belongs to vport2.
4. When VM is destroyed ovs receives dp_notify event but that can not
release device refcnt since netdev private data (dev1->ax25_ptr) is
NULL.
---

Therefore I have pushed his patch to branch-1.10.

On Tue, Jul 9, 2013 at 11:36 AM, Thomas Graf <tgraf at redhat.com> wrote:
> Moves the registration of the RHEL specific OVS hook to the compat
> backport of netdev_rx_handler_register(). This moves the hook
> unregistration from the RCU callback to the netdev_destroy()
> callback directly.
>
> This is purely cosmetic though, the RHEL hook is only used if the
> IFF_OVS_DATAPATH flag is present which was removed under RTNL
> protection before the RCU callback.
>
> Signed-off-by: Thomas Graf <tgraf at redhat.com>
> ---
> V2: - Move nr_bridges to compat/netdevice.c
>     - Don't use atomic_t for nr_bridges
>
>  datapath/linux/compat/include/linux/netdevice.h | 21 ++++++++++++++++++++-
>  datapath/linux/compat/netdevice.c               |  4 ++++
>  datapath/vport-netdev.c                         | 20 --------------------
>  3 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/datapath/linux/compat/include/linux/netdevice.h b/datapath/linux/compat/include/linux/netdevice.h
> index b051baf..176d1e6 100644
> --- a/datapath/linux/compat/include/linux/netdevice.h
> +++ b/datapath/linux/compat/include/linux/netdevice.h
> @@ -20,6 +20,11 @@ struct net;
>  #define to_net_dev(class) container_of(class, struct net_device, NETDEV_DEV_MEMBER)
>  #endif
>
> +#ifdef HAVE_RHEL_OVS_HOOK
> +extern struct sk_buff *(*openvswitch_handle_frame_hook)(struct sk_buff *skb);
> +extern int nr_bridges;
> +#endif
> +
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,26)
>  static inline
>  struct net *dev_net(const struct net_device *dev)
> @@ -84,19 +89,33 @@ extern int skb_checksum_help(struct sk_buff *skb, int);
>  extern int skb_checksum_help(struct sk_buff *skb);
>  #endif
>
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36)
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36) || \
> +    defined HAVE_RHEL_OVS_HOOK
>  static inline int netdev_rx_handler_register(struct net_device *dev,
>                                              void *rx_handler,
>                                              void *rx_handler_data)
>  {
> +#ifdef HAVE_RHEL_OVS_HOOK
> +       rcu_assign_pointer(dev->ax25_ptr, rx_handler_data);
> +       nr_bridges++;
> +       rcu_assign_pointer(openvswitch_handle_frame_hook, rx_handler_data);
> +#else
>         if (dev->br_port)
>                 return -EBUSY;
>         rcu_assign_pointer(dev->br_port, rx_handler_data);
> +#endif
>         return 0;
>  }
>  static inline void netdev_rx_handler_unregister(struct net_device *dev)
>  {
> +#ifdef HAVE_RHEL_OVS_HOOK
> +       rcu_assign_pointer(dev->ax25_ptr, NULL);
> +
> +       if (--nr_bridges <= 0)
> +               rcu_assign_pointer(openvswitch_handle_frame_hook, NULL);
> +#else
>         rcu_assign_pointer(dev->br_port, NULL);
> +#endif
>  }
>  #endif
>
> diff --git a/datapath/linux/compat/netdevice.c b/datapath/linux/compat/netdevice.c
> index d26fb5e..f03efde 100644
> --- a/datapath/linux/compat/netdevice.c
> +++ b/datapath/linux/compat/netdevice.c
> @@ -1,6 +1,10 @@
>  #include <linux/netdevice.h>
>  #include <linux/if_vlan.h>
>
> +#ifdef HAVE_RHEL_OVS_HOOK
> +int nr_bridges = 0;
> +#endif
> +
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,38)
>  #ifndef HAVE_CAN_CHECKSUM_PROTOCOL
>  static bool can_checksum_protocol(unsigned long features, __be16 protocol)
> diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
> index fe7e359..06598fa 100644
> --- a/datapath/vport-netdev.c
> +++ b/datapath/vport-netdev.c
> @@ -45,12 +45,6 @@ MODULE_PARM_DESC(vlan_tso, "Enable TSO for VLAN packets");
>  #define vlan_tso true
>  #endif
>
> -#ifdef HAVE_RHEL_OVS_HOOK
> -static atomic_t nr_bridges = ATOMIC_INIT(0);
> -
> -extern struct sk_buff *(*openvswitch_handle_frame_hook)(struct sk_buff *skb);
> -#endif
> -
>  static void netdev_port_receive(struct vport *vport, struct sk_buff *skb);
>
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
> @@ -171,16 +165,10 @@ static struct vport *netdev_create(const struct vport_parms *parms)
>         }
>
>         rtnl_lock();
> -#ifdef HAVE_RHEL_OVS_HOOK
> -       rcu_assign_pointer(netdev_vport->dev->ax25_ptr, vport);
> -       atomic_inc(&nr_bridges);
> -       rcu_assign_pointer(openvswitch_handle_frame_hook, netdev_frame_hook);
> -#else
>         err = netdev_rx_handler_register(netdev_vport->dev, netdev_frame_hook,
>                                          vport);
>         if (err)
>                 goto error_unlock;
> -#endif
>
>         dev_set_promiscuity(netdev_vport->dev, 1);
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,24)
> @@ -192,9 +180,7 @@ static struct vport *netdev_create(const struct vport_parms *parms)
>         netdev_init();
>         return vport;
>
> -#ifndef HAVE_RHEL_OVS_HOOK
>  error_unlock:
> -#endif
>         rtnl_unlock();
>  error_put:
>         dev_put(netdev_vport->dev);
> @@ -209,12 +195,6 @@ static void free_port_rcu(struct rcu_head *rcu)
>         struct netdev_vport *netdev_vport = container_of(rcu,
>                                         struct netdev_vport, rcu);
>
> -#ifdef HAVE_RHEL_OVS_HOOK
> -       rcu_assign_pointer(netdev_vport->dev->ax25_ptr, NULL);
> -
> -       if (atomic_dec_and_test(&nr_bridges))
> -               rcu_assign_pointer(openvswitch_handle_frame_hook, NULL);
> -#endif
>         dev_put(netdev_vport->dev);
>         ovs_vport_free(vport_from_priv(netdev_vport));
>  }
> --
> 1.8.3.1
>



More information about the dev mailing list