[02/29] net/ena/base: make allocation macros thread-safe
diff mbox series

Message ID 20200327101823.12646-3-mk@semihalf.com
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers show
Series
  • Update ENA driver to v2.1.0
Related show

Checks

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

Commit Message

Michał Krawczyk March 27, 2020, 10:17 a.m. UTC
From: Igor Chauskin <igorch@amazon.com>

Memory allocation region id could possibly be non-unique
due to non-atomic increment, causing allocation failure.

Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")
Cc: stable@dpdk.org

Signed-off-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_plat_dpdk.h | 8 +++++---
 drivers/net/ena/ena_ethdev.c         | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Stephen Hemminger March 27, 2020, 2:54 p.m. UTC | #1
On Fri, 27 Mar 2020 11:17:56 +0100
Michal Krawczyk <mk@semihalf.com> wrote:

> From: Igor Chauskin <igorch@amazon.com>
> 
> Memory allocation region id could possibly be non-unique
> due to non-atomic increment, causing allocation failure.
> 
> Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>

With DPDK all control operations are the device are supposed
to be single threaded by the caller. Do you have an allocation in
some datapath?
Michał Krawczyk March 31, 2020, 9:47 a.m. UTC | #2
pt., 27 mar 2020 o 15:54 Stephen Hemminger
<stephen@networkplumber.org> napisał(a):
>
> On Fri, 27 Mar 2020 11:17:56 +0100
> Michal Krawczyk <mk@semihalf.com> wrote:
>
> > From: Igor Chauskin <igorch@amazon.com>
> >
> > Memory allocation region id could possibly be non-unique
> > due to non-atomic increment, causing allocation failure.
> >
> > Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
>
> With DPDK all control operations are the device are supposed
> to be single threaded by the caller. Do you have an allocation in
> some datapath?

Currently, there aren't any allocations on the datapath. But if you
don't mind, we would like to keep the atomics there for future
robustness.

Patch
diff mbox series

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index b611fb204b..192bbaefcf 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -180,7 +180,7 @@  do {                                                                   \
  * Each rte_memzone should have unique name.
  * To satisfy it, count number of allocations and add it to name.
  */
-extern uint32_t ena_alloc_cnt;
+extern rte_atomic32_t ena_alloc_cnt;
 
 #define ENA_MEM_ALLOC_COHERENT(dmadev, size, virt, phys, handle)	\
 	do {								\
@@ -188,7 +188,8 @@  extern uint32_t ena_alloc_cnt;
 		char z_name[RTE_MEMZONE_NAMESIZE];			\
 		ENA_TOUCH(dmadev); ENA_TOUCH(handle);			\
 		snprintf(z_name, sizeof(z_name),			\
-				"ena_alloc_%d", ena_alloc_cnt++);	\
+			 "ena_alloc_%d",				\
+			 rte_atomic32_add_return(&ena_alloc_cnt, 1));	\
 		mz = rte_memzone_reserve(z_name, size, SOCKET_ID_ANY,	\
 				RTE_MEMZONE_IOVA_CONTIG);		\
 		handle = mz;						\
@@ -213,7 +214,8 @@  extern uint32_t ena_alloc_cnt;
 		char z_name[RTE_MEMZONE_NAMESIZE];			\
 		ENA_TOUCH(dmadev); ENA_TOUCH(dev_node);			\
 		snprintf(z_name, sizeof(z_name),			\
-				"ena_alloc_%d", ena_alloc_cnt++);	\
+			 "ena_alloc_%d",				\
+			 rte_atomic32_add_return(&ena_alloc_cnt, 1));	\
 		mz = rte_memzone_reserve(z_name, size, node,		\
 				RTE_MEMZONE_IOVA_CONTIG);		\
 		mem_handle = mz;					\
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index a8f8784a9f..cab38152a7 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -89,7 +89,7 @@  struct ena_stats {
  * Each rte_memzone should have unique name.
  * To satisfy it, count number of allocation and add it to name.
  */
-uint32_t ena_alloc_cnt;
+rte_atomic32_t ena_alloc_cnt;
 
 static const struct ena_stats ena_stats_global_strings[] = {
 	ENA_STAT_GLOBAL_ENTRY(wd_expired),