diff mbox series

[ovs-dev,v2] conntrack: Fix minimum connections to clean.

Message ID 1553896747-19775-1-git-send-email-dlu998@gmail.com
State Accepted
Commit c61a85f6c3fc22888ce73142a5aaa3f5ade83678
Headers show
Series [ovs-dev,v2] conntrack: Fix minimum connections to clean. | expand

Commit Message

Darrell Ball March 29, 2019, 9:59 p.m. UTC
If there is low maximum connection count configuration and less than 10
connections in a bucket, the calculation of the maximum number of
connections to clean for the bucket could be zero, leading to these
connections not being cleaned until and if the connection count in the
bucket increases.

Fix this by checking for low maximum connection count configuration
and do this outside of the buckets loop, thereby simplifying the loop.

Fixes: e6ef6cc6349b ("conntrack: Periodically delete expired connections.")
CC: Daniele Di Proietto <diproiettod@ovn.org>
Reported-by: Liujiaxin <liujiaxin.2019@bytedance.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-March/357703.html
Signed-off-by: Darrell Ball <dlu998@gmail.com>
---

v2: Fix local variable naming.

Backport to 2.6.

 lib/conntrack.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Ben Pfaff April 16, 2019, 8:45 p.m. UTC | #1
On Fri, Mar 29, 2019 at 02:59:07PM -0700, Darrell Ball wrote:
> If there is low maximum connection count configuration and less than 10
> connections in a bucket, the calculation of the maximum number of
> connections to clean for the bucket could be zero, leading to these
> connections not being cleaned until and if the connection count in the
> bucket increases.
> 
> Fix this by checking for low maximum connection count configuration
> and do this outside of the buckets loop, thereby simplifying the loop.
> 
> Fixes: e6ef6cc6349b ("conntrack: Periodically delete expired connections.")
> CC: Daniele Di Proietto <diproiettod@ovn.org>
> Reported-by: Liujiaxin <liujiaxin.2019@bytedance.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-March/357703.html
> Signed-off-by: Darrell Ball <dlu998@gmail.com>
> ---
> 
> v2: Fix local variable naming.
> 
> Backport to 2.6.

Thanks, applied to master and backported.
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 9c2abd1..da2dd1f 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1489,6 +1489,8 @@  conntrack_clean(struct conntrack *ct, long long now)
     size_t clean_count = 0;
 
     atomic_read_relaxed(&ct->n_conn_limit, &n_conn_limit);
+    size_t clean_max = n_conn_limit > CONNTRACK_BUCKETS * 10
+        ? n_conn_limit / (CONNTRACK_BUCKETS * 10) : 1;
 
     for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) {
         struct conntrack_bucket *ctb = &ct->buckets[i];
@@ -1506,8 +1508,7 @@  conntrack_clean(struct conntrack *ct, long long now)
          * limit to 10% of the global limit equally split among buckets. If
          * the bucket is busier than the others, we limit to 10% of its
          * current size. */
-        min_exp = sweep_bucket(ct, ctb, now,
-                MAX(prev_count/10, n_conn_limit/(CONNTRACK_BUCKETS*10)));
+        min_exp = sweep_bucket(ct, ctb, now, MAX(prev_count / 10, clean_max));
         clean_count += prev_count - hmap_count(&ctb->connections);
 
         if (min_exp > now) {