diff mbox series

[ovs-dev,v7] conntrack: support default timeout policy get/set cmd for netdev datapath

Message ID 1642570760-27962-1-git-send-email-wenxu@ucloud.cn
State Superseded
Headers show
Series [ovs-dev,v7] conntrack: support default timeout policy get/set cmd for netdev datapath | expand

Checks

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

Commit Message

wenxu Jan. 19, 2022, 5:39 a.m. UTC
From: wenxu <wenxu@ucloud.cn>

Now, the default timeout policy for netdev datapath is hard codeing. In
some case show or modify is needed.
Add command for get/set default timeout policy. Using like this:

ovs-appctl dpctl/ct-get-default-tp [dp]
ovs-appctl dpctl/ct-set-default-tp [dp] policies

Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 NEWS                             |  4 +++
 lib/conntrack-tp.c               | 11 +++++++
 lib/conntrack-tp.h               |  2 ++
 lib/ct-dpif.c                    | 56 ++++++++++++++++++++++++++++++++
 lib/ct-dpif.h                    |  9 ++++++
 lib/dpctl.c                      | 69 ++++++++++++++++++++++++++++++++++++++++
 lib/dpif-netdev.c                | 25 +++++++++++++++
 lib/dpif-netlink.c               |  2 ++
 lib/dpif-provider.h              |  8 +++++
 tests/system-kmod-macros.at      | 10 ++++++
 tests/system-traffic.at          | 67 ++++++++++++++++++++++++++++++++++++++
 tests/system-userspace-macros.at |  7 ++++
 12 files changed, 270 insertions(+)
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index e2cdc1a..122b1a7 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,10 @@  Post-v2.16.0
    - Ingress policing on Linux now uses 'matchall' classifier instead of
      'basic', if available.
    - Add User Statically-Defined Tracing (USDT) probe framework support.
+   - ovs-appctl dpctl/:
+     * New commands 'ct-set-default-tp' and
+       'ct-set-default-tp' that allows to get or configure
+       netdev datapath ct default timeout policy.
 
 
 v2.16.0 - 16 Aug 2021
diff --git a/lib/conntrack-tp.c b/lib/conntrack-tp.c
index a586d3a..4677d0b 100644
--- a/lib/conntrack-tp.c
+++ b/lib/conntrack-tp.c
@@ -230,6 +230,17 @@  tm_to_ct_dpif_tp(enum ct_timeout tm)
     return CT_DPIF_TP_ATTR_MAX;
 }
 
+void
+dpif_netdev_format_timeout_policy(const struct ct_dpif_timeout_policy *tp,
+                                  struct ds *ds)
+{
+    for (unsigned i = 0; i < N_CT_TM; i++) {
+        ds_put_format(ds, "\n\t%s = %"PRIu32, ct_timeout_str[i],
+                      tp->attrs[tm_to_ct_dpif_tp(i)]);
+    }
+}
+
+
 static void
 conn_update_expiration__(struct conntrack *ct, struct conn *conn,
                          enum ct_timeout tm, long long now,
diff --git a/lib/conntrack-tp.h b/lib/conntrack-tp.h
index 4d411d1..07dcb4e 100644
--- a/lib/conntrack-tp.h
+++ b/lib/conntrack-tp.h
@@ -27,4 +27,6 @@  void conn_init_expiration(struct conntrack *ct, struct conn *conn,
                           enum ct_timeout tm, long long now);
 void conn_update_expiration(struct conntrack *ct, struct conn *conn,
                             enum ct_timeout tm, long long now);
+void dpif_netdev_format_timeout_policy(const struct ct_dpif_timeout_policy *tp,
+                                       struct ds *ds);
 #endif
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index cfc2315..b061d28 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -180,6 +180,25 @@  ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool *enabled)
 }
 
 int
