ofp-util: Export (again) a generalized function for normalizing rules.

Feature #5029 requests that "ovs-ofctl add-flow" report an attempt to add
a flow that is not properly normalized, that is, a flow to which the switch
will add extra wildcards, ignoring some fields specified by the user.  This
requires that ofp-util make flow normalization directly available (again).

Until now, flow normalization has only been applied to OpenFlow 1.0 flows,
but the concept applies equally to NXM, so this commit generalizes the
implementation to NXM also.

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 9507d94a1571dac5efbbafe336a34a987624fa74..c1e98ba224a1125e1dc5b1c183b0261571557926 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -18,6 +18,7 @@
 #include "ofp-print.h"
 #include <errno.h>
 #include <inttypes.h>
+#include <netinet/icmp6.h>
 #include <stdlib.h>
 #include "autopath.h"
 #include "byte-order.h"
@@ -36,8 +37,6 @@
 
 VLOG_DEFINE_THIS_MODULE(ofp_util);
 
-static ovs_be32 normalize_wildcards(const struct ofp_match *);
-
 /* Rate limit for OpenFlow message parse errors.  These always indicate a bug
  * in the peer and so there's not much point in showing a lot of them. */
 static struct vlog_rate_limit bad_ofmsg_rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -905,7 +904,6 @@ ofputil_decode_flow_mod(struct flow_mod *fm, const struct ofp_header *oh,
         /* Standard OpenFlow flow_mod. */
         const struct ofp_flow_mod *ofm;
         uint16_t priority;
-        ovs_be32 wc;
         int error;
 
         /* Dissect the message. */
@@ -917,33 +915,16 @@ ofputil_decode_flow_mod(struct flow_mod *fm, const struct ofp_header *oh,
 
         /* Set priority based on original wildcards.  Normally we'd allow
          * ofputil_cls_rule_from_match() to do this for us, but
-         * normalize_wildcards() can put wildcards where the original flow
+         * ofputil_normalize_rule() can put wildcards where the original flow
          * didn't have them. */
         priority = ntohs(ofm->priority);
         if (!(ofm->match.wildcards & htonl(OFPFW_ALL))) {
             priority = UINT16_MAX;
         }
 
-        /* Normalize ofm->match.  If normalization actually changes anything,
-         * then log the differences. */
-        wc = normalize_wildcards(&ofm->match);
-        if (wc == ofm->match.wildcards) {
-            ofputil_cls_rule_from_match(&ofm->match, priority, &fm->cr);
-        } else {
-            struct ofp_match match = ofm->match;
-            match.wildcards = wc;
-            ofputil_cls_rule_from_match(&match, priority, &fm->cr);
-
-            if (!VLOG_DROP_INFO(&bad_ofmsg_rl)) {
-                char *pre = ofp_match_to_string(&ofm->match, 1);
-                char *post = ofp_match_to_string(&match, 1);
-                VLOG_INFO("normalization changed ofp_match, details:");
-                VLOG_INFO(" pre: %s", pre);
-                VLOG_INFO("post: %s", post);
-                free(pre);
-                free(post);
-            }
-        }
+        /* Translate the rule. */
+        ofputil_cls_rule_from_match(&ofm->match, priority, &fm->cr);
+        ofputil_normalize_rule(&fm->cr, NXFF_OPENFLOW10);
 
         /* Translate the message. */
         fm->cookie = ofm->cookie;
@@ -2079,29 +2060,115 @@ actions_next(struct actions_iterator *iter)
     }
 }
 
