Merge 'next' into 'master'.
[sliver-openvswitch.git] / lib / ofp-util.c
index 3487cba..4618567 100644 (file)
@@ -36,6 +36,8 @@
 
 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);
@@ -94,12 +96,11 @@ ofputil_netmask_to_wcbits(ovs_be32 netmask)
 
 /* WC_INVARIANTS is the invariant bits (as defined on WC_INVARIANT_LIST) all
  * OR'd together. */
-enum {
-    WC_INVARIANTS = 0
+static const flow_wildcards_t WC_INVARIANTS = 0
 #define WC_INVARIANT_BIT(NAME) | FWW_##NAME
     WC_INVARIANT_LIST
 #undef WC_INVARIANT_BIT
-};
+;
 
 /* Converts the ofp_match in 'match' into a cls_rule in 'rule', with the given
  * 'priority'. */
@@ -108,7 +109,7 @@ ofputil_cls_rule_from_match(const struct ofp_match *match,
                             unsigned int priority, struct cls_rule *rule)
 {
     struct flow_wildcards *wc = &rule->wc;
-    unsigned int ofpfw;
+    uint32_t ofpfw;
     ovs_be16 vid, pcp;
 
     /* Initialize rule->priority. */
@@ -117,7 +118,7 @@ ofputil_cls_rule_from_match(const struct ofp_match *match,
 
     /* Initialize most of rule->wc. */
     flow_wildcards_init_catchall(wc);
-    wc->wildcards = ofpfw & WC_INVARIANTS;
+    wc->wildcards = (OVS_FORCE flow_wildcards_t) ofpfw & WC_INVARIANTS;
 
     /* Wildcard fields that aren't defined by ofp_match or tun_id. */
     wc->wildcards |= (FWW_ARP_SHA | FWW_ARP_THA | FWW_ND_TARGET);
@@ -199,10 +200,10 @@ void
 ofputil_cls_rule_to_match(const struct cls_rule *rule, struct ofp_match *match)
 {
     const struct flow_wildcards *wc = &rule->wc;
-    unsigned int ofpfw;
+    uint32_t ofpfw;
 
     /* Figure out most OpenFlow wildcards. */
-    ofpfw = wc->wildcards & WC_INVARIANTS;
+    ofpfw = (OVS_FORCE uint32_t) (wc->wildcards & WC_INVARIANTS);
     ofpfw |= ofputil_netmask_to_wcbits(wc->nw_src_mask) << OFPFW_NW_SRC_SHIFT;
     ofpfw |= ofputil_netmask_to_wcbits(wc->nw_dst_mask) << OFPFW_NW_DST_SHIFT;
     if (wc->wildcards & FWW_NW_TOS) {
@@ -339,27 +340,16 @@ ofputil_lookup_openflow_message(const struct ofputil_msg_category *cat,
                                 const struct ofputil_msg_type **typep)
 {
     const struct ofputil_msg_type *type;
-    bool found;
 
-    found = false;
     for (type = cat->types; type < &cat->types[cat->n_types]; type++) {
         if (type->value == value) {
-            if (ofputil_length_ok(cat, type, size)) {
-                *typep = type;
-                return 0;
+            if (!ofputil_length_ok(cat, type, size)) {
+                return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
             }
-
-            /* We found a matching command type but it had the wrong length.
-             * Probably this is just an error.  However, a screwup means that
-             * NXT_SET_FLOW_FORMAT and NXT_FLOW_MOD_TABLE_ID have the same
-             * value.  They do have different lengths, so we can distinguish
-             * them that way. */
-            found = true;
+            *typep = type;
+            return 0;
         }
     }
-    if (found) {
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
-    }
 
     VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu32,
                  cat->name, value);
@@ -386,10 +376,6 @@ ofputil_decode_vendor(const struct ofp_header *oh,
           NXT_SET_FLOW_FORMAT, "NXT_SET_FLOW_FORMAT",
           sizeof(struct nxt_set_flow_format), 0 },
 
-        { OFPUTIL_NXT_FLOW_MOD_TABLE_ID,
-          NXT_FLOW_MOD_TABLE_ID, "NXT_FLOW_MOD_TABLE_ID",
-          sizeof(struct nxt_flow_mod_table_id), 0 },
-
         { OFPUTIL_NXT_FLOW_MOD,
           NXT_FLOW_MOD, "NXT_FLOW_MOD",
           sizeof(struct nx_flow_mod), 8 },
@@ -423,6 +409,21 @@ ofputil_decode_vendor(const struct ofp_header *oh,
     }
 
     nh = (const struct nicira_header *) oh;
+
+    if (nh->subtype == htonl(NXT_FLOW_MOD_TABLE_ID)
+        && oh->length == htons(sizeof(struct nxt_flow_mod_table_id))) {
+        /* NXT_SET_FLOW_FORMAT and NXT_FLOW_MOD_TABLE_ID accidentally have the
+         * same value but different lengths.  ofputil_lookup_openflow_message()
+         * doesn't support this case, so special case it here. */
+        static const struct ofputil_msg_type nxt_flow_mod_table_id =
+            { OFPUTIL_NXT_FLOW_MOD_TABLE_ID,
+              NXT_FLOW_MOD_TABLE_ID, "NXT_FLOW_MOD_TABLE_ID",
+              sizeof(struct nxt_flow_mod_table_id), 0 };
+
+        *typep = &nxt_flow_mod_table_id;
+        return 0;
+    }
+
     return ofputil_lookup_openflow_message(&nxt_category, ntohl(nh->subtype),
                                            ntohs(oh->length), typep);
 }
@@ -902,8 +903,9 @@ ofputil_decode_flow_mod(struct flow_mod *fm, const struct ofp_header *oh,
     ofputil_decode_msg_type(oh, &type);
     if (ofputil_msg_type_code(type) == OFPUTIL_OFPT_FLOW_MOD) {
         /* Standard OpenFlow flow_mod. */
-        struct ofp_match match, orig_match;
         const struct ofp_flow_mod *ofm;
+        uint16_t priority;
+        ovs_be32 wc;
         int error;
 
         /* Dissect the message. */
@@ -913,26 +915,37 @@ ofputil_decode_flow_mod(struct flow_mod *fm, const struct ofp_header *oh,
             return error;
         }
 
+        /* 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
+         * 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. */
-        match = ofm->match;
-        match.pad1[0] = match.pad2[0] = 0;
-        orig_match = match;
-        normalize_match(&match);
-        if (memcmp(&match, &orig_match, sizeof orig_match)) {
+        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 *old = ofp_match_to_literal_string(&orig_match);
-                char *new = ofp_match_to_literal_string(&match);
+                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", old);
-                VLOG_INFO("post: %s", new);
-                free(old);
-                free(new);
+                VLOG_INFO(" pre: %s", pre);
+                VLOG_INFO("post: %s", post);
+                free(pre);
+                free(post);
             }
         }
 
         /* Translate the message. */
-        ofputil_cls_rule_from_match(&match, ntohs(ofm->priority), &fm->cr);
         fm->cookie = ofm->cookie;
         command = ntohs(ofm->command);
         fm->idle_timeout = ntohs(ofm->idle_timeout);
@@ -1004,6 +1017,7 @@ ofputil_encode_flow_mod(const struct flow_mod *fm,
         msg = ofpbuf_new(sizeof *ofm + actions_len);
         ofm = put_openflow(sizeof *ofm, OFPT_FLOW_MOD, msg);
         ofputil_cls_rule_to_match(&fm->cr, &ofm->match);
+        ofm->cookie = fm->cookie;
         ofm->command = htons(fm->command);
         ofm->idle_timeout = htons(fm->idle_timeout);
         ofm->hard_timeout = htons(fm->hard_timeout);
@@ -2064,123 +2078,29 @@ actions_next(struct actions_iterator *iter)
     }
 }
 
-void
-normalize_match(struct ofp_match *m)
+static ovs_be32
+normalize_wildcards(const struct ofp_match *m)
 {
-    enum { OFPFW_NW = (OFPFW_NW_SRC_MASK | OFPFW_NW_DST_MASK | OFPFW_NW_PROTO
+    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 };
-    uint32_t wc;
+    ovs_be32 wc;
 
-    wc = ntohl(m->wildcards) & OFPFW_ALL;
-    if (wc & OFPFW_DL_TYPE) {
-        m->dl_type = 0;
-
-        /* Can't sensibly match on network or transport headers if the
-         * data link type is unknown. */
-        wc |= OFPFW_NW | OFPFW_TP;
-        m->nw_src = m->nw_dst = m->nw_proto = m->nw_tos = 0;
-        m->tp_src = m->tp_dst = 0;
+    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 & OFPFW_NW_PROTO) {
-            m->nw_proto = 0;
-
-            /* Can't sensibly match on transport headers if the network
-             * protocol is unknown. */
-            wc |= OFPFW_TP;
-            m->tp_src = m->tp_dst = 0;
-        } else if (m->nw_proto == IPPROTO_TCP ||
-                   m->nw_proto == IPPROTO_UDP ||
-                   m->nw_proto == IPPROTO_ICMP) {
-            if (wc & OFPFW_TP_SRC) {
-                m->tp_src = 0;
-            }
-            if (wc & OFPFW_TP_DST) {
-                m->tp_dst = 0;
-            }
-        } else {
-            /* Transport layer fields will always be extracted as zeros, so we
-             * can do an exact-match on those values.  */
-            wc &= ~OFPFW_TP;
-            m->tp_src = m->tp_dst = 0;
-        }
-        if (wc & OFPFW_NW_SRC_MASK) {
-            m->nw_src &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_SRC_SHIFT);
-        }
-        if (wc & OFPFW_NW_DST_MASK) {
-            m->nw_dst &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_DST_SHIFT);
-        }
-        if (wc & OFPFW_NW_TOS) {
-            m->nw_tos = 0;
-        } else {
-            m->nw_tos &= IP_DSCP_MASK;
+        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);
         }
     } else if (m->dl_type == htons(ETH_TYPE_ARP)) {
-        if (wc & OFPFW_NW_PROTO) {
-            m->nw_proto = 0;
-        }
-        if (wc & OFPFW_NW_SRC_MASK) {
-            m->nw_src &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_SRC_SHIFT);
-        }
-        if (wc & OFPFW_NW_DST_MASK) {
-            m->nw_dst &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_DST_SHIFT);
-        }
-        m->tp_src = m->tp_dst = m->nw_tos = 0;
-    } else if (m->dl_type == htons(ETH_TYPE_IPV6)) {
-        /* Don't normalize IPv6 traffic, since OpenFlow doesn't have a
-         * way to express it. */
+        wc |= htonl(OFPFW_TP);
     } else {
-        /* Network and transport layer fields will always be extracted as
-         * zeros, so we can do an exact-match on those values. */
-        wc &= ~(OFPFW_NW | OFPFW_TP);
-        m->nw_proto = m->nw_src = m->nw_dst = m->nw_tos = 0;
-        m->tp_src = m->tp_dst = 0;
-    }
-    if (wc & OFPFW_DL_SRC) {
-        memset(m->dl_src, 0, sizeof m->dl_src);
-    }
-    if (wc & OFPFW_DL_DST) {
-        memset(m->dl_dst, 0, sizeof m->dl_dst);
+        wc |= htonl(OFPFW_NW | OFPFW_TP);
     }
