diff mbox series

[ovs-dev] conntrack-tp: fix lock order in conn_update_expiration

Message ID 20200705020417.33668-1-hepeng.0320@bytedance.com
State Superseded
Headers show
Series [ovs-dev] conntrack-tp: fix lock order in conn_update_expiration | expand

Commit Message

.贺鹏 July 5, 2020, 2:04 a.m. UTC
*conn_update_expiration* violates the lock order of conn->lock and
ct->lock. In the comments of conntrack, the conn->lock should be
held after ct->lock when ct->lock needs to be taken.

Signed-off-by: hepeng.0320 <hepeng.0320@bytedance.com>
---
 lib/conntrack-tp.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

William Tu July 5, 2020, 12:59 p.m. UTC | #1
On Sat, Jul 4, 2020 at 7:04 PM hepeng.0320 <hepeng.0320@bytedance.com> wrote:
>
> *conn_update_expiration* violates the lock order of conn->lock and
> ct->lock. In the comments of conntrack, the conn->lock should be
> held after ct->lock when ct->lock needs to be taken.
>
> Signed-off-by: hepeng.0320 <hepeng.0320@bytedance.com>
> ---
Please take a look at
http://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/#tags

And put your name after the Signed-off-by tag.
Thanks
William
diff mbox series

Patch

diff --git a/lib/conntrack-tp.c b/lib/conntrack-tp.c
index 3a7604c0d..a586d3a8d 100644
--- a/lib/conntrack-tp.c
+++ b/lib/conntrack-tp.c
@@ -260,15 +260,20 @@  conn_update_expiration(struct conntrack *ct, struct conn *conn,
     struct timeout_policy *tp;
     uint32_t val;
 
+    ovs_mutex_unlock(&conn->lock);
+
     ovs_mutex_lock(&ct->ct_lock);
+    ovs_mutex_lock(&conn->lock);
     tp = timeout_policy_lookup(ct, conn->tp_id);
     if (tp) {
         val = tp->policy.attrs[tm_to_ct_dpif_tp(tm)];
     } else {
         val = ct_dpif_netdev_tp_def[tm_to_ct_dpif_tp(tm)];
     }
+    ovs_mutex_unlock(&conn->lock);
     ovs_mutex_unlock(&ct->ct_lock);
 
+    ovs_mutex_lock(&conn->lock);
     VLOG_DBG_RL(&rl, "Update timeout %s zone=%u with policy id=%d "
                 "val=%u sec.",
                 ct_timeout_str[tm], conn->key.zone, conn->tp_id, val);