[dpdk-dev] [PATCH 0/5] eal: check socket memory

[dpdk-dev] [PATCH 0/5] eal: check socket memory

[Pablo de Lara]

Several drivers, libraries and apps check if a socket
has reserved memory, by implementing their own function, 
which returns the total number of sockets that have memory.

First of all, this function is not completely correct,
as it really returns the highest socket id that has memory.
So, if all the sockets up to that one has memory, then it
really returns the total number of sockets with memory,
but it there is at least one in the middle without memory,
then total number of sockets should be less than the value returned.

Besides, this function is not really useful, as the main goal is
to check if memory on a specific socket is available,
rather than checking if a socket id is beyond the total
number of sockets.

Therefore, it looks more useful to have a function in EAL
that returns if a socket has memory reserved, which
can be used in these files.

Pablo de Lara (5):
  eal: check if socket has memory reserved
  cryptodev: check if socket id has memory
  crypto/scheduler: check if socket id has memory
  net/bonding: check if socket id has memory
  test/bonding: check if socket id has memory

 drivers/crypto/scheduler/scheduler_pmd.c        | 23 ++++-------------------
 drivers/net/bonding/rte_eth_bond_api.c          | 16 ----------------
 drivers/net/bonding/rte_eth_bond_args.c         |  2 +-
 drivers/net/bonding/rte_eth_bond_pmd.c          |  7 ++++---
 drivers/net/bonding/rte_eth_bond_private.h      |  3 ---
 lib/librte_cryptodev/rte_cryptodev.c            | 23 ++++-------------------
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  7 +++++++
 lib/librte_eal/common/eal_common_memory.c       | 17 +++++++++++++++++
 lib/librte_eal/common/include/rte_memory.h      |  9 +++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  7 +++++++
 test/test/virtual_pmd.c                         | 16 +---------------
 11 files changed, 54 insertions(+), 76 deletions(-)

-- 
2.9.4

[dpdk-dev] [PATCH 1/5] eal: check if socket has memory reserved

[Pablo de Lara]

Several drivers and apps check if a socket has reserved memory,
by implementing their own function, which returns the
total number of sockets that have memory.
This function is not really useful, as the main goal is
to check if memory on a specific socket is available,
rather than checking if a socket id is beyond the total
number of sockets (there could be a socket in the middle
with no memory).

Therefore, it looks more useful to have a function in EAL
that can be used in these files.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  7 +++++++
 lib/librte_eal/common/eal_common_memory.c       | 17 +++++++++++++++++
 lib/librte_eal/common/include/rte_memory.h      |  9 +++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  7 +++++++
 4 files changed, 40 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2e48a73..85813e3 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -193,3 +193,10 @@ DPDK_17.05 {
 	vfio_get_group_no;
 
 } DPDK_17.02;
+
+DPDK_17.08 {
+	global:
+
+	rte_eal_has_memory_socket;
+
+} DPDK_17.05;
diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
index 6155752..9811d06 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -120,6 +120,23 @@ unsigned rte_memory_get_nrank(void)
 	return rte_eal_get_configuration()->mem_config->nrank;
 }
 
