diff mbox series

[ovs-dev,v3] ovs-dpctl: Add new command dpctl/ct-[sg]et-sweep-interval.

Message ID 168077582293.1427477.13969080845416442479.stgit@fed.void
State Accepted
Headers show
Series [ovs-dev,v3] ovs-dpctl: Add new command dpctl/ct-[sg]et-sweep-interval. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Paolo Valerio April 6, 2023, 10:10 a.m. UTC
Since 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists
with rculists.") the sweep interval changed as well as the constraints
related to the sweeper.
Being able to change the default reschedule time may be convenient in
some conditions, like debugging.
This patch introduces new commands allowing to get and set the sweep
interval in ms.

Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
---
v3:
- rebased on top of the current master
- renamed commands to dpctl/ct-[sg]et-sweep-interval (Ilya)
- added simple get/set test in ofproto-dpif.at (Ilya)

v2:
- resolved conflict in NEWS
- added missing comment
- added missing '\' in dpctl.man
---
 NEWS                    |    3 ++
 lib/conntrack-private.h |    1 +
 lib/conntrack.c         |   18 +++++++++++++-
 lib/conntrack.h         |    2 ++
 lib/ct-dpif.c           |   14 +++++++++++
 lib/ct-dpif.h           |    1 +
 lib/dpctl.c             |   61 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/dpctl.man           |    9 +++++++
 lib/dpif-netdev.c       |   17 +++++++++++++
 lib/dpif-netlink.c      |    2 ++
 lib/dpif-provider.h     |    4 +++
 tests/ofproto-dpif.at   |   22 +++++++++++++++++
 12 files changed, 153 insertions(+), 1 deletion(-)

Comments

Simon Horman April 6, 2023, 1:17 p.m. UTC | #1
On Thu, Apr 06, 2023 at 12:10:22PM +0200, Paolo Valerio wrote:
> Since 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists
> with rculists.") the sweep interval changed as well as the constraints
> related to the sweeper.
> Being able to change the default reschedule time may be convenient in
> some conditions, like debugging.
> This patch introduces new commands allowing to get and set the sweep
> interval in ms.
> 
> Signed-off-by: Paolo Valerio <pvalerio@redhat.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Ilya Maximets April 6, 2023, 10:07 p.m. UTC | #2
On 4/6/23 15:17, Simon Horman wrote:
> On Thu, Apr 06, 2023 at 12:10:22PM +0200, Paolo Valerio wrote:
>> Since 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists
>> with rculists.") the sweep interval changed as well as the constraints
>> related to the sweeper.
>> Being able to change the default reschedule time may be convenient in
>> some conditions, like debugging.
>> This patch introduces new commands allowing to get and set the sweep
>> interval in ms.
>>
>> Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
> 
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> 

Applied.  Thanks!

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index b6418c36e..1155bfbb1 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@  Post-v3.1.0
    - ovs-appctl:
      * Add support for selecting the source address with the
        'ovs-appctl ovs/route/add' command.