+ct_dpif_set_default_timeout_policy(struct dpif *dpif,
+                                   struct ct_dpif_timeout_policy *tp)
+{
+    return (dpif->dpif_class->ct_set_default_timeout_policy
+            ? dpif->dpif_class->ct_set_default_timeout_policy(dpif, tp)
+            : EOPNOTSUPP);
+}
+
+int
+ct_dpif_get_default_timeout_policy(struct dpif *dpif,
+                                   struct ct_dpif_timeout_policy *tp,
+                                   struct ds *ds)
+{
+    return (dpif->dpif_class->ct_get_default_timeout_policy
+            ? dpif->dpif_class->ct_get_default_timeout_policy(dpif, tp, ds)
+            : EOPNOTSUPP);
+}
+
+int
 ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
                    const struct ovs_list *zone_limits)
 {
@@ -710,6 +729,43 @@  ct_dpif_free_zone_limits(struct ovs_list *zone_limits)
     }
 }
 
+
+/* Parses a specification of a timeout policy from 's' into '*tp'.
+ * Returns true on success.  Otherwise, returns false and puts the
+ * error message in 'ds'. */
+bool
+ct_dpif_parse_timeout_policy_tuple(const char *s, struct ds *ds,
+                                   struct ct_dpif_timeout_policy *tp)
+{
+    char *pos, *key, *value, *copy, *err;
+
+    pos = copy = xstrdup(s);
+    while (ofputil_parse_key_value(&pos, &key, &value)) {
+        uint32_t tmp;
+
+        if (!*value) {
+            ds_put_format(ds, "field %s missing value", key);
+            goto error;
+        }
+
+        err = str_to_u32(value, &tmp);
+        if (err) {
+          free(err);
+          goto error_with_msg;
+        }
+
+        ct_dpif_set_timeout_policy_attr_by_name(tp, key, tmp);
+    }
+    free(copy);
+
+    return true;
+
+error_with_msg:
+    ds_put_format(ds, "failed to parse field %s", key);
+error:
+    free(copy);
+    return false;
+}
 /* Parses a specification of a conntrack zone limit from 's' into '*pzone'
  * and '*plimit'.  Returns true on success.  Otherwise, returns false and
  * and puts the error message in 'ds'. */
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index b59cba9..f04182b 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -271,6 +271,8 @@  struct ct_dpif_timeout_policy {
                                                  * timeout attribute values */
 };
 
