DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix left shift extension of doorbell bitmap
@ 2020-05-15  8:13 Bing Zhao
  2020-05-17  5:45 ` Matan Azrad
  2020-05-17  9:18 ` Raslan Darawsheh
  0 siblings, 2 replies; 4+ messages in thread
From: Bing Zhao @ 2020-05-15  8:13 UTC (permalink / raw)
  To: viacheslavo, rasland; +Cc: orika, matan, dev, stable, dekelp

The doorbell record is organized with page and bitmap. When some new
doorbell needs to be associated with a queue, the bit will be set
in the bitmap to indicate the corresponding doorbell occupied. A
counter is used to record the number of doorbell occupied to speed
up the searching.
If the number reachs the maximal value of a pre-defined number of a
page, a new page will be allocated. If not, then the bitmap will be
checked to find a free one.
The LSHIFT and OR (AND NOT) operations are used to update the bitmap
of a page. But 1 will be treated as a signed integer when compiling.
When the shift number is 31, the shifted value will be considered as
negative. Then a wrong extension will be done when setting it to a
64-bits variable. All the upper 32-bits will be set to 1 by such
extension.
Then a wrong offset value will be calculated because of this. The
next 64 bits will be also treated as the bitmap and get corrupted
through the bit set operation.
The immediate value 1 needs to be used as 64 bits width explicitly.

Fixes: 21cae8580fd0 ("net/mlx5: allocate door-bells via DevX")
Cc: dekelp@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 1445809..ab4adec 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2174,7 +2174,7 @@ struct mlx5_flow_id_pool *
 	/* Find the first clear bit. */
 	MLX5_ASSERT(i < MLX5_DBR_BITMAP_SIZE);
 	j = rte_bsf64(~page->dbr_bitmap[i]);
-	page->dbr_bitmap[i] |= (1 << j);
+	page->dbr_bitmap[i] |= (UINT64_C(1) << j);
 	page->dbr_count++;
 	*dbr_page = page;
 	return (((i * 64) + j) * sizeof(uint64_t));
@@ -2219,7 +2219,7 @@ struct mlx5_flow_id_pool *
 		int i = offset / 64;
 		int j = offset % 64;
 
-		page->dbr_bitmap[i] &= ~(1 << j);
+		page->dbr_bitmap[i] &= ~(UINT64_C(1) << j);
 	}
 	return ret;
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] net/mlx5: fix left shift extension of doorbell bitmap
  2020-05-15  8:13 [dpdk-dev] [PATCH] net/mlx5: fix left shift extension of doorbell bitmap Bing Zhao
@ 2020-05-17  5:45 ` Matan Azrad
  2020-05-17  9:18 ` Raslan Darawsheh
  1 sibling, 0 replies; 4+ messages in thread
From: Matan Azrad @ 2020-05-17  5:45 UTC (permalink / raw)
  To: Bing Zhao, Slava Ovsiienko, Raslan Darawsheh
  Cc: Ori Kam, dev, stable, Dekel Peled


From: Bing Zhao:
> The doorbell record is organized with page and bitmap. When some new
> doorbell needs to be associated with a queue, the bit will be set in the
> bitmap to indicate the corresponding doorbell occupied. A counter is used to
> record the number of doorbell occupied to speed up the searching.
> If the number reachs the maximal value of a pre-defined number of a page, a
> new page will be allocated. If not, then the bitmap will be checked to find a
> free one.
> The LSHIFT and OR (AND NOT) operations are used to update the bitmap of a
> page. But 1 will be treated as a signed integer when compiling.
> When the shift number is 31, the shifted value will be considered as
> negative. Then a wrong extension will be done when setting it to a 64-bits
> variable. All the upper 32-bits will be set to 1 by such extension.
> Then a wrong offset value will be calculated because of this. The next 64 bits
> will be also treated as the bitmap and get corrupted through the bit set
> operation.
> The immediate value 1 needs to be used as 64 bits width explicitly.
> 
> Fixes: 21cae8580fd0 ("net/mlx5: allocate door-bells via DevX")
> Cc: dekelp@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>


Suggested title:
net/mlx5: fix doorbell bitmap management offsets

