On Tue, Apr 2, 2024 at 5:31 PM Dumitru Ceara <[email protected]> wrote:
> It's not used by any logical flow since v20.12.0. It was kept for
> backwards compatibility reasons in 4799f73fd6ed ("Fix OVN update
> issue when ovn-controller is updated first from 20.06 to 20.09.") but
> now there's no supported release that uses it.
>
> Therefore the action is safe to be removed. Update the tests to reflect
> the fact that the action is not supported anymore.
>
> Signed-off-by: Dumitru Ceara <[email protected]>
> ---
> include/ovn/actions.h | 9 +--
> lib/actions.c | 141 ------------------------------------------
> tests/ovn.at | 41 ++++--------
> utilities/ovn-trace.c | 1 -
> 4 files changed, 12 insertions(+), 180 deletions(-)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index dcacbb1fff..8e794450cf 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -66,7 +66,7 @@ struct collector_set_ids;
> OVNACT(EXCHANGE, ovnact_move) \
> OVNACT(DEC_TTL, ovnact_null) \
> OVNACT(CT_NEXT, ovnact_ct_next) \
> - OVNACT(CT_COMMIT_V1, ovnact_ct_commit_v1) \
> + /* CT_COMMIT_V1 is not supported anymore. */ \
> OVNACT(CT_COMMIT_V2, ovnact_nest) \
> OVNACT(CT_DNAT, ovnact_ct_nat) \
> OVNACT(CT_SNAT, ovnact_ct_nat) \
> @@ -260,13 +260,6 @@ struct ovnact_ct_next {
> uint8_t ltable; /* Logical table ID of next table. */
> };
>
> -/* OVNACT_CT_COMMIT_V1. */
> -struct ovnact_ct_commit_v1 {
> - struct ovnact ovnact;
> - uint32_t ct_mark, ct_mark_mask;
> - ovs_be128 ct_label, ct_label_mask;
> -};
> -
> /* Type of NAT used for the particular action.
> * UNSPEC translates to applying NAT that works for both directions. */
> enum ovnact_ct_nat_type {
> diff --git a/lib/actions.c b/lib/actions.c
> index 71615fc53c..7c20535717 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -729,71 +729,12 @@ ovnact_ct_next_free(struct ovnact_ct_next *a
> OVS_UNUSED)
> {
> }
>
> -static void
> -parse_ct_commit_v1_arg(struct action_context *ctx,
> - struct ovnact_ct_commit_v1 *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_V1(struct action_context *ctx)
> -{
> - add_prerequisite(ctx, "ip");
> -
> - struct ovnact_ct_commit_v1 *ct_commit =
> - ovnact_put_CT_COMMIT_V1(ctx->ovnacts);
> - if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> - while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
> - parse_ct_commit_v1_arg(ctx, ct_commit);
> - if (ctx->lexer->error) {
> - return;
> - }
> - lexer_match(ctx->lexer, LEX_T_COMMA);
> - }
> - }
> -}
> -
> static void
> parse_CT_COMMIT(struct action_context *ctx)
> {
> if (ctx->lexer->token.type == LEX_T_LCURLY) {
> parse_nested_action(ctx, OVNACT_CT_COMMIT_V2, "ip",
> WR_CT_COMMIT);
> - } else if (ctx->lexer->token.type == LEX_T_LPAREN) {
> - parse_CT_COMMIT_V1(ctx);
> } else {
> /* Add an empty nested action to allow for "ct_commit;" syntax */
> add_prerequisite(ctx, "ip");
> @@ -804,88 +745,6 @@ parse_CT_COMMIT(struct action_context *ctx)
> }
> }
>
> -static void
> -format_CT_COMMIT_V1(const struct ovnact_ct_commit_v1 *cc, 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, ';');
> -}
> -
> -static void
> -encode_CT_COMMIT_V1(const struct ovnact_ct_commit_v1 *cc,
> - 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.ofs = 0;
> - ct->zone_src.n_bits = 16;
> -
> - /* If the datapath supports all-zero SNAT then use it to avoid tuple
> - * collisions at commit time between NATed and firewalled-only
> sessions.
> - */
> -
> - if (ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)) {
> - size_t nat_offset = ofpacts->size;
> - ofpbuf_pull(ofpacts, nat_offset);
> -
> - struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
> - nat->flags = 0;
> - nat->range_af = AF_UNSPEC;
> - nat->flags |= NX_NAT_F_SRC;
> - ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
> - ct = ofpacts->header;
> - }
> -
> - 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);
> - }
> -
> - ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset);
> - ct = ofpacts->header;
> - ofpact_finish(ofpacts, &ct->ofpact);
> -}
> -
> -static void
> -ovnact_ct_commit_v1_free(struct ovnact_ct_commit_v1 *cc OVS_UNUSED)
> -{
> -}
> -
> -
> -
> static void
> format_CT_COMMIT_V2(const struct ovnact_nest *on, struct ds *s)
> {
> diff --git a/tests/ovn.at b/tests/ovn.at
> index dd2ebce98e..f05437b9cd 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1318,47 +1318,28 @@ ct_commit { ip4.dst = 192.168.0.1; };
>
> # Legact ct_commit_v1 action.
> ct_commit();
> - formats as ct_commit;
> - encodes as ct(commit,zone=NXM_NX_REG13[[0..15]])
> - has prereqs ip
> + Syntax error at `(' expecting `;'.
> ct_commit(ct_mark=1);
> - formats as ct_commit(ct_mark=0x1);
> - encodes as
> ct(commit,zone=NXM_NX_REG13[[0..15]],exec(set_field:0x1->ct_mark))
> - has prereqs ip
> + Syntax error at `(' expecting `;'.
> ct_commit(ct_mark=1/1);
> - formats as ct_commit(ct_mark=0x1/0x1);
> - encodes as
> ct(commit,zone=NXM_NX_REG13[[0..15]],exec(set_field:0x1/0x1->ct_mark))
> - has prereqs ip
> + Syntax error at `(' expecting `;'.
> ct_commit(ct_label=1);
> - formats as ct_commit(ct_label=0x1);
> - encodes as
> ct(commit,zone=NXM_NX_REG13[[0..15]],exec(set_field:0x1->ct_label))
> - has prereqs ip
> + Syntax error at `(' expecting `;'.
> ct_commit(ct_label=1/1);
> - formats as ct_commit(ct_label=0x1/0x1);
> - encodes as
> ct(commit,zone=NXM_NX_REG13[[0..15]],exec(set_field:0x1/0x1->ct_label))
> - has prereqs ip
> + Syntax error at `(' expecting `;'.
> ct_commit(ct_mark=1, ct_label=2);
> - formats as ct_commit(ct_mark=0x1, ct_label=0x2);
> - encodes as
> ct(commit,zone=NXM_NX_REG13[[0..15]],exec(set_field:0x1->ct_mark,set_field:0x2->ct_label))
> - has prereqs ip
> + Syntax error at `(' expecting `;'.
>
> 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
> + Syntax error at `(' expecting `;'.
> 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
> + Syntax error at `(' expecting `;'.
>
>
> 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
> + Syntax error at `(' expecting `;'.
> ct_commit(ct_label=18446744073709551615);
> - formats as ct_commit(ct_label=0xffffffffffffffff);
> - encodes as
> ct(commit,zone=NXM_NX_REG13[[0..15]],exec(set_field:0xffffffffffffffff->ct_label))
> - has prereqs ip
> + Syntax error at `(' expecting `;'.
> ct_commit(ct_label=18446744073709551616);
> - Decimal constants must be less than 2**64.
> + Syntax error at `(' expecting `;'.
>
> ct_mark = 12345
> Field ct_mark is not modifiable.
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index e0f1c3ec9a..5e55fbbcc0 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -3107,7 +3107,6 @@ trace_actions(const struct ovnact *ovnacts, size_t
> ovnacts_len,
> execute_ct_next(ovnact_get_CT_NEXT(a), dp, uflow, pipeline,
> super);
> break;
>
> - case OVNACT_CT_COMMIT_V1:
> case OVNACT_CT_COMMIT_V2:
> /* Nothing to do. */
> break;
> --
> 2.44.0
>
>
Makes sense, thanks!
Acked-by: Ales Musil <[email protected]>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