+     * New commands "dpctl/{ct-get-sweep-interval,ct-set-sweep-interval}" that
+       allow to get and set, for the userspace datapath, the sweep interval
+       for the conntrack garbage collector.
    - ovs-ctl:
      * Added new options --[ovsdb-server|ovs-vswitchd]-umask=MODE to set umask
        value when starting OVS daemons.  E.g., use --ovsdb-server-umask=0002
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index fae8b3a9b..bb326868e 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -224,6 +224,7 @@  struct conntrack {
     struct ipf *ipf; /* Fragmentation handling context. */
     uint32_t zone_limit_seq; /* Used to disambiguate zone limit counts. */
     atomic_bool tcp_seq_chk; /* Check TCP sequence numbers. */
+    atomic_uint32_t sweep_ms; /* Next sweep interval. */
 };
 
 /* Lock acquisition order:
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f86fa26f4..ce8a63de5 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -320,6 +320,7 @@  conntrack_init(void)
     atomic_count_init(&ct->n_conn, 0);
     atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT);
     atomic_init(&ct->tcp_seq_chk, true);
+    atomic_init(&ct->sweep_ms, 20000);
     latch_init(&ct->clean_thread_exit);
     ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, ct);
     ct->ipf = ipf_init();
@@ -1480,6 +1481,21 @@  set_label(struct dp_packet *pkt, struct conn *conn,
 }
 
 
+int
+conntrack_set_sweep_interval(struct conntrack *ct, uint32_t ms)
+{
+    atomic_store_relaxed(&ct->sweep_ms, ms);
+    return 0;
+}
+
+uint32_t
+conntrack_get_sweep_interval(struct conntrack *ct)
+{
+    uint32_t ms;
+    atomic_read_relaxed(&ct->sweep_ms, &ms);
+    return ms;
+}
+
 static size_t
 ct_sweep(struct conntrack *ct, struct rculist *list, long long now)
     OVS_NO_THREAD_SAFETY_ANALYSIS
@@ -1504,7 +1520,7 @@  ct_sweep(struct conntrack *ct, struct rculist *list, long long now)
 static long long
 conntrack_clean(struct conntrack *ct, long long now)
 {
-    long long next_wakeup = now + 20 * 1000;
+    long long next_wakeup = now + conntrack_get_sweep_interval(ct);
     unsigned int n_conn_limit, i;
     size_t clean_end, count = 0;
 
diff --git a/lib/conntrack.h b/lib/conntrack.h
index b064abc9f..524ec0acb 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -139,6 +139,8 @@  int conntrack_set_maxconns(struct conntrack *ct, uint32_t maxconns);
 int conntrack_get_maxconns(struct conntrack *ct, uint32_t *maxconns);
 int conntrack_get_nconns(struct conntrack *ct, uint32_t *nconns);
 int conntrack_set_tcp_seq_chk(struct conntrack *ct, bool enabled);
+int conntrack_set_sweep_interval(struct conntrack *ct, uint32_t ms);
+uint32_t conntrack_get_sweep_interval(struct conntrack *ct);
 bool conntrack_get_tcp_seq_chk(struct conntrack *ct);
 struct ipf *conntrack_ipf_ctx(struct conntrack *ct);
 struct conntrack_zone_limit zone_limit_get(struct conntrack *ct,
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index d3b2783ce..0c4b2964f 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -368,6 +368,20 @@  ct_dpif_del_limits(struct dpif *dpif, const struct ovs_list *zone_limits)
             : EOPNOTSUPP);
 }
 
+int
+ct_dpif_sweep(struct dpif *dpif, uint32_t *ms)
+{
+    if (*ms) {
+        return (dpif->dpif_class->ct_set_sweep_interval
+                ? dpif->dpif_class->ct_set_sweep_interval(dpif, *ms)
+                : EOPNOTSUPP);
+    } else {
+        return (dpif->dpif_class->ct_get_sweep_interval
+                ? dpif->dpif_class->ct_get_sweep_interval(dpif, ms)
+                : EOPNOTSUPP);
+    }
+}
+
 int
 ct_dpif_ipf_set_enabled(struct dpif *dpif, bool v6, bool enable)
 {
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 5edbbfd3b..1e265604f 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -298,6 +298,7 @@  int ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
 int ct_dpif_get_limits(struct dpif *dpif, uint32_t *default_limit,
                        const struct ovs_list *, struct ovs_list *);
 int ct_dpif_del_limits(struct dpif *dpif, const struct ovs_list *);
+int ct_dpif_sweep(struct dpif *dpif, uint32_t *ms);
 int ct_dpif_ipf_set_enabled(struct dpif *, bool v6, bool enable);
 int ct_dpif_ipf_set_min_frag(struct dpif *, bool v6, uint32_t min_frag);
 int ct_dpif_ipf_set_max_nfrags(struct dpif *, uint32_t max_frags);
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 59cc4f58c..3ba40fa8f 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -2300,6 +2300,65 @@  out:
     return error;
 }
 
+static int
+dpctl_ct_get_sweep(int argc, const char *argv[],
+                   struct dpctl_params *dpctl_p)
+{
+    uint32_t sweep_ms = 0;
+    struct dpif *dpif;
+
+    int error = opt_dpif_open(argc, argv, dpctl_p, 2, &dpif);
+    if (error) {
+        return error;
+    }
+
+    error = ct_dpif_sweep(dpif, &sweep_ms);
+    if (error) {
+        dpctl_error(dpctl_p, error, "failed to get the sweep interval");
+    } else {
+        dpctl_print(dpctl_p, "%"PRIu32, sweep_ms);
+    }
+
+    dpif_close(dpif);
+    return error;
+}
+
+static int
+dpctl_ct_set_sweep(int argc, const char *argv[],
+                   struct dpctl_params *dpctl_p)
+{
+    struct ds ds = DS_EMPTY_INITIALIZER;
+    uint32_t sweep_ms = 0;
+    struct dpif *dpif;
+
+    int error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
+    if (error) {
+        return error;
+    }
+
+    if (!ovs_scan(argv[argc - 1], "%"SCNu32, &sweep_ms) ||
+        sweep_ms == 0) {
+        ds_put_format(&ds, "invalid sweep value");
+        error = EINVAL;
+        goto error;
+    }
+
+    error = ct_dpif_sweep(dpif, &sweep_ms);
+    if (!error) {
+        dpctl_print(dpctl_p, "setting sweep interval successful\n");
+        goto out;
+    }
+
+    ds_put_format(&ds, "failed to set the sweep interval");
+
+error:
+    dpctl_error(dpctl_p, error, "%s", ds_cstr(&ds));
+    ds_destroy(&ds);
+out:
+    dpif_close(dpif);
+    return error;
+}
+
 static int
 ipf_set_enabled__(int argc, const char *argv[], struct dpctl_params *dpctl_p,
                   bool enabled)
@@ -2913,6 +2972,8 @@  static const struct dpctl_command all_commands[] = {
         DP_RO },
     { "ct-get-limits", "[dp] [zone=N1[,N2]...]", 0, 2, dpctl_ct_get_limits,
         DP_RO },
+    { "ct-get-sweep-interval", "[dp]", 0, 1, dpctl_ct_get_sweep, DP_RO },
+    { "ct-set-sweep-interval", "[dp] ms", 1, 2, dpctl_ct_set_sweep, DP_RW },
     { "ipf-set-enabled", "[dp] v4|v6", 1, 2, dpctl_ipf_set_enabled, DP_RW },
     { "ipf-set-disabled", "[dp] v4|v6", 1, 2, dpctl_ipf_set_disabled, DP_RW },
     { "ipf-set-min-frag", "[dp] v4|v6 minfragment", 2, 3,
diff --git a/lib/dpctl.man b/lib/dpctl.man
index 920446e8c..d448596d3 100644
--- a/lib/dpctl.man
+++ b/lib/dpctl.man
@@ -382,6 +382,15 @@  Prints whether TCP sequence checking is enabled or disabled on \fIdp\fR.  Only
 supported for the userspace datapath.
 .
 .TP
+\*(DX\fBct\-set\-sweep\-interval\fR [\fIdp\fR] \fIms\fR
+Sets the sweep interval. Only supported for the userspace datapath.
+.
+.TP
+\*(DX\fBct\-get\-sweep\-interval\fR [\fIdp\fR]
+Prints the current sweep interval in ms. Only supported for the userspace
+datapath.
+.
+.TP
 \*(DX\fBct\-set\-limits\fR [\fIdp\fR] [\fBdefault=\fIdefault_limit\fR] [\fBzone=\fIzone\fR,\fBlimit=\fIlimit\fR]...
 Sets the maximum allowed number of connections in a connection tracking
 zone.  A specific \fIzone\fR may be set to \fIlimit\fR, and multiple zones
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index aed2c8fbb..70b953ae6 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -9317,6 +9317,21 @@  dpif_netdev_ct_get_tcp_seq_chk(struct dpif *dpif, bool *enabled)
     return 0;
 }
 
+static int
+dpif_netdev_ct_set_sweep_interval(struct dpif *dpif, uint32_t ms)
+{
+    struct dp_netdev *dp = get_dp_netdev(dpif);
+    return conntrack_set_sweep_interval(dp->conntrack, ms);
+}
+
+static int
+dpif_netdev_ct_get_sweep_interval(struct dpif *dpif, uint32_t *ms)
+{
+    struct dp_netdev *dp = get_dp_netdev(dpif);
+    *ms = conntrack_get_sweep_interval(dp->conntrack);
+    return 0;
+}
+
 static int
 dpif_netdev_ct_set_limits(struct dpif *dpif,
                            const uint32_t *default_limits,
@@ -9668,6 +9683,8 @@  const struct dpif_class dpif_netdev_class = {
     dpif_netdev_ct_get_nconns,
     dpif_netdev_ct_set_tcp_seq_chk,
     dpif_netdev_ct_get_tcp_seq_chk,
+    dpif_netdev_ct_set_sweep_interval,
+    dpif_netdev_ct_get_sweep_interval,
     dpif_netdev_ct_set_limits,
     dpif_netdev_ct_get_limits,
     dpif_netdev_ct_del_limits,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 55b5b0a85..de0711277 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4572,6 +4572,8 @@  const struct dpif_class dpif_netlink_class = {
     NULL,                       /* ct_get_nconns */
     NULL,                       /* ct_set_tcp_seq_chk */
     NULL,                       /* ct_get_tcp_seq_chk */
