On Wed, Nov 11, 2020 at 6:49 AM Ben Pfaff <[email protected]> wrote: > > The example for the fwd_group action was obviously wrong (it needed a > closing parenthesis and semicolon) and then when I looked closer I saw > other issues: > > - It's weird to insist on "true" or "false" in quotation marks (why?) > and the example didn't show that. > > - The example didn't show that port names need to be quoted. > > - The implementation didn't allow specifying liveness=false (why?) > or liveness="false" and the test even checked for that. > > This fixes all that stuff. For backward compatibility, it still accepts > "true" (and "false") in quotation marks, but now accepts without as well > for future use. > > Signed-off-by: Ben Pfaff <[email protected]>
Oops. Thanks for fixing this. Acked-by: Numan Siddique <[email protected]> Numan > --- > include/ovn/lex.h | 1 + > lib/actions.c | 14 ++++++++------ > lib/lex.c | 13 +++++++++++++ > ovn-sb.xml | 21 ++++++++++++--------- > tests/ovn.at | 14 +++++++++++--- > 5 files changed, 45 insertions(+), 18 deletions(-) > > diff --git a/include/ovn/lex.h b/include/ovn/lex.h > index 1da6ccc8c021..ecb7ace24326 100644 > --- a/include/ovn/lex.h > +++ b/include/ovn/lex.h > @@ -137,6 +137,7 @@ enum lex_type lexer_lookahead(const struct lexer *); > bool lexer_match(struct lexer *, enum lex_type); > bool lexer_force_match(struct lexer *, enum lex_type); > bool lexer_match_id(struct lexer *, const char *id); > +bool lexer_match_string(struct lexer *, const char *s); > bool lexer_is_int(const struct lexer *); > bool lexer_get_int(struct lexer *, int *value); > bool lexer_force_int(struct lexer *, int *value); > diff --git a/lib/actions.c b/lib/actions.c > index 23e54ef2a6c4..6300fef2c48e 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -3344,15 +3344,17 @@ parse_fwd_group_action(struct action_context *ctx) > if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { > return; > } > - if (ctx->lexer->token.type != LEX_T_STRING) { > + if (lexer_match_string(ctx->lexer, "true") || > + lexer_match_id(ctx->lexer, "true")) { > + liveness = true; > + } else if (lexer_match_string(ctx->lexer, "false") || > + lexer_match_id(ctx->lexer, "false")) { > + liveness = false; > + } else { > lexer_syntax_error(ctx->lexer, > - "expecting true/false"); > + "expecting true or false"); > return; > } > - if (!strcmp(ctx->lexer->token.s, "true")) { > - liveness = true; > - lexer_get(ctx->lexer); > - } > lexer_force_match(ctx->lexer, LEX_T_COMMA); > } > if (lexer_match_id(ctx->lexer, "childports")) { > diff --git a/lib/lex.c b/lib/lex.c > index 4d921998877e..c84d52aa8d1d 100644 > --- a/lib/lex.c > +++ b/lib/lex.c > @@ -906,6 +906,19 @@ lexer_match_id(struct lexer *lexer, const char *id) > } > } > > +/* If 'lexer''s current token is the string 's', advances 'lexer' to the next > + * token and returns true. Otherwise returns false. */ > +bool > +lexer_match_string(struct lexer *lexer, const char *s) > +{ > + if (lexer->token.type == LEX_T_STRING && !strcmp(lexer->token.s, s)) { > + lexer_get(lexer); > + return true; > + } else { > + return false; > + } > +} > + > bool > lexer_is_int(const struct lexer *lexer) > { > diff --git a/ovn-sb.xml b/ovn-sb.xml > index b1480f218635..482df00777f3 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -2074,23 +2074,26 @@ > </dl> > </dd> > > - <dt><code>fwd_group(<var>P</var>);</code></dt> > + <dt><code>fwd_group(liveness=<var>bool</var>, > childports=<var>port</var>, ...);</code></dt> > <dd> > <p> > - <b>Parameters</b>: liveness, list of child ports <var>P</var>. > + <b>Parameters</b>: optional <code>liveness</code>, either > + <code>true</code> or <code>false</code>, defaulting to false; > + <code>childports</code>, a comma-delimited list of strings > denoting > + logical ports to load balance across. > </p> > > <p> > - It load balances traffic to one or more child ports in a > - logical switch. <code>ovn-controller</code> translates the > - <code>fwd_group</code> into openflow group with one bucket > - for each child port. If liveness is set to true, it also > - integrates the bucket selection with BFD status on the tunnel > + Load balance traffic to one or more child ports in a logical > + switch. <code>ovn-controller</code> translates the > + <code>fwd_group</code> into an OpenFlow group with one bucket for > + each child port. If <code>liveness=true</code> is specified, it > + also integrates the bucket selection with BFD status on the > tunnel > interface corresponding to child port. > </p> > > - <p><b>Example:</b> <code>fwd_group(liveness=true, childports=p1,p2 > - </code></p> > + <p><b>Example:</b> <code>fwd_group(liveness=true, childports="p1", > + "p2");</code></p> > </dd> > </dl> > > diff --git a/tests/ovn.at b/tests/ovn.at > index 9c23ae73ad86..bc731fefc47a 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1695,7 +1695,8 @@ ip.proto = select(1, 2, 3); > reg0[0..14] = select(1, 2, 3); > cannot use 15-bit field reg0[0..14] for "select", which requires at > least 16 bits. > > -fwd_group(liveness="true", childports="eth0", "lsp1"); > +fwd_group(liveness=true, childports="eth0", "lsp1"); > + formats as fwd_group(liveness="true", childports="eth0", "lsp1"); > encodes as group:11 > uses group: id(11), > name(type=select,selection_method=dp_hash,bucket=watch_port:5,load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=watch_port:17,load=0x17->NXM_NX_REG15[0..15],resubmit(,64)) > > @@ -1709,8 +1710,15 @@ fwd_group(childports=eth0); > fwd_group(); > Syntax error at `)' expecting `;'. > > -fwd_group(liveness="false", childports="eth0", "lsp1"); > - Syntax error at `"false"' expecting `,'. > +fwd_group(childports="eth0", "lsp1"); > + encodes as group:12 > + uses group: id(12), > name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=load=0x17->NXM_NX_REG15[0..15],resubmit(,64)) > + > +fwd_group(liveness=xyzzy, childports="eth0", "lsp1"); > + Syntax error at `xyzzy' expecting true or false. > + > +fwd_group(liveness=false childports="eth0", "lsp1"); > + Syntax error at `childports' expecting `,'. > > # prefix delegation > handle_dhcpv6_reply; > -- > 2.26.2 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
