For every port security defined for a logical port, add following lflows
in "ls_in_port_sec" and "ls_out_port_sec" stage
   - A priority 90 flow to allow ipv4 traffic for known ip addresses
     and (broadcast ip - for ingress, mainly for dhcp)
   - A priority 80 flow to drop all ipv4 traffic.
   - For ingress, a priority 90 flow to allow arp traffic for known
      ip addresses and priority 80 flow to drop all arp traffic
   - A priority 90 flow to allow ipv6 traffic for all ipv6 addresses if
     port security has ipv6 address(es) defined
     (next patch will address ipv6)
   - A priority 80 flow to drop all ipv6 traffic.
   - A priority 50 flow to allow all traffic on that port with the matching
     eth address

Eg. if the port security is "00:00:00:00:00:01 10.0.0.2"

priority=90, match=(inport == "portname" && eth.src == 00:00:00:00:00:01
&& arp && arp.sha == 00:00:00:00:00:01 && (arp.spa == 10.0.0.2)), action=(next;)

priority=90, match=(inport == "portname" && eth.src == 00:00:00:00:00:01
&& ip4 && ((ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255) ||
ip4.src == 10.0.0.3)), action=(next;)

priority=80, match=(inport == "portname" && eth.src == 00:00:00:00:00:01
&& (arp || ip4)), action=(drop;)

priority=80, match=(inport == "portname" && eth.src == 00:00:00:00:00:01
&& ip6), action=(drop;)

priority=50, match=(inport == "portname" && eth.src == 00:00:00:00:00:01),
action=(next;)

Signed-off-by: Numan Siddique <nusid...@redhat.com>
---
 lib/packets.c               | 102 ++++++++++++++++
 lib/packets.h               |  10 ++
 ovn/northd/ovn-northd.8.xml |  82 ++++++++++---
 ovn/northd/ovn-northd.c     | 283 ++++++++++++++++++++++++++++++++++++++------
 tests/ovn.at                | 255 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 680 insertions(+), 52 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index d82341d..b338846 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -455,6 +455,34 @@ ip_parse_masked(const char *s, ovs_be32 *ip, ovs_be32 
*mask)
     return NULL;
 }
 
+/*
+ * This function is similar to ip_parse_masked(), with an extra parameter
+ * `n` added to return the number of scanned characters.
+ */
+char * OVS_WARN_UNUSED_RESULT
+ip_parse_masked_len(const char *s, int *n, ovs_be32 *ip,
+                    ovs_be32 *mask)
+{
+    int prefix;
+
+    if (ovs_scan_len(s, n, IP_SCAN_FMT"/"IP_SCAN_FMT,
+                 IP_SCAN_ARGS(ip), IP_SCAN_ARGS(mask))) {
+        /* OK. */
+    } else if (ovs_scan_len(s, n, IP_SCAN_FMT"/%d",
+                            IP_SCAN_ARGS(ip), &prefix)) {
+        if (prefix <= 0 || prefix > 32) {
+            return xasprintf("%s: network prefix bits not between 0 and "
+                             "32", s);
+        }
+        *mask = be32_prefix_mask(prefix);
+    } else if (ovs_scan_len(s, n, IP_SCAN_FMT, IP_SCAN_ARGS(ip))) {
+        *mask = OVS_BE32_MAX;
+    } else {
+        return xasprintf("%s: invalid IP address", s);
+    }
+    return NULL;
+}
+
 /* Similar to ip_parse_masked(), but the mask, if present, must be a CIDR mask
  * and is returned as a prefix length in '*plen'. */
 char * OVS_WARN_UNUSED_RESULT
@@ -475,6 +503,26 @@ ip_parse_cidr(const char *s, ovs_be32 *ip, unsigned int 
*plen)
     return NULL;
 }
 
+/* Similar to ip_parse_cidr(), with an extra parameter `n` added to return
+ * the number of scanned characters. */
+char * OVS_WARN_UNUSED_RESULT
+ip_parse_cidr_len(const char *s, int *n, ovs_be32 *ip, unsigned int *plen)
+{
+    ovs_be32 mask;
+    char *error;
+
+    error = ip_parse_masked_len(s, n, ip, &mask);
+    if (error) {
+        return error;
+    }
+
+    if (!ip_is_cidr(mask)) {
+        return xasprintf("%s: CIDR network required", s);
+    }
+    *plen = ip_count_cidr_bits(mask);
+    return NULL;
+}
+
 /* Parses string 's', which must be an IPv6 address.  Stores the IPv6 address
  * into '*ip'.  Returns true if successful, otherwise false. */
 bool
