I noticed the pre-existing hash function, conn_key_hash() was missing hash_finish(), I added it locally and retested.
On 6/8/17, 8:34 PM, "[email protected] on behalf of Darrell Ball" <[email protected] on behalf of [email protected]> wrote: Acked-by: Darrell Ball [email protected] I added an incremental so we can use the new abstraction in the pre-existing hash function – conn_key_hash(). I am not sure if we need a separate patch ? diff --git a/lib/conntrack.c b/lib/conntrack.c index 55029c8..d3e514d 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1509,20 +1509,30 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, return false; } + +static uint32_t +ct_addr_hash_add(uint32_t hash, const struct ct_addr *addr) +{ + BUILD_ASSERT_DECL(sizeof *addr % 4 == 0); + return hash_add_bytes32(hash, (const uint32_t *) addr, sizeof *addr); +} + +static uint32_t +ct_endpoint_hash_add(uint32_t hash, const struct ct_endpoint *ep) +{ + BUILD_ASSERT_DECL(sizeof *ep % 4 == 0); + return hash_add_bytes32(hash, (const uint32_t *) ep, sizeof *ep); +} ^L /* Symmetric */ static uint32_t conn_key_hash(const struct conn_key *key, uint32_t basis) { uint32_t hsrc, hdst, hash; - int i; hsrc = hdst = basis; - - for (i = 0; i < sizeof(key->src) / sizeof(uint32_t); i++) { - hsrc = hash_add(hsrc, ((uint32_t *) &key->src)[i]); - hdst = hash_add(hdst, ((uint32_t *) &key->dst)[i]); - } + hsrc = ct_endpoint_hash_add(hsrc, &key->src); + hdst = ct_endpoint_hash_add(hdst, &key->dst); /* Even if source and destination are swapped the hash will be the same. */ hash = hsrc ^ hdst; @@ -1613,20 +1623,6 @@ nat_ipv6_addr_increment(struct in6_addr *ipv6_aligned, uint32_t increment) } static uint32_t -ct_addr_hash_add(uint32_t hash, const struct ct_addr *addr) -{ - BUILD_ASSERT_DECL(sizeof *addr % 4 == 0); - return hash_add_bytes32(hash, (const uint32_t *) addr, sizeof *addr); -} - -static uint32_t -ct_endpoint_hash_add(uint32_t hash, const struct ct_endpoint *ep) -{ - BUILD_ASSERT_DECL(sizeof *ep % 4 == 0); - return hash_add_bytes32(hash, (const uint32_t *) ep, sizeof *ep); -} - -static uint32_t nat_range_hash(const struct conn *conn, uint32_t basis) { uint32_t hash = basis; (END) On 6/7/17, 9:37 AM, "[email protected] on behalf of Ben Pfaff" <[email protected] on behalf of [email protected]> wrote: From: Darrell Ball <[email protected]> Part of the hash input for nat_range_hash() was accidentally omitted, so this fixes the problem. Also, add a missing call to hash_finish(). Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.") Signed-off-by: Darrell Ball <[email protected]> Signed-off-by: Ben Pfaff <[email protected]> --- lib/conntrack-private.h | 4 ++++ lib/conntrack.c | 47 ++++++++++++++++++++++++++--------------------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index bfa88f0fe7e2..55084d3ea64e 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -42,6 +42,10 @@ struct ct_endpoint { }; }; +/* Verify that there is no padding in struct ct_endpoint, to facilitate + * hashing in ct_endpoint_hash_add(). */ +BUILD_ASSERT_DECL(sizeof(struct ct_endpoint) == sizeof(struct ct_addr) + 4); + /* Changes to this structure need to be reflected in conn_key_hash() */ struct conn_key { struct ct_endpoint src; diff --git a/lib/conntrack.c b/lib/conntrack.c index 44a6bc4e3ccf..55029c82e7a6 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1613,36 +1613,41 @@ nat_ipv6_addr_increment(struct in6_addr *ipv6_aligned, uint32_t increment) } static uint32_t -nat_range_hash(const struct conn *conn, uint32_t basis) +ct_addr_hash_add(uint32_t hash, const struct ct_addr *addr) { - uint32_t hash = basis; - int i; - uint16_t port; + BUILD_ASSERT_DECL(sizeof *addr % 4 == 0); + return hash_add_bytes32(hash, (const uint32_t *) addr, sizeof *addr); +} - for (i = 0; - i < sizeof(conn->nat_info->min_addr) / sizeof(uint32_t); - i++) { - hash = hash_add(hash, ((uint32_t *) &conn->nat_info->min_addr)[i]); - hash = hash_add(hash, ((uint32_t *) &conn->nat_info->max_addr)[i]); - } +static uint32_t +ct_endpoint_hash_add(uint32_t hash, const struct ct_endpoint *ep) +{ + BUILD_ASSERT_DECL(sizeof *ep % 4 == 0); + return hash_add_bytes32(hash, (const uint32_t *) ep, sizeof *ep); +} - memcpy(&port, &conn->nat_info->min_port, sizeof port); - hash = hash_add(hash, port); +static uint32_t +nat_range_hash(const struct conn *conn, uint32_t basis) +{ + uint32_t hash = basis; - for (i = 0; i < sizeof(conn->key.src.addr) / sizeof(uint32_t); i++) { - hash = hash_add(hash, ((uint32_t *) &conn->key.src)[i]); - hash = hash_add(hash, ((uint32_t *) &conn->key.dst)[i]); - } + hash = ct_addr_hash_add(hash, &conn->nat_info->min_addr); + hash = ct_addr_hash_add(hash, &conn->nat_info->max_addr); + hash = hash_add(hash, + (conn->nat_info->max_port << 16) + | conn->nat_info->min_port); - memcpy(&port, &conn->key.src.port, sizeof port); - hash = hash_add(hash, port); - memcpy(&port, &conn->key.dst.port, sizeof port); - hash = hash_add(hash, port); + hash = ct_endpoint_hash_add(hash, &conn->key.src); + hash = ct_endpoint_hash_add(hash, &conn->key.dst); hash = hash_add(hash, (OVS_FORCE uint32_t) conn->key.dl_type); hash = hash_add(hash, conn->key.nw_proto); hash = hash_add(hash, conn->key.zone); - return hash; + + /* The purpose of the second parameter is to distinguish hashes of data of + * different length; our data always has the same length so there is no + * value in counting. */ + return hash_finish(hash, 0); } static bool -- 2.10.2 _______________________________________________ dev mailing list [email protected] https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=0-D9j1lxPv8FuvIlzvLsBxXMvnqlIWhS_O_iGeh0O_E&s=DMIKKrfIU2O9SgvyTV0Yvs6CSu3pnRc6u4-ZBL_pRho&e= _______________________________________________ dev mailing list [email protected] https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=0-D9j1lxPv8FuvIlzvLsBxXMvnqlIWhS_O_iGeh0O_E&s=DMIKKrfIU2O9SgvyTV0Yvs6CSu3pnRc6u4-ZBL_pRho&e= _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
