@@ -75,7 +75,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_nat) \
+ OVNACT(CT_COMMIT_NAT, ovnact_ct_commit_nat) \
OVNACT(CLONE, ovnact_nest) \
OVNACT(ARP, ovnact_nest) \
OVNACT(ICMP4, ovnact_nest) \
@@ -274,7 +274,7 @@ enum ovnact_ct_nat_type {
OVNACT_CT_NAT_UNSPEC,
};
-/* OVNACT_CT_DNAT, OVNACT_CT_SNAT, OVNACT_CT_COMMIT_NAT. */
+/* OVNACT_CT_DNAT, OVNACT_CT_SNAT. */
struct ovnact_ct_nat {
struct ovnact ovnact;
int family;
@@ -296,6 +296,14 @@ struct ovnact_ct_nat {
uint8_t ltable; /* Logical table ID of next table. */
};
+/* OVNACT_CT_COMMIT_NAT. */
+struct ovnact_ct_commit_nat {
+ struct ovnact ovnact;
+
+ bool dnat_zone;
+ uint8_t ltable;
+};
+
enum ovnact_ct_lb_flag {
OVNACT_CT_LB_FLAG_NONE,
OVNACT_CT_LB_FLAG_SKIP_SNAT,
@@ -1020,16 +1020,29 @@ parse_CT_COMMIT_NAT(struct action_context *ctx)
if (ctx->pp->cur_ltable >= ctx->pp->n_tables) {
lexer_error(ctx->lexer,
- "\"ct_commit_related\" action not allowed in last table.");
+ "\"ct_commit_nat\" action not allowed in last table.");
return;
}
- struct ovnact_ct_nat *cn = ovnact_put_CT_COMMIT_NAT(ctx->ovnacts);
- cn->commit = true;
+ struct ovnact_ct_commit_nat *cn = ovnact_put_CT_COMMIT_NAT(ctx->ovnacts);
cn->ltable = ctx->pp->cur_ltable + 1;
- cn->family = AF_UNSPEC;
- cn->type = OVNACT_CT_NAT_UNSPEC;
- cn->port_range.exists = false;
+ 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;
+ }
+
+ lexer_force_match(ctx->lexer, LEX_T_RPAREN);
}
static void
@@ -1082,9 +1095,10 @@ format_CT_SNAT_IN_CZONE(const struct ovnact_ct_nat *cn, struct ds *s)
}
static void
-format_CT_COMMIT_NAT(const struct ovnact_ct_nat *cn OVS_UNUSED, struct ds *s)
+format_CT_COMMIT_NAT(const struct ovnact_ct_commit_nat *cn, struct ds *s)
{
- ds_put_cstr(s, "ct_commit_nat;");
+ ds_put_cstr(s, "ct_commit_nat");
+ ds_put_cstr(s, cn->dnat_zone ? "(dnat);" : "(snat);");
}
static void
@@ -1177,20 +1191,45 @@ encode_CT_SNAT_IN_CZONE(const struct ovnact_ct_nat *cn,
}
static void
-encode_CT_COMMIT_NAT(const struct ovnact_ct_nat *cn,
- const struct ovnact_encode_params *ep,
- struct ofpbuf *ofpacts)
+encode_CT_COMMIT_NAT(const struct ovnact_ct_commit_nat *cn,
+ const struct ovnact_encode_params *ep,
+ struct ofpbuf *ofpacts)
{
- enum mf_field_id zone = ep->is_switch
- ? MFF_LOG_CT_ZONE
- : MFF_LOG_DNAT_ZONE;
- encode_ct_nat(cn, ep, zone, 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;
+
+ ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct);
+ ofpacts->header = ct;
+ ofpact_finish_CT(ofpacts, &ct);
}
static void
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)
+{
+}
static void
parse_ct_lb_action(struct action_context *ctx, bool ct_lb_mark)
@@ -1498,9 +1498,30 @@ ct_clear;
# ct_commit_nat
ct_commit_nat;
+ formats as ct_commit_nat(dnat);
encodes as ct(commit,table=19,zone=NXM_NX_REG13[0..15],nat)
has prereqs ip
+ct_commit_nat(snat);
+ encodes as ct(commit,table=19,zone=NXM_NX_REG13[0..15],nat)
+ has prereqs ip
+
+ct_commit_nat(dnat);
+ encodes as ct(commit,table=19,zone=NXM_NX_REG13[0..15],nat)
+ has prereqs ip
+
+ct_commit_nat(snat, dnat);
+ Syntax error at `,' expecting `)'.
+
+ct_commit_nat(dnat, ignore);
+ Syntax error at `,' expecting `)'.
+
+ct_commit_nat(ignore);
+ "ct_commit_nat" action accepts only "dnat" or "snat" parameter.
+
+ct_commit_nat();
+ "ct_commit_nat" action accepts only "dnat" or "snat" parameter.
+
# clone
clone { ip4.dst = 255.255.255.255; output; }; next;
encodes as clone(set_field:255.255.255.255->ip_dst,resubmit(,64)),resubmit(,19)
@@ -2462,7 +2462,7 @@ execute_ct_nat(const struct ovnact_ct_nat *ct_nat,
}
static void
-execute_ct_commit_nat(const struct ovnact_ct_nat *ct_nat,
+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)
{
The ct_commit nat was hardcoded to use DNAT zone in router pipeline. Extend it that it accepts two new arguments (snat/dnat) which will determine the zone for router pipeline. The switch pipeline has only one, so it resolves to the same for both arguments. In order to keep backward compatibility the ct_commit_nat without any arguments is the same as ct_commit_nat(dnat). Signed-off-by: Ales Musil <amusil@redhat.com> --- v2: Rebase on top of current main. Address comment from Dumitru: - Make the ct_commit_nat to accept only snat/dnat parameter. --- include/ovn/actions.h | 12 ++++++-- lib/actions.c | 69 +++++++++++++++++++++++++++++++++---------- tests/ovn.at | 21 +++++++++++++ utilities/ovn-trace.c | 2 +- 4 files changed, 86 insertions(+), 18 deletions(-)