@@ -519,6 +567,39 @@ ipv6_parse_masked(const char *s, struct in6_addr *ip, 
struct in6_addr *mask)
     return xasprintf("%s: invalid IPv6 address", s);
 }
 
+/*
+ * This function is similar to ipv6_parse_masked(), with an extra parameter
+ * `n` added to return the number of scanned characters.
+ */
+char * OVS_WARN_UNUSED_RESULT
+ipv6_parse_masked_len(const char *s, int *n, struct in6_addr *ip,
+                      struct in6_addr *mask)
+{
+    char ipv6_s[IPV6_SCAN_LEN + 1];
+    int prefix;
+
+    if (ovs_scan_len(s, n, " "IPV6_SCAN_FMT, ipv6_s)
+        && ipv6_parse(ipv6_s, ip)) {
+        if (ovs_scan_len(s, n, "/%d", &prefix)) {
+            if (prefix <= 0 || prefix > 128) {
+                return xasprintf("%s: IPv6 network prefix bits not between 0 "
+                                 "and 128", s);
+            }
+            *mask = ipv6_create_mask(prefix);
+        } else if (ovs_scan_len(s, n, "/"IPV6_SCAN_FMT, ipv6_s)) {
+             if (!ipv6_parse(ipv6_s, mask)) {
+                 return xasprintf("%s: Invalid IPv6 mask", s);
+             }
+            /* OK. */
+        } else {
+            /* OK. No mask */
+            *mask = in6addr_exact;
+        }
+        return NULL;
+    }
+    return xasprintf("%s: invalid IPv6 address", s);
+}
+
 /* Similar to ipv6_parse_masked(), but the mask, if present, must be a CIDR
  * mask and is returned as a prefix length in '*plen'. */
 char * OVS_WARN_UNUSED_RESULT
@@ -539,6 +620,27 @@ ipv6_parse_cidr(const char *s, struct in6_addr *ip, 
unsigned int *plen)
     return NULL;
 }
 
+/* Similar to ipv6_parse_cidr(), with an extra parameter `n` added to return
+ * the number of scanned characters. */
+char * OVS_WARN_UNUSED_RESULT
+ipv6_parse_cidr_len(const char *s, int *n, struct in6_addr *ip,
+                    unsigned int *plen)
+{
+    struct in6_addr mask;
+    char *error;
+
+    error = ipv6_parse_masked_len(s, n, ip, &mask);
+    if (error) {
+        return error;
+    }
+
+    if (!ipv6_is_cidr(&mask)) {
+        return xasprintf("%s: IPv6 CIDR network required", s);
+    }
+    *plen = ipv6_count_cidr_bits(&mask);
+    return NULL;
+}
+
 /* Stores the string representation of the IPv6 address 'addr' into the
  * character array 'addr_str', which must be at least INET6_ADDRSTRLEN
  * bytes long. */
diff --git a/lib/packets.h b/lib/packets.h
index 834e8a4..31a4e92 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -586,6 +586,11 @@ char *ip_parse_masked(const char *s, ovs_be32 *ip, 
ovs_be32 *mask)
     OVS_WARN_UNUSED_RESULT;
 char *ip_parse_cidr(const char *s, ovs_be32 *ip, unsigned int *plen)
     OVS_WARN_UNUSED_RESULT;
+char *ip_parse_masked_len(const char *s, int *n, ovs_be32 *ip, ovs_be32 *mask)
+    OVS_WARN_UNUSED_RESULT;
+char *ip_parse_cidr_len(const char *s, int *n, ovs_be32 *ip,
+                        unsigned int *plen)
+    OVS_WARN_UNUSED_RESULT;
 
 #define IP_VER(ip_ihl_ver) ((ip_ihl_ver) >> 4)
 #define IP_IHL(ip_ihl_ver) ((ip_ihl_ver) & 15)
@@ -1033,6 +1038,11 @@ char *ipv6_parse_masked(const char *s, struct in6_addr 
*ipv6,
                         struct in6_addr *mask);
 char *ipv6_parse_cidr(const char *s, struct in6_addr *ip, unsigned int *plen)
     OVS_WARN_UNUSED_RESULT;