-    m->wildcards = htonl(wc);
-}
-
-/* Returns a string that describes 'match' in a very literal way, without
- * interpreting its contents except in a very basic fashion.  The returned
- * string is intended to be fixed-length, so that it is easy to see differences
- * between two such strings if one is put above another.  This is useful for
- * describing changes made by normalize_match().
- *
- * The caller must free the returned string (with free()). */
-char *
-ofp_match_to_literal_string(const struct ofp_match *match)
-{
-    return xasprintf("wildcards=%#10"PRIx32" "
-                     " in_port=%5"PRId16" "
-                     " dl_src="ETH_ADDR_FMT" "
-                     " dl_dst="ETH_ADDR_FMT" "
-                     " dl_vlan=%5"PRId16" "
-                     " dl_vlan_pcp=%3"PRId8" "
-                     " dl_type=%#6"PRIx16" "
-                     " nw_tos=%#4"PRIx8" "
-                     " nw_proto=%#4"PRIx16" "
-                     " nw_src=%#10"PRIx32" "
-                     " nw_dst=%#10"PRIx32" "
-                     " tp_src=%5"PRId16" "
-                     " tp_dst=%5"PRId16,
-                     ntohl(match->wildcards),
-                     ntohs(match->in_port),
-                     ETH_ADDR_ARGS(match->dl_src),
-                     ETH_ADDR_ARGS(match->dl_dst),
-                     ntohs(match->dl_vlan),
-                     match->dl_vlan_pcp,
-                     ntohs(match->dl_type),
-                     match->nw_tos,
-                     match->nw_proto,
-                     ntohl(match->nw_src),
-                     ntohl(match->nw_dst),
-                     ntohs(match->tp_src),
-                     ntohs(match->tp_dst));
+    return wc;
 }
 
 static uint32_t