+/**
+ * Return if socket has memory reserved.
+ */
+unsigned int
+rte_eal_has_memory_socket(uint8_t socket_id)
+{
+	unsigned int i;
+	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
+
+	for (i = 0; ((i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL)); i++) {
+		if (ms[i].socket_id == (int) socket_id)
+			return 1;
+	}
+
+	return 0;
+}
+
 static int
 rte_eal_memdevice_init(void)
 {
diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
index 4aa5d1f..e5fa902 100644
--- a/lib/librte_eal/common/include/rte_memory.h
+++ b/lib/librte_eal/common/include/rte_memory.h
@@ -195,6 +195,15 @@ unsigned rte_memory_get_nchannel(void);
  */
 unsigned rte_memory_get_nrank(void);
 
+/**
+ * Return if socket has memory reserved.
+ *
+ * @return
+ *   - 0 if socket has no memory reserved.
+ *   - 1 if socket has memory reserved..
+ */
+unsigned int rte_eal_has_memory_socket(uint8_t socket_id);
+
 #ifdef RTE_LIBRTE_XEN_DOM0
 
 /**< Internal use only - should DOM0 memory mapping be used */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 670bab3..0b3ba7f 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -198,3 +198,10 @@ DPDK_17.05 {
 	vfio_get_group_no;
 
 } DPDK_17.02;
+
+DPDK_17.08 {
+	global:
+
+	rte_eal_has_memory_socket;
+
+} DPDK_17.05;
-- 
2.9.4

[dpdk-dev] [PATCH 2/5] cryptodev: check if socket id has memory

[Pablo de Lara]

Use new function to check if socket id has reserved memory,
instead of implementing a local function that checks
total number of sockets, to verify if selected socket id
is beyond the range of sockets.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 lib/librte_cryptodev/rte_cryptodev.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index b65cd9c..20dcaf2 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -233,22 +233,6 @@ rte_crypto_auth_operation_strings[] = {
 		[RTE_CRYPTO_AUTH_OP_GENERATE]	= "generate"
 };
 
-static uint8_t
-number_of_sockets(void)
-{
-	int sockets = 0;
-	int i;
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-
-	for (i = 0; ((i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL)); i++) {
-		if (sockets < ms[i].socket_id)
-			sockets = ms[i].socket_id;
-	}
-
-	/* Number of sockets = maximum socket_id + 1 */
-	return ++sockets;
-}
-
 /** Parse integer from integer argument */
 static int
 parse_integer_arg(const char *key __rte_unused,
@@ -326,9 +310,10 @@ rte_cryptodev_parse_vdev_init_params(struct rte_crypto_vdev_init_params *params,
 		if (ret < 0)
 			goto free_kvlist;
 
-		if (params->socket_id >= number_of_sockets()) {
-			CDEV_LOG_ERR("Invalid socket id specified to create "
-				"the virtual crypto device on");
+		if (!rte_eal_has_memory_socket(params->socket_id)) {
+			CDEV_LOG_ERR("Socket ID specified to create the "
+				"the virtual crypto device on does not have "
+				"available memory");
 			goto free_kvlist;
 		}
 	}
-- 
2.9.4

[dpdk-dev] [PATCH 3/5] crypto/scheduler: check if socket id has memory

[Pablo de Lara]

Use new function to check if socket id has reserved memory,
instead of implementing a local function that checks
total number of sockets, to verify if selected socket id
is beyond the range of sockets.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/scheduler/scheduler_pmd.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/crypto/scheduler/scheduler_pmd.c b/drivers/crypto/scheduler/scheduler_pmd.c
index 0b63c20..e2b0aa7 100644
--- a/drivers/crypto/scheduler/scheduler_pmd.c
+++ b/drivers/crypto/scheduler/scheduler_pmd.c
@@ -219,22 +219,6 @@ cryptodev_scheduler_remove(struct rte_vdev_device *vdev)
 	return 0;
 }
 
-static uint8_t
-number_of_sockets(void)
-{
-	int sockets = 0;
-	int i;
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-
-	for (i = 0; ((i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL)); i++) {
-		if (sockets < ms[i].socket_id)
-			sockets = ms[i].socket_id;
-	}
-
-	/* Number of sockets = maximum socket_id + 1 */
-	return ++sockets;
-}
-
 /** Parse integer from integer argument */
 static int
 parse_integer_arg(const char *key __rte_unused,
@@ -391,9 +375,10 @@ scheduler_parse_init_params(struct scheduler_init_params *params,
 		if (ret < 0)
 			goto free_kvlist;
 
-		if (params->def_p.socket_id >= number_of_sockets()) {
-			CDEV_LOG_ERR("Invalid socket id specified to create "
-				"the virtual crypto device on");
+		if (!rte_eal_has_memory_socket(params->def_p.socket_id)) {
+			CDEV_LOG_ERR("Socket ID specified to create the "
+				"the virtual crypto device on does not have "
+				"available memory");
 			goto free_kvlist;
 		}
 	}
-- 
2.9.4

[dpdk-dev] [PATCH 4/5] net/bonding: check if socket id has memory

[Pablo de Lara]

Use new function to check if socket id has reserved memory,
instead of implementing a local function that checks
total number of sockets, to verify if selected socket id
is beyond the range of sockets.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/bonding/rte_eth_bond_api.c     | 16 ----------------
 drivers/net/bonding/rte_eth_bond_args.c    |  2 +-
 drivers/net/bonding/rte_eth_bond_pmd.c     |  7 ++++---
 drivers/net/bonding/rte_eth_bond_private.h |  3 ---
 4 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 36ec65d..6c7dbae 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -143,22 +143,6 @@ deactivate_slave(struct rte_eth_dev *eth_dev, uint8_t port_id)
 	}
 }
 
-uint8_t
-number_of_sockets(void)
-{
-	int sockets = 0;
-	int i;
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-
-	for (i = 0; ((i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL)); i++) {
-		if (sockets < ms[i].socket_id)
-			sockets = ms[i].socket_id;
-	}
-
-	/* Number of sockets = maximum socket_id + 1 */
-	return ++sockets;
-}
-
 int
 rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 {
diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
index e3bdad9..66c3795 100644
--- a/drivers/net/bonding/rte_eth_bond_args.c
+++ b/drivers/net/bonding/rte_eth_bond_args.c
@@ -205,7 +205,7 @@ bond_ethdev_parse_socket_id_kvarg(const char *key __rte_unused,
 		return -1;
 
 	/* validate mode value */
-	if (socket_id >= 0 && socket_id < number_of_sockets()) {
+	if (socket_id >= 0 && rte_eal_has_memory_socket(socket_id)) {
 		*(uint8_t *)extra_args = (uint8_t)socket_id;
 		return 0;
 	}
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 82959ab..500e59b 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2272,9 +2272,10 @@ bond_alloc(struct rte_vdev_device *dev, uint8_t mode)
 	 * and internal (private) data
 	 */
 
-	if (socket_id >= number_of_sockets()) {
-		RTE_BOND_LOG(ERR,
-				"Invalid socket id specified to create bonded device on.");
+	if (!rte_eal_has_memory_socket(socket_id)) {
+		RTE_BOND_LOG(ERR, "Socket ID specified to create the "
+				"the bonded device on does not have "
+				"available memory");
 		goto err;
 	}
 
diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h
index c8db090..e37d463 100644
--- a/drivers/net/bonding/rte_eth_bond_private.h
+++ b/drivers/net/bonding/rte_eth_bond_private.h
@@ -232,9 +232,6 @@ mac_address_get(struct rte_eth_dev *eth_dev, struct ether_addr *dst_mac_addr);
 int
 mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev);
 
-uint8_t
-number_of_sockets(void);
-
 int
 bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, int mode);
 
-- 
2.9.4

[dpdk-dev] [PATCH 5/5] test/bonding: check if socket id has memory

[Pablo de Lara]

Use new function to check if socket id has reserved memory,
instead of implementing a local function that checks
total number of sockets, to verify if selected socket id
is beyond the range of sockets.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 test/test/virtual_pmd.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/test/test/virtual_pmd.c b/test/test/virtual_pmd.c
index e9dd8ac..a874cd4 100644
--- a/test/test/virtual_pmd.c
+++ b/test/test/virtual_pmd.c
@@ -511,20 +511,6 @@ virtual_ethdev_get_mbufs_from_tx_queue(uint8_t port_id,
 		burst_length, NULL);
 }
 
-static uint8_t
-get_number_of_sockets(void)
-{
-	int sockets = 0;
-	int i;
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-
-	for (i = 0; i < RTE_MAX_MEMSEG && ms[i].addr != NULL; i++) {
-		if (sockets < ms[i].socket_id)
-			sockets = ms[i].socket_id;
-	}
-	/* Number of sockets = maximum socket_id + 1 */
-	return ++sockets;
-}
 
 int
 virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
@@ -542,7 +528,7 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
 	 * and internal (dev_private) data
 	 */
 
-	if (socket_id >= get_number_of_sockets())
+	if (!rte_eal_has_memory_socket(socket_id))
 		goto err;
 
 	pci_dev = rte_zmalloc_socket(name, sizeof(*pci_dev), 0, socket_id);
-- 
2.9.4

[dpdk-dev] [PATCH v2 0/4] Socket ID check removal

[Pablo de Lara]

Several libraries, drivers and tests check if a socket
is within the range of available sockets, by
implementig their own function, which returns the total 
number of sockets that have memory.

First of all, this function is not completely correct,
as it really returns the highest socket id that has memory.
So, if all the sockets up to that one has memory, then it
really returns the total number of sockets with memory,
but if there is at least one in the middle without memory,
then total number of sockets should be less than the value returned.

Besides, this function is not really useful, as the main goal is
to check if memory on a specific socket is available, rather than
checking if a socket id is beyond the total number of sockets.

Therefore, it is better to remove the check and let the memory
allocation function handle an incorrect socket.

Changes in v2:

- Removed new EAL function and just removed the socket id check,
  instead of creating a new function.

Pablo de Lara (4):
  cryptodev: remove socket id check
  crypto/scheduler: remove socket id check
  net/bonding: remove socket id check
  test/bonding: remove socket id check

 drivers/crypto/scheduler/scheduler_pmd.c   | 22 ----------------------
 drivers/net/bonding/rte_eth_bond_api.c     | 16 ----------------
 drivers/net/bonding/rte_eth_bond_args.c    |  4 ++--
 drivers/net/bonding/rte_eth_bond_pmd.c     |  6 ------
 drivers/net/bonding/rte_eth_bond_private.h |  3 ---
 lib/librte_cryptodev/rte_cryptodev.c       | 22 ----------------------
 test/test/virtual_pmd.c                    | 17 -----------------
 7 files changed, 2 insertions(+), 88 deletions(-)

-- 
2.9.4

[dpdk-dev] [PATCH v2 1/4] cryptodev: remove socket id check

[Pablo de Lara]

Socket id parsed from the user was checked
if it was in the range of available sockets.
This check is unnecessary, as the socket specified
might not have memory anyway, so it will fail
at memory allocation.

Therefore, the best solution is to remove this check.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 lib/librte_cryptodev/rte_cryptodev.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index b65cd9c..81fd2bf 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -233,22 +233,6 @@ rte_crypto_auth_operation_strings[] = {
 		[RTE_CRYPTO_AUTH_OP_GENERATE]	= "generate"
 };
 
-static uint8_t
-number_of_sockets(void)
-{
-	int sockets = 0;
-	int i;
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-
-	for (i = 0; ((i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL)); i++) {
-		if (sockets < ms[i].socket_id)
-			sockets = ms[i].socket_id;
-	}
-
-	/* Number of sockets = maximum socket_id + 1 */
-	return ++sockets;
-}
-
 /** Parse integer from integer argument */
 static int
 parse_integer_arg(const char *key __rte_unused,
@@ -325,12 +309,6 @@ rte_cryptodev_parse_vdev_init_params(struct rte_crypto_vdev_init_params *params,
 					params);
 		if (ret < 0)
 			goto free_kvlist;
-
-		if (params->socket_id >= number_of_sockets()) {
-			CDEV_LOG_ERR("Invalid socket id specified to create "
-				"the virtual crypto device on");
-			goto free_kvlist;
-		}
 	}
 
 free_kvlist:
-- 
2.9.4

Re: [dpdk-dev] [PATCH v2 1/4] cryptodev: remove socket id check

[Declan Doherty]

On 21/06/2017 6:07 AM, Pablo de Lara wrote:
> Socket id parsed from the user was checked
> if it was in the range of available sockets.
> This check is unnecessary, as the socket specified
> might not have memory anyway, so it will fail
> at memory allocation.
>
> Therefore, the best solution is to remove this check.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  lib/librte_cryptodev/rte_cryptodev.c | 22 ----------------------
...
>

Acked-by: Declan Doherty <declan.doherty@intel.com>

[dpdk-dev] [PATCH v2 2/4] crypto/scheduler: remove socket id check

[Pablo de Lara]

Socket id parsed from the user was checked
if it was in the range of available sockets.
This check is unnecessary, as the socket specified
might not have memory anyway, so it will fail
at memory allocation.

Therefore, the best solution is to remove this check.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/scheduler/scheduler_pmd.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/crypto/scheduler/scheduler_pmd.c b/drivers/crypto/scheduler/scheduler_pmd.c
index 0b63c20..29b16c9 100644
--- a/drivers/crypto/scheduler/scheduler_pmd.c
+++ b/drivers/crypto/scheduler/scheduler_pmd.c
@@ -219,22 +219,6 @@ cryptodev_scheduler_remove(struct rte_vdev_device *vdev)
 	return 0;
 }
 
-static uint8_t
-number_of_sockets(void)
-{
-	int sockets = 0;
-	int i;
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-
-	for (i = 0; ((i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL)); i++) {
-		if (sockets < ms[i].socket_id)
-			sockets = ms[i].socket_id;
-	}
-
-	/* Number of sockets = maximum socket_id + 1 */
-	return ++sockets;
-}
-
 /** Parse integer from integer argument */
 static int
 parse_integer_arg(const char *key __rte_unused,
@@ -390,12 +374,6 @@ scheduler_parse_init_params(struct scheduler_init_params *params,
 				&parse_ordering_arg, params);
 		if (ret < 0)
 			goto free_kvlist;
-
-		if (params->def_p.socket_id >= number_of_sockets()) {
-			CDEV_LOG_ERR("Invalid socket id specified to create "
-				"the virtual crypto device on");
-			goto free_kvlist;
-		}
 	}
 
 free_kvlist:
-- 
2.9.4

Re: [dpdk-dev] [PATCH v2 2/4] crypto/scheduler: remove socket id check

[Declan Doherty]

On 21/06/2017 6:07 AM, Pablo de Lara wrote:
> Socket id parsed from the user was checked
> if it was in the range of available sockets.
> This check is unnecessary, as the socket specified
> might not have memory anyway, so it will fail
> at memory allocation.
>
> Therefore, the best solution is to remove this check.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
...
>
Acked-by: Declan Doherty <declan.doherty@intel.com>

[dpdk-dev] [PATCH v2 3/4] net/bonding: remove socket id check

[Pablo de Lara]

Socket id parsed from the user was checked
if it was in the range of available sockets.
This check is unnecessary, as the socket specified
might not have memory anyway, so it will fail
at memory allocation.

Therefore, the best solution is to remove this check.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/bonding/rte_eth_bond_api.c     | 16 ----------------
 drivers/net/bonding/rte_eth_bond_args.c    |  4 ++--
 drivers/net/bonding/rte_eth_bond_pmd.c     |  6 ------
 drivers/net/bonding/rte_eth_bond_private.h |  3 ---
 4 files changed, 2 insertions(+), 27 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 36ec65d..6c7dbae 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -143,22 +143,6 @@ deactivate_slave(struct rte_eth_dev *eth_dev, uint8_t port_id)
 	}
 }
 
