diff mbox series

lib/librte_ip_frag:fix ip_frag_key_cmp bug

Message ID 1533175263-4642-1-git-send-email-han.li1@zte.com.cn (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series lib/librte_ip_frag:fix ip_frag_key_cmp bug |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Li Han Aug. 2, 2018, 2:01 a.m. UTC 
  in struct ip_frag_key,src_dst[] type is uint64_t.
but "val" which to store the calc restult ,type is uint32_t.
we may lost high 32 bit key. and function return value is int,
but it won't return < 0.

Signed-off-by: Li Han <han.li1@zte.com.cn>
---
 lib/librte_ip_frag/ip_frag_common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Monjalon Aug. 5, 2018, 12:59 p.m. UTC | #1 
Konstantin,
Is it candidate for last minute fix in 18.08?


02/08/2018 04:01, Li Han:
> in struct ip_frag_key,src_dst[] type is uint64_t.
> but "val" which to store the calc restult ,type is uint32_t.
> we may lost high 32 bit key. and function return value is int,
> but it won't return < 0.
> 
> Signed-off-by: Li Han <han.li1@zte.com.cn>
> ---
>  lib/librte_ip_frag/ip_frag_common.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_frag_common.h
> index 197acf8..ca69680 100644
> --- a/lib/librte_ip_frag/ip_frag_common.h
> +++ b/lib/librte_ip_frag/ip_frag_common.h
> @@ -69,10 +69,10 @@ struct ip_frag_pkt * ip_frag_lookup(struct rte_ip_frag_tbl *tbl,
>  }
>  
>  /* compare two keys */
> -static inline int
> +static inline uint64_t
>  ip_frag_key_cmp(const struct ip_frag_key * k1, const struct ip_frag_key * k2)
>  {
> -	uint32_t i, val;
> +	uint64_t i, val;
>  	val = k1->id ^ k2->id;
>  	for (i = 0; i < k1->key_len; i++)
>  		val |= k1->src_dst[i] ^ k2->src_dst[i];
>
Ferruh Yigit Aug. 9, 2018, 12:40 p.m. UTC | #2 
On 8/2/2018 3:01 AM, Li Han wrote:
> in struct ip_frag_key,src_dst[] type is uint64_t.
> but "val" which to store the calc restult ,type is uint32_t.
> we may lost high 32 bit key. and function return value is int,
> but it won't return < 0.
> 
> Signed-off-by: Li Han <han.li1@zte.com.cn>

Fix looks good to me [1], but not sure if it worth getting risk to merge it
after rc3, taking into account that original code is from 2014.

[1] perhaps no need to make "i" uint64_t

> ---
>  lib/librte_ip_frag/ip_frag_common.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_frag_common.h
> index 197acf8..ca69680 100644
> --- a/lib/librte_ip_frag/ip_frag_common.h
> +++ b/lib/librte_ip_frag/ip_frag_common.h
> @@ -69,10 +69,10 @@ struct ip_frag_pkt * ip_frag_lookup(struct rte_ip_frag_tbl *tbl,
>  }
>  
>  /* compare two keys */
> -static inline int
> +static inline uint64_t
>  ip_frag_key_cmp(const struct ip_frag_key * k1, const struct ip_frag_key * k2)
>  {
> -	uint32_t i, val;
> +	uint64_t i, val;
>  	val = k1->id ^ k2->id;
>  	for (i = 0; i < k1->key_len; i++)
>  		val |= k1->src_dst[i] ^ k2->src_dst[i];
>
Ananyev, Konstantin Aug. 24, 2018, 11:04 a.m. UTC | #3 
> -----Original Message-----
> From: Li Han [mailto:han.li1@zte.com.cn]
> Sent: Thursday, August 2, 2018 3:01 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; Li Han <han.li1@zte.com.cn>
> Subject: [PATCH] [dpdk-dev] lib/librte_ip_frag:fix ip_frag_key_cmp bug
> 
> in struct ip_frag_key,src_dst[] type is uint64_t.
> but "val" which to store the calc restult ,type is uint32_t.
> we may lost high 32 bit key. and function return value is int,
> but it won't return < 0.
> 
> Signed-off-by: Li Han <han.li1@zte.com.cn>
> ---
>  lib/librte_ip_frag/ip_frag_common.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_frag_common.h
> index 197acf8..ca69680 100644
> --- a/lib/librte_ip_frag/ip_frag_common.h
> +++ b/lib/librte_ip_frag/ip_frag_common.h
> @@ -69,10 +69,10 @@ struct ip_frag_pkt * ip_frag_lookup(struct rte_ip_frag_tbl *tbl,
>  }
> 
>  /* compare two keys */
> -static inline int
> +static inline uint64_t
>  ip_frag_key_cmp(const struct ip_frag_key * k1, const struct ip_frag_key * k2)
>  {
> -	uint32_t i, val;
> +	uint64_t i, val;
>  	val = k1->id ^ k2->id;
>  	for (i = 0; i < k1->key_len; i++)
>  		val |= k1->src_dst[i] ^ k2->src_dst[i];
> --

As a nit - no point to make *i* 64-bit.
Apart from that:
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 1.8.3.1
Thomas Monjalon Oct. 28, 2018, 10:18 a.m. UTC | #4 
24/08/2018 13:04, Ananyev, Konstantin:
> From: Li Han [mailto:han.li1@zte.com.cn]
> > 
> > in struct ip_frag_key,src_dst[] type is uint64_t.
> > but "val" which to store the calc restult ,type is uint32_t.
> > we may lost high 32 bit key. and function return value is int,
> > but it won't return < 0.
> > 
> > Signed-off-by: Li Han <han.li1@zte.com.cn>
> > ---
> >  lib/librte_ip_frag/ip_frag_common.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_frag_common.h
> > index 197acf8..ca69680 100644
> > --- a/lib/librte_ip_frag/ip_frag_common.h
> > +++ b/lib/librte_ip_frag/ip_frag_common.h
> > @@ -69,10 +69,10 @@ struct ip_frag_pkt * ip_frag_lookup(struct rte_ip_frag_tbl *tbl,
> >  }
> > 
> >  /* compare two keys */
> > -static inline int
> > +static inline uint64_t
> >  ip_frag_key_cmp(const struct ip_frag_key * k1, const struct ip_frag_key * k2)
> >  {
> > -	uint32_t i, val;
> > +	uint64_t i, val;
> >  	val = k1->id ^ k2->id;
> >  	for (i = 0; i < k1->key_len; i++)
> >  		val |= k1->src_dst[i] ^ k2->src_dst[i];
> > --
> 
> As a nit - no point to make *i* 64-bit.
> Apart from that:
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied with suggested change, thanks
diff mbox series

Patch

diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_frag_common.h
index 197acf8..ca69680 100644
--- a/lib/librte_ip_frag/ip_frag_common.h
+++ b/lib/librte_ip_frag/ip_frag_common.h
@@ -69,10 +69,10 @@  struct ip_frag_pkt * ip_frag_lookup(struct rte_ip_frag_tbl *tbl,
 }
 
 /* compare two keys */
-static inline int
+static inline uint64_t
 ip_frag_key_cmp(const struct ip_frag_key * k1, const struct ip_frag_key * k2)
 {
-	uint32_t i, val;
+	uint64_t i, val;
 	val = k1->id ^ k2->id;
 	for (i = 0; i < k1->key_len; i++)
 		val |= k1->src_dst[i] ^ k2->src_dst[i];