Other than that:
Acked-by: Matan Azrad <matan@mellanox.com>

>  drivers/net/mlx5/mlx5.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 1445809..ab4adec 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -2174,7 +2174,7 @@ struct mlx5_flow_id_pool *
>  	/* Find the first clear bit. */
>  	MLX5_ASSERT(i < MLX5_DBR_BITMAP_SIZE);
>  	j = rte_bsf64(~page->dbr_bitmap[i]);
> -	page->dbr_bitmap[i] |= (1 << j);
> +	page->dbr_bitmap[i] |= (UINT64_C(1) << j);
>  	page->dbr_count++;
>  	*dbr_page = page;
>  	return (((i * 64) + j) * sizeof(uint64_t)); @@ -2219,7 +2219,7 @@
> struct mlx5_flow_id_pool *
>  		int i = offset / 64;
>  		int j = offset % 64;
> 
> -		page->dbr_bitmap[i] &= ~(1 << j);
> +		page->dbr_bitmap[i] &= ~(UINT64_C(1) << j);
>  	}
>  	return ret;
>  }
> --
> 1.8.3.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] net/mlx5: fix left shift extension of doorbell bitmap
  2020-05-15  8:13 [dpdk-dev] [PATCH] net/mlx5: fix left shift extension of doorbell bitmap Bing Zhao
  2020-05-17  5:45 ` Matan Azrad
@ 2020-05-17  9:18 ` Raslan Darawsheh
  2020-05-17  9:54   ` Raslan Darawsheh
  1 sibling, 1 reply; 4+ messages in thread
From: Raslan Darawsheh @ 2020-05-17  9:18 UTC (permalink / raw)
  To: Bing Zhao, Slava Ovsiienko; +Cc: Ori Kam, Matan Azrad, dev, stable, Dekel Peled

Hi,

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Friday, May 15, 2020 11:14 AM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
> dev@dpdk.org; stable@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> Subject: [PATCH] net/mlx5: fix left shift extension of doorbell bitmap
> 
> The doorbell record is organized with page and bitmap. When some new
> doorbell needs to be associated with a queue, the bit will be set
> in the bitmap to indicate the corresponding doorbell occupied. A
> counter is used to record the number of doorbell occupied to speed
> up the searching.
> If the number reachs the maximal value of a pre-defined number of a
> page, a new page will be allocated. If not, then the bitmap will be
> checked to find a free one.
> The LSHIFT and OR (AND NOT) operations are used to update the bitmap
> of a page. But 1 will be treated as a signed integer when compiling.
> When the shift number is 31, the shifted value will be considered as
> negative. Then a wrong extension will be done when setting it to a
> 64-bits variable. All the upper 32-bits will be set to 1 by such
> extension.
> Then a wrong offset value will be calculated because of this. The
> next 64 bits will be also treated as the bitmap and get corrupted
> through the bit set operation.
> The immediate value 1 needs to be used as 64 bits width explicitly.
> 
> Fixes: 21cae8580fd0 ("net/mlx5: allocate door-bells via DevX")
> Cc: dekelp@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 1445809..ab4adec 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -2174,7 +2174,7 @@ struct mlx5_flow_id_pool *
>  	/* Find the first clear bit. */
>  	MLX5_ASSERT(i < MLX5_DBR_BITMAP_SIZE);
>  	j = rte_bsf64(~page->dbr_bitmap[i]);
> -	page->dbr_bitmap[i] |= (1 << j);
> +	page->dbr_bitmap[i] |= (UINT64_C(1) << j);
>  	page->dbr_count++;
>  	*dbr_page = page;
>  	return (((i * 64) + j) * sizeof(uint64_t));
> @@ -2219,7 +2219,7 @@ struct mlx5_flow_id_pool *
>  		int i = offset / 64;
>  		int j = offset % 64;
> 
> -		page->dbr_bitmap[i] &= ~(1 << j);
> +		page->dbr_bitmap[i] &= ~(UINT64_C(1) << j);
>  	}
>  	return ret;
>  }
> --
> 1.8.3.1

Commit title changed to:
net/mlx5: fix left shift extension of doorbell bitmap

Patch applied to next-net-mlx,

Kindest regards
Raslan Darawsheh

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] net/mlx5: fix left shift extension of doorbell bitmap
  2020-05-17  9:18 ` Raslan Darawsheh
