diff mbox

[ovs-dev,patch_v1] dpdk: Recover hash input for nat_range_hash.

Message ID 1496522294-26104-1-git-send-email-dlu998@gmail.com
State Superseded
Headers show

Commit Message

Darrell Ball June 3, 2017, 8:38 p.m. UTC
Part of the hash input for nat_range_hash() was accidently
truncated, so recover it here.  Also update the local variable
name to better reflect the intention and add missing comments.

Signed-off-by: Darrell Ball <dlu998@gmail.com>
---
 lib/conntrack.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Comments

Ben Pfaff June 7, 2017, 4:37 p.m. UTC | #1
On Sat, Jun 03, 2017 at 01:38:14PM -0700, Darrell Ball wrote:
> Part of the hash input for nat_range_hash() was accidently
> truncated, so recover it here.  Also update the local variable
> name to better reflect the intention and add missing comments.
> 
> Signed-off-by: Darrell Ball <dlu998@gmail.com>

Thanks a lot for the improvement.  I'm sorry I screwed this up on
review.

I feel like this could benefit from a little more abstraction.  I sent
out a 2-patch series for you to take a look at:
        https://mail.openvswitch.org/pipermail/ovs-dev/2017-June/333573.html
        https://mail.openvswitch.org/pipermail/ovs-dev/2017-June/333574.html
Darrell Ball June 7, 2017, 5:23 p.m. UTC | #2
Thanks Ben

I had a quick look; I will test out today and get back.

Darrell

On 6/7/17, 9:37 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ben Pfaff" <ovs-dev-bounces@openvswitch.org on behalf of blp@ovn.org> wrote:

    On Sat, Jun 03, 2017 at 01:38:14PM -0700, Darrell Ball wrote:
    > Part of the hash input for nat_range_hash() was accidently
    > truncated, so recover it here.  Also update the local variable
    > name to better reflect the intention and add missing comments.
    > 
    > Signed-off-by: Darrell Ball <dlu998@gmail.com>
    
    Thanks a lot for the improvement.  I'm sorry I screwed this up on
    review.
    
    I feel like this could benefit from a little more abstraction.  I sent
    out a 2-patch series for you to take a look at:
            https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DJune_333573.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=DYoxK1j8Y-F_qsiBKKfBPXvMiD5Sso-WINF5q-UvqBI&s=Cn4cLI5MDgHTEtgx9-5DB09ZDuY-7TbYfnUwOGUnZl4&e= 
            https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DJune_333574.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=DYoxK1j8Y-F_qsiBKKfBPXvMiD5Sso-WINF5q-UvqBI&s=1vffunQcO8aXlL2OD08qZUjFv9UUz7-bisYGGGMMYM4&e= 
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=DYoxK1j8Y-F_qsiBKKfBPXvMiD5Sso-WINF5q-UvqBI&s=J7BbzKyPMkSADLePw4ZFt9U2aw2XRCRYUEAAIlRZqf4&e=
diff mbox

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 44a6bc4..aceaf74 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1617,7 +1617,7 @@  nat_range_hash(const struct conn *conn, uint32_t basis)
 {
     uint32_t hash = basis;
     int i;
-    uint16_t port;
+    uint32_t ports;
 
     for (i = 0;
          i < sizeof(conn->nat_info->min_addr) / sizeof(uint32_t);
@@ -1626,18 +1626,22 @@  nat_range_hash(const struct conn *conn, uint32_t basis)
         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);
+
+    /* Fold in the 4 bytes starting at the address of min_port. */
+    memcpy(&ports, &conn->nat_info->min_port, sizeof ports);
+    hash = hash_add(hash, ports);
 
     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);
+    /* Fold in the 4 bytes starting at the address of src.port. */
+    memcpy(&ports, &conn->key.src.port, sizeof ports);
+    hash = hash_add(hash, ports);
+    /* Fold in the 4 bytes starting at the address of dst.port. */
+    memcpy(&ports, &conn->key.dst.port, sizeof ports);
+    hash = hash_add(hash, ports);
 
     hash = hash_add(hash, (OVS_FORCE uint32_t) conn->key.dl_type);
     hash = hash_add(hash, conn->key.nw_proto);