-uint8_t
-number_of_sockets(void)
-{
-	int sockets = 0;
-	int i;
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-
-	for (i = 0; ((i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL)); i++) {
-		if (sockets < ms[i].socket_id)
-			sockets = ms[i].socket_id;
-	}
-
-	/* Number of sockets = maximum socket_id + 1 */
-	return ++sockets;
-}
-
 int
 rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 {
diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
index e3bdad9..baaab35 100644
--- a/drivers/net/bonding/rte_eth_bond_args.c
+++ b/drivers/net/bonding/rte_eth_bond_args.c
@@ -204,8 +204,8 @@ bond_ethdev_parse_socket_id_kvarg(const char *key __rte_unused,
 	if (*endptr != 0 || errno != 0)
 		return -1;
 
-	/* validate mode value */
-	if (socket_id >= 0 && socket_id < number_of_sockets()) {
+	/* validate socket id value */
+	if (socket_id >= 0) {
 		*(uint8_t *)extra_args = (uint8_t)socket_id;
 		return 0;
 	}
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 82959ab..c5efd06 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2272,12 +2272,6 @@ bond_alloc(struct rte_vdev_device *dev, uint8_t mode)
 	 * and internal (private) data
 	 */
 
-	if (socket_id >= number_of_sockets()) {
-		RTE_BOND_LOG(ERR,
-				"Invalid socket id specified to create bonded device on.");
-		goto err;
-	}
-
 	/* reserve an ethdev entry */
 	eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internals));
 	if (eth_dev == NULL) {
diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h
index c8db090..e37d463 100644
--- a/drivers/net/bonding/rte_eth_bond_private.h
+++ b/drivers/net/bonding/rte_eth_bond_private.h
@@ -232,9 +232,6 @@ mac_address_get(struct rte_eth_dev *eth_dev, struct ether_addr *dst_mac_addr);
 int
 mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev);
 
-uint8_t
-number_of_sockets(void);
-
 int
 bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, int mode);
 