+char *ipv6_parse_masked_len(const char *s, int *n, struct in6_addr *ipv6,
+                            struct in6_addr *mask);
+char *ipv6_parse_cidr_len(const char *s, int *n, struct in6_addr *ip,
+                          unsigned int *plen)
+    OVS_WARN_UNUSED_RESULT;
 
 void *eth_compose(struct dp_packet *, const struct eth_addr eth_dst,
                   const struct eth_addr eth_src, uint16_t eth_type,
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 1b2912e..376356e 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -124,12 +124,63 @@
       </li>
 
       <li>
-        Priority 50 flows that implement ingress port security for each enabled
-        logical port.  For logical ports on which port security is enabled,
-        these match the <code>inport</code> and the valid <code>eth.src</code>
-        address(es) and advance only those packets to the next flow table.  For
-        logical ports on which port security is not enabled, these advance all
-        packets that match the <code>inport</code>.
+        <p>
+          Port security flows to allow traffic for each enabled logical port.
+          For logical ports on which port security is not enabled, a priority 
50
+          flow is added to allow all packets that match the 
<code>inport</code>.
+        </p>
+
+        <p>
+          For logical ports on which port security is enabled, following flows
+          are added.
+        </p>
+
+        <p>
+          For each element in the port security set,
+        </p>
+
+        <ul>
+          <li>
+            Priority 90 flow is added to allow ipv4 traffic if it has ipv4
+            addresses which match the <code>inport</code>, valid
+            <code>eth.src</code> and valid <code>ip4.src</code> address(es).
+          </li>
+
+          <li>
+            Priority 90 flow is added to allow arp traffic if it has ipv4
+            addresses which match the <code>inport</code>, valid
+            <code>eth.src</code>, valid <code>arp.sha</code> and valid
+            <code>arp.spa</code> address(es).
+          </li>
+
+          <li>
+            Priority 90 flow is added to allow all ipv6 traffic if it has
+            ipv6 address(es).
+          </li>
+
+          <li>
+            Priority 80 flow is added to drop all ipv4 traffic if it has
+            ipv4/ipv6 address(es) which match the <code>inport</code> and
+            <code>eth.src</code>
+          </li>
+
+          <li>
+            Priority 80 flow is added to drop all arp traffic if it has ipv4
+            addresses which match the <code>inport</code> and
+            <code>eth.src</code>
+          </li>
+
+          <li>
+            Priority 80 flow is added to drop all ipv6 traffic if it has
+            ipv4/ipv6 address(es) which match the <code>inport</code> and
+            <code>eth.src</code>
+          </li>
+
+          <li>
+            Priority 50 flow is added to allow all traffic which match the
+            <code>inport</code> and the valid <code>eth.src</code>
+          </li>
+        </ul>
       </li>
     </ul>
 
@@ -270,15 +321,16 @@ output;
     <p>
       This is similar to the ingress port security logic in ingress table 0,
       but with important differences.  Most obviously, <code>outport</code> and
-      <code>eth.dst</code> are checked instead of <code>inport</code> and
-      <code>eth.src</code>.  Second, packets directed to broadcast or multicast
-      <code>eth.dst</code> are always accepted instead of being subject to the
-      port security rules; this is implemented through a priority-100 flow that
-      matches on <code>eth.mcast</code> with action <code>output;</code>.
-      Finally, to ensure that even broadcast and multicast packets are not
-      delivered to disabled logical ports, a priority-150 flow for each
-      disabled logical <code>outport</code> overrides the priority-100 flow
-      with a <code>drop;</code> action.
+      <code>eth.dst</code> and <code>ip4.dst</code> are checked instead of
+      <code>inport</code>, <code>eth.src</code>, and <code>ip4.src</code>.
+      Second, packets directed to broadcast or multicast <code>eth.dst</code>
+      are always accepted instead of being subject to the port security rules;
+      this is implemented through a priority-100 flow that matches on
+      <code>eth.mcast</code> with action <code>output;</code>. Third, flows
+      related to <code>arp</code> are not added. Finally, to ensure that even
+      broadcast and multicast packets are not delivered to disabled logical
+      ports, a priority-150 flow for each disabled logical <code>outport</code>
+      overrides the priority-100 flow with a <code>drop;</code> action.
     </p>
 
     <h2>Logical Router Datapaths</h2>
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4f03287..b10b2e3 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -914,33 +914,252 @@ ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow 
*lflow)
     }
 }
 
