Skip to content

Commit

Permalink
actions: Adjust the ct_commit_nat action.
Browse files Browse the repository at this point in the history 
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>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit b252f45)
  • Loading branch information
@almusil @dceara
almusil authored and dceara committed Feb 8, 2024
1 parent f424fdc commit c76cb23
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 18 deletions.
12 changes: 10 additions & 2 deletions include/ovn/actions.h
View file
Expand Up @@ -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) \
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down
69 changes: 54 additions & 15 deletions lib/actions.c
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1189,20 +1203,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)
Expand Down
21 changes: 21 additions & 0 deletions tests/ovn.at
View file
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion utilities/ovn-trace.c
View file
Expand Up @@ -2463,7 +2463,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)
{
Expand Down

0 comments on commit c76cb23

Please sign in to comment.