[ovs-dev] [PATCH ovn v2 3/5] ovn: Back off on port_security specification and implementation.

[Ben Pfaff]

The implementation of port_security was partial, and the specification was
vague.  This reduces both to what was actually meant to be supported at
this point.  It shouldn't be hard to fully implement port security when we
have a clear specification.

Reported-by: Justin Pettit <jpettit at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ovn/northd/ovn-northd.c | 67 +++++--------------------------------------------
 ovn/ovn-nb.xml          | 10 +++-----
 2 files changed, 10 insertions(+), 67 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index b0c6b4e..be6430d 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -202,66 +202,10 @@ pipeline_add(struct pipeline_ctx *ctx,
     sbrec_pipeline_set_actions(pipeline, actions);
 }
 
-/* A single port security constraint.  This is a parsed version of a single
- * member of the port_security column in the OVN_NB Logical_Port table.
- *
- * Each token has type LEX_T_END if that field is missing, otherwise
- * LEX_T_INTEGER or LEX_T_MASKED_INTEGER. */
-struct ps_constraint {
-    struct lex_token eth;
-    struct lex_token ip4;
-    struct lex_token ip6;
-};
-
-/* Parses a member of the port_security column 'ps' into 'c'.  Returns true if
- * successful, false on syntax error. */
-static bool
-parse_port_security(const char *ps, struct ps_constraint *c)
-{
-    c->eth.type = LEX_T_END;
-    c->ip4.type = LEX_T_END;
-    c->ip6.type = LEX_T_END;
-
-    struct lexer lexer;
-    lexer_init(&lexer, ps);
-    do {
-        if (lexer.token.type == LEX_T_INTEGER ||
-            lexer.token.type == LEX_T_MASKED_INTEGER) {
-            struct lex_token *t;
-
-            t = (lexer.token.format == LEX_F_IPV4 ? &c->ip4
-                 : lexer.token.format == LEX_F_IPV6 ? &c->ip6
-                 : lexer.token.format == LEX_F_ETHERNET ? &c->eth
-                 : NULL);
-            if (t) {
-                if (t->type == LEX_T_END) {
-                    *t = lexer.token;
-                } else {
-                    VLOG_INFO("%s: port_security has duplicate %s address",
-                              ps, lex_format_to_string(lexer.token.format));
-                }
-                lexer_get(&lexer);
-                lexer_match(&lexer, LEX_T_COMMA);
-                continue;
-            }
-        }
-
-        VLOG_INFO("%s: syntax error in port_security", ps);
-        lexer_destroy(&lexer);
-        return false;
-    } while (lexer.token.type != LEX_T_END);
-    lexer_destroy(&lexer);
-
-    return true;
-}
-
 /* Appends port security constraints on L2 address field 'eth_addr_field'
  * (e.g. "eth.src" or "eth.dst") to 'match'.  'port_security', with
  * 'n_port_security' elements, is the collection of port_security constraints
- * from an OVN_NB Logical_Port row.
- *
- * (This is naive; it's not yet possible to express complete L2 and L3 port
- * security constraints as a single Boolean expression.) */
+ * from an OVN_NB Logical_Port row. */
 static void
 build_port_security(const char *eth_addr_field,
                     char **port_security, size_t n_port_security,
@@ -272,14 +216,15 @@ build_port_security(const char *eth_addr_field,
 
     size_t n = 0;
     for (size_t i = 0; i < n_port_security; i++) {
-        struct ps_constraint c;
-        if (parse_port_security(port_security[i], &c)
-            && c.eth.type != LEX_T_END) {
-            lex_token_format(&c.eth, match);
+        uint8_t ea[ETH_ADDR_LEN];
+
+        if (eth_addr_from_string(port_security[i], ea)) {
+            ds_put_format(match, ETH_ADDR_FMT, ETH_ADDR_ARGS(ea));
             ds_put_char(match, ' ');
             n++;
         }
     }
+    ds_chomp(match, ' ');
     ds_put_cstr(match, "}");
 
     if (!n) {
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 2e792a7..2335349 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -137,7 +137,7 @@
 
     <column name="port_security">
       <p>
-        A set of L2 (Ethernet) or L3 (IPv4 or IPv6) addresses or L2+L3 pairs
+        A set of L2 (Ethernet) addresses
         from which the logical port is allowed to send packets and to which it
         is allowed to receive packets.  If this column is empty, all addresses
         are permitted.  Logical ports are always allowed to receive packets
@@ -145,14 +145,12 @@
       </p>
 
       <p>
-        Each member of the set is a comma- or space-separated list.  A single
-        set member may have an Ethernet address, an IPv4 address, and an IPv6
-        address, or any subset.  Order is not significant.
+        Each member of the set is an Ethernet address in the form
+        <var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>.
       </p>
 
       <p>
-        TBD: exact semantics.  For now only Ethernet port security is
-        implemented.
+	This specification will be extended to support L3 port security.
       </p>
     </column>
 
-- 
2.1.3