diff mbox series

[ovs-dev,2/2] datapath: ovs_ct_exit to be done under ovs_lock

Message ID 20201021164940.19894-3-gvrose8192@gmail.com
State Accepted
Headers show
Series Backport upstream lockdep bug fix | expand

Commit Message

Gregory Rose Oct. 21, 2020, 4:49 p.m. UTC
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Upstream commit:
    commit 27de77cec985233bdf6546437b9761853265c505
    Author: Tonghao Zhang <xiangxia.m.yue@gmail.com>
    Date:   Fri Apr 17 02:57:31 2020 +0800

    net: openvswitch: ovs_ct_exit to be done under ovs_lock

    syzbot wrote:
    | =============================
    | WARNING: suspicious RCU usage
    | 5.7.0-rc1+ #45 Not tainted
    | -----------------------------
    | net/openvswitch/conntrack.c:1898 RCU-list traversed in non-reader section!!
    |
    | other info that might help us debug this:
    | rcu_scheduler_active = 2, debug_locks = 1
    | ...
    |
    | stack backtrace:
    | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-0-ga698c8995f-prebuilt.qemu.org 04/01/2014
    | Workqueue: netns cleanup_net
    | Call Trace:
    | ...
    | ovs_ct_exit
    | ovs_exit_net
    | ops_exit_list.isra.7
    | cleanup_net
    | process_one_work
    | worker_thread

    To avoid that warning, invoke the ovs_ct_exit under ovs_lock and add
    lockdep_ovsl_is_held as optional lockdep expression.

    Link: https://lore.kernel.org/lkml/000000000000e642a905a0cbee6e@google.com
    Fixes: 11efd5cb04a1 ("openvswitch: Support conntrack zone limit")
    Cc: Pravin B Shelar <pshelar@ovn.org>
    Cc: Yi-Hung Wei <yihung.wei@gmail.com>
    Reported-by: syzbot+7ef50afd3a211f879112@syzkaller.appspotmail.com
    Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
    Acked-by: Pravin B Shelar <pshelar@ovn.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Fixes: cb2a5486a3a3 ("datapath: conntrack: Support conntrack zone limit")
Signed-off-by: Greg Rose <gvrose8192@gmail.com>
---
 datapath/conntrack.c | 3 ++-
 datapath/datapath.c  | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

0-day Robot Oct. 21, 2020, 5:03 p.m. UTC | #1
Bleep bloop.  Greetings Greg Rose, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Tonghao Zhang <xiangxia.m.yue@gmail.com> needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Greg Rose <gvrose8192@gmail.com>
Lines checked: 88, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Tonghao Zhang Oct. 23, 2020, 1:41 a.m. UTC | #2
On Thu, Oct 22, 2020 at 12:49 AM Greg Rose <gvrose8192@gmail.com> wrote:
>
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Upstream commit:
>     commit 27de77cec985233bdf6546437b9761853265c505
>     Author: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>     Date:   Fri Apr 17 02:57:31 2020 +0800
>
>     net: openvswitch: ovs_ct_exit to be done under ovs_lock
>
>     syzbot wrote:
>     | =============================
>     | WARNING: suspicious RCU usage
>     | 5.7.0-rc1+ #45 Not tainted
>     | -----------------------------
>     | net/openvswitch/conntrack.c:1898 RCU-list traversed in non-reader section!!
>     |
>     | other info that might help us debug this:
>     | rcu_scheduler_active = 2, debug_locks = 1
>     | ...
>     |
>     | stack backtrace:
>     | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-0-ga698c8995f-prebuilt.qemu.org 04/01/2014
>     | Workqueue: netns cleanup_net
>     | Call Trace:
>     | ...
>     | ovs_ct_exit
>     | ovs_exit_net
>     | ops_exit_list.isra.7
>     | cleanup_net
>     | process_one_work
>     | worker_thread
>
>     To avoid that warning, invoke the ovs_ct_exit under ovs_lock and add
>     lockdep_ovsl_is_held as optional lockdep expression.
>
>     Link: https://lore.kernel.org/lkml/000000000000e642a905a0cbee6e@google.com
>     Fixes: 11efd5cb04a1 ("openvswitch: Support conntrack zone limit")
>     Cc: Pravin B Shelar <pshelar@ovn.org>
>     Cc: Yi-Hung Wei <yihung.wei@gmail.com>
>     Reported-by: syzbot+7ef50afd3a211f879112@syzkaller.appspotmail.com
>     Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>     Acked-by: Pravin B Shelar <pshelar@ovn.org>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Fixes: cb2a5486a3a3 ("datapath: conntrack: Support conntrack zone limit")
> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
Acked-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Thanks Greg

> ---
>  datapath/conntrack.c | 3 ++-
>  datapath/datapath.c  | 4 +++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> index c7a318baf..50b4d7bd6 100644
> --- a/datapath/conntrack.c
> +++ b/datapath/conntrack.c
> @@ -1984,7 +1984,8 @@ static void ovs_ct_limit_exit(struct net *net, struct ovs_net *ovs_net)
>                 struct hlist_head *head = &info->limits[i];
>                 struct ovs_ct_limit *ct_limit;
>
> -               hlist_for_each_entry_rcu(ct_limit, head, hlist_node)
> +               hlist_for_each_entry_rcu(ct_limit, head, hlist_node,
> +                                        lockdep_ovsl_is_held())
>                         kfree_rcu(ct_limit, rcu);
>         }
>         kfree(ovs_net->ct_limit_info->limits);
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 52a59f135..97ba5fb72 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -2576,8 +2576,10 @@ static void __net_exit ovs_exit_net(struct net *dnet)
>
>         ovs_netns_frags6_exit(dnet);
>         ovs_netns_frags_exit(dnet);
> -       ovs_ct_exit(dnet);
>         ovs_lock();
> +
> +       ovs_ct_exit(dnet);
> +
>         list_for_each_entry_safe(dp, dp_next, &ovs_net->dps, list_node)
>                 __dp_destroy(dp);
>
> --
> 2.17.1
>
diff mbox series

Patch

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index c7a318baf..50b4d7bd6 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -1984,7 +1984,8 @@  static void ovs_ct_limit_exit(struct net *net, struct ovs_net *ovs_net)
 		struct hlist_head *head = &info->limits[i];
 		struct ovs_ct_limit *ct_limit;
 
-		hlist_for_each_entry_rcu(ct_limit, head, hlist_node)
+		hlist_for_each_entry_rcu(ct_limit, head, hlist_node,
+					 lockdep_ovsl_is_held())
 			kfree_rcu(ct_limit, rcu);
 	}
 	kfree(ovs_net->ct_limit_info->limits);
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 52a59f135..97ba5fb72 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -2576,8 +2576,10 @@  static void __net_exit ovs_exit_net(struct net *dnet)
 
 	ovs_netns_frags6_exit(dnet);
 	ovs_netns_frags_exit(dnet);
-	ovs_ct_exit(dnet);
 	ovs_lock();
+
+	ovs_ct_exit(dnet);
+
 	list_for_each_entry_safe(dp, dp_next, &ovs_net->dps, list_node)
 		__dp_destroy(dp);