flow: Move functions for dealing with wildcard bit counts to ofp-util.

These functions are really OpenFlow-specific, and they will not be used
directly by the flow code soon, so move them to ofp-util.

lib/flow.c

@@ -24,6 +24,7 @@
 #include "coverage.h"
 #include "dynamic-string.h"
 #include "hash.h"
+#include "ofp-util.h"
 #include "ofpbuf.h"
 #include "openflow/openflow.h"
 #include "openvswitch/datapath-protocol.h"
@@ -358,22 +359,6 @@ flow_print(FILE *stream, const struct flow *flow)
 
 /* flow_wildcards functions. */
 
-/* Given the wildcard bit count in bits 'shift' through 'shift + 5' (inclusive)
- * of 'wildcards', returns a 32-bit bit mask with a 1 in each bit that must
- * match and a 0 in each bit that is wildcarded.
- *
- * The bits in 'wildcards' are in the format used in enum ofp_flow_wildcards: 0
- * is exact match, 1 ignores the LSB, 2 ignores the 2 least-significant bits,
- * ..., 32 and higher wildcard the entire field.  This is the *opposite* of the
- * usual convention where e.g. /24 indicates that 8 bits (not 24 bits) are
- * wildcarded. */
-ovs_be32
-flow_nw_bits_to_mask(uint32_t wildcards, int shift)
-{
-    wildcards = (wildcards >> shift) & 0x3f;
-    return wildcards < 32 ? htonl(~((1u << wildcards) - 1)) : 0;
-}
-
 /* Return 'wildcards' in "normal form":
  *
  *   - Forces unknown bits to 0.
@@ -404,8 +389,8 @@ void
 flow_wildcards_init(struct flow_wildcards *wc, uint32_t wildcards)
 {
     wc->wildcards = flow_wildcards_normalize(wildcards) | FWW_REGS;
-    wc->nw_src_mask = flow_nw_bits_to_mask(wc->wildcards, OFPFW_NW_SRC_SHIFT);
+    wc->nw_src_mask = ofputil_wcbits_to_netmask(wildcards >> OFPFW_NW_SRC_SHIFT);
-    wc->nw_dst_mask = flow_nw_bits_to_mask(wc->wildcards, OFPFW_NW_DST_SHIFT);
+    wc->nw_dst_mask = ofputil_wcbits_to_netmask(wildcards >> OFPFW_NW_DST_SHIFT);
     memset(wc->reg_masks, 0, sizeof wc->reg_masks);
 }
 
@@ -512,30 +497,13 @@ flow_wildcards_has_extra(const struct flow_wildcards *a,
             || (a->nw_dst_mask & b->nw_dst_mask) != b->nw_dst_mask);
 }
 
-static int
-count_ones(ovs_be32 mask)
-{
-#if __GNUC__ >= 4
-    return __builtin_popcount(mask);
-#else
-    int bits;
-
-    for (bits = 0; mask; bits++) {
-        mask &= mask - 1;
-    }
-
-    return bits;
-#endif
-}
-
 static bool
 set_nw_mask(struct flow_wildcards *wc, ovs_be32 mask,
             ovs_be32 *maskp, int shift)
 {
-    int wcbits = 32 - count_ones(mask);
+    if (ip_is_cidr(mask)) {
-    if (flow_nw_bits_to_mask(wcbits, 0) == mask) {
         wc->wildcards &= ~(0x3f << shift);
-        wc->wildcards |= wcbits << shift;
+        wc->wildcards |= ofputil_netmask_to_wcbits(mask) << shift;
         *maskp = mask;
         return true;
     } else {

lib/flow.h

@@ -146,7 +146,6 @@ struct flow_wildcards {
     ovs_be32 nw_dst_mask;       /* 1-bit in each significant nw_dst bit. */
 };
 
-ovs_be32 flow_nw_bits_to_mask(uint32_t wildcards, int shift);
 void flow_wildcards_init(struct flow_wildcards *, uint32_t wildcards);
 void flow_wildcards_init_exact(struct flow_wildcards *);
 

lib/ofp-util.c

@@ -32,6 +32,41 @@ VLOG_DEFINE_THIS_MODULE(ofp_util);
  * 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);
 
+/* Given the wildcard bit count in the least-significant 6 of 'wcbits', returns
+ * an IP netmask with a 1 in each bit that must match and a 0 in each bit that
+ * is wildcarded.
+ *
+ * The bits in 'wcbits' are in the format used in enum ofp_flow_wildcards: 0
+ * is exact match, 1 ignores the LSB, 2 ignores the 2 least-significant bits,
+ * ..., 32 and higher wildcard the entire field.  This is the *opposite* of the
+ * usual convention where e.g. /24 indicates that 8 bits (not 24 bits) are
+ * wildcarded. */
+ovs_be32
+ofputil_wcbits_to_netmask(int wcbits)
+{
+    wcbits &= 0x3f;
+    return wcbits < 32 ? htonl(~((1u << wcbits) - 1)) : 0;
+}
+
+/* Given the IP netmask 'netmask', returns the number of bits of the IP address
+ * that it wildcards.  'netmask' must be a CIDR netmask (see ip_is_cidr()). */
+int
+ofputil_netmask_to_wcbits(ovs_be32 netmask)
+{
+    assert(ip_is_cidr(netmask));
+#if __GNUC__ >= 4
+    return netmask == htonl(0) ? 32 : __builtin_ctz(ntohl(netmask));
+#else
+    int wcbits;
+
+    for (wcbits = 32; netmask; wcbits--) {
+        netmask &= netmask - 1;
+    }
+
+    return wcbits;
+#endif
+}
+
 /* Returns a transaction ID to use for an outgoing OpenFlow message. */
 static ovs_be32
 alloc_xid(void)
