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

wenxu wenxu at ucloud.cn
Sat Nov 13 14:21:11 UTC 2021


Will do . Thanks Paolo






From: Paolo Valerio <pvalerio at redhat.com>
Date: 2021-11-08 02:24:29
To:  wenxu at ucloud.cn,i.maximets at ovn.org,aconole at redhat.com
Cc:  dev at openvswitch.org
Subject: Re: [PATCH] conntrack: support default timeout policy get/set cmd for netdev datapath>Hi Wenxu,
>
>wenxu at ucloud.cn writes:
>
>> 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>
>> ---
>>  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
>






More information about the dev mailing list