| Message ID | 1564097054-72663-12-git-send-email-yihung.wei@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Support zone-based conntrack timeout policy | expand |
Thanks for the patch Few comments inline On Thu, Jul 25, 2019 at 4:30 PM Yi-Hung Wei <yihung.wei@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@gmail.com> > --- > lib/ct-dpif.c | 10 ++++++++++ > lib/ct-dpif.h | 3 +++ > lib/datapath-config.c | 30 ++++++++++++++++++++++++++++++ > lib/datapath-config.h | 2 ++ > lib/dpif-netdev.c | 1 + > lib/dpif-netlink.c | 10 ++++++++++ > lib/dpif-provider.h | 5 +++++ > ofproto/ofproto-dpif-xlate.c | 23 +++++++++++++++++++++++ > 8 files changed, 84 insertions(+) > > diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c > index 1625754e2441..e2c96b29b17f 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 de032cc416ce..a4e91f975a9b 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/datapath-config.c b/lib/datapath-config.c > index cdd2128a60bc..2f3852100b78 100644 > --- a/lib/datapath-config.c > +++ b/lib/datapath-config.c > @@ -377,3 +377,33 @@ destroy_all_datapaths(void) > datapath_destroy(dp); > } > } > + > +/* If timeout policy is found in datapath '*dp_type' and in 'zone', > + * sets timeout policy id in '*tp_id' and returns true. Otherwise, > + * returns false. */ > +bool > +datapath_get_zone_timeout_policy_id(const char *dp_type, uint16_t zone, > + uint32_t *tp_id) > +{ > + struct datapath *dp; > + struct ct_zone *ct_zone; > + struct ct_timeout_policy *ct_tp; > + > + dp = datapath_lookup(dp_type); > + if (!dp) { > + return false; > + } > + > + ct_zone = ct_zone_lookup(&dp->ct_zones, zone); > + if (!ct_zone) { > + return false; > + } > + > + ct_tp = ct_timeout_policy_lookup(&dp->ct_tps, &ct_zone->tp_uuid); > + if (!ct_tp) { > + return false; > + } > + > + *tp_id = ct_tp->cdtp.id; > + return true; > +} > diff --git a/lib/datapath-config.h b/lib/datapath-config.h > index d9a90e4f8312..0e0cd7eaad2f 100644 > --- a/lib/datapath-config.h > +++ b/lib/datapath-config.h > @@ -21,5 +21,7 @@ > void reconfigure_datapath(const struct ovsrec_open_vswitch *, > unsigned int idl_seqno); > void destroy_all_datapaths(void); > +bool datapath_get_zone_timeout_policy_id(const char *dp_type, uint16_t > zone, > + uint32_t *tp_id); > > #endif /* datapath-config.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 abfad9543c3b..8e6f2ebb51e1 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -3076,6 +3076,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_TO_NL_TP_TCP_MAPPINGS \ > CT_DPIF_TO_NL_TP_MAPPING(TCP, TCP, SYN_SENT, SYN_SENT) \ > CT_DPIF_TO_NL_TP_MAPPING(TCP, TCP, SYN_RECV, SYN_RECV) \ > @@ -3860,6 +3869,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 3460ef8aa98d..f01f3abee5ab 100644 > --- a/lib/dpif-provider.h > +++ b/lib/dpif-provider.h > @@ -541,6 +541,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); > + > The above generic API implies that 'dl_type' and 'nw_proto' are always needed, which is not true in general. I think these parameters could have generic names in this base class. The corresponding types could be uint32_t as well so as not to imply any particular context. If this is going to delay things, it can be done later from my POV. > /* 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..12fa9684d0e4 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -28,10 +28,12 @@ > #include "bond.h" > #include "bundle.h" > #include "byte-order.h" > +#include "ct-dpif.h" > #include "cfm.h" > #include "connmgr.h" > #include "coverage.h" > #include "csum.h" > +#include "datapath-config.h" > #include "dp-packet.h" > #include "dpif.h" > #include "in-band.h" > @@ -5977,6 +5979,25 @@ put_ct_helper(struct xlate_ctx *ctx, > } > > static void > +put_ct_timeout(struct ofpbuf *odp_actions, const char *dp_type, > + struct dpif *dpif, const struct flow *flow, > + struct flow_wildcards *wc, uint16_t zone_id) > +{ > + uint32_t tp_id; > + > + if (datapath_get_zone_timeout_policy_id(dp_type, zone_id, &tp_id)) { > + struct ds ds = DS_EMPTY_INITIALIZER; > + int err = ct_dpif_format_timeout_policy_name( > + dpif, tp_id, ntohs(flow->dl_type), flow->nw_proto, > &ds); > + if (!err) { > > + memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); > I think the above unwildcarding should be wrapped in a datapath type specific (i.e. 'system' in this case) function. Also a note should be added to the function describing the why the additional datapath flows are needed (i.e. bcoz of Netfilter timeouts implementation). Note that these datapath flows will remain if kept minimally active, which is generally undesirable, hence needs to be flagged. If this is going to delay things, it can be done later from my POV. > + 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 +6092,8 @@ 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); > + put_ct_timeout(ctx->odp_actions, ctx->xbridge->ofproto->backer->type, > + ctx->xbridge->dpif, &ctx->xin->flow, ctx->wc, zone); > put_ct_nat(ctx); > ctx->ct_nat_action = NULL; > nl_msg_end_nested(ctx->odp_actions, ct_offset); > -- > 2.7.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
One more comment inline On Thu, Jul 25, 2019 at 10:02 PM Darrell Ball <dlu998@gmail.com> wrote: > Thanks for the patch > > Few comments inline > > On Thu, Jul 25, 2019 at 4:30 PM Yi-Hung Wei <yihung.wei@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@gmail.com> >> --- >> lib/ct-dpif.c | 10 ++++++++++ >> lib/ct-dpif.h | 3 +++ >> lib/datapath-config.c | 30 ++++++++++++++++++++++++++++++ >> lib/datapath-config.h | 2 ++ >> lib/dpif-netdev.c | 1 + >> lib/dpif-netlink.c | 10 ++++++++++ >> lib/dpif-provider.h | 5 +++++ >> ofproto/ofproto-dpif-xlate.c | 23 +++++++++++++++++++++++ >> 8 files changed, 84 insertions(+) >> >> diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c >> index 1625754e2441..e2c96b29b17f 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 de032cc416ce..a4e91f975a9b 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/datapath-config.c b/lib/datapath-config.c >> index cdd2128a60bc..2f3852100b78 100644 >> --- a/lib/datapath-config.c >> +++ b/lib/datapath-config.c >> @@ -377,3 +377,33 @@ destroy_all_datapaths(void) >> datapath_destroy(dp); >> } >> } >> + >> +/* If timeout policy is found in datapath '*dp_type' and in 'zone', >> + * sets timeout policy id in '*tp_id' and returns true. Otherwise, >> + * returns false. */ >> +bool >> +datapath_get_zone_timeout_policy_id(const char *dp_type, uint16_t zone, >> + uint32_t *tp_id) >> +{ >> + struct datapath *dp; >> + struct ct_zone *ct_zone; >> + struct ct_timeout_policy *ct_tp; >> + >> + dp = datapath_lookup(dp_type); >> + if (!dp) { >> + return false; >> + } >> + >> + ct_zone = ct_zone_lookup(&dp->ct_zones, zone); >> + if (!ct_zone) { >> + return false; >> + } >> + >> + ct_tp = ct_timeout_policy_lookup(&dp->ct_tps, &ct_zone->tp_uuid); >> + if (!ct_tp) { >> + return false; >> + } >> + >> + *tp_id = ct_tp->cdtp.id; >> + return true; >> +} >> diff --git a/lib/datapath-config.h b/lib/datapath-config.h >> index d9a90e4f8312..0e0cd7eaad2f 100644 >> --- a/lib/datapath-config.h >> +++ b/lib/datapath-config.h >> @@ -21,5 +21,7 @@ >> void reconfigure_datapath(const struct ovsrec_open_vswitch *, >> unsigned int idl_seqno); >> void destroy_all_datapaths(void); >> +bool datapath_get_zone_timeout_policy_id(const char *dp_type, uint16_t >> zone, >> + uint32_t *tp_id); >> >> #endif /* datapath-config.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 abfad9543c3b..8e6f2ebb51e1 100644 >> --- a/lib/dpif-netlink.c >> +++ b/lib/dpif-netlink.c >> @@ -3076,6 +3076,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_TO_NL_TP_TCP_MAPPINGS \ >> CT_DPIF_TO_NL_TP_MAPPING(TCP, TCP, SYN_SENT, SYN_SENT) \ >> CT_DPIF_TO_NL_TP_MAPPING(TCP, TCP, SYN_RECV, SYN_RECV) \ >> @@ -3860,6 +3869,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 3460ef8aa98d..f01f3abee5ab 100644 >> --- a/lib/dpif-provider.h >> +++ b/lib/dpif-provider.h >> @@ -541,6 +541,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); >> + >> > > The above generic API implies that 'dl_type' and 'nw_proto' are always > needed, which is > not true in general. > I think these parameters could have generic names in this base class. > The corresponding types could be uint32_t as well so as not to imply any > particular context. > > If this is going to delay things, it can be done later from my POV. > > > >> /* 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..12fa9684d0e4 100644 >> --- a/ofproto/ofproto-dpif-xlate.c >> +++ b/ofproto/ofproto-dpif-xlate.c >> @@ -28,10 +28,12 @@ >> #include "bond.h" >> #include "bundle.h" >> #include "byte-order.h" >> +#include "ct-dpif.h" >> #include "cfm.h" >> #include "connmgr.h" >> #include "coverage.h" >> #include "csum.h" >> +#include "datapath-config.h" >> #include "dp-packet.h" >> #include "dpif.h" >> #include "in-band.h" >> @@ -5977,6 +5979,25 @@ put_ct_helper(struct xlate_ctx *ctx, >> } >> >> static void >> +put_ct_timeout(struct ofpbuf *odp_actions, const char *dp_type, >> + struct dpif *dpif, const struct flow *flow, >> + struct flow_wildcards *wc, uint16_t zone_id) >> +{ >> + uint32_t tp_id; >> + >> + if (datapath_get_zone_timeout_policy_id(dp_type, zone_id, &tp_id)) { >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + int err = ct_dpif_format_timeout_policy_name( >> + dpif, tp_id, ntohs(flow->dl_type), flow->nw_proto, >> &ds); >> + if (!err) { >> > > > >> + memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); >> > > I think the above unwildcarding should be wrapped in a datapath type > specific (i.e. 'system' in this case) > function. > > Also a note should be added to the function describing the why the > additional datapath flows are > needed (i.e. bcoz of Netfilter timeouts implementation). Note that these > datapath flows will remain if kept > minimally active, which is generally undesirable, hence needs to be > flagged. > > If this is going to delay things, it can be done later from my POV. > > > >> + 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 +6092,8 @@ 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); >> + put_ct_timeout(ctx->odp_actions, ctx->xbridge->ofproto->backer->type, >> + ctx->xbridge->dpif, &ctx->xin->flow, ctx->wc, zone); >> > Let us change: put_ct_timeout(ctx->odp_actions, ctx->xbridge->ofproto->backer->type, ctx->xbridge->dpif, &ctx->xin->flow, ctx->wc, zone); to if (ofc->flags & NX_CT_F_COMMIT) { put_ct_timeout(ctx->odp_actions, ctx->xbridge->ofproto->backer->type, ctx->xbridge->dpif, &ctx->xin->flow, ctx->wc, zone); } since we only need the unwildcarding for 'commit' related flows This will save lots of unnecessary datapath flows. > put_ct_nat(ctx); >> ctx->ct_nat_action = NULL; >> nl_msg_end_nested(ctx->odp_actions, ct_offset); >> -- >> 2.7.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c index 1625754e2441..e2c96b29b17f 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 de032cc416ce..a4e91f975a9b 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/datapath-config.c b/lib/datapath-config.c index cdd2128a60bc..2f3852100b78 100644 --- a/lib/datapath-config.c +++ b/lib/datapath-config.c @@ -377,3 +377,33 @@ destroy_all_datapaths(void) datapath_destroy(dp); } } + +/* If timeout policy is found in datapath '*dp_type' and in 'zone', + * sets timeout policy id in '*tp_id' and returns true. Otherwise, + * returns false. */ +bool +datapath_get_zone_timeout_policy_id(const char *dp_type, uint16_t zone, + uint32_t *tp_id) +{ + struct datapath *dp; + struct ct_zone *ct_zone; + struct ct_timeout_policy *ct_tp; + + dp = datapath_lookup(dp_type); + if (!dp) { + return false; + } + + ct_zone = ct_zone_lookup(&dp->ct_zones, zone); + if (!ct_zone) { + return false; + } + + ct_tp = ct_timeout_policy_lookup(&dp->ct_tps, &ct_zone->tp_uuid); + if (!ct_tp) { + return false; + } + + *tp_id = ct_tp->cdtp.id; + return true; +} diff --git a/lib/datapath-config.h b/lib/datapath-config.h index d9a90e4f8312..0e0cd7eaad2f 100644 --- a/lib/datapath-config.h +++ b/lib/datapath-config.h @@ -21,5 +21,7 @@ void reconfigure_datapath(const struct ovsrec_open_vswitch *, unsigned int idl_seqno); void destroy_all_datapaths(void); +bool datapath_get_zone_timeout_policy_id(const char *dp_type, uint16_t zone, + uint32_t *tp_id); #endif /* datapath-config.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 abfad9543c3b..8e6f2ebb51e1 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -3076,6 +3076,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_TO_NL_TP_TCP_MAPPINGS \ CT_DPIF_TO_NL_TP_MAPPING(TCP, TCP, SYN_SENT, SYN_SENT) \ CT_DPIF_TO_NL_TP_MAPPING(TCP, TCP, SYN_RECV, SYN_RECV) \ @@ -3860,6 +3869,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 3460ef8aa98d..f01f3abee5ab 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -541,6 +541,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..12fa9684d0e4 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -28,10 +28,12 @@ #include "bond.h" #include "bundle.h" #include "byte-order.h" +#include "ct-dpif.h" #include "cfm.h" #include "connmgr.h" #include "coverage.h" #include "csum.h" +#include "datapath-config.h" #include "dp-packet.h" #include "dpif.h" #include "in-band.h" @@ -5977,6 +5979,25 @@ put_ct_helper(struct xlate_ctx *ctx, } static void +put_ct_timeout(struct ofpbuf *odp_actions, const char *dp_type, + struct dpif *dpif, const struct flow *flow, + struct flow_wildcards *wc, uint16_t zone_id) +{ + uint32_t tp_id; + + if (datapath_get_zone_timeout_policy_id(dp_type, zone_id, &tp_id)) { + struct ds ds = DS_EMPTY_INITIALIZER; + int err = ct_dpif_format_timeout_policy_name( + 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 +6092,8 @@ 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); + put_ct_timeout(ctx->odp_actions, ctx->xbridge->ofproto->backer->type, + ctx->xbridge->dpif, &ctx->xin->flow, ctx->wc, zone); put_ct_nat(ctx); ctx->ct_nat_action = NULL; nl_msg_end_nested(ctx->odp_actions, ct_offset);
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@gmail.com> --- lib/ct-dpif.c | 10 ++++++++++ lib/ct-dpif.h | 3 +++ lib/datapath-config.c | 30 ++++++++++++++++++++++++++++++ lib/datapath-config.h | 2 ++ lib/dpif-netdev.c | 1 + lib/dpif-netlink.c | 10 ++++++++++ lib/dpif-provider.h | 5 +++++ ofproto/ofproto-dpif-xlate.c | 23 +++++++++++++++++++++++ 8 files changed, 84 insertions(+)