[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 9 Oct 2013 20:02:33 -0700
From: Pravin Shelar <pshelar@...ira.com>
To: Alexei Starovoitov <ast@...mgrid.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Jesse Gross <jesse@...ira.com>, Jiri Pirko <jiri@...nulli.us>,
"dev@...nvswitch.org" <dev@...nvswitch.org>,
netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] openvswitch: fix vport-netdev unregister
On Tue, Oct 8, 2013 at 8:07 PM, Alexei Starovoitov <ast@...mgrid.com> wrote:
> The combination of two commits
>
> commit 8e4e1713e4
> ("openvswitch: Simplify datapath locking.")
>
> and
>
> commit 2537b4dd0a
> ("openvswitch:: link upper device for port devices")
>
> introduced a bug where upper_dev wasn't unlinked upon
> netdev_unregister notification
>
> The following steps:
>
> modprobe openvswitch
> ovs-dpctl add-dp test
> ip tuntap add dev tap1 mode tap
> ovs-dpctl add-if test tap1
> ip tuntap del dev tap1 mode tap
>
> are causing multiple warnings:
>
> [ 62.747557] gre: GRE over IPv4 demultiplexor driver
> [ 62.749579] openvswitch: Open vSwitch switching datapath
> [ 62.755087] device test entered promiscuous mode
> [ 62.765911] device tap1 entered promiscuous mode
> [ 62.766033] IPv6: ADDRCONF(NETDEV_UP): tap1: link is not ready
> [ 62.769017] ------------[ cut here ]------------
> [ 62.769022] WARNING: CPU: 1 PID: 3267 at net/core/dev.c:5501 rollback_registered_many+0x20f/0x240()
> [ 62.769023] Modules linked in: openvswitch gre vxlan ip_tunnel libcrc32c ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack xt_CHECKSUM iptable_mangle ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc vhost_net macvtap macvlan vhost kvm_intel kvm dm_crypt iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi hid_generic mxm_wmi eeepc_wmi asus_wmi sparse_keymap dm_multipath psmouse serio_raw usbhid hid parport_pc ppdev firewire_ohci lpc_ich firewire_core e1000e crc_itu_t binfmt_misc igb dca ptp pps_core mac_hid wmi lp parport i2o_config i2o_block video
> [ 62.769051] CPU: 1 PID: 3267 Comm: ip Not tainted 3.12.0-rc3+ #60
> [ 62.769052] Hardware name: System manufacturer System Product Name/P8Z77 WS, BIOS 3007 07/26/2012
> [ 62.769053] 0000000000000009 ffff8807f25cbd28 ffffffff8175e575 0000000000000006
> [ 62.769055] 0000000000000000 ffff8807f25cbd68 ffffffff8105314c ffff8807f25cbd58
> [ 62.769057] ffff8807f2634000 ffff8807f25cbdc8 ffff8807f25cbd88 ffff8807f25cbdc8
> [ 62.769059] Call Trace:
> [ 62.769062] [<ffffffff8175e575>] dump_stack+0x55/0x76
> [ 62.769065] [<ffffffff8105314c>] warn_slowpath_common+0x8c/0xc0
> [ 62.769067] [<ffffffff8105319a>] warn_slowpath_null+0x1a/0x20
> [ 62.769069] [<ffffffff8162a04f>] rollback_registered_many+0x20f/0x240
> [ 62.769071] [<ffffffff8162a101>] rollback_registered+0x31/0x40
> [ 62.769073] [<ffffffff8162a488>] unregister_netdevice_queue+0x58/0x90
> [ 62.769075] [<ffffffff8154f900>] __tun_detach+0x140/0x340
> [ 62.769077] [<ffffffff8154fb36>] tun_chr_close+0x36/0x60
> [ 62.769080] [<ffffffff811bddaf>] __fput+0xff/0x260
> [ 62.769082] [<ffffffff811bdf5e>] ____fput+0xe/0x10
> [ 62.769084] [<ffffffff8107b515>] task_work_run+0xb5/0xe0
> [ 62.769087] [<ffffffff810029b9>] do_notify_resume+0x59/0x80
> [ 62.769089] [<ffffffff813a41fe>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [ 62.769091] [<ffffffff81770f5a>] int_signal+0x12/0x17
> [ 62.769093] ---[ end trace 838756c62e156ffb ]---
> [ 62.769481] ------------[ cut here ]------------
> [ 62.769485] WARNING: CPU: 1 PID: 92 at fs/sysfs/inode.c:325 sysfs_hash_and_remove+0xa9/0xb0()
> [ 62.769486] sysfs: can not remove 'master', no directory
> [ 62.769486] Modules linked in: openvswitch gre vxlan ip_tunnel libcrc32c ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack xt_CHECKSUM iptable_mangle ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc vhost_net macvtap macvlan vhost kvm_intel kvm dm_crypt iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi hid_generic mxm_wmi eeepc_wmi asus_wmi sparse_keymap dm_multipath psmouse serio_raw usbhid hid parport_pc ppdev firewire_ohci lpc_ich firewire_core e1000e crc_itu_t binfmt_misc igb dca ptp pps_core mac_hid wmi lp parport i2o_config i2o_block video
> [ 62.769514] CPU: 1 PID: 92 Comm: kworker/1:2 Tainted: G W 3.12.0-rc3+ #60
> [ 62.769515] Hardware name: System manufacturer System Product Name/P8Z77 WS, BIOS 3007 07/26/2012
> [ 62.769518] Workqueue: events ovs_dp_notify_wq [openvswitch]
> [ 62.769519] 0000000000000009 ffff880807ad3ac8 ffffffff8175e575 0000000000000006
> [ 62.769521] ffff880807ad3b18 ffff880807ad3b08 ffffffff8105314c ffff880807ad3b28
> [ 62.769523] 0000000000000000 ffffffff81a87a1f ffff8807f2634000 ffff880037038500
> [ 62.769525] Call Trace:
> [ 62.769528] [<ffffffff8175e575>] dump_stack+0x55/0x76
> [ 62.769529] [<ffffffff8105314c>] warn_slowpath_common+0x8c/0xc0
> [ 62.769531] [<ffffffff81053236>] warn_slowpath_fmt+0x46/0x50
> [ 62.769533] [<ffffffff8123e7e9>] sysfs_hash_and_remove+0xa9/0xb0
> [ 62.769535] [<ffffffff81240e96>] sysfs_remove_link+0x26/0x30
> [ 62.769538] [<ffffffff81631ef7>] __netdev_adjacent_dev_remove+0xf7/0x150
> [ 62.769540] [<ffffffff81632037>] __netdev_adjacent_dev_unlink_lists+0x27/0x50
> [ 62.769542] [<ffffffff8163213a>] __netdev_adjacent_dev_unlink_neighbour+0x3a/0x50
> [ 62.769544] [<ffffffff8163218d>] netdev_upper_dev_unlink+0x3d/0x140
> [ 62.769548] [<ffffffffa033c2db>] netdev_destroy+0x4b/0x80 [openvswitch]
> [ 62.769550] [<ffffffffa033b696>] ovs_vport_del+0x46/0x60 [openvswitch]
> [ 62.769552] [<ffffffffa0335314>] ovs_dp_detach_port+0x44/0x60 [openvswitch]
> [ 62.769555] [<ffffffffa0336574>] ovs_dp_notify_wq+0xb4/0x150 [openvswitch]
> [ 62.769557] [<ffffffff81075c28>] process_one_work+0x1d8/0x6a0
> [ 62.769559] [<ffffffff81075bc8>] ? process_one_work+0x178/0x6a0
> [ 62.769562] [<ffffffff8107659b>] worker_thread+0x11b/0x370
> [ 62.769564] [<ffffffff81076480>] ? rescuer_thread+0x350/0x350
> [ 62.769566] [<ffffffff8107f44a>] kthread+0xea/0xf0
> [ 62.769568] [<ffffffff8107f360>] ? flush_kthread_worker+0x150/0x150
> [ 62.769570] [<ffffffff81770bac>] ret_from_fork+0x7c/0xb0
> [ 62.769572] [<ffffffff8107f360>] ? flush_kthread_worker+0x150/0x150
> [ 62.769573] ---[ end trace 838756c62e156ffc ]---
> [ 62.769574] ------------[ cut here ]------------
> [ 62.769576] WARNING: CPU: 1 PID: 92 at fs/sysfs/inode.c:325 sysfs_hash_and_remove+0xa9/0xb0()
> [ 62.769577] sysfs: can not remove 'upper_test', no directory
> [ 62.769577] Modules linked in: openvswitch gre vxlan ip_tunnel libcrc32c ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack xt_CHECKSUM iptable_mangle ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc vhost_net macvtap macvlan vhost kvm_intel kvm dm_crypt iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi hid_generic mxm_wmi eeepc_wmi asus_wmi sparse_keymap dm_multipath psmouse serio_raw usbhid hid parport_pc ppdev firewire_ohci lpc_ich firewire_core e1000e crc_itu_t binfmt_misc igb dca ptp pps_core mac_hid wmi lp parport i2o_config i2o_block video
> [ 62.769603] CPU: 1 PID: 92 Comm: kworker/1:2 Tainted: G W 3.12.0-rc3+ #60
> [ 62.769604] Hardware name: System manufacturer System Product Name/P8Z77 WS, BIOS 3007 07/26/2012
> [ 62.769606] Workqueue: events ovs_dp_notify_wq [openvswitch]
> [ 62.769607] 0000000000000009 ffff880807ad3ac8 ffffffff8175e575 0000000000000006
> [ 62.769609] ffff880807ad3b18 ffff880807ad3b08 ffffffff8105314c ffff880807ad3b58
> [ 62.769611] 0000000000000000 ffff880807ad3bd9 ffff8807f2634000 ffff880037038500
> [ 62.769613] Call Trace:
> [ 62.769615] [<ffffffff8175e575>] dump_stack+0x55/0x76
> [ 62.769617] [<ffffffff8105314c>] warn_slowpath_common+0x8c/0xc0
> [ 62.769619] [<ffffffff81053236>] warn_slowpath_fmt+0x46/0x50
> [ 62.769621] [<ffffffff8123e7e9>] sysfs_hash_and_remove+0xa9/0xb0
> [ 62.769622] [<ffffffff81240e96>] sysfs_remove_link+0x26/0x30
> [ 62.769624] [<ffffffff81631f22>] __netdev_adjacent_dev_remove+0x122/0x150
> [ 62.769627] [<ffffffff81632037>] __netdev_adjacent_dev_unlink_lists+0x27/0x50
> [ 62.769629] [<ffffffff8163213a>] __netdev_adjacent_dev_unlink_neighbour+0x3a/0x50
> [ 62.769631] [<ffffffff8163218d>] netdev_upper_dev_unlink+0x3d/0x140
> [ 62.769633] [<ffffffffa033c2db>] netdev_destroy+0x4b/0x80 [openvswitch]
> [ 62.769636] [<ffffffffa033b696>] ovs_vport_del+0x46/0x60 [openvswitch]
> [ 62.769638] [<ffffffffa0335314>] ovs_dp_detach_port+0x44/0x60 [openvswitch]
> [ 62.769640] [<ffffffffa0336574>] ovs_dp_notify_wq+0xb4/0x150 [openvswitch]
> [ 62.769642] [<ffffffff81075c28>] process_one_work+0x1d8/0x6a0
> [ 62.769644] [<ffffffff81075bc8>] ? process_one_work+0x178/0x6a0
> [ 62.769646] [<ffffffff8107659b>] worker_thread+0x11b/0x370
> [ 62.769648] [<ffffffff81076480>] ? rescuer_thread+0x350/0x350
> [ 62.769650] [<ffffffff8107f44a>] kthread+0xea/0xf0
> [ 62.769652] [<ffffffff8107f360>] ? flush_kthread_worker+0x150/0x150
> [ 62.769654] [<ffffffff81770bac>] ret_from_fork+0x7c/0xb0
> [ 62.769656] [<ffffffff8107f360>] ? flush_kthread_worker+0x150/0x150
> [ 62.769657] ---[ end trace 838756c62e156ffd ]---
> [ 62.769724] device tap1 left promiscuous mode
>
> Signed-off-by: Alexei Starovoitov <ast@...mgrid.com>
> ---
> net/openvswitch/dp_notify.c | 5 +++++
> net/openvswitch/vport-netdev.c | 16 +++++++++++++---
> net/openvswitch/vport-netdev.h | 1 +
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/net/openvswitch/dp_notify.c b/net/openvswitch/dp_notify.c
> index c323567..e9380bd 100644
> --- a/net/openvswitch/dp_notify.c
> +++ b/net/openvswitch/dp_notify.c
> @@ -88,6 +88,11 @@ static int dp_device_event(struct notifier_block *unused, unsigned long event,
> return NOTIFY_DONE;
>
> if (event == NETDEV_UNREGISTER) {
> + /* rx_handler_unregister and upper_dev_unlink immediately */
> + if (dev->reg_state == NETREG_UNREGISTERING)
> + ovs_netdev_unlink_dev(vport);
> +
Rather than doing vport destroy here, we can just unlink upper device
and let workq do rest of work.
Thanks.
> + /* schedule vport destroy, dev_put and genl notification */
> ovs_net = net_generic(dev_net(dev), ovs_net_id);
> queue_work(system_wq, &ovs_net->dp_notify_work);
> }
> diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
> index 09d93c1..cce933a 100644
> --- a/net/openvswitch/vport-netdev.c
> +++ b/net/openvswitch/vport-netdev.c
> @@ -150,15 +150,25 @@ static void free_port_rcu(struct rcu_head *rcu)
> ovs_vport_free(vport_from_priv(netdev_vport));
> }
>
> -static void netdev_destroy(struct vport *vport)
> +void ovs_netdev_unlink_dev(struct vport *vport)
> {
> struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
>
> - rtnl_lock();
> + ASSERT_RTNL();
> netdev_vport->dev->priv_flags &= ~IFF_OVS_DATAPATH;
> netdev_rx_handler_unregister(netdev_vport->dev);
> - netdev_upper_dev_unlink(netdev_vport->dev, get_dpdev(vport->dp));
> + netdev_upper_dev_unlink(netdev_vport->dev,
> + netdev_master_upper_dev_get(netdev_vport->dev));
> dev_set_promiscuity(netdev_vport->dev, -1);
> +}
> +
> +static void netdev_destroy(struct vport *vport)
> +{
> + struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
> +
> + rtnl_lock();
> + if (netdev_vport->dev->reg_state != NETREG_UNREGISTERING)
> + ovs_netdev_unlink_dev(vport);
> rtnl_unlock();
>
> call_rcu(&netdev_vport->rcu, free_port_rcu);
> diff --git a/net/openvswitch/vport-netdev.h b/net/openvswitch/vport-netdev.h
> index dd298b5..21e3770 100644
> --- a/net/openvswitch/vport-netdev.h
> +++ b/net/openvswitch/vport-netdev.h
> @@ -39,5 +39,6 @@ netdev_vport_priv(const struct vport *vport)
> }
>
> const char *ovs_netdev_get_name(const struct vport *);
> +void ovs_netdev_unlink_dev(struct vport *);
>
> #endif /* vport_netdev.h */
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists