wenxu <we...@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: we...@ucloud.cn > Date: 2021-11-16 12:50:54 > To: i.maxim...@ovn.org,pvale...@redhat.com > Cc: d...@openvswitch.org > Subject: [PATCH v3] conntrack: support default timeout policy get/set cmd for > netdev datapath>From: wenxu <we...@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 <we...@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 >> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev