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 <b...@ovn.org>
---
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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
