| Message ID | 1505730091-19042-3-git-send-email-antonio.fischetti@intel.com |
|---|---|
| State | Deferred |
| Delegated to: | Darrell Ball |
| Headers | show |
| Series | [ovs-dev,1/5] conntrack: add commands to r/w conntrack parameters. | expand |
Hi Antonio
What is the motivation for this ?
I don’t think this is a good idea, as it should not be needed under normal
usage and has the potential to create unnecessary issues for the user and
also maintenance issues.
Thanks Darrell
On 9/18/17, 3:23 AM, "ovs-dev-bounces@openvswitch.org on behalf of antonio.fischetti@intel.com" <ovs-dev-bounces@openvswitch.org on behalf of antonio.fischetti@intel.com> wrote:
Read/Write conntrack clean-up interval used by
the clean_thread_main() thread.
Example:
ovs-appctl dpctl/ct-set cleanup=4000 # Set a new value
ovs-appctl dpctl/ct-get cleanup # Read
Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>
---
lib/conntrack.c | 27 ++++++++++++++++++++++++---
lib/conntrack.h | 2 ++
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 6d86625..60eb376 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -225,6 +225,9 @@ conn_key_cmp(const struct conn_key *key1, const struct conn_key *key2)
return 1;
}
+#define CT_CLEAN_INTERVAL 5000 /* 5 seconds */
+#define CT_CLEAN_MIN_INTERVAL 200 /* 0.2 seconds */
+
/* Initializes the connection tracker 'ct'. The caller is responsible for
* calling 'conntrack_destroy()', when the instance is not needed anymore */
void
@@ -258,6 +261,7 @@ conntrack_init(struct conntrack *ct)
ct->hash_basis = random_uint32();
atomic_count_init(&ct->n_conn, 0);
atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT);
+ ct->clean_interval = CT_CLEAN_INTERVAL;
latch_init(&ct->clean_thread_exit);
ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, ct);
}
@@ -1327,8 +1331,6 @@ next_bucket:
* behind, there is at least some 200ms blocks of time when buckets will be
* left alone, so the datapath can operate unhindered.
*/
-#define CT_CLEAN_INTERVAL 5000 /* 5 seconds */
-#define CT_CLEAN_MIN_INTERVAL 200 /* 0.2 seconds */
static void *
clean_thread_main(void *f_)
@@ -1344,7 +1346,7 @@ clean_thread_main(void *f_)
if (next_wake < now) {
poll_timer_wait_until(now + CT_CLEAN_MIN_INTERVAL);
} else {
- poll_timer_wait_until(MAX(next_wake, now + CT_CLEAN_INTERVAL));
+ poll_timer_wait_until(MAX(next_wake, now + ct->clean_interval));
}
latch_wait(&ct->clean_thread_exit);
poll_block();
@@ -2398,6 +2400,21 @@ conntrack_flush(struct conntrack *ct, const uint16_t *zone)
return 0;
}
+/* Set an interval value to be used by clean_thread_main. */
+static int
+wr_clean_int(struct conntrack *ct, uint32_t new_val) {
+ ct->clean_interval = new_val;
+ VLOG_DBG("Set clean interval to %d", new_val);
+ return 0;
+}
+
+/* Read current clean-up interval used by clean_thread_main. */
+static int
+rd_clean_int(struct conntrack *ct, uint32_t *cur_val) {
+ *cur_val = ct->clean_interval;
+ return 0;
+}
+
/* Set a new value for the upper limit of connections. */
static int
wr_max_conn(struct conntrack *ct, uint32_t new_val) {
@@ -2414,11 +2431,15 @@ rd_max_conn(struct conntrack *ct, uint32_t *cur_val) {
}
/* List of managed parameters. */
+/* Max nr of connections managed by CT module. */
#define CT_RW_MAX_CONN "maxconn"
+/* Clean-up interval used by clean_thread_main() thread. */
+#define CT_RW_CLEAN_INTERVAL "cleanup"
/* List of parameters that can be read/written at run-time. */
struct ct_wk_params wk_params[] = {
{CT_RW_MAX_CONN, wr_max_conn, rd_max_conn},
+ {CT_RW_CLEAN_INTERVAL, wr_clean_int, rd_clean_int},
};
int
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 4eb9a9a..ba9d3f1 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -261,6 +261,8 @@ struct conntrack {
pthread_t clean_thread;
/* Latch to destroy the 'clean_thread' */
struct latch clean_thread_exit;
+ /* Clean interval. */
+ uint32_t clean_interval;
/* Number of connections currently in the connection tracker. */
atomic_count n_conn;
--
2.4.11
_______________________________________________
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=KufLQwhfzaOAv5qQm_yEbJGussIlaYUvadeEqpKvDwQ&s=q6sQTxV7hxR1iqdLjnyaom-K3njxys4KHag_KG8uoHM&e=
> -----Original Message----- > From: Darrell Ball [mailto:dball@vmware.com] > Sent: Tuesday, September 19, 2017 9:28 PM > To: Fischetti, Antonio <antonio.fischetti@intel.com>; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH 3/5] conntrack: r/w clean-up interval. > > Hi Antonio > > What is the motivation for this ? > I don’t think this is a good idea, as it should not be needed under normal > usage and has the potential to create unnecessary issues for the user and > also maintenance issues. [Antonio] Agree, this would be more for debugging/experimenting purposes. I will remove this patch from the series. BTW let me know if you think some other CT cfg parameters could be read/written - or just read - by this ct-set-glbl-cfg command. For example, the Alg expectation timeout? Thanks, Antonio > > Thanks Darrell > > On 9/18/17, 3:23 AM, "ovs-dev-bounces@openvswitch.org on behalf of > antonio.fischetti@intel.com" <ovs-dev-bounces@openvswitch.org on behalf of > antonio.fischetti@intel.com> wrote: > > Read/Write conntrack clean-up interval used by > the clean_thread_main() thread. > > Example: > ovs-appctl dpctl/ct-set cleanup=4000 # Set a new value > ovs-appctl dpctl/ct-get cleanup # Read > > Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> > --- > lib/conntrack.c | 27 ++++++++++++++++++++++++--- > lib/conntrack.h | 2 ++ > 2 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 6d86625..60eb376 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -225,6 +225,9 @@ conn_key_cmp(const struct conn_key *key1, const struct > conn_key *key2) > return 1; > } > > +#define CT_CLEAN_INTERVAL 5000 /* 5 seconds */ > +#define CT_CLEAN_MIN_INTERVAL 200 /* 0.2 seconds */ > + > /* Initializes the connection tracker 'ct'. The caller is responsible for > * calling 'conntrack_destroy()', when the instance is not needed anymore > */ > void > @@ -258,6 +261,7 @@ conntrack_init(struct conntrack *ct) > ct->hash_basis = random_uint32(); > atomic_count_init(&ct->n_conn, 0); > atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT); > + ct->clean_interval = CT_CLEAN_INTERVAL; > latch_init(&ct->clean_thread_exit); > ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, > ct); > } > @@ -1327,8 +1331,6 @@ next_bucket: > * behind, there is at least some 200ms blocks of time when buckets will > be > * left alone, so the datapath can operate unhindered. > */ > -#define CT_CLEAN_INTERVAL 5000 /* 5 seconds */ > -#define CT_CLEAN_MIN_INTERVAL 200 /* 0.2 seconds */ > > static void * > clean_thread_main(void *f_) > @@ -1344,7 +1346,7 @@ clean_thread_main(void *f_) > if (next_wake < now) { > poll_timer_wait_until(now + CT_CLEAN_MIN_INTERVAL); > } else { > - poll_timer_wait_until(MAX(next_wake, now + > CT_CLEAN_INTERVAL)); > + poll_timer_wait_until(MAX(next_wake, now + ct- > >clean_interval)); > } > latch_wait(&ct->clean_thread_exit); > poll_block(); > @@ -2398,6 +2400,21 @@ conntrack_flush(struct conntrack *ct, const uint16_t > *zone) > return 0; > } > > +/* Set an interval value to be used by clean_thread_main. */ > +static int > +wr_clean_int(struct conntrack *ct, uint32_t new_val) { > + ct->clean_interval = new_val; > + VLOG_DBG("Set clean interval to %d", new_val); > + return 0; > +} > + > +/* Read current clean-up interval used by clean_thread_main. */ > +static int > +rd_clean_int(struct conntrack *ct, uint32_t *cur_val) { > + *cur_val = ct->clean_interval; > + return 0; > +} > + > /* Set a new value for the upper limit of connections. */ > static int > wr_max_conn(struct conntrack *ct, uint32_t new_val) { > @@ -2414,11 +2431,15 @@ rd_max_conn(struct conntrack *ct, uint32_t > *cur_val) { > } > > /* List of managed parameters. */ > +/* Max nr of connections managed by CT module. */ > #define CT_RW_MAX_CONN "maxconn" > +/* Clean-up interval used by clean_thread_main() thread. */ > +#define CT_RW_CLEAN_INTERVAL "cleanup" > > /* List of parameters that can be read/written at run-time. */ > struct ct_wk_params wk_params[] = { > {CT_RW_MAX_CONN, wr_max_conn, rd_max_conn}, > + {CT_RW_CLEAN_INTERVAL, wr_clean_int, rd_clean_int}, > }; > > int > diff --git a/lib/conntrack.h b/lib/conntrack.h > index 4eb9a9a..ba9d3f1 100644 > --- a/lib/conntrack.h > +++ b/lib/conntrack.h > @@ -261,6 +261,8 @@ struct conntrack { > pthread_t clean_thread; > /* Latch to destroy the 'clean_thread' */ > struct latch clean_thread_exit; > + /* Clean interval. */ > + uint32_t clean_interval; > > /* Number of connections currently in the connection tracker. */ > atomic_count n_conn; > -- > 2.4.11 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__mail.openvswitch.org_mailman_listinfo_ovs- > 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih- > uZnsw&m=KufLQwhfzaOAv5qQm_yEbJGussIlaYUvadeEqpKvDwQ&s=q6sQTxV7hxR1iqdLjnyaom- > K3njxys4KHag_KG8uoHM&e= >
On 9/20/17, 11:33 AM, "Fischetti, Antonio" <antonio.fischetti@intel.com> wrote: > -----Original Message----- > From: Darrell Ball [mailto:dball@vmware.com] > Sent: Tuesday, September 19, 2017 9:28 PM > To: Fischetti, Antonio <antonio.fischetti@intel.com>; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH 3/5] conntrack: r/w clean-up interval. > > Hi Antonio > > What is the motivation for this ? > I don’t think this is a good idea, as it should not be needed under normal > usage and has the potential to create unnecessary issues for the user and > also maintenance issues. [Antonio] Agree, this would be more for debugging/experimenting purposes. I will remove this patch from the series. BTW let me know if you think some other CT cfg parameters could be read/written - or just read - by this ct-set-glbl-cfg command. For example, the Alg expectation timeout? [Darrell] I left the expectation timeout simple for now as the area can be dicey; timeouts are probably best left as read-only for now. Thanks, Antonio > > Thanks Darrell > > On 9/18/17, 3:23 AM, "ovs-dev-bounces@openvswitch.org on behalf of > antonio.fischetti@intel.com" <ovs-dev-bounces@openvswitch.org on behalf of > antonio.fischetti@intel.com> wrote: > > Read/Write conntrack clean-up interval used by > the clean_thread_main() thread. > > Example: > ovs-appctl dpctl/ct-set cleanup=4000 # Set a new value > ovs-appctl dpctl/ct-get cleanup # Read > > Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> > --- > lib/conntrack.c | 27 ++++++++++++++++++++++++--- > lib/conntrack.h | 2 ++ > 2 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 6d86625..60eb376 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -225,6 +225,9 @@ conn_key_cmp(const struct conn_key *key1, const struct > conn_key *key2) > return 1; > } > > +#define CT_CLEAN_INTERVAL 5000 /* 5 seconds */ > +#define CT_CLEAN_MIN_INTERVAL 200 /* 0.2 seconds */ > + > /* Initializes the connection tracker 'ct'. The caller is responsible for > * calling 'conntrack_destroy()', when the instance is not needed anymore > */ > void > @@ -258,6 +261,7 @@ conntrack_init(struct conntrack *ct) > ct->hash_basis = random_uint32(); > atomic_count_init(&ct->n_conn, 0); > atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT); > + ct->clean_interval = CT_CLEAN_INTERVAL; > latch_init(&ct->clean_thread_exit); > ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, > ct); > } > @@ -1327,8 +1331,6 @@ next_bucket: > * behind, there is at least some 200ms blocks of time when buckets will > be > * left alone, so the datapath can operate unhindered. > */ > -#define CT_CLEAN_INTERVAL 5000 /* 5 seconds */ > -#define CT_CLEAN_MIN_INTERVAL 200 /* 0.2 seconds */ > > static void * > clean_thread_main(void *f_) > @@ -1344,7 +1346,7 @@ clean_thread_main(void *f_) > if (next_wake < now) { > poll_timer_wait_until(now + CT_CLEAN_MIN_INTERVAL); > } else { > - poll_timer_wait_until(MAX(next_wake, now + > CT_CLEAN_INTERVAL)); > + poll_timer_wait_until(MAX(next_wake, now + ct- > >clean_interval)); > } > latch_wait(&ct->clean_thread_exit); > poll_block(); > @@ -2398,6 +2400,21 @@ conntrack_flush(struct conntrack *ct, const uint16_t > *zone) > return 0; > } > > +/* Set an interval value to be used by clean_thread_main. */ > +static int > +wr_clean_int(struct conntrack *ct, uint32_t new_val) { > + ct->clean_interval = new_val; > + VLOG_DBG("Set clean interval to %d", new_val); > + return 0; > +} > + > +/* Read current clean-up interval used by clean_thread_main. */ > +static int > +rd_clean_int(struct conntrack *ct, uint32_t *cur_val) { > + *cur_val = ct->clean_interval; > + return 0; > +} > + > /* Set a new value for the upper limit of connections. */ > static int > wr_max_conn(struct conntrack *ct, uint32_t new_val) { > @@ -2414,11 +2431,15 @@ rd_max_conn(struct conntrack *ct, uint32_t > *cur_val) { > } > > /* List of managed parameters. */ > +/* Max nr of connections managed by CT module. */ > #define CT_RW_MAX_CONN "maxconn" > +/* Clean-up interval used by clean_thread_main() thread. */ > +#define CT_RW_CLEAN_INTERVAL "cleanup" > > /* List of parameters that can be read/written at run-time. */ > struct ct_wk_params wk_params[] = { > {CT_RW_MAX_CONN, wr_max_conn, rd_max_conn}, > + {CT_RW_CLEAN_INTERVAL, wr_clean_int, rd_clean_int}, > }; > > int > diff --git a/lib/conntrack.h b/lib/conntrack.h > index 4eb9a9a..ba9d3f1 100644 > --- a/lib/conntrack.h > +++ b/lib/conntrack.h > @@ -261,6 +261,8 @@ struct conntrack { > pthread_t clean_thread; > /* Latch to destroy the 'clean_thread' */ > struct latch clean_thread_exit; > + /* Clean interval. */ > + uint32_t clean_interval; > > /* Number of connections currently in the connection tracker. */ > atomic_count n_conn; > -- > 2.4.11 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__mail.openvswitch.org_mailman_listinfo_ovs- > 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih- > uZnsw&m=KufLQwhfzaOAv5qQm_yEbJGussIlaYUvadeEqpKvDwQ&s=q6sQTxV7hxR1iqdLjnyaom- > K3njxys4KHag_KG8uoHM&e= >
diff --git a/lib/conntrack.c b/lib/conntrack.c index 6d86625..60eb376 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -225,6 +225,9 @@ conn_key_cmp(const struct conn_key *key1, const struct conn_key *key2) return 1; } +#define CT_CLEAN_INTERVAL 5000 /* 5 seconds */ +#define CT_CLEAN_MIN_INTERVAL 200 /* 0.2 seconds */ + /* Initializes the connection tracker 'ct'. The caller is responsible for * calling 'conntrack_destroy()', when the instance is not needed anymore */ void @@ -258,6 +261,7 @@ conntrack_init(struct conntrack *ct) ct->hash_basis = random_uint32(); atomic_count_init(&ct->n_conn, 0); atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT); + ct->clean_interval = CT_CLEAN_INTERVAL; latch_init(&ct->clean_thread_exit); ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, ct); } @@ -1327,8 +1331,6 @@ next_bucket: * behind, there is at least some 200ms blocks of time when buckets will be * left alone, so the datapath can operate unhindered. */ -#define CT_CLEAN_INTERVAL 5000 /* 5 seconds */ -#define CT_CLEAN_MIN_INTERVAL 200 /* 0.2 seconds */ static void * clean_thread_main(void *f_) @@ -1344,7 +1346,7 @@ clean_thread_main(void *f_) if (next_wake < now) { poll_timer_wait_until(now + CT_CLEAN_MIN_INTERVAL); } else { - poll_timer_wait_until(MAX(next_wake, now + CT_CLEAN_INTERVAL)); + poll_timer_wait_until(MAX(next_wake, now + ct->clean_interval)); } latch_wait(&ct->clean_thread_exit); poll_block(); @@ -2398,6 +2400,21 @@ conntrack_flush(struct conntrack *ct, const uint16_t *zone) return 0; } +/* Set an interval value to be used by clean_thread_main. */ +static int +wr_clean_int(struct conntrack *ct, uint32_t new_val) { + ct->clean_interval = new_val; + VLOG_DBG("Set clean interval to %d", new_val); + return 0; +} + +/* Read current clean-up interval used by clean_thread_main. */ +static int +rd_clean_int(struct conntrack *ct, uint32_t *cur_val) { + *cur_val = ct->clean_interval; + return 0; +} + /* Set a new value for the upper limit of connections. */ static int wr_max_conn(struct conntrack *ct, uint32_t new_val) { @@ -2414,11 +2431,15 @@ rd_max_conn(struct conntrack *ct, uint32_t *cur_val) { } /* List of managed parameters. */ +/* Max nr of connections managed by CT module. */ #define CT_RW_MAX_CONN "maxconn" +/* Clean-up interval used by clean_thread_main() thread. */ +#define CT_RW_CLEAN_INTERVAL "cleanup" /* List of parameters that can be read/written at run-time. */ struct ct_wk_params wk_params[] = { {CT_RW_MAX_CONN, wr_max_conn, rd_max_conn}, + {CT_RW_CLEAN_INTERVAL, wr_clean_int, rd_clean_int}, }; int diff --git a/lib/conntrack.h b/lib/conntrack.h index 4eb9a9a..ba9d3f1 100644 --- a/lib/conntrack.h +++ b/lib/conntrack.h @@ -261,6 +261,8 @@ struct conntrack { pthread_t clean_thread; /* Latch to destroy the 'clean_thread' */ struct latch clean_thread_exit; + /* Clean interval. */ + uint32_t clean_interval; /* Number of connections currently in the connection tracker. */ atomic_count n_conn;
Read/Write conntrack clean-up interval used by the clean_thread_main() thread. Example: ovs-appctl dpctl/ct-set cleanup=4000 # Set a new value ovs-appctl dpctl/ct-get cleanup # Read Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> --- lib/conntrack.c | 27 ++++++++++++++++++++++++--- lib/conntrack.h | 2 ++ 2 files changed, 26 insertions(+), 3 deletions(-)