-/* 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. */
+struct port_security_options {
+    struct eth_addr ea;
+    int n_ipv4_addrs;
+    ovs_be32 *ipv4_addrs;
+    int n_ipv6_addrs;
+    struct in6_addr *ipv6_addrs;
+};
+
+/*
+ * Extracts the mac, ipv4 and ipv6 addresses from the input
+ * param 'port_security' and returns the struct port_security_options *.
+ */
+static struct port_security_options *
+extract_port_security_options(char *port_security)
+{
+    struct port_security_options *ps;
+    int n = 0;
+    char *p = port_security;
+    ps = xzalloc(sizeof (*ps));
+    if (!ovs_scan_len(p, &n, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ps->ea))) {
+        free(ps);
+        return NULL;
+    }
+
+    ovs_be32 ip4;
+    unsigned int plen;
+    int n1 = n;
+    int i = 0;
+    char *error;
+    while (!(error = ip_parse_cidr_len(p, &n1, &ip4, &plen))) {
+         ps->n_ipv4_addrs++;
+    }
+
+    if (error) {
+        free(error);
+    }
+
+    struct in6_addr ip6;
+    while (!(error = ipv6_parse_cidr_len(p, &n1, &ip6, &plen))) {
+         ps->n_ipv6_addrs++;
+    }
+
+    if (error) {
+        free(error);
+    }
+
+    n1 = n;
+    if (ps->n_ipv4_addrs) {
+        ps->ipv4_addrs = xmalloc(ps->n_ipv4_addrs * sizeof (ovs_be32));
+        i = 0;
+        while (!(error = ip_parse_cidr_len(p, &n1, &ip4, &plen))) {
+            ps->ipv4_addrs[i++] = ip4;
+        }
+
+        if (error) {
+            free(error);
+        }
+    }
+
+    if (ps->n_ipv6_addrs) {
+        ps->ipv6_addrs = xmalloc(ps->n_ipv6_addrs * sizeof (struct in6_addr));
+        i = 0;
+        while (!(error = ipv6_parse_cidr_len(p, &n1, &ps->ipv6_addrs[i],
+                                             &plen))) {
+            i++;
+        }
+
+        if (error) {
+            free(error);
+        }
+    }
+    return ps;
+}
+
 static void
-build_port_security(const char *eth_addr_field,
-                    char **port_security, size_t n_port_security,
-                    struct ds *match)
+build_port_security_arp_flow(struct ds *match, char *port_name,
+                             struct eth_addr ea, ovs_be32 *ipv4_addrs,
+                             int n_ipv4_addrs)
 {
-    size_t base_len = match->length;
-    ds_put_format(match, " && %s == {", eth_addr_field);
+    ds_put_format(match, "inport == %s && eth.src == "ETH_ADDR_FMT" && arp"
+                  " && arp.sha == "ETH_ADDR_FMT" && (", port_name,
+                  ETH_ADDR_ARGS(ea), ETH_ADDR_ARGS(ea));
+    for(int i = 0; i < n_ipv4_addrs; i++) {
+        if (i) {
+            ds_put_cstr(match, " || ");
+        }
+        ds_put_format(match, "arp.spa == "IP_FMT, IP_ARGS(ipv4_addrs[i]));
+    }
+    ds_put_cstr(match, ")");
+}
 
