On Fri, Nov 22, 2013 at 2:57 PM, Thomas Graf <tg...@redhat.com> wrote:
> On 11/22/2013 11:47 PM, Pravin Shelar wrote:
>>
>> 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.
>
>
> The following is also needed:
>
> commit 0d1d26db42b5d206ebb75cc6dc93507f4e5b6a8a
> Author: Chris Wright <chr...@sous-sol.org>
> Date: Tue Aug 13 18:02:48 2013 -0700
>
>
right. I pushed it too.
Thanks.
>
>>
>> On Tue, Jul 9, 2013 at 11:36 AM, Thomas Graf <tg...@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 <tg...@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
>>>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