+extern const char *ct_dpif_timeout_string[];
+
 /* Conntrack Features. */
 enum ct_features {
     CONNTRACK_F_ZERO_SNAT = 1 << 0,  /* All-zero SNAT support. */
@@ -292,6 +294,13 @@  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_set_default_timeout_policy(struct dpif *dpif,
+                                       struct ct_dpif_timeout_policy *);
+int ct_dpif_get_default_timeout_policy(struct dpif *dpif,
+                                       struct ct_dpif_timeout_policy *tp,
+                                       struct ds *ds);
+bool ct_dpif_parse_timeout_policy_tuple(const char *s, struct ds *ds,
+                                        struct ct_dpif_timeout_policy *);
 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 1ba1a96..e59a621 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -2074,6 +2074,71 @@  dpctl_ct_get_tcp_seq_chk(int argc, const char *argv[],
 }
 
 static int
+dpctl_ct_set_default_timeout_policy(int argc, const char *argv[],
+                                    struct dpctl_params *dpctl_p)
+{
+    int i =  dp_arg_exists(argc, argv) ? 2 : 1;
+    struct ds ds = DS_EMPTY_INITIALIZER;
+    struct ct_dpif_timeout_policy tp;
+    struct dpif *dpif;
+
+    int error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
+    if (error) {
+        return error;
+    }
+
+    memset(&tp, 0, sizeof tp);
+
+    /* Parse timeout policy tuples */
+    if (!ct_dpif_parse_timeout_policy_tuple(argv[i], &ds, &tp)) {
+        error = EINVAL;
+        goto error;
+    }
+
+    error = ct_dpif_set_default_timeout_policy(dpif, &tp);
+    if (!error) {
+        dpif_close(dpif);
+        return 0;
+    } else {
+        ds_put_cstr(&ds, "failed to set timeout policy");
+    }
+
+error:
+    dpctl_error(dpctl_p, error, "%s", ds_cstr(&ds));
+    ds_destroy(&ds);
+    dpif_close(dpif);
+    return error;
+}
+
+static int
+dpctl_ct_get_default_timeout_policy(int argc, const char *argv[],
+                                    struct dpctl_params *dpctl_p)
+{
+    struct ds ds = DS_EMPTY_INITIALIZER;
+    struct ct_dpif_timeout_policy tp;
+    struct dpif *dpif;
+
+    int error = opt_dpif_open(argc, argv, dpctl_p, INT_MAX, &dpif);
+    if (error) {
+        return error;
+    }
+
+    error = ct_dpif_get_default_timeout_policy(dpif, &tp, &ds);
+    if (!error) {
+        ds_put_format(&ds, "default timeout policy (s): ");
+        dpctl_print(dpctl_p, "%s\n", ds_cstr(&ds));
+    } else {
+        ds_put_format(&ds, "failed to get conntrack timeout policy %s",
+                      ovs_strerror(error));
+        dpctl_error(dpctl_p, error, "%s", ds_cstr(&ds));
+    }
+
+    ds_destroy(&ds);
+    dpif_close(dpif);
+    return error;
+}
+
+static int
 dpctl_ct_set_limits(int argc, const char *argv[],
                     struct dpctl_params *dpctl_p)
 {
@@ -2842,6 +2907,10 @@  static const struct dpctl_command all_commands[] = {
     { "ct-disable-tcp-seq-chk", "[dp]", 0, 1, dpctl_ct_disable_tcp_seq_chk,
        DP_RW },
     { "ct-get-tcp-seq-chk", "[dp]", 0, 1, dpctl_ct_get_tcp_seq_chk, DP_RO },
+    { "ct-set-default-tp", "[dp]", 1, 2,
+       dpctl_ct_set_default_timeout_policy, DP_RW },
+    { "ct-get-default-tp", "[dp]", 0, 1,
+       dpctl_ct_get_default_timeout_policy, DP_RO },
     { "ct-set-limits", "[dp] [default=L] [zone=N,limit=L]...", 1, INT_MAX,
         dpctl_ct_set_limits, DP_RO },
     { "ct-del-limits", "[dp] zone=N1[,N2]...", 1, 2, dpctl_ct_del_limits,
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 31cf117..0315d7f 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -8967,6 +8967,29 @@  dpif_netdev_ct_get_timeout_policy_name(struct dpif *dpif OVS_UNUSED,
 }
 
 static int
+dpif_netdev_ct_set_default_timeout_policy(struct dpif *dpif,
+                                          struct ct_dpif_timeout_policy *tp)
+{
+    tp->id = DEFAULT_TP_ID;
+    return dpif_netdev_ct_set_timeout_policy(dpif, tp);
+}
+
+static int
+dpif_netdev_ct_get_default_timeout_policy(struct dpif *dpif,
+                                          struct ct_dpif_timeout_policy *tp,
+                                          struct ds *ds)
+{
+    int err;
+
+    err = dpif_netdev_ct_get_timeout_policy(dpif, DEFAULT_TP_ID, tp);
+    if (!err && ds) {
+        dpif_netdev_format_timeout_policy(tp, ds);
+    }
+
+    return err;
+}
+
+static int
 dpif_netdev_ipf_set_enabled(struct dpif *dpif, bool v6, bool enable)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
@@ -9176,6 +9199,8 @@  const struct dpif_class dpif_netdev_class = {
     NULL,                       /* ct_timeout_policy_dump_next */
     NULL,                       /* ct_timeout_policy_dump_done */
     dpif_netdev_ct_get_timeout_policy_name,
+    dpif_netdev_ct_set_default_timeout_policy,
+    dpif_netdev_ct_get_default_timeout_policy,
     dpif_netdev_ct_get_features,
     dpif_netdev_ipf_set_enabled,
     dpif_netdev_ipf_set_min_frag,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 08b6d7b..ebcd280 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4476,6 +4476,8 @@  const struct dpif_class dpif_netlink_class = {
     dpif_netlink_ct_timeout_policy_dump_next,
     dpif_netlink_ct_timeout_policy_dump_done,
     dpif_netlink_ct_get_timeout_policy_name,
+    NULL,                       /* ct_set_default_timeout_policy */
+    NULL,                       /* ct_get_default_timeout_policy */
     dpif_netlink_ct_get_features,
     NULL,                       /* ipf_set_enabled */
     NULL,                       /* ipf_set_min_frag */
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 27e3a76..0465211 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -565,6 +565,14 @@  struct dpif_class {
                                       uint16_t dl_type, uint8_t nw_proto,
                                       char **tp_name, bool *is_generic);
 
+    /* Sets default timeout policy '*tp' into the datapath. */
+    int (*ct_set_default_timeout_policy)(struct dpif *,
+                                         struct ct_dpif_timeout_policy *);
+    /* Gets the default timeout policy and stores it into '*tp'. */
+    int (*ct_get_default_timeout_policy)(struct dpif *,
+                                         struct ct_dpif_timeout_policy *,
+                                         struct ds *);
+
     /* Stores the conntrack features supported by 'dpif' into features.
      * The value is a bitmap of CONNTRACK_F_* bits. */
     int (*ct_get_features)(struct dpif *, enum ct_features *features);
diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
index 86d633a..21fb259 100644
--- a/tests/system-kmod-macros.at
+++ b/tests/system-kmod-macros.at
@@ -121,6 +121,16 @@  m4_define([CHECK_CONNTRACK_TIMEOUT],
     on_exit 'modprobe -r nfnetlink_cttimeout'
 ])
 
+# CHECK_CONNTRACK_DEFAULT_TIMEOUT()
+#
+# Perform requirements checks for running ovs-dpctl ct-set-default-tp or
+# ovs-dpctl ct-get-default-tp. The kernel datapath does not support this
+# feature.
+m4_define([CHECK_CONNTRACK_DEFAULT_TIMEOUT],
+[
+    AT_SKIP_IF([:])
+])
+
 # CHECK_CT_DPIF_SET_GET_MAXCONNS()
 #
 # Perform requirements checks for running ovs-dpctl ct-set-maxconns or
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 9da84a0..8f6e1ed 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -4009,6 +4009,73 @@  udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - default timeout policy])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_DEFAULT_TIMEOUT()
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+AT_DATA([flows.txt], [dnl
+priority=1,action=drop
+priority=10,arp,action=normal
+priority=100,in_port=1,ip,action=ct(zone=5, table=1)
+priority=100,in_port=2,ip,action=ct(zone=5, table=1)
+table=1,in_port=2,ip,ct_state=+trk+est,action=1
+table=1,in_port=1,ip,ct_state=+trk+new,action=ct(commit,zone=5),2
+table=1,in_port=1,ip,ct_state=+trk+est,action=2
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl Test with origin default timeout
+
+dnl Send ICMP and UDP traffic
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
+
+sleep 4
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sort], [0], [dnl
+icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0),zone=5
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),zone=5
+])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+
+dnl Shorten the udp_first udp_single and
+dnl icmp_first icmp_reply default timeout
+VSCTL_ADD_DATAPATH_TABLE()
+
+dnl Modifing default timeout policies
+AT_CHECK([ovs-appctl dpctl/ct-set-default-tp "udp_first=1,udp_single=1,icmp_first=1,icmp_reply=1"])
+
+dnl Send ICMP and UDP traffic
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sort], [0], [dnl
+icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0),zone=5
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),zone=5
+])
+
+dnl Wait until the timeout expire.
+dnl We intend to wait a bit longer, because conntrack does not recycle the entry right after it is expired.
+sleep 6
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 dnl Check kernel datapath to make sure conntrack fills in L3 and L4
 dnl protocol information
 AT_SETUP([conntrack - fragment reassembly with L3 L4 protocol information])
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index f639ba5..acd9643 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -110,6 +110,13 @@  m4_define([CHECK_CONNTRACK_ZEROIP_SNAT])
 #
 m4_define([CHECK_CONNTRACK_TIMEOUT])
 
+# CHECK_CONNTRACK_DEFAULT_TIMEOUT()
+#
+# Perform requirements checks for running conntrack customized
+# default timeout tests.
+#
+m4_define([CHECK_CONNTRACK_DEFAULT_TIMEOUT])
+
 # CHECK_CT_DPIF_SET_GET_MAXCONNS()
 #
 # Perform requirements checks for running ovs-dpctl ct-set-maxconns or