From 92edd073ce6ca72636600a64e0645bf23027660a Mon Sep 17 00:00:00 2001 From: Darrell Ball Date: Fri, 9 Jun 2017 15:30:43 -0700 Subject: [PATCH] conntrack: Hash entire NAT data structure in nat_range_hash(). 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.") Co-authored-by: Ben Pfaff Signed-off-by: Ben Pfaff Signed-off-by: Darrell Ball Signed-off-by: Ben Pfaff --- 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 bfa88f0fe7e..55084d3ea64 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 44a6bc4e3cc..9584a0a354f 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1509,6 +1509,20 @@ 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); +} /* Symmetric */ static uint32_t @@ -1616,33 +1630,24 @@ static uint32_t nat_range_hash(const struct conn *conn, uint32_t basis) { uint32_t hash = basis; - int i; - uint16_t port; - - 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]); - } - memcpy(&port, &conn->nat_info->min_port, sizeof port); - hash = hash_add(hash, port); + 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); - 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]); - } - - 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