On Tue, Nov 11, 2014 at 3:37 PM, Jarno Rajahalme <[email protected]> wrote: > 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 <[email protected]>
LGTM Acked-by: Pravin B Shelar <[email protected]> > --- > 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 > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
