| Message ID | 20201120105253.2160203-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] Fix OVN update issue when ovn-controller is updated first from 20.06 to 20.09. | expand |
Acked-by: Flavio Fernandes <flavio@flaviof.com> > On Nov 20, 2020, at 5:52 AM, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > The commit in the Fixes tag changed the ct_commit signature from > ct_commit(..) to ct_commit{ ..}. When ovn-controllers are updated > to a vesion which has this change and if ovn-northd is still not > udated to this version, then the logical flow with the action > ct_commit(..) will be rejected by ovn-controllers resulting in > datapath disruptions. OVN recommends ovn-controller updates before > ovn-northd, but it is broken now. This patch fixes this issue by > adding back the support for the old ct_commit(..). We now support > both the versions of ct_commit. > > Fixes: 6cfb44a76c61("Used nested actions in ct_commit) > CC: Mark Michelson <mmichels@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > include/ovn/actions.h | 10 ++- > lib/actions.c | 137 ++++++++++++++++++++++++++++++++++++++++-- > tests/ovn.at | 44 ++++++++++++++ > utilities/ovn-trace.c | 3 +- > 4 files changed, 186 insertions(+), 8 deletions(-) > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index 630bbe79e..7bec6a6f8 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -57,7 +57,8 @@ struct ovn_extend_table; > OVNACT(EXCHANGE, ovnact_move) \ > OVNACT(DEC_TTL, ovnact_null) \ > OVNACT(CT_NEXT, ovnact_ct_next) \ > - OVNACT(CT_COMMIT, ovnact_nest) \ > + OVNACT(CT_COMMIT_V1, ovnact_ct_commit_v1) \ > + OVNACT(CT_COMMIT_V2, ovnact_nest) \ > OVNACT(CT_DNAT, ovnact_ct_nat) \ > OVNACT(CT_SNAT, ovnact_ct_nat) \ > OVNACT(CT_LB, ovnact_ct_lb) \ > @@ -226,6 +227,13 @@ 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; > +}; > + > /* OVNACT_CT_DNAT, OVNACT_CT_SNAT. */ > struct ovnact_ct_nat { > struct ovnact ovnact; > diff --git a/lib/actions.c b/lib/actions.c > index 82b35ccf9..36c8078b0 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -627,16 +627,75 @@ 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, "ip", > + 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"); > - struct ovnact_nest *on = ovnact_put(ctx->ovnacts, OVNACT_CT_COMMIT, > + struct ovnact_nest *on = ovnact_put(ctx->ovnacts, OVNACT_CT_COMMIT_V2, > OVNACT_ALIGN(sizeof *on)); > on->nested_len = 0; > on->nested = NULL; > @@ -644,7 +703,73 @@ parse_CT_COMMIT(struct action_context *ctx) > } > > static void > -format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s) > +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; > + > + 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) > { > if (on->nested_len) { > format_nested_action(on, "ct_commit", s); > @@ -654,9 +779,9 @@ format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s) > } > > static void > -encode_CT_COMMIT(const struct ovnact_nest *on, > - const struct ovnact_encode_params *ep OVS_UNUSED, > - struct ofpbuf *ofpacts) > +encode_CT_COMMIT_V2(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; > diff --git a/tests/ovn.at b/tests/ovn.at > index 2253d0bde..e2568bb66 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1102,6 +1102,50 @@ ct_commit { ct_label=0x181716151413121110090807060504030201; }; > ct_commit { ip4.dst = 192.168.0.1; }; > Field ip4.dst is not modifiable. > > +# Legact ct_commit_v1 action. > +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); > + 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); > + 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); > + 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); > + 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); > + 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); > + 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); > + 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); > + encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0xffffffffffffffff->ct_label)) > + has prereqs ip > +ct_commit(ct_label=18446744073709551616); > + Decimal constants must be less than 2**64. > + > ct_mark = 12345 > Field ct_mark is not modifiable. > ct_label = 0xcafe > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > index 5d92188ab..8eb7263b3 100644 > --- a/utilities/ovn-trace.c > +++ b/utilities/ovn-trace.c > @@ -2336,7 +2336,8 @@ 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: > + case OVNACT_CT_COMMIT_V1: > + case OVNACT_CT_COMMIT_V2: > /* Nothing to do. */ > break; > > -- > 2.28.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Nov 23, 2020 at 4:21 AM Flavio Fernandes <flavio@flaviof.com> wrote: > > Acked-by: Flavio Fernandes <flavio@flaviof.com> Thanks Flavio for the reviews. I applied this patch to master. I think this should also be backported to branch-20.09. I will do it in sometime. Thanks Numan > > > > On Nov 20, 2020, at 5:52 AM, numans@ovn.org wrote: > > > > From: Numan Siddique <numans@ovn.org> > > > > The commit in the Fixes tag changed the ct_commit signature from > > ct_commit(..) to ct_commit{ ..}. When ovn-controllers are updated > > to a vesion which has this change and if ovn-northd is still not > > udated to this version, then the logical flow with the action > > ct_commit(..) will be rejected by ovn-controllers resulting in > > datapath disruptions. OVN recommends ovn-controller updates before > > ovn-northd, but it is broken now. This patch fixes this issue by > > adding back the support for the old ct_commit(..). We now support > > both the versions of ct_commit. > > > > Fixes: 6cfb44a76c61("Used nested actions in ct_commit) > > CC: Mark Michelson <mmichels@redhat.com> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > include/ovn/actions.h | 10 ++- > > lib/actions.c | 137 ++++++++++++++++++++++++++++++++++++++++-- > > tests/ovn.at | 44 ++++++++++++++ > > utilities/ovn-trace.c | 3 +- > > 4 files changed, 186 insertions(+), 8 deletions(-) > > > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > > index 630bbe79e..7bec6a6f8 100644 > > --- a/include/ovn/actions.h > > +++ b/include/ovn/actions.h > > @@ -57,7 +57,8 @@ struct ovn_extend_table; > > OVNACT(EXCHANGE, ovnact_move) \ > > OVNACT(DEC_TTL, ovnact_null) \ > > OVNACT(CT_NEXT, ovnact_ct_next) \ > > - OVNACT(CT_COMMIT, ovnact_nest) \ > > + OVNACT(CT_COMMIT_V1, ovnact_ct_commit_v1) \ > > + OVNACT(CT_COMMIT_V2, ovnact_nest) \ > > OVNACT(CT_DNAT, ovnact_ct_nat) \ > > OVNACT(CT_SNAT, ovnact_ct_nat) \ > > OVNACT(CT_LB, ovnact_ct_lb) \ > > @@ -226,6 +227,13 @@ 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; > > +}; > > + > > /* OVNACT_CT_DNAT, OVNACT_CT_SNAT. */ > > struct ovnact_ct_nat { > > struct ovnact ovnact; > > diff --git a/lib/actions.c b/lib/actions.c > > index 82b35ccf9..36c8078b0 100644 > > --- a/lib/actions.c > > +++ b/lib/actions.c > > @@ -627,16 +627,75 @@ 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, "ip", > > + 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"); > > - struct ovnact_nest *on = ovnact_put(ctx->ovnacts, OVNACT_CT_COMMIT, > > + struct ovnact_nest *on = ovnact_put(ctx->ovnacts, OVNACT_CT_COMMIT_V2, > > OVNACT_ALIGN(sizeof *on)); > > on->nested_len = 0; > > on->nested = NULL; > > @@ -644,7 +703,73 @@ parse_CT_COMMIT(struct action_context *ctx) > > } > > > > static void > > -format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s) > > +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; > > + > > + 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) > > { > > if (on->nested_len) { > > format_nested_action(on, "ct_commit", s); > > @@ -654,9 +779,9 @@ format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s) > > } > > > > static void > > -encode_CT_COMMIT(const struct ovnact_nest *on, > > - const struct ovnact_encode_params *ep OVS_UNUSED, > > - struct ofpbuf *ofpacts) > > +encode_CT_COMMIT_V2(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; > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 2253d0bde..e2568bb66 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -1102,6 +1102,50 @@ ct_commit { ct_label=0x181716151413121110090807060504030201; }; > > ct_commit { ip4.dst = 192.168.0.1; }; > > Field ip4.dst is not modifiable. > > > > +# Legact ct_commit_v1 action. > > +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); > > + 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); > > + 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); > > + 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); > > + 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); > > + 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); > > + 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); > > + 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); > > + encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0xffffffffffffffff->ct_label)) > > + has prereqs ip > > +ct_commit(ct_label=18446744073709551616); > > + Decimal constants must be less than 2**64. > > + > > ct_mark = 12345 > > Field ct_mark is not modifiable. > > ct_label = 0xcafe > > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > > index 5d92188ab..8eb7263b3 100644 > > --- a/utilities/ovn-trace.c > > +++ b/utilities/ovn-trace.c > > @@ -2336,7 +2336,8 @@ 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: > > + case OVNACT_CT_COMMIT_V1: > > + case OVNACT_CT_COMMIT_V2: > > /* Nothing to do. */ > > break; > > > > -- > > 2.28.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 630bbe79e..7bec6a6f8 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -57,7 +57,8 @@ struct ovn_extend_table; OVNACT(EXCHANGE, ovnact_move) \ OVNACT(DEC_TTL, ovnact_null) \ OVNACT(CT_NEXT, ovnact_ct_next) \ - OVNACT(CT_COMMIT, ovnact_nest) \ + OVNACT(CT_COMMIT_V1, ovnact_ct_commit_v1) \ + OVNACT(CT_COMMIT_V2, ovnact_nest) \ OVNACT(CT_DNAT, ovnact_ct_nat) \ OVNACT(CT_SNAT, ovnact_ct_nat) \ OVNACT(CT_LB, ovnact_ct_lb) \ @@ -226,6 +227,13 @@ 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; +}; + /* OVNACT_CT_DNAT, OVNACT_CT_SNAT. */ struct ovnact_ct_nat { struct ovnact ovnact; diff --git a/lib/actions.c b/lib/actions.c index 82b35ccf9..36c8078b0 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -627,16 +627,75 @@ 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, "ip", + 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"); - struct ovnact_nest *on = ovnact_put(ctx->ovnacts, OVNACT_CT_COMMIT, + struct ovnact_nest *on = ovnact_put(ctx->ovnacts, OVNACT_CT_COMMIT_V2, OVNACT_ALIGN(sizeof *on)); on->nested_len = 0; on->nested = NULL; @@ -644,7 +703,73 @@ parse_CT_COMMIT(struct action_context *ctx) } static void -format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s) +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; + + 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) { if (on->nested_len) { format_nested_action(on, "ct_commit", s); @@ -654,9 +779,9 @@ format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s) } static void -encode_CT_COMMIT(const struct ovnact_nest *on, - const struct ovnact_encode_params *ep OVS_UNUSED, - struct ofpbuf *ofpacts) +encode_CT_COMMIT_V2(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; diff --git a/tests/ovn.at b/tests/ovn.at index 2253d0bde..e2568bb66 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1102,6 +1102,50 @@ ct_commit { ct_label=0x181716151413121110090807060504030201; }; ct_commit { ip4.dst = 192.168.0.1; }; Field ip4.dst is not modifiable. +# Legact ct_commit_v1 action. +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); + 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); + 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); + 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); + 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); + 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); + 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); + 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); + encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(set_field:0xffffffffffffffff->ct_label)) + has prereqs ip +ct_commit(ct_label=18446744073709551616); + Decimal constants must be less than 2**64. + ct_mark = 12345 Field ct_mark is not modifiable. ct_label = 0xcafe diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index 5d92188ab..8eb7263b3 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -2336,7 +2336,8 @@ 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: + case OVNACT_CT_COMMIT_V1: + case OVNACT_CT_COMMIT_V2: /* Nothing to do. */ break;