[ovs-dev] [PATCH v2] Allow bare ct_commits when no nested actions are required.

[Mark Michelson]

On 8/6/20 12:16 PM, Numan Siddique wrote:
> 
> 
> On Thu, Aug 6, 2020 at 8:22 PM Mark Michelson <mmichels at redhat.com 
> <mailto:mmichels at redhat.com>> wrote:
> 
>     In the fixes commit below, ct_commit was changed to use nested actions.
>     This requires that curly braces be present for all ct_commits. When
>     adjusting ovn-northd, some ct_commits were not updated to have them.
>     This commit changes the behavior of the ct_commit action not to require
>     curly braces if there are no nested actions required.
> 
>     Fixes: 6cfb44a76c61("Used nested actions in ct_commit")
>     Signed-off-by: Mark Michelson <mmichels at redhat.com
>     <mailto:mmichels at redhat.com>>
> 
> 
> Thanks for the fix.
> Acked-by: Numan Siddique <numans at ovn.org <mailto:numans at ovn.org>>
> 
> The system test case - 29: ovn --Test packet drops due to incorrect 
> flows in physical table 33 FAILED (system-ovn.at:4538 
> <http://system-ovn.at:4538>) is failing
> on my setup. But this patch is not the issue. I see failure with the 
> master too. Just FYI.

OK. Just to throw out another data point, that test passes on my setup.

> 
> Thanks
> Numan
> 
>     ---
>       lib/actions.c | 20 ++++++++++++++++----
>       tests/ovn.at <http://ovn.at>  |  5 ++++-
>       2 files changed, 20 insertions(+), 5 deletions(-)
> 
>     diff --git a/lib/actions.c b/lib/actions.c
>     index 05fa44b60..4afc23d66 100644
>     --- a/lib/actions.c
>     +++ b/lib/actions.c
>     @@ -630,15 +630,27 @@ ovnact_ct_next_free(struct ovnact_ct_next *a
>     OVS_UNUSED)
>       static void
>       parse_CT_COMMIT(struct action_context *ctx)
>       {
>     -
>     -    parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
>     -                        WR_CT_COMMIT);
>     +    if (ctx->lexer->token.type == LEX_T_LCURLY) {
>     +        parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
>     +                            WR_CT_COMMIT);
>     +    } 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,
>     +                                            OVNACT_ALIGN(sizeof *on));
>     +        on->nested_len = 0;
>     +        on->nested = NULL;
>     +    }
>       }
> 
>       static void
>       format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s)
>       {
>     -    format_nested_action(on, "ct_commit", s);
>     +    if (on->nested_len) {
>     +        format_nested_action(on, "ct_commit", s);
>     +    } else {
>     +        ds_put_cstr(s, "ct_commit;");
>     +    }
>       }
> 
>       static void
>     diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at <http://ovn.at>
>     index b0179a8db..7236eeb8e 100644
>     --- a/tests/ovn.at <http://ovn.at>
>     +++ b/tests/ovn.at <http://ovn.at>
>     @@ -1050,8 +1050,11 @@ ct_next;
>           has prereqs ip
> 
>       # ct_commit
>     +ct_commit;
>     +    encodes as ct(commit,zone=NXM_NX_REG13[0..15])
>     +    has prereqs ip
>       ct_commit { };
>     -    formats as ct_commit { drop; };
>     +    formats as ct_commit;
>           encodes as ct(commit,zone=NXM_NX_REG13[0..15])
>           has prereqs ip
>       ct_commit { ct_mark=1; };
>     -- 
>     2.25.4
> 
>     _______________________________________________
>     dev mailing list
>     dev at openvswitch.org <mailto:dev at openvswitch.org>
>     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>