-- 
2.9.4

Re: [dpdk-dev] [PATCH v2 3/4] net/bonding: remove socket id check

[Declan Doherty]

On 21/06/2017 6:07 AM, Pablo de Lara wrote:
> Socket id parsed from the user was checked
> if it was in the range of available sockets.
> This check is unnecessary, as the socket specified
> might not have memory anyway, so it will fail
> at memory allocation.
>
> Therefore, the best solution is to remove this check.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
...
>

Acked-by: Declan Doherty <declan.doherty@intel.com>

[dpdk-dev] [PATCH v2 4/4] test/bonding: remove socket id check

[Pablo de Lara]

When creating a virtual pmd to test link bonding,
the socket id was checked, if it was in the range
of available sockets.
This check is unnecessary, as the socket specified
might not have memory anyway, so it will fail
at memory allocation.

Therefore, the best solution is to remove this check.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 test/test/virtual_pmd.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/test/test/virtual_pmd.c b/test/test/virtual_pmd.c
index e9dd8ac..f1ea3e1 100644
--- a/test/test/virtual_pmd.c
+++ b/test/test/virtual_pmd.c
@@ -511,20 +511,6 @@ virtual_ethdev_get_mbufs_from_tx_queue(uint8_t port_id,
 		burst_length, NULL);
 }
 
