| Message ID | 1637906780-18675-1-git-send-email-wenxu@ucloud.cn |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v4] conntrack: support default timeout policy get/set cmd for netdev datapath | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
wenxu@ucloud.cn writes: > 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-timeout-policy [dp] > ovs-appctl dpctl/ct-set-default-timeout-policy [dp] policies > Looking at the other timeout policy commands, they all use "-tp" instead of "-timeout-policy". Maybe we can stay consistent with those. WDYT? > Signed-off-by: wenxu <wenxu@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 | 67 +++++++++++++++++++++++++++++++++++++++ > 7 files changed, 237 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" This doesn't look right. This way you are coupling the abstraction with the implementation. > #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); > +} > + timeout_policy_dump() is userspace dp specific (and the reason of the includes above, I suppose). You have to use dpif_class ops instead of calling implementation specific functions directly. > 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) nit: this should be aligned > +{ > + 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); nit: prefer reverse xmas tree > + if (error) { > + return error; > + } > + > + if (!strstr(dpif->full_name, "netdev@")) { > + error = EOPNOTSUPP; > + dpctl_print(dpctl_p, "not support set default timeout policy"); > + goto error; > + } > + This results in: "not support set default timeout policyovs-vswitchd: (Operation not supported)" Besides that, it is a bit unusual (if I'm not missing something) to exclude a datapath in this way. Usually, that happens implicitly if the related dpif_class op is not supported. I think it should be possible to do something like that. > + 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) nit: this should be aligned > +{ > + struct dpif *dpif; > + struct ds ds = DS_EMPTY_INITIALIZER; > + struct ct_dpif_timeout_policy tp; nit: prefer reverse xmas tree > + > + 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; > + } > + same remark as above for dpctl_ct_set_default_timeout_policy(). > + 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; you don't need this goto, right? > + } 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..b171a77 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -3632,6 +3632,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_TIMEOUT() if CONFIG_NF_CONNTRACK_TIMEOUT is set, this test fails for check-kernel. 00125|unixctl|DBG|received request dpctl/ct-set-default-timeout-policy["udp_first=1 udp_single=1 icmp_first=1 icmp_reply=1"], id=0 00127|unixctl|DBG|replying with error, id=0: "not support set default timeout policyovs-vswitchd: (Operation not supported) You should add a macro like CHECK_CONNTRACK_DEFAULT_TIMEOUT() and skip the test for check-kernel target. > +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() Is the line above needed? > + > +dnl Modifing default timeout policies > +AT_CHECK([ovs-appctl dpctl/ct-set-default-timeout-policy udp_first=1 udp_single=1 icmp_first=1 icmp_reply=1]) > + This line above fails with: 00131|unixctl|DBG|received request dpctl/ct-set-default-timeout-policy["udp_first=1","udp_single=1","icmp_first=1","icmp_reply=1"], id=0 00132|unixctl|DBG|replying with error, id=0: ""dpctl/ct-set-default-timeout-policy" command takes at most 2 arguments" > +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]) > -- > 1.8.3.1
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..b171a77 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -3632,6 +3632,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_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-timeout-policy 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])