Will do . Thanks Paolo
From: Paolo Valerio <[email protected]> Date: 2021-11-08 02:24:29 To: [email protected],[email protected],[email protected] Cc: [email protected] Subject: Re: [PATCH] conntrack: support default timeout policy get/set cmd for netdev datapath>Hi Wenxu, > >[email protected] writes: > >> From: wenxu <[email protected]> >> >> 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 <[email protected]> >> --- >> lib/conntrack-tp.c | 9 ++++++ >> lib/conntrack-tp.h | 2 ++ >> lib/ct-dpif.c | 64 +++++++++++++++++++++++++++++++++++++++++ >> lib/ct-dpif.h | 8 ++++++ >> lib/dpctl.c | 84 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 167 insertions(+) >> > >not a full review, although I did some preliminary tests. NEWS should >be updated, and it would be good to have a test to verify the commands. > >Paolo > >> 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..ccc4caa 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,25 @@ 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 +731,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 +849,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 ef8ae74..9c28266 100644 >> --- a/lib/dpctl.c >> +++ b/lib/dpctl.c >> @@ -2033,6 +2033,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) >> { >> @@ -2713,6 +2793,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, >> -- >> 1.8.3.1 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