-static uint8_t
-get_number_of_sockets(void)
-{
-	int sockets = 0;
-	int i;
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-
-	for (i = 0; i < RTE_MAX_MEMSEG && ms[i].addr != NULL; i++) {
-		if (sockets < ms[i].socket_id)
-			sockets = ms[i].socket_id;
-	}
-	/* Number of sockets = maximum socket_id + 1 */
-	return ++sockets;
-}
 
 int
 virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
@@ -542,9 +528,6 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
 	 * and internal (dev_private) data
 	 */
 
-	if (socket_id >= get_number_of_sockets())
-		goto err;
-
 	pci_dev = rte_zmalloc_socket(name, sizeof(*pci_dev), 0, socket_id);
 	if (pci_dev == NULL)
 		goto err;
-- 
2.9.4

Re: [dpdk-dev] [PATCH v2 4/4] test/bonding: remove socket id check

[Declan Doherty]

On 21/06/2017 6:07 AM, Pablo de Lara wrote:
> When creating a virtual pmd to test link bonding,
> the socket id was checked, if it was in the range
> of available sockets.
> This check is unnecessary, as the socket specified
> might not have memory anyway, so it will fail
> at memory allocation.
>
> Therefore, the best solution is to remove this check.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
...
>

Acked-by: Declan Doherty <declan.doherty@intel.com>

