Thanks for a quick review! Applied to master and backported to branch-2.3.
Jarno On Nov 11, 2014, at 3:41 PM, Pravin Shelar <[email protected]> wrote: > 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