-static ovs_be32
-normalize_wildcards(const struct ofp_match *m)
-{
-    enum { OFPFW_NW = (OFPFW_NW_SRC_ALL | OFPFW_NW_DST_ALL | OFPFW_NW_PROTO
-                       | OFPFW_NW_TOS) };
-    enum { OFPFW_TP = OFPFW_TP_SRC | OFPFW_TP_DST };
-    ovs_be32 wc;
-
-    wc = m->wildcards;
-    if (wc & htonl(OFPFW_DL_TYPE)) {
-        wc |= htonl(OFPFW_NW | OFPFW_TP);
-    } else if (m->dl_type == htons(ETH_TYPE_IP)) {
-        if (wc & htonl(OFPFW_NW_PROTO) || (m->nw_proto != IPPROTO_TCP &&
-                                           m->nw_proto != IPPROTO_UDP &&
-                                           m->nw_proto != IPPROTO_ICMP)) {
-            wc |= htonl(OFPFW_TP);
+/* "Normalizes" the wildcards in 'rule'.  That means:
+ *
+ *    1. If the type of level N is known, then only the valid fields for that
+ *       level may be specified.  For example, ARP does not have a TOS field,
+ *       so nw_tos must be wildcarded if 'rule' specifies an ARP flow.
+ *       Similarly, IPv4 does not have any IPv6 addresses, so ipv6_src and
+ *       ipv6_dst (and other fields) must be wildcarded if 'rule' specifies an
+ *       IPv4 flow.
+ *
+ *    2. If the type of level N is not known (or not understood by Open
+ *       vSwitch), then no fields at all for that level may be specified.  For
+ *       example, Open vSwitch does not understand SCTP, an L4 protocol, so the
+ *       L4 fields tp_src and tp_dst must be wildcarded if 'rule' specifies an
+ *       SCTP flow.
+ *
+ * 'flow_format' specifies the format of the flow as received or as intended to
+ * be sent.  This is important for IPv6 and ARP, for which NXM supports more
+ * detailed matching. */
+void
+ofputil_normalize_rule(struct cls_rule *rule, enum nx_flow_format flow_format)
+{
+    enum {
+        MAY_NW_ADDR     = 1 << 0, /* nw_src, nw_dst */
+        MAY_TP_ADDR     = 1 << 1, /* tp_src, tp_dst */
+        MAY_NW_PROTO    = 1 << 2, /* nw_proto */
+        MAY_NW_TOS      = 1 << 3, /* nw_tos */
+        MAY_ARP_SHA     = 1 << 4, /* arp_sha */
+        MAY_ARP_THA     = 1 << 5, /* arp_tha */
+        MAY_IPV6_ADDR   = 1 << 6, /* ipv6_src, ipv6_dst */
+        MAY_ND_TARGET   = 1 << 7  /* nd_target */
+    } may_match;
+
+    struct flow_wildcards wc;
+
+    /* Figure out what fields may be matched. */
+    if (rule->flow.dl_type == htons(ETH_TYPE_IP)) {
+        may_match = MAY_NW_PROTO | MAY_NW_TOS | MAY_NW_ADDR;
+        if (rule->flow.nw_proto == IPPROTO_TCP ||
+            rule->flow.nw_proto == IPPROTO_UDP ||
+            rule->flow.nw_proto == IPPROTO_ICMP) {
+            may_match |= MAY_TP_ADDR;
+        }
+    } else if (rule->flow.dl_type == htons(ETH_TYPE_IPV6)
+               && flow_format == NXFF_NXM) {
+        may_match = MAY_NW_PROTO | MAY_NW_TOS | MAY_IPV6_ADDR;
+        if (rule->flow.nw_proto == IPPROTO_TCP ||
+            rule->flow.nw_proto == IPPROTO_UDP) {
+            may_match |= MAY_TP_ADDR;
+        } else if (rule->flow.nw_proto == IPPROTO_ICMPV6) {
+            may_match |= MAY_TP_ADDR;
+            if (rule->flow.tp_src == htons(ND_NEIGHBOR_SOLICIT)) {
+                may_match |= MAY_ND_TARGET | MAY_ARP_SHA;
+            } else if (rule->flow.tp_src == htons(ND_NEIGHBOR_ADVERT)) {
+                may_match |= MAY_ND_TARGET | MAY_ARP_THA;
+            }
+        }
+    } else if (rule->flow.dl_type == htons(ETH_TYPE_ARP)) {
+        may_match = MAY_NW_PROTO | MAY_NW_ADDR;
+        if (flow_format == NXFF_NXM) {
+            may_match |= MAY_ARP_SHA | MAY_ARP_THA;
         }
-    } else if (m->dl_type == htons(ETH_TYPE_ARP)) {
-        wc |= htonl(OFPFW_TP | OFPFW_NW_TOS);
     } else {
-        wc |= htonl(OFPFW_NW | OFPFW_TP);
+        may_match = 0;
+    }
+
+    /* Clear the fields that may not be matched. */
+    wc = rule->wc;
+    if (!(may_match & MAY_NW_ADDR)) {
+        wc.nw_src_mask = wc.nw_dst_mask = htonl(0);
+    }
+    if (!(may_match & MAY_TP_ADDR)) {
+        wc.wildcards |= FWW_TP_SRC | FWW_TP_DST;
+    }
+    if (!(may_match & MAY_NW_PROTO)) {
+        wc.wildcards |= FWW_NW_PROTO;
+    }
+    if (!(may_match & MAY_NW_TOS)) {
+        wc.wildcards |= FWW_NW_TOS;
+    }
+    if (!(may_match & MAY_ARP_SHA)) {
+        wc.wildcards |= FWW_ARP_SHA;
+    }
+    if (!(may_match & MAY_ARP_THA)) {
+        wc.wildcards |= FWW_ARP_THA;
+    }
+    if (!(may_match & MAY_IPV6_ADDR)) {
+        wc.ipv6_src_mask = wc.ipv6_dst_mask = in6addr_any;
+    }
+    if (!(may_match & MAY_ND_TARGET)) {
+        wc.wildcards |= FWW_ND_TARGET;
+    }
+
+    /* Log any changes. */
+    if (!flow_wildcards_equal(&wc, &rule->wc)) {
+        bool log = !VLOG_DROP_INFO(&bad_ofmsg_rl);
+        char *pre = log ? cls_rule_to_string(rule) : NULL;
+
+        rule->wc = wc;
+        cls_rule_zero_wildcarded_fields(rule);
+
+        if (log) {
+            char *post = cls_rule_to_string(rule);
+            VLOG_INFO("normalization changed ofp_match, details:");
+            VLOG_INFO(" pre: %s", pre);
+            VLOG_INFO("post: %s", post);
+            free(pre);
+            free(post);
+        }
     }
-    return wc;
 }
 
 static uint32_t

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 975d8d2f4aca33f88754f9f7a669d73c01a376df..152f4dd805e299d39217f98a39a403afa3e66f40 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -101,6 +101,7 @@ int ofputil_netmask_to_wcbits(ovs_be32 netmask);
 /* Work with OpenFlow 1.0 ofp_match. */
 void ofputil_cls_rule_from_match(const struct ofp_match *,
                                  unsigned int priority, struct cls_rule *);
+void ofputil_normalize_rule(struct cls_rule *, enum nx_flow_format);
 void ofputil_cls_rule_to_match(const struct cls_rule *, struct ofp_match *);
 
 /* dl_type translation between OpenFlow and 'struct flow' format. */

diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 604267a07f7ac9efb3abf89ef1b1d6b0acc14dda..b5653d93d27a190553e1c88028de59bd1fe64a8e 100644 (file)
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -290,8 +290,8 @@ AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\
 OFPT_FLOW_MOD (xid=0x0): ADD priority=65535,arp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2 idle:5 buf:0x10e actions=output:3
 ], [dnl
 ofp_util|INFO|normalization changed ofp_match, details:
-ofp_util|INFO| pre: arp,in_port=1,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2,nw_tos=0,tp_src=0,tp_dst=0
-ofp_util|INFO|post: arp,in_port=1,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2
+ofp_util|INFO| pre: priority=65535,arp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2,nw_tos=0,tp_src=0,tp_dst=0
+ofp_util|INFO|post: priority=65535,arp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2
 ])
 AT_CLEANUP
 
@@ -308,8 +308,8 @@ AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\
 OFPT_FLOW_MOD (xid=0x0): ADD arp,in_port=1,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2,nw_tos=0,tp_src=0,tp_dst=0 idle:5 pri:65535 buf:0x10e actions=output:3
 ], [dnl
 ofp_util|INFO|normalization changed ofp_match, details:
-ofp_util|INFO| pre: arp,in_port=1,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2,nw_tos=0,tp_src=0,tp_dst=0
-ofp_util|INFO|post: arp,in_port=1,dl_vlan=65535,dl_vlan_pcp=0,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2
+ofp_util|INFO| pre: priority=65535,arp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2,nw_tos=0,tp_src=0,tp_dst=0
+ofp_util|INFO|post: priority=65535,arp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,opcode=2
 ])
 AT_CLEANUP