-    size_t n = 0;
-    for (size_t i = 0; i < n_port_security; i++) {
-        struct eth_addr ea;
+static void
+build_port_security_ipv4_flow(enum ovn_pipeline pipeline, struct ds *match,
+                              ovs_be32 *ipv4_addrs, int n_ipv4_addrs)
+{
+    char *ipv4_addr_field;
+    if (pipeline == P_IN) {
+        ipv4_addr_field = "ip4.src";
+        /* For ingress pipeline allow broadcast traffic (for dhcp) */
+        ds_put_cstr(match, " && ip4 && ((ip4.src == 0.0.0.0 &&"
+                    " ip4.dst == 255.255.255.255) || ");
+    }
+    else {
+        ipv4_addr_field = "ip4.dst";
+        ds_put_cstr(match, " && ip4 && (");
+    }
 
-        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++;
+    for(int i = 0; i < n_ipv4_addrs; i++) {
+        if (i) {
+            ds_put_cstr(match, " || ");
         }
+        ds_put_format(match, "%s == "IP_FMT, ipv4_addr_field,
+                      IP_ARGS(ipv4_addrs[i]));
     }
-    ds_chomp(match, ' ');
-    ds_put_cstr(match, "}");
+    ds_put_cstr(match, ")");
+}
 
-    if (!n) {
-        match->length = base_len;
+/*
+ * Build port security constraints on L2 and L3 address fields and add
+ * logical flows to S_SWITCH_(IN/OUT)_PORT_SEC stage.
+ *
+ * If there is no port security enabled for the logical port add a flow
+ * to allow all the traffic.
+ * For each port security of the logical port
+ *    - For ingress pipine, add a priority 90 flow to allow arp packets for
+ *      known ipv4 addresses.
+ *    - Add a priority 90 flow to allow ipv4 packets for known ipv4 addresses
+ *    - Add a priority 80 flow to drop arp (for ingress) and ip packets
+ *    - Add a priority 90 flow to allow all ipv6 packets if port security
+ *      has ipv6 address(es).
+ *    - Add a priority 80 flow to drop ipv6 packets.
+ *    - Add a priority 50 flow to allow all other traffic with the matching
+ *      ethernet address.
+ */
+static void
+build_port_security(enum ovn_pipeline pipeline, struct ovn_port *op,
+                    struct hmap *lflows)
+{
+    char *eth_addr_field;
+    char *port_direction;
+    enum ovn_stage stage;
+    char *action;
+    if (pipeline == P_IN) {
+        eth_addr_field = "eth.src";
+        port_direction = "inport";
+        stage = S_SWITCH_IN_PORT_SEC;
+        action = "next;";
+    }
+    else {
+        eth_addr_field = "eth.dst";
+        port_direction = "outport";
+        stage = S_SWITCH_OUT_PORT_SEC;
+        action = "output;";
+    }
+
+    if (!op->nbs->n_port_security) {
+        /* port security is disabled on this lport. */
+        char *match = xasprintf("%s == %s", port_direction, op->json_key);
+        ovn_lflow_add(lflows, op->od, stage, 50, match, action);
+        free(match);
+        return;
+    }
+
+    struct ds match = DS_EMPTY_INITIALIZER;
+    for (size_t i = 0; i < op->nbs->n_port_security; i++) {
+        struct port_security_options *ps = extract_port_security_options(
+                op->nbs->port_security[i]);
+        if (!ps) {
+            continue;
+        }
+
+        /* Allow all traffic with priority 50 */
+        ds_init(&match);
+        ds_put_format(&match, "%s == %s && %s == "ETH_ADDR_FMT,
+                      port_direction, op->json_key, eth_addr_field,
+                      ETH_ADDR_ARGS(ps->ea));
+        ovn_lflow_add(lflows, op->od, stage, 50, ds_cstr(&match), action);
+        ds_destroy(&match);
+
+        if (!ps->n_ipv4_addrs && !ps->n_ipv6_addrs) {
+            /* port security option has only l2 address.
+             * Add port security constraints only on eth address which the 
above
+             * priority 50 flow takes care of*/
+            free(ps);
+            continue;
+        }
+
+        if (ps->n_ipv4_addrs) {
+            if (pipeline == P_IN) {
+                ds_init(&match);
+                /* Allow arp traffic for known ipv4 addresses */
+                build_port_security_arp_flow(&match, op->json_key, ps->ea,
+                                             ps->ipv4_addrs, ps->n_ipv4_addrs);
+                ovn_lflow_add(lflows, op->od, S_SWITCH_IN_PORT_SEC, 90,
+                              ds_cstr(&match), "next;");
+                ds_destroy(&match);
+            }
+
+            /* Allow traffic for known ipv4 addresses */
+            ds_init(&match);
+            ds_put_format(&match, "%s == %s && %s == "ETH_ADDR_FMT,
+                          port_direction, op->json_key, eth_addr_field,
+                          ETH_ADDR_ARGS(ps->ea));
+            build_port_security_ipv4_flow(pipeline, &match, ps->ipv4_addrs,
+                                          ps->n_ipv4_addrs);
+            ovn_lflow_add(lflows, op->od, stage, 90, ds_cstr(&match),
+                          action);
+            ds_destroy(&match);
+            free(ps->ipv4_addrs);
+        }
+
+        /* Drop all ip and arp packets (for ingress pipline) with
+         * priority 80. */
+        ds_init(&match);
+        ds_put_format(&match, "%s == %s && %s == "ETH_ADDR_FMT,
+                      port_direction, op->json_key, eth_addr_field,
+                      ETH_ADDR_ARGS(ps->ea));
+        if (pipeline == P_IN) {
+            ds_put_cstr(&match, " && (arp || ip4)");
+        }
+        else {
+            ds_put_cstr(&match, " && ip4");
+        }
+        ovn_lflow_add(lflows, op->od, stage, 80, ds_cstr(&match), "drop;");
+        ds_destroy(&match);
+
+        if (ps->n_ipv6_addrs) {
+            /* XXX Need to add port security flows for ipv6.
+             * For now add a priority 90 flow to allow all ipv6 traffic */
+            ds_init(&match);
+            ds_put_format(&match, "%s == %s && %s == "ETH_ADDR_FMT" && ip6",
+                          port_direction, op->json_key, eth_addr_field,
+                          ETH_ADDR_ARGS(ps->ea));
+            ovn_lflow_add(lflows, op->od, stage, 90, ds_cstr(&match), action);
+            ds_destroy(&match);
+            free(ps->ipv6_addrs);
+        }
+
+        /* Drop all ipv6 traffic with priority 80. */
+        ds_init(&match);
+        ds_put_format(&match, "%s == %s && %s == "ETH_ADDR_FMT" && ip6",
+                      port_direction, op->json_key, eth_addr_field,
+                      ETH_ADDR_ARGS(ps->ea));
+        ovn_lflow_add(lflows, op->od, stage, 80, ds_cstr(&match), "drop;");
+        ds_destroy(&match);
+        free(ps);
     }
 }
 
@@ -1154,7 +1373,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
          * to the next table if packet source is acceptable. */
     }
 
-    /* Logical switch ingress table 0: Ingress port security (priority 50). */
+    /* Logical switch ingress table 0: Ingress port security. */
     struct ovn_port *op;
     HMAP_FOR_EACH (op, key_node, ports) {
         if (!op->nbs) {
@@ -1167,14 +1386,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
             continue;
         }
 
-        struct ds match = DS_EMPTY_INITIALIZER;
-        ds_put_format(&match, "inport == %s", op->json_key);
-        build_port_security("eth.src",
-                            op->nbs->port_security, op->nbs->n_port_security,
-                            &match);
-        ovn_lflow_add(lflows, op->od, S_SWITCH_IN_PORT_SEC, 50,
-                      ds_cstr(&match), "next;");
-        ds_destroy(&match);
+        build_port_security(P_IN, op, lflows);
     }
 
     /* Ingress table 3: Destination lookup, ARP reply for known IPs.
@@ -1315,19 +1527,16 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
             continue;
         }
 
-        struct ds match = DS_EMPTY_INITIALIZER;
-        ds_put_format(&match, "outport == %s", op->json_key);
         if (lport_is_enabled(op->nbs)) {
-            build_port_security("eth.dst", op->nbs->port_security,
-                                op->nbs->n_port_security, &match);
-            ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC, 50,
-                          ds_cstr(&match), "output;");
+            build_port_security(P_OUT, op, lflows);
         } else {
+            struct ds match = DS_EMPTY_INITIALIZER;
+            ds_put_format(&match, "outport == %s", op->json_key);
+
             ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC, 150,
                           ds_cstr(&match), "drop;");
+            ds_destroy(&match);
         }
-
-        ds_destroy(&match);
     }
 }
 
diff --git a/tests/ovn.at b/tests/ovn.at
index b990116..72b6643 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1314,3 +1314,258 @@ for i in 1 2 3; do
     done
 done
 AT_CLEANUP
+
+# 3 hypervisors, one logical switch, 3 logical ports per hypervisor
+AT_SETUP([ovn -- 3 HVs, 1 LS, 3 lports/HV])
+AT_KEYWORDS([portsecurity])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# Create hypervisors hv[123].
+# Add vif1[123] to hv1, vif2[123] to hv2, vif3[123] to hv3.
+# Add all of the vifs to a single logical switch lsw0.
+# Turn off port security on vifs vif[123]1
+# Turn on l2 port security on vifs vif[123]2
+# Turn of l2 and l3 port security on vifs vif[123]3
+# Make vif13, vif2[23], vif3[123] destinations for unknown MACs.
+ovn-nbctl lswitch-add lsw0
+net_add n1
+for i in 1 2 3; do
+    sim_add hv$i
+    as hv$i
+    ovs-vsctl add-br br-phys
+    ovn_attach n1 br-phys 192.168.0.$i
+
+    for j in 1 2 3; do
+        ovs-vsctl add-port br-int vif$i$j -- set Interface vif$i$j 
external-ids:iface-id=lp$i$j options:tx_pcap=hv$i/vif$i$j-tx.pcap 
options:rxq_pcap=hv$i/vif$i$j-rx.pcap ofport-request=$i$j
+        ovn-nbctl lport-add lsw0 lp$i$j
+        if test $j = 1; then
+            ovn-nbctl lport-set-addresses lp$i$j "f0:00:00:00:00:$i$j 
192.168.0.$i$j" unknown
+        elif test $j = 2; then
+            ovn-nbctl lport-set-addresses lp$i$j "f0:00:00:00:00:$i$j 
192.168.0.$i$j"
+            ovn-nbctl lport-set-port-security lp$i$j f0:00:00:00:00:$i$j
+        else
+            ovn-nbctl lport-set-addresses lp$i$j "f0:00:00:00:00:$i$j 
192.168.0.$i$j"
+            ovn-nbctl lport-set-port-security lp$i$j "f0:00:00:00:00:$i$j 
192.168.0.$i$j"
+        fi
+    done
+done
+
+
+# Pre-populate the hypervisors' ARP tables so that we don't lose any
+# packets for ARP resolution (native tunneling doesn't queue packets
+# for ARP resolution).
+ovn_populate_arp
+
+# Allow some time for ovn-northd and ovn-controller to catch up.
+# XXX This should be more systematic.
+sleep 1
+ovn-sbctl dump-flows -- list multicast_group
+
+# Given the name of a logical port, prints the name of the hypervisor
+# on which it is located.
+vif_to_hv() {
+    echo hv${1%?}
+}
+
+
+trim_zeros() {
+    sed 's/\(00\)\{1,\}$//'
+}
+for i in 1 2 3; do
+    for j in 1 2 3; do
+        : > $i$j.expected
+    done
+done
+
+# test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT...
+#
+# This shell function causes an ip packet to be received on INPORT.
+# The packet's content has Ethernet destination DST and source SRC
+# (each exactly 12 hex digits) and Ethernet type ETHTYPE (4 hex digits).
+# The OUTPORTs (zero or more) list the VIFs on which the packet should
+# be received.  INPORT and the OUTPORTs are specified as lport numbers,
+# e.g. 11 for vif11.
+test_ip() {
+    # This packet has bad checksums but logical L3 routing doesn't check.
+    local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5
+    local 
packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}003511110008
+    shift; shift; shift; shift; shift
+    hv=`vif_to_hv $inport`
+    as $hv ovs-appctl netdev-dummy/receive vif$inport $packet
+    #as $hv ovs-appctl ofproto/trace br-int in_port=$inport $packet
+    for outport; do
+        echo $packet | trim_zeros >> $outport.expected
+    done
+}
+
+# test_arp INPORT SHA SPA TPA DROP [REPLY_HA]
+#
+# Causes a packet to be received on INPORT.  The packet is an ARP
+# request with SHA, SPA, and TPA as specified.  If REPLY_HA is provided, then
+# it should be the hardware address of the target to expect to receive in an
+# ARP reply; otherwise no reply is expected.
+#
+# INPORT is an lport number, e.g. 11 for vif11.
+# SHA and REPLY_HA are each 12 hex digits.
+# SPA and TPA are each 8 hex digits.
+test_arp() {
+    local inport=$1 sha=$2 spa=$3 tpa=$4 drop=$5 reply_ha=$6
+    local 
request=ffffffffffff${sha}08060001080006040001${sha}${spa}ffffffffffff${tpa}
+    hv=`vif_to_hv $inport`
+    as $hv ovs-appctl netdev-dummy/receive vif$inport $request
+    #as $hv ovs-appctl ofproto/trace br-int in_port=$inport $request
+    if test $drop != 1; then
+        if test X$reply_ha == X; then
+            # Expect to receive the broadcast ARP on the other logical switch 
ports
+            # if no reply is expected.
+            local i j
+            for i in 1 2 3; do
+                for j in 1 2 3; do
+                    if test $i$j != $inport; then
+                        echo $request >> $i$j.expected
+                    fi
+                done
+            done
+        else
+            # Expect to receive the reply, if any.
+            local 
reply=${sha}${reply_ha}08060001080006040002${reply_ha}${tpa}${sha}${spa}
+            echo $reply >> $inport.expected
+        fi
+    fi
+}
+
+# test_ipv6 INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT...
+# This function is similar to test_ip() except that it sends
+# ipv6 packet
+test_ipv6() {
+    local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5
+    local 
packet=${dst_mac}${src_mac}86dd6000000000083aff${src_ip}${dst_ip}0000000000000000
+    shift; shift; shift; shift; shift
+    hv=`vif_to_hv $inport`
+    as $hv ovs-appctl netdev-dummy/receive vif$inport $packet
+    #as $hv ovs-appctl ofproto/trace br-int in_port=$inport $packet
+    for outport; do
+        echo $packet | trim_zeros >> $outport.expected
+    done
+}
+
+ip_to_hex() {
+    printf "%02x%02x%02x%02x" "$@"
+}
+
+# no port security
+sip=`ip_to_hex 192 168 0 12`
+tip=`ip_to_hex 192 168 0 13`
+# the arp packet should be allowed even if lp[123]1 is
+# not configured with mac f00000000023 and ip 192.168.0.12
+for i in 1 2 3; do
+    test_arp ${i}1 f00000000023 $sip $tip 0 f00000000013
+    for j in 1 2 3; do
+        if test $i != $j; then
+            test_ip ${i}1 f000000000${i}1 f000000000${j}1 $sip $tip ${j}1
+            echo 'hi'
+        fi
+    done
+done
+
+# l2 port security
+sip=`ip_to_hex 192 168 0 12`
+tip=`ip_to_hex 192 168 0 13`
+
+# arp packet should be allowed since lp22 is configured with
+# mac f00000000022
+test_arp 22 f00000000022 $sip $tip 0 f00000000013
+
+# arp packet should not be allowed since lp32 is not configured with
+# mac f00000000021
+test_arp 32 f00000000021 $sip $tip 1
+
+# ip packets should be allowed and received since lp[123]2 do not
+# have l3 port security
+sip=`ip_to_hex 192 168 0 55`
+tip=`ip_to_hex 192 168 0 66`
+for i in 1 2 3; do
+    for j in 1 2 3; do
+        if test $i != $j; then
+            # test_ip $i2 f000000000$i2 f000000000$j2 $sip $tip $j2
+            echo 'bye'
+        fi
+    done
+done
+
+# ipv6 packets should be received by lp[123]2
+# lp[123]1 can send ipv6 traffic as there is no port security
+sip=fe800000000000000000000000000000
+tip=ff020000000000000000000000000000
+
+for i in 1 2 3; do
+    test_ipv6 ${i}1 f000000000${i}2 f000000000{i}2 $sip $tip {i}2
+done
+
+
+# l2 and l3 port security
+sip=`ip_to_hex 192 168 0 13`
+tip=`ip_to_hex 192 168 0 22`
+# arp packet should be allowed since lp13 is configured with
+# f00000000013 and 192.168.0.13
+test_arp 13 f00000000013 $sip $tip 0 f00000000022
+
+# the arp packet should be dropped because lp23 is not configured
+# with mac f00000000022
+sip=`ip_to_hex 192 168 0 13`
+tip=`ip_to_hex 192 168 0 22`
+test_arp 23 f00000000022 $sip $tip 1
+
+
+# the arp packet should be dropped because lp33 is not configured
+# with ip 192.168.0.55
+spa=`ip_to_hex 192 168 0 55`
+tpa=`ip_to_hex 192 168 0 22`
+test_arp 33 f00000000031 $spa $tpa 1
+
+
+# ip packets should not be received by lp[123]3 since
+# l3 port security is enabled
+sip=`ip_to_hex 192 168 0 55`
+tip=`ip_to_hex 192 168 0 66`
+for i in 1 2 3; do
+    for j in 1 2 3; do
+        test_ip ${i}2 f000000000${i}2 f000000000${j}3 $sip $tip
+        echo 'see'
+    done
+done
+
+# ipv6 packets should be dropped for lp[123]3 since
+# it is configured with only ipv4 address
+sip=fe800000000000000000000000000000
+tip=ff020000000000000000000000000000
+
+for i in 1 2 3; do
+    test_ipv6 ${i}3 f000000000${i}3 f00000000022 $sip $tip
+done
+
+# ipv6 packets should not be received by lp[123]3
+# lp[123]1 can send ipv6 traffic as there is no port security
+for i in 1 2 3; do
+    test_ipv6 ${i}1 f000000000${i}1 f000000000${i}3 $sip $tip
+done
+
+# Allow some time for packet forwarding.
+# XXX This can be improved.
+sleep 1
+
+# Now check the packets actually received against the ones expected.
+for i in 1 2 3; do
+    for j in 1 2 3; do
+        file=hv$i/vif$i$j-tx.pcap
+        echo $file
+        $PYTHON "$top_srcdir/utilities/ovs-pcap.in" $file | trim_zeros > 
$i$j.packets
+        sort $i$j.expected > expout
+        AT_CHECK([sort $i$j.packets], [0], [expout])
+        echo
+    done
+done
+
+
+AT_CLEANUP
-- 
2.5.0


_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to