@@ -57,7 +57,7 @@ struct ovn_extend_table;
OVNACT(EXCHANGE, ovnact_move) \
OVNACT(DEC_TTL, ovnact_null) \
OVNACT(CT_NEXT, ovnact_ct_next) \
- OVNACT(CT_COMMIT, ovnact_ct_commit) \
+ OVNACT(CT_COMMIT, ovnact_nest) \
OVNACT(CT_DNAT, ovnact_ct_nat) \
OVNACT(CT_SNAT, ovnact_ct_nat) \
OVNACT(CT_LB, ovnact_ct_lb) \
@@ -220,13 +220,6 @@ struct ovnact_ct_next {
uint8_t ltable; /* Logical table ID of next table. */
};
-/* OVNACT_CT_COMMIT. */
-struct ovnact_ct_commit {
- struct ovnact ovnact;
- uint32_t ct_mark, ct_mark_mask;
- ovs_be128 ct_label, ct_label_mask;
-};
-
/* OVNACT_CT_DNAT, OVNACT_CT_SNAT. */
struct ovnact_ct_nat {
struct ovnact ovnact;
@@ -85,6 +85,7 @@ enum expr_level {
enum expr_write_scope {
WR_DEFAULT = (1 << 0), /* Writeable at "global" level */
+ WR_CT_COMMIT = (1 << 1), /* Writeable in "ct_commit" action */
};
const char *expr_level_to_string(enum expr_level);
@@ -200,6 +200,15 @@ struct action_context {
static void parse_actions(struct action_context *, enum lex_type sentinel);
+static void parse_nested_action(struct action_context *ctx,
+ enum ovnact_type type,
+ const char *prereq,
+ enum expr_write_scope scope);
+
+static void format_nested_action(const struct ovnact_nest *on,
+ const char *name,
+ struct ds *s);
+
static bool
action_parse_field(struct action_context *ctx,
int n_bits, bool rw, struct expr_field *f)
@@ -618,125 +627,42 @@ ovnact_ct_next_free(struct ovnact_ct_next *a OVS_UNUSED)
{
}
-static void
-parse_ct_commit_arg(struct action_context *ctx,
- struct ovnact_ct_commit *cc)
-{
- if (lexer_match_id(ctx->lexer, "ct_mark")) {
- if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
- return;
- }
- if (ctx->lexer->token.type == LEX_T_INTEGER) {
- cc->ct_mark = ntohll(ctx->lexer->token.value.integer);
- cc->ct_mark_mask = UINT32_MAX;
- } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) {
- cc->ct_mark = ntohll(ctx->lexer->token.value.integer);
- cc->ct_mark_mask = ntohll(ctx->lexer->token.mask.integer);
- } else {
- lexer_syntax_error(ctx->lexer, "expecting integer");
- return;
- }
- lexer_get(ctx->lexer);
- } else if (lexer_match_id(ctx->lexer, "ct_label")) {
- if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
- return;
- }
- if (ctx->lexer->token.type == LEX_T_INTEGER) {
- cc->ct_label = ctx->lexer->token.value.be128_int;
- cc->ct_label_mask = OVS_BE128_MAX;
- } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) {
- cc->ct_label = ctx->lexer->token.value.be128_int;
- cc->ct_label_mask = ctx->lexer->token.mask.be128_int;
- } else {
- lexer_syntax_error(ctx->lexer, "expecting integer");
- return;
- }
- lexer_get(ctx->lexer);
- } else {
- lexer_syntax_error(ctx->lexer, NULL);
- }
-}
-
static void
parse_CT_COMMIT(struct action_context *ctx)
{
- add_prerequisite(ctx, "ip");
- struct ovnact_ct_commit *ct_commit = ovnact_put_CT_COMMIT(ctx->ovnacts);
- if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
- while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
- parse_ct_commit_arg(ctx, ct_commit);
- if (ctx->lexer->error) {
- return;
- }
- lexer_match(ctx->lexer, LEX_T_COMMA);
- }
- }
+ parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
+ WR_CT_COMMIT);
}
static void
-format_CT_COMMIT(const struct ovnact_ct_commit *cc, struct ds *s)
+format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s)
{
- ds_put_cstr(s, "ct_commit(");
- if (cc->ct_mark_mask) {
- ds_put_format(s, "ct_mark=%#"PRIx32, cc->ct_mark);
- if (cc->ct_mark_mask != UINT32_MAX) {
- ds_put_format(s, "/%#"PRIx32, cc->ct_mark_mask);
- }
- }
- if (!ovs_be128_is_zero(cc->ct_label_mask)) {
- if (ds_last(s) != '(') {
- ds_put_cstr(s, ", ");
- }
-
- ds_put_format(s, "ct_label=");
- ds_put_hex(s, &cc->ct_label, sizeof cc->ct_label);
- if (!ovs_be128_equals(cc->ct_label_mask, OVS_BE128_MAX)) {
- ds_put_char(s, '/');
- ds_put_hex(s, &cc->ct_label_mask, sizeof cc->ct_label_mask);
- }
- }
- if (!ds_chomp(s, '(')) {
- ds_put_char(s, ')');
- }
- ds_put_char(s, ';');
+ format_nested_action(on, "ct_commit", s);
}
static void
-encode_CT_COMMIT(const struct ovnact_ct_commit *cc,
+encode_CT_COMMIT(const struct ovnact_nest *on,
const struct ovnact_encode_params *ep OVS_UNUSED,
struct ofpbuf *ofpacts)
{
struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
ct->flags = NX_CT_F_COMMIT;
ct->recirc_table = NX_CT_RECIRC_NONE;
- ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE);
+ ct->zone_src.field = ep->is_switch
+ ? mf_from_id(MFF_LOG_CT_ZONE)
+ : mf_from_id(MFF_LOG_DNAT_ZONE);
ct->zone_src.ofs = 0;
ct->zone_src.n_bits = 16;
size_t set_field_offset = ofpacts->size;
ofpbuf_pull(ofpacts, set_field_offset);
- if (cc->ct_mark_mask) {
- const ovs_be32 value = htonl(cc->ct_mark);
- const ovs_be32 mask = htonl(cc->ct_mark_mask);
- ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_MARK), &value, &mask);
- }
-
- if (!ovs_be128_is_zero(cc->ct_label_mask)) {
- ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_LABEL), &cc->ct_label,
- &cc->ct_label_mask);
- }
-
+ ovnacts_encode(on->nested, on->nested_len, ep, ofpacts);
ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset);
ct = ofpacts->header;
ofpact_finish(ofpacts, &ct->ofpact);
}
-
-static void
-ovnact_ct_commit_free(struct ovnact_ct_commit *cc OVS_UNUSED)
-{
-}
static void
parse_ct_nat(struct action_context *ctx, const char *name,
@@ -123,10 +123,13 @@ ovn_init_symtab(struct shash *symtab)
flags_str);
/* Connection tracking state. */
- expr_symtab_add_field(symtab, "ct_mark", MFF_CT_MARK, NULL, false);
+ expr_symtab_add_field_scoped(symtab, "ct_mark", MFF_CT_MARK, NULL, false,
+ WR_CT_COMMIT);
- expr_symtab_add_field(symtab, "ct_label", MFF_CT_LABEL, NULL, false);
- expr_symtab_add_subfield(symtab, "ct_label.blocked", NULL, "ct_label[0]");
+ expr_symtab_add_field_scoped(symtab, "ct_label", MFF_CT_LABEL, NULL, false,
+ WR_CT_COMMIT);
+ expr_symtab_add_subfield_scoped(symtab, "ct_label.blocked", NULL,
+ "ct_label[0]", WR_CT_COMMIT);
expr_symtab_add_field(symtab, "ct_state", MFF_CT_STATE, NULL, false);
@@ -5357,7 +5357,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
ds_clear(&match);
ds_clear(&actions);
ds_put_cstr(&match, "ct.est && ct_label.blocked == 0");
- ds_put_cstr(&actions, "ct_commit(ct_label=1/1); ");
+ ds_put_cstr(&actions, "ct_commit { ct_label.blocked = 1; }; ");
if (!strcmp(acl->action, "reject")) {
build_reject_acl_rules(od, lflows, stage, acl, &match,
&actions, &acl->header_);
@@ -5881,9 +5881,11 @@ build_stateful(struct ovn_datapath *od, struct hmap *lflows, struct hmap *lbs)
* any packet that makes it this far is part of a connection we
* want to allow to continue. */
ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100,
- REGBIT_CONNTRACK_COMMIT" == 1", "ct_commit(ct_label=0/1); next;");
+ REGBIT_CONNTRACK_COMMIT" == 1",
+ "ct_commit { ct_label.blocked = 0; }; next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 100,
- REGBIT_CONNTRACK_COMMIT" == 1", "ct_commit(ct_label=0/1); next;");
+ REGBIT_CONNTRACK_COMMIT" == 1",
+ "ct_commit { ct_label.blocked = 0; }; next;");
/* If REGBIT_CONNTRACK_NAT is set as 1, then packets should just be sent
* through nat (without committing).
@@ -1261,10 +1261,10 @@
</p>
</dd>
- <dt><code>ct_commit;</code></dt>
- <dt><code>ct_commit(ct_mark=<var>value[/mask]</var>);</code></dt>
- <dt><code>ct_commit(ct_label=<var>value[/mask]</var>);</code></dt>
- <dt><code>ct_commit(ct_mark=<var>value[/mask]</var>, ct_label=<var>value[/mask]</var>);</code></dt>
+ <dt><code>ct_commit { };</code></dt>
+ <dt><code>ct_commit { ct_mark=<var>value[/mask]</var>; };</code></dt>
+ <dt><code>ct_commit { ct_label=<var>value[/mask]</var>; };</code></dt>
+ <dt><code>ct_commit { ct_mark=<var>value[/mask]</var>; ct_label=<var>value[/mask]</var>; };</code></dt>
<dd>
<p>
Commit the flow to the connection tracking entry associated with it
@@ -1276,6 +1276,9 @@
tracking entry. <code>ct_mark</code> is a 32-bit field.
<code>ct_label</code> is a 128-bit field. The <var>value[/mask]</var>
should be specified in hex string if more than 64bits are to be used.
+ Registers and other named fields can be used for <var>value</var>.
+ <code>ct_mark</code> and <code>ct_label</code> may be sub-addressed
+ in order to have specific bits set.
</p>
<p>
@@ -1048,51 +1048,60 @@ ct_next;
has prereqs ip
# ct_commit
-ct_commit;
+ct_commit { };
+ formats as ct_commit { drop; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15])
has prereqs ip
-ct_commit();
- formats as ct_commit;
- encodes as ct(commit,zone=NXM_NX_REG13[0..15])
- has prereqs ip
-ct_commit(ct_mark=1);
- formats as ct_commit(ct_mark=0x1);
+ct_commit { ct_mark=1; };
+ formats as ct_commit { ct_mark = 1; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1->ct_mark))
has prereqs ip
-ct_commit(ct_mark=1/1);
- formats as ct_commit(ct_mark=0x1/0x1);
+ct_commit { ct_mark=1/1; };
+ formats as ct_commit { ct_mark = 1/1; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1/0x1->ct_mark))
has prereqs ip
-ct_commit(ct_label=1);
- formats as ct_commit(ct_label=0x1);
+ct_commit { ct_label=1; };
+ formats as ct_commit { ct_label = 1; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1->ct_label))
has prereqs ip
-ct_commit(ct_label=1/1);
- formats as ct_commit(ct_label=0x1/0x1);
+ct_commit { ct_label=1/1; };
+ formats as ct_commit { ct_label = 1/1; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1/0x1->ct_label))
has prereqs ip
-ct_commit(ct_mark=1, ct_label=2);
- formats as ct_commit(ct_mark=0x1, ct_label=0x2);
+ct_commit { ct_mark=1; ct_label=2; };
+ formats as ct_commit { ct_mark = 1; ct_label = 2; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1->ct_mark,set_field:0x2->ct_label))
has prereqs ip
-ct_commit(ct_label=0x01020304050607080910111213141516);
- formats as ct_commit(ct_label=0x1020304050607080910111213141516);
+ct_commit { ct_label=0x01020304050607080910111213141516; };
+ formats as ct_commit { ct_label = 0x1020304050607080910111213141516; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1020304050607080910111213141516->ct_label))
has prereqs ip
-ct_commit(ct_label=0x181716151413121110090807060504030201);
- formats as ct_commit(ct_label=0x16151413121110090807060504030201);
- encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x16151413121110090807060504030201->ct_label))
- has prereqs ip
-ct_commit(ct_label=0x1000000000000000000000000000000/0x1000000000000000000000000000000);
+ct_commit { ct_label=0x1000000000000000000000000000000/0x1000000000000000000000000000000; };
+ formats as ct_commit { ct_label = 0x1000000000000000000000000000000/0x1000000000000000000000000000000; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0x1000000000000000000000000000000/0x1000000000000000000000000000000->ct_label))
has prereqs ip
-ct_commit(ct_label=18446744073709551615);
- formats as ct_commit(ct_label=0xffffffffffffffff);
+ct_commit { ct_label=18446744073709551615; };
+ formats as ct_commit { ct_label = 18446744073709551615; };
encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0xffffffffffffffff->ct_label))
has prereqs ip
-ct_commit(ct_label=18446744073709551616);
+ct_commit { ct_label[0..47] = 0x00000f040201; ct_label[48..63] = 0x0002; };
+ formats as ct_commit { ct_label[0..47] = 0xf040201; ct_label[48..63] = 0x2; };
+ encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0xf040201/0xffffffffffff->ct_label,set_field:0x2000000000000/0xffff000000000000->ct_label))
+ has prereqs ip
+ct_commit { ct_label=18446744073709551616; };
Decimal constants must be less than 2**64.
+ct_commit { ct_label=0x181716151413121110090807060504030201; };
+ 141-bit constant is not compatible with 128-bit field ct_label.
+ct_commit { ip4.dst = 192.168.0.1; };
+ Field ip4.dst is not modifiable.
+
+ct_mark = 12345
+ Field ct_mark is not modifiable.
+ct_label = 0xcafe
+ Field ct_label is not modifiable.
+ct_label.blocked = 1/1
+ Field ct_label.blocked is not modifiable.
# ct_dnat
ct_dnat;
ct_commit allows for ct_label and ct_mark to be set within. However, there are some restrictions with the current implementation: * It is not possible to address the indiviual bits within the ct_mark or ct_label. * It is not possible to set these to the value of a register. Only explicit integer setting can be used. With this change, ct_commit now can have arbitrary nested actions inside. This makes it similar to how the "exec" option works in OVS's ct() action. ct_commit now also sets a writeability scope so that ct_mark and ct_label are the only symbols that are writeable. The positive side effect is that ct_mark and ct_label are no longer writeable except for inside ct_commit. In this commit, the only noticeable effect is that it allows for slightly more expressive setting of ct_label.blocked. A future commit will take further advantage of this. Signed-off-by: Mark Michelson <mmichels@redhat.com> --- include/ovn/actions.h | 9 +--- include/ovn/expr.h | 1 + lib/actions.c | 110 +++++++----------------------------------- lib/logical-fields.c | 9 ++-- northd/ovn-northd.c | 8 +-- ovn-sb.xml | 11 +++-- tests/ovn.at | 59 ++++++++++++---------- 7 files changed, 72 insertions(+), 135 deletions(-)