@ 2020-05-17  9:54   ` Raslan Darawsheh
  0 siblings, 0 replies; 4+ messages in thread
From: Raslan Darawsheh @ 2020-05-17  9:54 UTC (permalink / raw)
  To: Bing Zhao, Slava Ovsiienko; +Cc: Ori Kam, Matan Azrad, dev, stable, Dekel Peled


> -----Original Message-----
> From: Raslan Darawsheh
> Sent: Sunday, May 17, 2020 12:19 PM
> To: Bing Zhao <bingz@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
> dev@dpdk.org; stable@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> Subject: RE: [PATCH] net/mlx5: fix left shift extension of doorbell bitmap
> 
> Hi,
> 
> > -----Original Message-----
> > From: Bing Zhao <bingz@mellanox.com>
> > Sent: Friday, May 15, 2020 11:14 AM
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>; Raslan Darawsheh
> > <rasland@mellanox.com>
> > Cc: Ori Kam <orika@mellanox.com>; Matan Azrad
> <matan@mellanox.com>;
> > dev@dpdk.org; stable@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> > Subject: [PATCH] net/mlx5: fix left shift extension of doorbell bitmap
> >
> > The doorbell record is organized with page and bitmap. When some new
> > doorbell needs to be associated with a queue, the bit will be set
> > in the bitmap to indicate the corresponding doorbell occupied. A
> > counter is used to record the number of doorbell occupied to speed
> > up the searching.
> > If the number reachs the maximal value of a pre-defined number of a
> > page, a new page will be allocated. If not, then the bitmap will be
> > checked to find a free one.
> > The LSHIFT and OR (AND NOT) operations are used to update the bitmap
> > of a page. But 1 will be treated as a signed integer when compiling.
> > When the shift number is 31, the shifted value will be considered as
> > negative. Then a wrong extension will be done when setting it to a
> > 64-bits variable. All the upper 32-bits will be set to 1 by such
> > extension.
> > Then a wrong offset value will be calculated because of this. The
> > next 64 bits will be also treated as the bitmap and get corrupted
> > through the bit set operation.
> > The immediate value 1 needs to be used as 64 bits width explicitly.
> >
> > Fixes: 21cae8580fd0 ("net/mlx5: allocate door-bells via DevX")
> > Cc: dekelp@mellanox.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bing Zhao <bingz@mellanox.com>
> > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> > index 1445809..ab4adec 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -2174,7 +2174,7 @@ struct mlx5_flow_id_pool *
> >  	/* Find the first clear bit. */
> >  	MLX5_ASSERT(i < MLX5_DBR_BITMAP_SIZE);
> >  	j = rte_bsf64(~page->dbr_bitmap[i]);
> > -	page->dbr_bitmap[i] |= (1 << j);
> > +	page->dbr_bitmap[i] |= (UINT64_C(1) << j);
> >  	page->dbr_count++;
> >  	*dbr_page = page;
> >  	return (((i * 64) + j) * sizeof(uint64_t));
> > @@ -2219,7 +2219,7 @@ struct mlx5_flow_id_pool *
> >  		int i = offset / 64;
> >  		int j = offset % 64;
> >
> > -		page->dbr_bitmap[i] &= ~(1 << j);
> > +		page->dbr_bitmap[i] &= ~(UINT64_C(1) << j);
> >  	}
> >  	return ret;
> >  }
> > --
> > 1.8.3.1
> 
> Commit title changed to:
> net/mlx5: fix left shift extension of doorbell bitmap
Commit title sent incorrectly, the fix title is:
net/mlx5: fix doorbell bitmap management offsets

> 
> Patch applied to next-net-mlx,
> 
> Kindest regards
> Raslan Darawsheh
Kindest regards
Raslan Darawsheh

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-05-17  9:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15  8:13 [dpdk-dev] [PATCH] net/mlx5: fix left shift extension of doorbell bitmap Bing Zhao
2020-05-17  5:45 ` Matan Azrad
2020-05-17  9:18 ` Raslan Darawsheh
2020-05-17  9:54   ` Raslan Darawsheh

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git