[ovs-dev] [PATCH v3] conntrack: support default timeout policy get/set cmd for netdev datapath
Paolo Valerio
pvalerio at redhat.com
Thu Nov 25 22:01:19 UTC 2021
wenxu <wenxu at ucloud.cn> writes:
> Hi Paolo,
>
> Any suggestion for this version. I run all the test case success.
> But the robot build show 1091: ofproto-dpif - controller action without megaflows FAILED (ovs-macros.at:217)
>
> Maybe there are some problem? This patch is not matter with this tescase
>
It seems unrelated.
Regarding the test instead, I think it's better to add a new one that
besides checking the commands, it also checks the functionality.
You could take "zone-based timeout policy" as a reference for that.
> BR
> wenxu
> From: wenxu at ucloud.cn
> Date: 2021-11-16 12:50:54
> To: i.maximets at ovn.org,pvalerio at redhat.com
> Cc: dev at openvswitch.org
> Subject: [PATCH v3] conntrack: support default timeout policy get/set cmd for netdev datapath>From: wenxu <wenxu at 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-timeout-policy [dp]
>>ovs-appctl dpctl/ct-set-default-timeout-policy [dp] policies
>>
>>Signed-off-by: wenxu <wenxu at ucloud.cn>
>>---
>> NEWS | 4 +++
>> lib/conntrack-tp.c | 9 ++++++
>> lib/conntrack-tp.h | 2 ++
>> lib/ct-dpif.c | 63 +++++++++++++++++++++++++++++++++++++
>> lib/ct-dpif.h | 8 +++++
>> lib/dpctl.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/system-traffic.at | 6 ++++
>> 7 files changed, 176 insertions(+)
>>
>>diff --git a/NEWS b/NEWS
>>index 434ee57..c6a2eda 100644
>>--- a/NEWS
>>+++ b/NEWS
>>@@ -16,6 +16,10 @@ Post-v2.16.0
>> - ovs-dpctl and 'ovs-appctl dpctl/':
>> * New commands 'cache-get-size' and 'cache-set-size' that allows to
>> get or configure linux kernel datapath cache sizes.
>>+ - ovs-appctl dpctl/:
>>+ * New commands 'ct-set-default-timeout-policy' and
>>+ 'ct-set-default-timeout-policy' 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..726b854 100644
>>--- a/lib/conntrack-tp.c
>>+++ b/lib/conntrack-tp.c
>>@@ -230,6 +230,15 @@ tm_to_ct_dpif_tp(enum ct_timeout tm)
>> return CT_DPIF_TP_ATTR_MAX;
>> }
>>
>>+void
>>+timeout_policy_dump(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..bd22242 100644
>>--- a/lib/conntrack-tp.h
>>+++ b/lib/conntrack-tp.h
>>@@ -22,6 +22,8 @@ enum ct_timeout;
>> void timeout_policy_init(struct conntrack *ct);
>> int timeout_policy_update(struct conntrack *ct, struct timeout_policy *tp);
>> int timeout_policy_delete(struct conntrack *ct, uint32_t tp_id);
>>+void timeout_policy_dump(const struct ct_dpif_timeout_policy *tp,
>>+ struct ds *ds);
>> struct timeout_policy *timeout_policy_get(struct conntrack *ct, int32_t tp_id);
>> void conn_init_expiration(struct conntrack *ct, struct conn *conn,
>> enum ct_timeout tm, long long now);
>>diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
>>index cfc2315..6e36da2 100644
>>--- a/lib/ct-dpif.c
>>+++ b/lib/ct-dpif.c
>>@@ -20,6 +20,8 @@
>> #include <errno.h>
>>
>> #include "ct-dpif.h"
>>+#include "conntrack-private.h"
>>+#include "conntrack-tp.h"
>> #include "openvswitch/ofp-parse.h"
>> #include "openvswitch/vlog.h"
>>
>>@@ -180,6 +182,24 @@ ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool *enabled)
>> }
>>
>> int
>>+ct_dpif_set_default_timeout_policy(struct dpif *dpif,
>>+ const struct ct_dpif_timeout_policy *tp)
>>+{
>>+ return (dpif->dpif_class->ct_set_timeout_policy
>>+ ? dpif->dpif_class->ct_set_timeout_policy(dpif, tp)
>>+ : EOPNOTSUPP);
>>+}
>>+
>>+int
>>+ct_dpif_get_default_timeout_policy(struct dpif *dpif,
>>+ struct ct_dpif_timeout_policy *tp)
>>+{
>>+ return (dpif->dpif_class->ct_get_timeout_policy
>>+ ? dpif->dpif_class->ct_get_timeout_policy(dpif, DEFAULT_TP_ID, tp)
>>+ : EOPNOTSUPP);
>>+}
>>+
>>+int
>> ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
>> const struct ovs_list *zone_limits)
>> {
>>@@ -710,6 +730,42 @@ 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
>>+ * 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'. */
>>@@ -792,6 +848,13 @@ static const char *const ct_dpif_tp_attr_string[] = {
>> #undef CT_DPIF_TP_ICMP_ATTR
>> };
>>
>>+void
>>+ct_dpif_format_timeout_policy(const struct ct_dpif_timeout_policy *tp,
>>+ struct ds *ds)
>>+{
>>+ timeout_policy_dump(tp, ds);
>>+}
>>+
>> static bool
>> ct_dpif_set_timeout_policy_attr(struct ct_dpif_timeout_policy *tp,
>> uint32_t attr, uint32_t value)
>>diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
>>index b59cba9..9f09ee0 100644
>>--- a/lib/ct-dpif.h
>>+++ b/lib/ct-dpif.h
>>@@ -292,6 +292,12 @@ 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,
>>+ const struct ct_dpif_timeout_policy *);
>>+int ct_dpif_get_default_timeout_policy(struct dpif *dpif,
>>+ struct ct_dpif_timeout_policy *tp);
>>+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);
>>@@ -315,6 +321,8 @@ bool ct_dpif_parse_zone_limit_tuple(const char *s, uint16_t *pzone,
>> uint32_t *plimit, struct ds *);
>> void ct_dpif_format_zone_limits(uint32_t default_limit,
>> const struct ovs_list *, struct ds *);
>>+void ct_dpif_format_timeout_policy(const struct ct_dpif_timeout_policy *tp,
>>+ struct ds *ds);
>> bool ct_dpif_set_timeout_policy_attr_by_name(struct ct_dpif_timeout_policy *tp,
>> const char *key, uint32_t value);
>> bool ct_dpif_timeout_policy_support_ipproto(uint8_t ipproto);
>>diff --git a/lib/dpctl.c b/lib/dpctl.c
>>index 1ba1a96..e7442c9 100644
>>--- a/lib/dpctl.c
>>+++ b/lib/dpctl.c
>>@@ -2074,6 +2074,86 @@ 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)
>>+{
>>+ struct dpif *dpif;
>>+ struct ds ds = DS_EMPTY_INITIALIZER;
>>+ int i = dp_arg_exists(argc, argv) ? 2 : 1;
>>+ struct ct_dpif_timeout_policy tp;
>>+ int error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
>>+ if (error) {
>>+ return error;
>>+ }
>>+
>>+ if (!strstr(dpif->full_name, "netdev@")) {
>>+ error = EOPNOTSUPP;
>>+ dpctl_print(dpctl_p, "not support set default timeout policy");
>>+ goto error;
>>+ }
>>+
>>+ memset(&tp, 0, sizeof tp);
>>+ tp.id = DEFAULT_TP_ID;
>>+
>>+ /* 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 dpif *dpif;
>>+ struct ds ds = DS_EMPTY_INITIALIZER;
>>+ struct ct_dpif_timeout_policy tp;
>>+
>>+ int error = opt_dpif_open(argc, argv, dpctl_p, INT_MAX, &dpif);
>>+ if (error) {
>>+ return error;
>>+ }
>>+
>>+ if (!strstr(dpif->full_name, "netdev@")) {
>>+ error = EOPNOTSUPP;
>>+ dpctl_print(dpctl_p, "not support get default timeout policy, ");
>>+ goto out;
>>+ }
>>+
>>+ error = ct_dpif_get_default_timeout_policy(dpif, &tp);
>>+
>>+ if (!error) {
>>+ ds_put_format(&ds, "default timeout policy (s): ");
>>+ ct_dpif_format_timeout_policy(&tp, &ds);
>>+ dpctl_print(dpctl_p, "%s\n", ds_cstr(&ds));
>>+ goto out;
>>+ } else {
>>+ ds_put_format(&ds, "failed to get conntrack timeout policy %s",
>>+ ovs_strerror(error));
>>+ }
>>+
>>+out:
>>+ 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 +2922,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-timeout-policy", "[dp]", 1, 2,
>>+ dpctl_ct_set_default_timeout_policy, DP_RW },
>>+ { "ct-get-default-timeout-policy", "[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/tests/system-traffic.at b/tests/system-traffic.at
>>index a69896d..867cab9 100644
>>--- a/tests/system-traffic.at
>>+++ b/tests/system-traffic.at
>>@@ -1786,6 +1786,12 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
>> 10
>> ])
>>
>>+AT_CHECK([ovs-appctl dpctl/ct-set-default-timeout-policy tcp_syn_sent=60])
>>+
>>+AT_CHECK([ovs-appctl dpctl/ct-get-default-timeout-policy | grep "TCP_FIRST_PACKET" | sed -n 's/.*TCP/TCP/p'], [], [dnl
>>+TCP_FIRST_PACKET = 60
>>+])
>>+
>> OVS_TRAFFIC_VSWITCHD_STOP
>> AT_CLEANUP
>>
>>--
>>1.8.3.1
>>
More information about the dev
mailing list