Shifting a 32-bit entity by 32 bits is undefined behavior.  As we have 3
cases where we may hit this, it is a time to introduce a helper for
this.

VMware-BZ: #1355026
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
 lib/classifier.c |    2 +-
 lib/meta-flow.c  |    8 +++-----
 lib/ovs-router.c |    2 +-
 lib/util.h       |    9 +++++++++
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index 82aaf3e..c19d81c 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1333,7 +1333,7 @@ find_match_wc(const struct cls_subtable *subtable, const 
struct flow *flow,
         mbits = trie_lookup_value(&subtable->ports_trie, &value, &plens, 32);
 
         ((OVS_FORCE ovs_be32 *)&wc->masks)[TP_PORTS_OFS32] |=
-            mask & htonl(~0 << (32 - mbits));
+            mask & be32_prefix_mask(mbits);
 
         /* Unwildcard all bits in the mask upto the ports, as they were used
          * to determine there is no match. */
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 4342337..9aa71a3 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -34,6 +34,7 @@
 #include "shash.h"
 #include "socket-util.h"
 #include "unaligned.h"
+#include "util.h"
 #include "vlog.h"
 
 VLOG_DEFINE_THIS_MODULE(meta_flow);
@@ -1669,13 +1670,10 @@ mf_from_ipv4_string(const struct mf_field *mf, const 
char *s,
         /* OK. */
     } else if (ovs_scan(s, IP_SCAN_FMT"/%d", IP_SCAN_ARGS(ip), &prefix)) {
         if (prefix <= 0 || prefix > 32) {
-            return xasprintf("%s: network prefix bits not between 1 and "
+            return xasprintf("%s: network prefix bits not between 0 and "
                              "32", s);
-        } else if (prefix == 32) {
-            *mask = OVS_BE32_MAX;
-        } else {
-            *mask = htonl(((1u << prefix) - 1) << (32 - prefix));
         }
+        *mask = be32_prefix_mask(prefix);
     } else if (ovs_scan(s, IP_SCAN_FMT, IP_SCAN_ARGS(ip))) {
         *mask = OVS_BE32_MAX;
     } else {
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index 2aa4a9a..26ae18f 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -87,7 +87,7 @@ static void rt_init_match(struct match *match, ovs_be32 
ip_dst, uint8_t plen)
 {
     ovs_be32 mask;
 
-    mask = htonl(UINT32_MAX << (32 - plen));
+    mask = be32_prefix_mask(plen);
 
     ip_dst &= mask; /* Clear out insignificant bits. */
     memset(match, 0, sizeof *match);
diff --git a/lib/util.h b/lib/util.h
index 79501af..e0e9e07 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -26,6 +26,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include "byte-order.h"
 #include "compiler.h"
 #include "openvswitch/types.h"
 
@@ -528,6 +529,14 @@ leftmost_1bit_idx(uint32_t x)
 {
     return x ? log_2_floor(x) : 32;
 }
+
+/* Return a ovs_be32 prefix in network byte order with 'plen' highest bits set.
+ * Shift with 32 is undefined behavior, but we rather use 64-bit shift than
+ * compare. */
+static inline ovs_be32 be32_prefix_mask(int plen)
+{
+    return htonl((uint64_t)UINT32_MAX << (32 - plen));
+}
 
 bool is_all_zeros(const void *, size_t);
 bool is_all_ones(const void *, size_t);
-- 
1.7.10.4

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

Reply via email to