diff mbox

[ovs-dev,patch_v2,3/3] conntrack: Add hash_finish() to conn_key_hash().

Message ID 1497047444-80332-3-git-send-email-dlu998@gmail.com
State Accepted
Headers show

Commit Message

Darrell Ball June 9, 2017, 10:30 p.m. UTC
The function conn_key_hash() is updated to include
a call to hash_finish() and also to make use of a
new hash abstraction - ct_endpoint_hash_add().

Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
Signed-off-by: Darrell Ball <dlu998@gmail.com>
---
 lib/conntrack.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Fischetti, Antonio June 13, 2017, 1:15 p.m. UTC | #1
Hi Darrell,
it seems in lib/hash.h there's already a hash_finish() function for the 
Intrinsic mode where the 1st parm is a uint64_t:
static inline uint32_t hash_finish(uint64_t hash, uint64_t final)

so I'm getting some errors when building with CFLAGS="-O2 -march=native -g" 

lib/hash.h:180:24: error: conflicting types for 'hash_finish'
 static inline uint32_t hash_finish(uint64_t hash, uint64_t final)

lib/hash.h:95:24: note: previous declaration of 'hash_finish' was here
 static inline uint32_t hash_finish(uint32_t hash, uint32_t final);


Antonio

> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> bounces@openvswitch.org] On Behalf Of Darrell Ball
> Sent: Friday, June 9, 2017 11:31 PM
> To: dev@openvswitch.org
> Subject: [ovs-dev] [patch_v2 3/3] conntrack: Add hash_finish() to
> conn_key_hash().
> 
> The function conn_key_hash() is updated to include
> a call to hash_finish() and also to make use of a
> new hash abstraction - ct_endpoint_hash_add().
> 
> Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
> Signed-off-by: Darrell Ball <dlu998@gmail.com>
> ---
>  lib/conntrack.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 9584a0a..146edd7 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1529,14 +1529,10 @@ 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;
> @@ -1546,7 +1542,7 @@ conn_key_hash(const struct conn_key *key, uint32_t
> basis)
>                        (uint32_t *) (key + 1) - (uint32_t *) (&key->dst +
> 1),
>                        hash);
> 
> -    return hash;
> +    return hash_finish(hash, 0);
>  }
> 
>  static void
> --
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff June 13, 2017, 3:31 p.m. UTC | #2
Oops, that's my fault.  Can you confirm that this patch solves the
problem?
        https://patchwork.ozlabs.org/patch/775320/

On Tue, Jun 13, 2017 at 01:15:24PM +0000, Fischetti, Antonio wrote:
> Hi Darrell,
> it seems in lib/hash.h there's already a hash_finish() function for the 
> Intrinsic mode where the 1st parm is a uint64_t:
> static inline uint32_t hash_finish(uint64_t hash, uint64_t final)
> 
> so I'm getting some errors when building with CFLAGS="-O2 -march=native -g" 
> 
> lib/hash.h:180:24: error: conflicting types for 'hash_finish'
>  static inline uint32_t hash_finish(uint64_t hash, uint64_t final)
> 
> lib/hash.h:95:24: note: previous declaration of 'hash_finish' was here
>  static inline uint32_t hash_finish(uint32_t hash, uint32_t final);
> 
> 
> Antonio
> 
> > -----Original Message-----
> > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> > bounces@openvswitch.org] On Behalf Of Darrell Ball
> > Sent: Friday, June 9, 2017 11:31 PM
> > To: dev@openvswitch.org
> > Subject: [ovs-dev] [patch_v2 3/3] conntrack: Add hash_finish() to
> > conn_key_hash().
> > 
> > The function conn_key_hash() is updated to include
> > a call to hash_finish() and also to make use of a
> > new hash abstraction - ct_endpoint_hash_add().
> > 
> > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
> > Signed-off-by: Darrell Ball <dlu998@gmail.com>
> > ---
> >  lib/conntrack.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index 9584a0a..146edd7 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -1529,14 +1529,10 @@ 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;
> > @@ -1546,7 +1542,7 @@ conn_key_hash(const struct conn_key *key, uint32_t
> > basis)
> >                        (uint32_t *) (key + 1) - (uint32_t *) (&key->dst +
> > 1),
> >                        hash);
> > 
> > -    return hash;
> > +    return hash_finish(hash, 0);
> >  }
> > 
> >  static void
> > --
> > 1.9.1
> > 
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 9584a0a..146edd7 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1529,14 +1529,10 @@  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;
@@ -1546,7 +1542,7 @@  conn_key_hash(const struct conn_key *key, uint32_t basis)
                       (uint32_t *) (key + 1) - (uint32_t *) (&key->dst + 1),
                       hash);
 
-    return hash;
+    return hash_finish(hash, 0);
 }
 
 static void