+    NULL,                       /* ct_set_sweep_interval */
+    NULL,                       /* ct_get_sweep_interval */
     dpif_netlink_ct_set_limits,
     dpif_netlink_ct_get_limits,
     dpif_netlink_ct_del_limits,
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index b8ead8a02..44d799198 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -493,6 +493,10 @@  struct dpif_class {
     int (*ct_set_tcp_seq_chk)(struct dpif *, bool enabled);
     /* Get the TCP sequence checking configuration. */
     int (*ct_get_tcp_seq_chk)(struct dpif *, bool *enabled);
+    /* Updates the sweep interval for the CT sweeper */
+    int (*ct_set_sweep_interval)(struct dpif *, uint32_t ms);
+    /* Get the current value of the sweep interval for the CT sweeper */
+    int (*ct_get_sweep_interval)(struct dpif *, uint32_t *ms);
 
 
     /* Connection tracking per zone limit */
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 222415ac0..d4d1d9fc1 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -11721,6 +11721,28 @@  AT_CHECK([tail -1 stdout], [0],
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+dnl Checks the get/set sweep interval
+AT_SETUP([ofproto-dpif - conntrack - change sweep interval])
+OVS_VSWITCHD_START
+
+# Check the default value.
+AT_CHECK([ovs-appctl dpctl/ct-get-sweep-interval], [0], [dnl
+20000
+])
+
+# Set the interval to 5s.
+AT_CHECK([ovs-appctl dpctl/ct-set-sweep-interval 5000], [0], [dnl
+setting sweep interval successful
+])
+
+# Verify that the previous value has been applied.
+AT_CHECK([ovs-appctl dpctl/ct-get-sweep-interval], [0], [dnl
+5000
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto - set mtu])
 OVS_VSWITCHD_START