Re: [dpdk-dev] [PATCH v2 0/4] Socket ID check removal

[Thomas Monjalon]

> Pablo de Lara (4):
>   cryptodev: remove socket id check
>   crypto/scheduler: remove socket id check
>   net/bonding: remove socket id check
>   test/bonding: remove socket id check

Applied, thanks

Re: [dpdk-dev] [PATCH 0/5] eal: check socket memory

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Wednesday, June 21, 2017 12:26 AM
> To: thomas@monjalon.net; Doherty, Declan <declan.doherty@intel.com>
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH 0/5] eal: check socket memory
> 
> Several drivers, libraries and apps check if a socket has reserved memory, by
> implementing their own function, which returns the total number of sockets that
> have memory.
> 
> First of all, this function is not completely correct, as it really returns the highest
> socket id that has memory.
> So, if all the sockets up to that one has memory, then it really returns the total
> number of sockets with memory, but it there is at least one in the middle
> without memory, then total number of sockets should be less than the value
> returned.
> 
> Besides, this function is not really useful, as the main goal is to check if memory
> on a specific socket is available, rather than checking if a socket id is beyond the
> total number of sockets.
> 
> Therefore, it looks more useful to have a function in EAL that returns if a socket
> has memory reserved, which can be used in these files.

Looking at this again, probably having this check is not necessary,
as this will be checked when attempting to allocate the memory.
Will send a v2, to remove the check completely.