[ovs-dev] [PATCH v2 8/9] ofproto-dpif-xlate: Translate timeout policy in ct action
Darrell Ball
dlu998 at gmail.com
Tue Aug 6 03:51:18 UTC 2019
Thanks for the patch
The main comment I had from the V1 patch was adding the check
+ if (ofc->flags & NX_CT_F_COMMIT) {
in compose_conntrack_action()
I see that was done.
After a quick scan, I had one minor comment inline.
On Thu, Aug 1, 2019 at 3:12 PM Yi-Hung Wei <yihung.wei at gmail.com> wrote:
> This patch derives the timeout policy based on ct zone from the
> internal data structure that reads the configuration from ovsdb.
>
> Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
> ---
> lib/ct-dpif.c | 10 ++++++++++
> lib/ct-dpif.h | 3 +++
> lib/dpif-netdev.c | 1 +
> lib/dpif-netlink.c | 10 ++++++++++
> lib/dpif-provider.h | 5 +++++
> ofproto/ofproto-dpif-xlate.c | 29 +++++++++++++++++++++++++++++
> ofproto/ofproto-dpif.c | 24 ++++++++++++++++++++++++
> ofproto/ofproto-provider.h | 5 +++++
> ofproto/ofproto.c | 11 +++++++++++
> ofproto/ofproto.h | 2 ++
> 10 files changed, 100 insertions(+)
>
> diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
> index 7f9ce0a561f7..5d2acfd7810b 100644
> --- a/lib/ct-dpif.c
> +++ b/lib/ct-dpif.c
> @@ -864,3 +864,13 @@ ct_dpif_timeout_policy_dump_done(struct dpif *dpif,
> void *state)
> ? dpif->dpif_class->ct_timeout_policy_dump_done(dpif, state)
> : EOPNOTSUPP);
> }
> +
> +int
> +ct_dpif_format_timeout_policy_name(struct dpif *dpif, uint32_t tp_id,
> + uint16_t dl_type, uint8_t nw_proto,
> + struct ds *ds)
> +{
> + return (dpif->dpif_class->ct_format_timeout_policy_name
> + ? dpif->dpif_class->ct_format_timeout_policy_name(
> + dpif, tp_id, dl_type, nw_proto, ds) : EOPNOTSUPP);
> +}
> diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
> index 8dacb1c7c253..0a27568880c0 100644
> --- a/lib/ct-dpif.h
> +++ b/lib/ct-dpif.h
> @@ -318,5 +318,8 @@ int ct_dpif_timeout_policy_dump_start(struct dpif
> *dpif, void **statep);
> int ct_dpif_timeout_policy_dump_next(struct dpif *dpif, void *state,
> struct ct_dpif_timeout_policy *tp);
> int ct_dpif_timeout_policy_dump_done(struct dpif *dpif, void *state);
> +int ct_dpif_format_timeout_policy_name(struct dpif *dpif, uint32_t tp_id,
> + uint16_t dl_type, uint8_t nw_proto,
> + struct ds *ds);
>
> #endif /* CT_DPIF_H */
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 7240a3e6f3c8..19cf9f21ec85 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -7539,6 +7539,7 @@ const struct dpif_class dpif_netdev_class = {
> NULL, /* ct_timeout_policy_dump_start */
> NULL, /* ct_timeout_policy_dump_next */
> NULL, /* ct_timeout_policy_dump_done */
> + NULL, /* ct_format_timeout_policy_name */
> dpif_netdev_ipf_set_enabled,
> dpif_netdev_ipf_set_min_frag,
> dpif_netdev_ipf_set_max_nfrags,
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index b859508f718a..92da87027c58 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -3071,6 +3071,15 @@ dpif_netlink_format_tp_name(uint32_t id, uint16_t
> l3num, uint8_t l4num,
> ovs_assert(tp_name->length < CTNL_TIMEOUT_NAME_MAX);
> }
>
> +static int
> +dpif_netlink_ct_format_timeout_policy_name(struct dpif *dpif OVS_UNUSED,
> + uint32_t tp_id, uint16_t dl_type, uint8_t nw_proto, struct ds *ds)
> +{
> + dpif_netlink_format_tp_name(tp_id,
> + dl_type == ETH_TYPE_IP ? AF_INET : AF_INET6, nw_proto, ds);
> + return 0;
> +}
> +
> #define CT_DPIF_NL_TP_TCP_MAPPINGS \
> CT_DPIF_NL_TP_MAPPING(TCP, TCP, SYN_SENT, SYN_SENT) \
> CT_DPIF_NL_TP_MAPPING(TCP, TCP, SYN_RECV, SYN_RECV) \
> @@ -3891,6 +3900,7 @@ const struct dpif_class dpif_netlink_class = {
> dpif_netlink_ct_timeout_policy_dump_start,
> dpif_netlink_ct_timeout_policy_dump_next,
> dpif_netlink_ct_timeout_policy_dump_done,
> + dpif_netlink_ct_format_timeout_policy_name,
> NULL, /* ipf_set_enabled */
> NULL, /* ipf_set_min_frag */
> NULL, /* ipf_set_max_nfrags */
> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> index 79a2314500cf..57b32ccb610f 100644
> --- a/lib/dpif-provider.h
> +++ b/lib/dpif-provider.h
> @@ -536,6 +536,11 @@ struct dpif_class {
> struct ct_dpif_timeout_policy *tp);
> int (*ct_timeout_policy_dump_done)(struct dpif *, void *state);
>
> + /* Get timeout policy name (OVS_CT_ATTR_TIMEOUT) from datapath. */
> + int (*ct_format_timeout_policy_name)(struct dpif *, uint32_t tp_id,
> + uint16_t dl_type, uint8_t
> nw_proto,
> + struct ds *ds);
> +
> /* IP Fragmentation. */
>
> /* Disables or enables conntrack fragment reassembly. The default
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 28a7fdd842a6..f9b517aaa270 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -28,6 +28,7 @@
> #include "bond.h"
> #include "bundle.h"
> #include "byte-order.h"
> +#include "ct-dpif.h"
>
move further down
> #include "cfm.h"
> #include "connmgr.h"
> #include "coverage.h"
> @@ -5977,6 +5978,30 @@ put_ct_helper(struct xlate_ctx *ctx,
> }
>
> static void
> +put_ct_timeout(struct ofpbuf *odp_actions, const char *dp_type,
> + const struct ofproto_dpif *ofproto, const struct flow
> *flow,
> + struct flow_wildcards *wc, uint16_t zone_id)
> +{
> + struct dpif_backer *backer = ofproto->backer;
> + uint32_t tp_id;
> +
> + if (ofproto_ct_zone_timeout_policy_get(&ofproto->up, zone_id,
> &tp_id)) {
> + if (!strcmp(dp_type, "system")) {
> + struct ds ds = DS_EMPTY_INITIALIZER;
> + int err = ct_dpif_format_timeout_policy_name(
> + backer->dpif, tp_id, ntohs(flow->dl_type),
> + flow->nw_proto, &ds);
> + if (!err) {
> + memset(&wc->masks.nw_proto, 0xff, sizeof
> wc->masks.nw_proto);
> + nl_msg_put_string(odp_actions, OVS_CT_ATTR_TIMEOUT,
> + ds_cstr(&ds));
> + }
> + ds_destroy(&ds);
> + }
> + }
> +}
> +
> +static void
> put_ct_nat(struct xlate_ctx *ctx)
> {
> struct ofpact_nat *ofn = ctx->ct_nat_action;
> @@ -6071,6 +6096,10 @@ compose_conntrack_action(struct xlate_ctx *ctx,
> struct ofpact_conntrack *ofc,
> put_ct_mark(&ctx->xin->flow, ctx->odp_actions, ctx->wc);
> put_ct_label(&ctx->xin->flow, ctx->odp_actions, ctx->wc);
> put_ct_helper(ctx, ctx->odp_actions, ofc);
> + if (ofc->flags & NX_CT_F_COMMIT) {
> + put_ct_timeout(ctx->odp_actions,
> ctx->xbridge->ofproto->backer->type,
> + ctx->xbridge->ofproto, &ctx->xin->flow, ctx->wc,
> zone);
> + }
> put_ct_nat(ctx);
> ctx->ct_nat_action = NULL;
> nl_msg_end_nested(ctx->odp_actions, ct_offset);
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 6336494e0bc8..f31162f4481d 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5351,6 +5351,29 @@ ct_zone_timeout_policy_reconfig(const struct
> ofproto *ofproto,
> }
>
> static bool
> +ct_zone_timeout_policy_get(const struct ofproto *ofproto_,
> + uint16_t zone, uint32_t *tp_id)
> +{
> + struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> + struct dpif_backer *backer = ofproto->backer;
> + struct ct_zone *ct_zone;
> + struct ct_timeout_policy *ct_tp;
> +
> + ct_zone = ct_zone_lookup(&backer->ct_zones, zone);
> + if (!ct_zone) {
> + return false;
> + }
> +
> + ct_tp = ct_timeout_policy_lookup(&backer->ct_tps, &ct_zone->tp_uuid);
> + if (!ct_tp) {
> + return false;
> + }
> +
> + *tp_id = ct_tp->cdtp.id;
> + return true;
> +}
> +
> +static bool
> set_frag_handling(struct ofproto *ofproto_,
> enum ofputil_frag_handling frag_handling)
> {
> @@ -6455,4 +6478,5 @@ const struct ofproto_class ofproto_dpif_class = {
> ct_flush, /* ct_flush */
> ct_zone_timeout_policy_reconfig,
> ct_zone_timeout_policy_sweep,
> + ct_zone_timeout_policy_get,
> };
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 41e07f0ee23e..1a2fc4a6a084 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1880,6 +1880,11 @@ struct ofproto_class {
> const struct ovsrec_datapath *dp_cfg, unsigned int idl_seqno);
> /* Cleans up the to be deleted timeout policy in the pending kill
> list. */
> void (*ct_zone_timeout_policy_sweep)(const struct ofproto *ofproto_);
> +
> + /* Returns true if timeout policy for 'zone' is configured and stores
> the
> + * timeout policy id in '*tp_id'. */
> + bool (*ct_zone_timeout_policy_get)(const struct ofproto *ofproto_,
> + uint16_t zone, uint32_t *tp_id);
> };
>
> extern const struct ofproto_class ofproto_dpif_class;
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 373b8a4eba0c..cef0690cf466 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -955,6 +955,17 @@ ofproto_ct_zone_timeout_policy_sweep(const struct
> ofproto *ofproto)
> }
> }
>
> +bool
> +ofproto_ct_zone_timeout_policy_get(const struct ofproto *ofproto,
> + uint16_t zone, uint32_t *tp_id)
> +{
> + if (ofproto->ofproto_class->ct_zone_timeout_policy_get) {
> + return ofproto->ofproto_class->ct_zone_timeout_policy_get(
> + ofproto, zone, tp_id);
> + }
> + return false;
> +}
> +
>
> /* Spanning Tree Protocol (STP) configuration. */
>
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 2ae42374be36..8749cabdb7bd 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -366,6 +366,8 @@ void ofproto_set_vlan_limit(int vlan_limit);
> void ofproto_ct_zone_timeout_policy_reconfig(const struct ofproto
> *ofproto,
> const struct ovsrec_datapath *dp_cfg, unsigned int idl_seqno);
> void ofproto_ct_zone_timeout_policy_sweep(const struct ofproto *ofproto);
> +bool ofproto_ct_zone_timeout_policy_get(const struct ofproto *ofproto,
> + uint16_t zone, uint32_t *tp_id);
>
> /* Configuration of ports. */
> void ofproto_port_unregister(struct ofproto *, ofp_port_t ofp_port);
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list