[ovs-dev] [PATCH v7] ovn: Add a new logical switch port type - 'virtual'

Ben Pfaff blp at ovn.org
Wed Jul 17 23:45:16 UTC 2019


Guru has some comments and questions for you.  I have the following
suggested incremental to improve the parsing and error checking:

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 318d80d5d1b7..66916a837fd3 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -2607,17 +2607,16 @@ parse_bind_vport(struct action_context *ctx)
     }
 
     if (ctx->lexer->token.type != LEX_T_STRING) {
-        lexer_error(ctx->lexer,
-                    "bind_vport requires port name to be specified.");
+        lexer_syntax_error(ctx->lexer, "expecting port name string");
         return;
     }
 
     struct ovnact_bind_vport *bind_vp = ovnact_put_BIND_VPORT(ctx->ovnacts);
     bind_vp->vport = xstrdup(ctx->lexer->token.s);
     lexer_get(ctx->lexer);
-    lexer_force_match(ctx->lexer, LEX_T_COMMA);
-    action_parse_field(ctx, 0, false, &bind_vp->vport_parent);
-    lexer_force_match(ctx->lexer, LEX_T_RPAREN);
+    (void) (lexer_force_match(ctx->lexer, LEX_T_COMMA)
+            && action_parse_field(ctx, 0, false, &bind_vp->vport_parent)
+            && lexer_force_match(ctx->lexer, LEX_T_RPAREN));
 }
 
 static void
diff --git a/tests/ovn.at b/tests/ovn.at
index 2837be1671b6..5d6c90c5fb6f 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1372,10 +1372,19 @@ reg0[0] = check_pkt_larger(foo);
 # lsp1's port key is 0x11.
 bind_vport("lsp1", inport);
     encodes as controller(userdata=00.00.00.11.00.00.00.00.11.00.00.00)
-
 # lsp2 doesn't exist. So it should be encoded as drop.
 bind_vport("lsp2", inport);
     encodes as drop
+bind_vport;
+    Syntax error at `;' expecting `('.
+bind_vport(;
+    Syntax error at `;' expecting port name string.
+bind_vport("xyzzy";
+    Syntax error at `;' expecting `,'.
+bind_vport("xyzzy",;
+    Syntax error at `;' expecting field name.
+bind_vport("xyzzy", inport;
+    Syntax error at `;' expecting `)'.
 
 # Miscellaneous negative tests.
 ;


More information about the dev mailing list