ct_commit allow 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. 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 <[email protected]> --- include/ovn/actions.h | 9 +--- lib/actions.c | 109 +++++++----------------------------------- northd/ovn-northd.c | 8 ++-- 3 files changed, 22 insertions(+), 104 deletions(-) diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 0f7f4cdb8..12b7ab0df 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -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; diff --git a/lib/actions.c b/lib/actions.c index e14907e3d..330fd616b 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -199,6 +199,14 @@ 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); + +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) @@ -617,125 +625,40 @@ 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"); } 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, diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 192198272..d10e5ee5d 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -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). -- 2.25.4 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
