This change adds a new action 'ct_commit_to_zone' that enables users to commit the flow into a specific zone in the connection tracker.
A new feature flag, OVN_FEATURE_CT_COMMIT_TO_ZONE, is also included to avoid issues during upgrade in case the northd is upgraded to a version that supports this action before the controller is upgraded. Note that this action is meaningful only in the context of Logical Router datapath. Logical Switch datapath does not use multiple zones and this action falls back to committing the connection into the default zone for the Logical Switch. Signed-off-by: Martin Kalcok <[email protected]> --- controller/chassis.c | 8 ++ include/ovn/actions.h | 8 +- include/ovn/features.h | 1 + lib/actions.c | 155 +++++++++++++++++++++++++------------- northd/en-global-config.c | 10 +++ northd/en-global-config.h | 1 + ovn-sb.xml | 22 +++++- tests/ovn.at | 16 ++++ utilities/ovn-trace.c | 45 ++++++++--- 9 files changed, 199 insertions(+), 67 deletions(-) diff --git a/controller/chassis.c b/controller/chassis.c index ad75df288..9bb2eba95 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -371,6 +371,7 @@ chassis_build_other_config(const struct ovs_chassis_cfg *ovs_cfg, smap_replace(config, OVN_FEATURE_FDB_TIMESTAMP, "true"); smap_replace(config, OVN_FEATURE_LS_DPG_COLUMN, "true"); smap_replace(config, OVN_FEATURE_CT_COMMIT_NAT_V2, "true"); + smap_replace(config, OVN_FEATURE_CT_COMMIT_TO_ZONE, "true"); } /* @@ -516,6 +517,12 @@ chassis_other_config_changed(const struct ovs_chassis_cfg *ovs_cfg, return true; } + if (!smap_get_bool(&chassis_rec->other_config, + OVN_FEATURE_CT_COMMIT_TO_ZONE, + false)) { + return true; + } + return false; } @@ -648,6 +655,7 @@ update_supported_sset(struct sset *supported) sset_add(supported, OVN_FEATURE_FDB_TIMESTAMP); sset_add(supported, OVN_FEATURE_LS_DPG_COLUMN); sset_add(supported, OVN_FEATURE_CT_COMMIT_NAT_V2); + sset_add(supported, OVN_FEATURE_CT_COMMIT_TO_ZONE); } static void diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 8e794450c..aa5e4ab89 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -68,6 +68,7 @@ struct collector_set_ids; OVNACT(CT_NEXT, ovnact_ct_next) \ /* CT_COMMIT_V1 is not supported anymore. */ \ OVNACT(CT_COMMIT_V2, ovnact_nest) \ + OVNACT(CT_COMMIT_TO_ZONE, ovnact_ct_commit_to_zone) \ OVNACT(CT_DNAT, ovnact_ct_nat) \ OVNACT(CT_SNAT, ovnact_ct_nat) \ OVNACT(CT_DNAT_IN_CZONE, ovnact_ct_nat) \ @@ -76,7 +77,7 @@ struct collector_set_ids; OVNACT(CT_LB_MARK, ovnact_ct_lb) \ OVNACT(SELECT, ovnact_select) \ OVNACT(CT_CLEAR, ovnact_null) \ - OVNACT(CT_COMMIT_NAT, ovnact_ct_commit_nat) \ + OVNACT(CT_COMMIT_NAT, ovnact_ct_commit_to_zone) \ OVNACT(CLONE, ovnact_nest) \ OVNACT(ARP, ovnact_nest) \ OVNACT(ICMP4, ovnact_nest) \ @@ -290,11 +291,12 @@ struct ovnact_ct_nat { uint8_t ltable; /* Logical table ID of next table. */ }; -/* OVNACT_CT_COMMIT_NAT. */ -struct ovnact_ct_commit_nat { +/* OVNACT_CT_COMMIT_TO_ZONE, OVNACT_CT_COMMIT_NAT. */ +struct ovnact_ct_commit_to_zone { struct ovnact ovnact; bool dnat_zone; + bool do_nat; uint8_t ltable; }; diff --git a/include/ovn/features.h b/include/ovn/features.h index 08f1d8288..35a5d8ba0 100644 --- a/include/ovn/features.h +++ b/include/ovn/features.h @@ -28,6 +28,7 @@ #define OVN_FEATURE_FDB_TIMESTAMP "fdb-timestamp" #define OVN_FEATURE_LS_DPG_COLUMN "ls-dpg-column" #define OVN_FEATURE_CT_COMMIT_NAT_V2 "ct-commit-nat-v2" +#define OVN_FEATURE_CT_COMMIT_TO_ZONE "ct-commit-to-zone" /* OVS datapath supported features. Based on availability OVN might generate * different types of openflows. diff --git a/lib/actions.c b/lib/actions.c index 39bb5bc8a..b9bdcd48d 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -870,6 +870,45 @@ parse_ct_nat(struct action_context *ctx, const char *name, } } +static void +parse_ct_commit_to_zone(struct action_context *ctx, const char *name, + bool do_nat, bool require_param, + struct ovnact_ct_commit_to_zone *cn) +{ + add_prerequisite(ctx, "ip"); + + if (ctx->pp->cur_ltable >= ctx->pp->n_tables) { + lexer_error(ctx->lexer, + "\"%s\" action not allowed in last table.", name); + return; + } + + cn->ltable = ctx->pp->cur_ltable + 1; + cn->do_nat = do_nat; + cn->dnat_zone = true; + + if (require_param) { + lexer_force_match(ctx->lexer, LEX_T_LPAREN); + } else { + if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) { + return; + } + } + + if (lexer_match_id(ctx->lexer, "dnat")) { + cn->dnat_zone = true; + } else if (lexer_match_id(ctx->lexer, "snat")) { + cn->dnat_zone = false; + } else { + lexer_error(ctx->lexer, "\"%s\" action accepts" + " only \"dnat\" or \"snat\" parameter.", name); + return; + } + + lexer_force_match(ctx->lexer, LEX_T_RPAREN); +} + + static void parse_CT_DNAT(struct action_context *ctx) { @@ -901,33 +940,17 @@ parse_CT_SNAT_IN_CZONE(struct action_context *ctx) static void parse_CT_COMMIT_NAT(struct action_context *ctx) { - add_prerequisite(ctx, "ip"); - - if (ctx->pp->cur_ltable >= ctx->pp->n_tables) { - lexer_error(ctx->lexer, - "\"ct_commit_nat\" action not allowed in last table."); - return; - } - - struct ovnact_ct_commit_nat *cn = ovnact_put_CT_COMMIT_NAT(ctx->ovnacts); - cn->ltable = ctx->pp->cur_ltable + 1; - cn->dnat_zone = true; - - if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) { - return; - } - - if (lexer_match_id(ctx->lexer, "dnat")) { - cn->dnat_zone = true; - } else if (lexer_match_id(ctx->lexer, "snat")) { - cn->dnat_zone = false; - } else { - lexer_error(ctx->lexer, "\"ct_commit_nat\" action accepts" - " only \"dnat\" or \"snat\" parameter."); - return; - } + parse_ct_commit_to_zone(ctx, "ct_commit_nat", + true, false, + ovnact_put_CT_COMMIT_NAT(ctx->ovnacts)); +} - lexer_force_match(ctx->lexer, LEX_T_RPAREN); +static void +parse_CT_COMMIT_TO_ZONE(struct action_context *ctx) +{ + parse_ct_commit_to_zone(ctx, "ct_commit_to_zone", + false, true, + ovnact_put_CT_COMMIT_TO_ZONE(ctx->ovnacts)); } static void @@ -980,12 +1003,20 @@ format_CT_SNAT_IN_CZONE(const struct ovnact_ct_nat *cn, struct ds *s) } static void -format_CT_COMMIT_NAT(const struct ovnact_ct_commit_nat *cn, struct ds *s) +format_CT_COMMIT_NAT(const struct ovnact_ct_commit_to_zone *cn, struct ds *s) { ds_put_cstr(s, "ct_commit_nat"); ds_put_cstr(s, cn->dnat_zone ? "(dnat);" : "(snat);"); } +static void +format_CT_COMMIT_TO_ZONE(const struct ovnact_ct_commit_to_zone *cn, + struct ds *s) +{ + ds_put_cstr(s, "ct_commit_to_zone"); + ds_put_cstr(s, cn->dnat_zone ? "(dnat);" : "(snat);"); +} + static void encode_ct_nat(const struct ovnact_ct_nat *cn, const struct ovnact_encode_params *ep, @@ -1055,6 +1086,39 @@ encode_ct_nat(const struct ovnact_ct_nat *cn, ofpbuf_push_uninit(ofpacts, ct_offset); } +static void +encode_ct_commit_to_zone(const struct ovnact_ct_commit_to_zone *cn, + const struct ovnact_encode_params *ep, + struct ofpbuf *ofpacts) +{ + const size_t ct_offset = ofpacts->size; + + struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); + ct->recirc_table = cn->ltable + first_ptable(ep, ep->pipeline); + ct->zone_src.ofs = 0; + ct->zone_src.n_bits = 16; + ct->flags = NX_CT_F_COMMIT; + ct->alg = 0; + + if (ep->is_switch) { + ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE); + } else { + ct->zone_src.field = mf_from_id(cn->dnat_zone + ? MFF_LOG_DNAT_ZONE + : MFF_LOG_SNAT_ZONE); + } + + if (cn->do_nat) { + struct ofpact_nat *nat = ofpact_put_NAT(ofpacts); + nat->range_af = AF_UNSPEC; + nat->flags = 0; + } + + ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); + ofpacts->header = ct; + ofpact_finish_CT(ofpacts, &ct); +} + static void encode_CT_DNAT(const struct ovnact_ct_nat *cn, const struct ovnact_encode_params *ep, @@ -1088,34 +1152,19 @@ encode_CT_SNAT_IN_CZONE(const struct ovnact_ct_nat *cn, } static void -encode_CT_COMMIT_NAT(const struct ovnact_ct_commit_nat *cn, +encode_CT_COMMIT_NAT(const struct ovnact_ct_commit_to_zone *cn, const struct ovnact_encode_params *ep, struct ofpbuf *ofpacts) { - const size_t ct_offset = ofpacts->size; - - struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); - ct->recirc_table = cn->ltable + first_ptable(ep, ep->pipeline); - ct->zone_src.ofs = 0; - ct->zone_src.n_bits = 16; - ct->flags = NX_CT_F_COMMIT; - ct->alg = 0; - - if (ep->is_switch) { - ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE); - } else { - ct->zone_src.field = mf_from_id(cn->dnat_zone - ? MFF_LOG_DNAT_ZONE - : MFF_LOG_SNAT_ZONE); - } - - struct ofpact_nat *nat = ofpact_put_NAT(ofpacts); - nat->range_af = AF_UNSPEC; - nat->flags = 0; + encode_ct_commit_to_zone(cn, ep, ofpacts); +} - ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); - ofpacts->header = ct; - ofpact_finish_CT(ofpacts, &ct); +static void +encode_CT_COMMIT_TO_ZONE(const struct ovnact_ct_commit_to_zone *cn, + const struct ovnact_encode_params *ep, + struct ofpbuf *ofpacts) +{ + encode_ct_commit_to_zone(cn, ep, ofpacts); } static void @@ -1124,7 +1173,7 @@ ovnact_ct_nat_free(struct ovnact_ct_nat *ct_nat OVS_UNUSED) } static void -ovnact_ct_commit_nat_free(struct ovnact_ct_commit_nat *cn OVS_UNUSED) +ovnact_ct_commit_to_zone_free(struct ovnact_ct_commit_to_zone *cn OVS_UNUSED) { } @@ -5306,6 +5355,8 @@ parse_action(struct action_context *ctx) parse_CT_NEXT(ctx); } else if (lexer_match_id(ctx->lexer, "ct_commit")) { parse_CT_COMMIT(ctx); + } else if (lexer_match_id(ctx->lexer, "ct_commit_to_zone")) { + parse_CT_COMMIT_TO_ZONE(ctx); } else if (lexer_match_id(ctx->lexer, "ct_dnat")) { parse_CT_DNAT(ctx); } else if (lexer_match_id(ctx->lexer, "ct_snat")) { diff --git a/northd/en-global-config.c b/northd/en-global-config.c index 34e393b33..193deaebc 100644 --- a/northd/en-global-config.c +++ b/northd/en-global-config.c @@ -370,6 +370,7 @@ northd_enable_all_features(struct ed_type_global_config *data) .fdb_timestamp = true, .ls_dpg_column = true, .ct_commit_nat_v2 = true, + .ct_commit_to_zone = true, }; } @@ -439,6 +440,15 @@ build_chassis_features(const struct sbrec_chassis_table *sbrec_chassis_table, chassis_features->ct_commit_nat_v2) { chassis_features->ct_commit_nat_v2 = false; } + + bool ct_commit_to_zone = + smap_get_bool(&chassis->other_config, + OVN_FEATURE_CT_COMMIT_TO_ZONE, + false); + if (!ct_commit_to_zone && + chassis_features->ct_commit_to_zone) { + chassis_features->ct_commit_to_zone = false; + } } } diff --git a/northd/en-global-config.h b/northd/en-global-config.h index 38d732808..842bcee70 100644 --- a/northd/en-global-config.h +++ b/northd/en-global-config.h @@ -20,6 +20,7 @@ struct chassis_features { bool fdb_timestamp; bool ls_dpg_column; bool ct_commit_nat_v2; + bool ct_commit_to_zone; }; struct global_config_tracked_data { diff --git a/ovn-sb.xml b/ovn-sb.xml index 4c26c6714..ab0c37c8d 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -1432,13 +1432,31 @@ </p> </dd> + <dt><code>ct_commit_to_zone(dnat);</code></dt> + <dt><code>ct_commit_to_zone(snat);</code></dt> + <dd> + <p> + Commit the flow to the specific zone in the connection tracker. + The packet is then automatically sent to the next tables as if + followed by <code>next;</code> action. The next tables will + see the changes in the packet caused by the connection tracker. + </p> + + <p> + Note that this action is meaningful only in the Logical Router + Datapath as the Logical Switch Datapath does not use separate + connection tracking zones. Using this action in Logical Switch + Datapath falls back to committing the flow into the logical port's + conntrack zone. + </p> + </dd> <dt><code>ct_dnat;</code></dt> <dt><code>ct_dnat(<var>IP</var>);</code></dt> <dd> <p> <code>ct_dnat</code> sends the packet through the DNAT zone in connection tracking table to unDNAT any packet that was DNATed in - the opposite direction. The packet is then automatically sent to + the opposite direction. The packet is then automatically sent to the next tables as if followed by <code>next;</code> action. The next tables will see the changes in the packet caused by the connection tracker. @@ -1448,7 +1466,7 @@ DNAT zone to change the destination IP address of the packet to the one provided inside the parentheses and commits the connection. The packet is then automatically sent to the next tables as if - followed by <code>next;</code> action. The next tables will see + followed by <code>next;</code> action. The next tables will see the changes in the packet caused by the connection tracker. </p> </dd> diff --git a/tests/ovn.at b/tests/ovn.at index c8cc1d37f..1852457e1 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1316,6 +1316,22 @@ ct_commit { ct_label=0x181716151413121110090807060504030201; }; ct_commit { ip4.dst = 192.168.0.1; }; Field ip4.dst is not modifiable. +# ct_commit_to_zone +ct_commit_to_zone(dnat); + encodes as ct(commit,table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) + has prereqs ip +ct_commit_to_zone(snat); + encodes as ct(commit,table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) + has prereqs ip +ct_commit_to_zone; + Syntax error at `;' expecting `('. +ct_commit_to_zone(); + "ct_commit_to_zone" action accepts only "dnat" or "snat" parameter. +ct_commit_to_zone(foo); + "ct_commit_to_zone" action accepts only "dnat" or "snat" parameter. +ct_commit_to_zone(dnat; + Syntax error at `;' expecting `)'. + # Legact ct_commit_v1 action. ct_commit(); Syntax error at `(' expecting `;'. diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index ee086a7ae..7aa6e2ca9 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -2463,24 +2463,20 @@ execute_ct_nat(const struct ovnact_ct_nat *ct_nat, } static void -execute_ct_commit_nat(const struct ovnact_ct_commit_nat *ct_nat, - const struct ovntrace_datapath *dp, struct flow *uflow, - enum ovnact_pipeline pipeline, struct ovs_list *super) +ct_commit_to_zone__(const struct ovnact_ct_commit_to_zone *ct_nat, + const struct ovntrace_datapath *dp, struct flow *uflow, + enum ovnact_pipeline pipeline, struct ovs_list *super, + struct ds *action) { struct flow ct_flow = *uflow; - struct ds s = DS_EMPTY_INITIALIZER; - - ds_put_cstr(&s, "ct_commit_nat /* assuming no" - " un-nat entry, so no change */"); /* ct(nat) implies ct(). */ if (!(ct_flow.ct_state & CS_TRACKED)) { - ct_flow.ct_state |= next_ct_state(&s); + ct_flow.ct_state |= next_ct_state(action); } struct ovntrace_node *node = ovntrace_node_append( - super, OVNTRACE_NODE_TRANSFORMATION, "%s", ds_cstr(&s)); - ds_destroy(&s); + super, OVNTRACE_NODE_TRANSFORMATION, "%s", ds_cstr(action)); /* Trace the actions in the next table. */ trace__(dp, &ct_flow, ct_nat->ltable, pipeline, &node->subs); @@ -2490,6 +2486,30 @@ execute_ct_commit_nat(const struct ovnact_ct_commit_nat *ct_nat, * flow, not ct_flow. */ } +static void +execute_ct_commit_nat(const struct ovnact_ct_commit_to_zone *ct_nat, + const struct ovntrace_datapath *dp, struct flow *uflow, + enum ovnact_pipeline pipeline, struct ovs_list *super) +{ + struct ds s = DS_EMPTY_INITIALIZER; + ds_put_cstr(&s, "ct_commit_nat /* assuming no" + " un-nat entry, so no change */"); + ct_commit_to_zone__(ct_nat, dp, uflow, pipeline, super, &s); + ds_destroy(&s); +} + +static void +execute_ct_commit_to_zone(const struct ovnact_ct_commit_to_zone *ct_commit, + const struct ovntrace_datapath *dp, + struct flow *uflow, enum ovnact_pipeline pipeline, + struct ovs_list *super) +{ + struct ds s = DS_EMPTY_INITIALIZER; + ds_put_format(&s, "ct_commit_to_zone(%s)", + ct_commit->dnat_zone ? "dnat" : "snat"); + ct_commit_to_zone__(ct_commit, dp, uflow, pipeline, super, &s); + ds_destroy(&s); +} static void execute_ct_lb(const struct ovnact_ct_lb *ct_lb, @@ -3147,6 +3167,11 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, flow_clear_conntrack(uflow); break; + case OVNACT_CT_COMMIT_TO_ZONE: + execute_ct_commit_to_zone(ovnact_get_CT_COMMIT_TO_ZONE(a), dp, + uflow, pipeline, super); + break; + case OVNACT_CT_COMMIT_NAT: execute_ct_commit_nat(ovnact_get_CT_COMMIT_NAT(a), dp, uflow, pipeline, super); -- 2.40.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