@@ -747,10 +782,10 @@ normalize_match(struct ofp_match *m)
             m->tp_src = m->tp_dst = 0;
         }
         if (wc & OFPFW_NW_SRC_MASK) {
-            m->nw_src &= flow_nw_bits_to_mask(wc, OFPFW_NW_SRC_SHIFT);
+            m->nw_src &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_SRC_SHIFT);
         }
         if (wc & OFPFW_NW_DST_MASK) {
-            m->nw_dst &= flow_nw_bits_to_mask(wc, OFPFW_NW_DST_SHIFT);
+            m->nw_dst &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_DST_SHIFT);
         }
         if (wc & OFPFW_NW_TOS) {
             m->nw_tos = 0;
@@ -762,10 +797,10 @@ normalize_match(struct ofp_match *m)
             m->nw_proto = 0;
         }
         if (wc & OFPFW_NW_SRC_MASK) {
-            m->nw_src &= flow_nw_bits_to_mask(wc, OFPFW_NW_SRC_SHIFT);
+            m->nw_src &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_SRC_SHIFT);
         }
         if (wc & OFPFW_NW_DST_MASK) {
-            m->nw_dst &= flow_nw_bits_to_mask(wc, OFPFW_NW_DST_SHIFT);
+            m->nw_dst &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_DST_SHIFT);
         }
         m->tp_src = m->tp_dst = m->nw_tos = 0;
     } else {

lib/ofp-util.h

@@ -30,6 +30,11 @@ struct ofp_action_header;
 /* Alignment of ofp_actions. */
 #define OFP_ACTION_ALIGN 8
 
+/* Converting OFPFW_NW_SRC_MASK and OFPFW_NW_DST_MASK wildcard bit counts to
+ * and from IP bitmasks. */
+ovs_be32 ofputil_wcbits_to_netmask(int wcbits);
+int ofputil_netmask_to_wcbits(ovs_be32 netmask);
+
 /* OpenFlow protocol utility functions. */
 void *make_openflow(size_t openflow_len, uint8_t type, struct ofpbuf **);
 void *make_nxmsg(size_t openflow_len, uint32_t subtype, struct ofpbuf **);

lib/packets.h

@@ -23,6 +23,7 @@
 #include <stdint.h>
 #include <string.h>
 #include "compiler.h"
+#include "openvswitch/types.h"
 #include "random.h"
 #include "util.h"
 
@@ -246,6 +247,15 @@ BUILD_ASSERT_DECL(VLAN_ETH_HEADER_LEN == sizeof(struct vlan_eth_header));
         ((uint8_t *) ip)[2],                    \
         ((uint8_t *) ip)[3]
 
+/* Returns true if 'netmask' is a CIDR netmask, that is, if it consists of N
+ * high-order 1-bits and 32-N low-order 0-bits. */
+static inline bool
+ip_is_cidr(ovs_be32 netmask)
+{
+    uint32_t x = ~ntohl(netmask);
+    return !(x & (x + 1));
+}
+
 #define IP_VER(ip_ihl_ver) ((ip_ihl_ver) >> 4)
 #define IP_IHL(ip_ihl_ver) ((ip_ihl_ver) & 15)
 #define IP_IHL_VER(ihl, ver) (((ver) << 4) | (ihl))

tests/test-classifier.c

@@ -32,6 +32,7 @@
 #include "byte-order.h"
 #include "command-line.h"
 #include "flow.h"
+#include "ofp-util.h"
 #include "packets.h"
 #include "unaligned.h"
 
@@ -200,9 +201,12 @@ match(const struct cls_rule *wild, const struct flow *fixed)
         if (wild->wc.wildcards & f->wildcards) {
             uint32_t test = get_unaligned_u32(wild_field);
             uint32_t ip = get_unaligned_u32(fixed_field);
-            int shift = (f_idx == CLS_F_IDX_NW_SRC
+            uint32_t mask;
+            int shift;
+
+            shift = (f_idx == CLS_F_IDX_NW_SRC
                      ? OFPFW_NW_SRC_SHIFT : OFPFW_NW_DST_SHIFT);
-            uint32_t mask = flow_nw_bits_to_mask(wild->wc.wildcards, shift);
+            mask = ofputil_wcbits_to_netmask(wild->wc.wildcards >> shift);
             if (!((test ^ ip) & mask)) {
                 continue;
             }