[dpdk-dev] [PATCH] test: Ring PMD unit test rollback

[dpdk-dev] [PATCH] test: Ring PMD unit test rollback

[Pablo de Lara]

From: Pablo de Lara <pablo.de.lara.guarch@intel.com>

New ring PMD unit test requires extra EAL option (vdev),
in order to pass, which I believe is incorrect, as this
test should be focused on testing the API, without needing
to pass any argument.

Added again all functions that create all the ring devices
necessary for the test, and remove the last two tests
(test_pmd_ring_pair_create and test_pmd_ring_pair_attach),
as they call functions that are deprecated.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 app/test/test_pmd_ring.c |  217 +++++++++++++++-------------------------------
 1 files changed, 72 insertions(+), 145 deletions(-)

diff --git a/app/test/test_pmd_ring.c b/app/test/test_pmd_ring.c
index 0d3d95c..6988277 100644
--- a/app/test/test_pmd_ring.c
+++ b/app/test/test_pmd_ring.c
@@ -42,6 +42,7 @@
 /* two test rings, r1 is used by two ports, r2 just by one */
 static struct rte_ring *r1[2], *r2;
 
+static struct rte_ring *nullring = NULL;
 static struct rte_mempool *mp;
 static uint8_t start_idx; /* will store the port id of the first of our new ports */
 
@@ -49,8 +50,6 @@ static uint8_t start_idx; /* will store the port id of the first of our new port
 #define RX_PORT (uint8_t)(start_idx + 2)
 #define RXTX_PORT (uint8_t)(start_idx + 3)
 #define RXTX_PORT2 (uint8_t)(start_idx + 4)
-#define RXTX_PORT4 (uint8_t)(start_idx + 6)
-#define RXTX_PORT5 (uint8_t)(start_idx + 7)
 #define SOCKET0 0
 
 #define RING_SIZE 256
@@ -58,6 +57,58 @@ static uint8_t start_idx; /* will store the port id of the first of our new port
 #define MBUF_SIZE (2048 + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
 #define NB_MBUF   512
 
+
+static int
+test_ring_ethdev_create(void)
+{
+	int retval;
+	printf("Testing ring pmd create\n");
+
+	retval = rte_eth_from_rings(NULL, 0, NULL, 0, SOCKET0);
+	if (retval < 0) {
+		printf("Failure, failed to create zero-sized RXTX ring pmd\n");
+		return -1;
+	}
+
+	retval = rte_eth_from_rings(NULL, 0, NULL, 0, RTE_MAX_NUMA_NODES);
+	if (retval >= 0) {
+		printf("Failure, can create ring pmd on socket %d\n", RTE_MAX_NUMA_NODES);
+		return -1;
+	}
+
+	retval = rte_eth_from_rings(NULL, 1, &r2, 1, SOCKET0);
+	if (retval >= 0) {
+		printf("Failure, can create pmd with null rx rings\n");
+		return -1;
+	}
+
+	retval = rte_eth_from_rings(r1, 1, NULL, 1, SOCKET0);
+	if (retval >= 0) {
+		printf("Failure, can create pmd with null tx rings\n");
+		return -1;
+	}
+
+	retval = rte_eth_from_rings(&nullring, 1, r1, 2, SOCKET0);
+	if (retval < 0) {
+		printf("Failure, failed to create TX-only ring pmd\n");
+		return -1;
+	}
+
+	retval = rte_eth_from_rings(r1, 1, &nullring, 1, SOCKET0);
+	if (retval < 0) {
+		printf("Failure, failed to create RX-only ring pmd\n");
+		return -1;
+	}
+
+	retval = rte_eth_from_rings(&r2, 1, &r2, 1, SOCKET0);
+	if (retval < 0) {
+		printf("Failure, failed to create RXTX ring pmd\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 static int
 test_ethdev_configure(void)
 {
@@ -252,12 +303,26 @@ test_stats_reset(void)
 static int
 test_pmd_ring_init(void)
 {
+	const char * name1 = "R3";
+	const char * name2 = "R4";
+	const char * params_null = NULL;
+	const char * params = "PARAMS";
 	struct rte_eth_stats stats;
 	struct rte_mbuf buf, *pbuf = &buf;
 	struct rte_eth_conf null_conf;
 
 	printf("Testing ring pmd init\n");
 
+	if (rte_pmd_ring_devinit(name1, params_null) < 0) {
+		printf("Testing ring pmd init fail\n");
+		return -1;
+	}
+
+	if (rte_pmd_ring_devinit(name2, params) < 0) {
+		printf("Testing ring pmd init fail\n");
+		return -1;
+	}
+
 	if (RXTX_PORT2 >= RTE_MAX_ETHPORTS) {
 		printf(" TX/RX port exceed max eth ports\n");
 		return -1;
@@ -304,144 +369,8 @@ test_pmd_ring_init(void)
 
 	rte_eth_dev_stop(RXTX_PORT2);
 
-	return 0;
-}
-
-static int
-test_pmd_ring_pair_create(void)
-{
-	struct rte_eth_stats stats, stats2;
-	struct rte_mbuf buf, *pbuf = &buf;
-	struct rte_eth_conf null_conf;
-
-	if ((RXTX_PORT4 >= RTE_MAX_ETHPORTS) || (RXTX_PORT5 >= RTE_MAX_ETHPORTS)) {
-		printf(" TX/RX port exceed max eth ports\n");
-		return -1;
-	}
-	if ((rte_eth_dev_configure(RXTX_PORT4, 1, 1, &null_conf) < 0)
-		|| (rte_eth_dev_configure(RXTX_PORT5, 1, 1, &null_conf) < 0)) {
-		printf("Configure failed for RXTX port\n");
-		return -1;
-	}
-
-	if ((rte_eth_tx_queue_setup(RXTX_PORT4, 0, RING_SIZE, SOCKET0, NULL) < 0)
-		|| (rte_eth_tx_queue_setup(RXTX_PORT5, 0, RING_SIZE, SOCKET0, NULL) < 0)) {
-		printf("TX queue setup failed\n");
-		return -1;
-	}
-
-	if ((rte_eth_rx_queue_setup(RXTX_PORT4, 0, RING_SIZE, SOCKET0, NULL, mp) < 0)
-		|| (rte_eth_rx_queue_setup(RXTX_PORT5, 0, RING_SIZE, SOCKET0, NULL, mp) < 0)) {
-		printf("RX queue setup failed\n");
-		return -1;
-	}
-
-	if ((rte_eth_dev_start(RXTX_PORT4) < 0)
-		|| (rte_eth_dev_start(RXTX_PORT5) < 0)) {
-		printf("Error starting RXTX port\n");
-		return -1;
-	}
-
-	/* send and receive 1 packet and check for stats update */
-	if (rte_eth_tx_burst(RXTX_PORT4, 0, &pbuf, 1) != 1) {
-		printf("Error sending packet to RXTX port\n");
-		return -1;
-	}
-
-	if (rte_eth_rx_burst(RXTX_PORT5, 0, &pbuf, 1) != 1) {
-		printf("Error receiving packet from RXTX port\n");
-		return -1;
-	}
-
-	rte_eth_stats_get(RXTX_PORT4, &stats);
-	rte_eth_stats_get(RXTX_PORT5, &stats2);
-	if (stats.ipackets != 0 || stats.opackets != 1 ||
-			stats.ibytes != 0 || stats.obytes != 0 ||
-			stats.ierrors != 0 || stats.oerrors != 0) {
-		printf("Error: RXTX port stats are not as expected\n");
-		return -1;
-	}
-
-	if (stats2.ipackets != 1 || stats2.opackets != 0 ||
-			stats2.ibytes != 0 || stats2.obytes != 0 ||
-			stats2.ierrors != 0 || stats2.oerrors != 0) {
-		printf("Error: RXTX port stats are not as expected\n");
-		return -1;
-	}
-
-	rte_eth_dev_stop(RXTX_PORT4);
-	rte_eth_dev_stop(RXTX_PORT5);
-
-	return 0;
-}
-
-static int
-test_pmd_ring_pair_attach(void)
-{
-	struct rte_eth_stats stats, stats2;
-	struct rte_mbuf buf, *pbuf = &buf;
-	struct rte_eth_conf null_conf;
-
-	if ((RXTX_PORT4 >= RTE_MAX_ETHPORTS) || (RXTX_PORT5 >= RTE_MAX_ETHPORTS)) {
-		printf(" TX/RX port exceed max eth ports\n");
-		return -1;
-	}
-	if ((rte_eth_dev_configure(RXTX_PORT4, 1, 1, &null_conf) < 0)
-		|| (rte_eth_dev_configure(RXTX_PORT5, 1, 1, &null_conf) < 0)) {
-		printf("Configure failed for RXTX port\n");
-		return -1;
-	}
-
-	if ((rte_eth_tx_queue_setup(RXTX_PORT4, 0, RING_SIZE, SOCKET0, NULL) < 0)
-		|| (rte_eth_tx_queue_setup(RXTX_PORT5, 0, RING_SIZE, SOCKET0, NULL) < 0)) {
-		printf("TX queue setup failed\n");
-		return -1;
-	}
-
-	if ((rte_eth_rx_queue_setup(RXTX_PORT4, 0, RING_SIZE, SOCKET0, NULL, mp) < 0)
-		|| (rte_eth_rx_queue_setup(RXTX_PORT5, 0, RING_SIZE, SOCKET0, NULL, mp) < 0)) {
-		printf("RX queue setup failed\n");
-		return -1;
-	}
-
-	if ((rte_eth_dev_start(RXTX_PORT4) < 0)
-		|| (rte_eth_dev_start(RXTX_PORT5) < 0)) {
-		printf("Error starting RXTX port\n");
-		return -1;
-	}
-
-	rte_eth_stats_reset(RXTX_PORT4);
-	rte_eth_stats_reset(RXTX_PORT5);
-
-	/* send and receive 1 packet and check for stats update */
-	if (rte_eth_tx_burst(RXTX_PORT4, 0, &pbuf, 1) != 1) {
-		printf("Error sending packet to RXTX port\n");
-		return -1;
-	}
-	if (rte_eth_rx_burst(RXTX_PORT5, 0, &pbuf, 1) != 1) {
-		printf("Error receiving packet from RXTX port\n");
-		return -1;
-	}
-
-	rte_eth_stats_get(RXTX_PORT4, &stats);
-	rte_eth_stats_get(RXTX_PORT5, &stats2);
-	if (stats.ipackets != 0 || stats.opackets != 1 ||
-			stats.ibytes != 0 || stats.obytes != 0 ||
-			stats.ierrors != 0 || stats.oerrors != 0) {
-		printf("Error: RXTX port stats are not as expected\n");
-		return -1;
-	}
-
-	if (stats2.ipackets != 1 || stats2.opackets != 0 ||
-			stats2.ibytes != 0 || stats2.obytes != 0 ||
-			stats2.ierrors != 0 || stats2.oerrors != 0) {
-		printf("Error: RXTX port stats are not as expected\n");
-		return -1;
-	}
-
-	rte_eth_dev_stop(RXTX_PORT4);
-	rte_eth_dev_stop(RXTX_PORT5);
-
+	/* Test init same name pmd ring */
+	rte_pmd_ring_devinit(name1, params_null);
 	return 0;
 }
 
@@ -476,6 +405,9 @@ test_pmd_ring(void)
 		return -1;
 	}
 
+	if (test_ring_ethdev_create() < 0)
+		return -1;
+
 	if (test_ethdev_configure() < 0)
 		return -1;
 
@@ -495,11 +427,6 @@ test_pmd_ring(void)
 	if (test_pmd_ring_init() < 0)
 		return -1;
 
-	if (test_pmd_ring_pair_create() < 0)
-		return -1;
-
-	if (test_pmd_ring_pair_attach() < 0)
-		return -1;
 	return 0;
 }
 
-- 
1.7.0.7

Re: [dpdk-dev] [PATCH] test: Ring PMD unit test rollback

[Neil Horman]

On Fri, Jun 27, 2014 at 03:46:39PM +0100, Pablo de Lara wrote:
> From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> New ring PMD unit test requires extra EAL option (vdev),
> in order to pass, which I believe is incorrect, as this
> test should be focused on testing the API, without needing
> to pass any argument.
> 
> Added again all functions that create all the ring devices
> necessary for the test, and remove the last two tests
> (test_pmd_ring_pair_create and test_pmd_ring_pair_attach),
> as they call functions that are deprecated.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Nak, this also re-adds the need to link the pmd to the application, either
statically or at runtime (i.e. this implicitly obviates the -d option in
rte_eal_parse_args).  If you really want to re-add these api calls, I recommend
that you split them into a separate ring library and ring_pmd library.

Neil

P.S. you did remember to use the -d option in your testing, right?  Without it
the pmd won't load and the vdev option won't find the proper pmd to parse it.


> ---
>  app/test/test_pmd_ring.c |  217 +++++++++++++++-------------------------------
>  1 files changed, 72 insertions(+), 145 deletions(-)
> 
> diff --git a/app/test/test_pmd_ring.c b/app/test/test_pmd_ring.c
> index 0d3d95c..6988277 100644
> --- a/app/test/test_pmd_ring.c
> +++ b/app/test/test_pmd_ring.c
> @@ -42,6 +42,7 @@
>  /* two test rings, r1 is used by two ports, r2 just by one */
>  static struct rte_ring *r1[2], *r2;
>  
> +static struct rte_ring *nullring = NULL;
>  static struct rte_mempool *mp;
>  static uint8_t start_idx; /* will store the port id of the first of our new ports */
>  
> @@ -49,8 +50,6 @@ static uint8_t start_idx; /* will store the port id of the first of our new port
>  #define RX_PORT (uint8_t)(start_idx + 2)
>  #define RXTX_PORT (uint8_t)(start_idx + 3)
>  #define RXTX_PORT2 (uint8_t)(start_idx + 4)
> -#define RXTX_PORT4 (uint8_t)(start_idx + 6)
> -#define RXTX_PORT5 (uint8_t)(start_idx + 7)
>  #define SOCKET0 0
>  
>  #define RING_SIZE 256
> @@ -58,6 +57,58 @@ static uint8_t start_idx; /* will store the port id of the first of our new port
>  #define MBUF_SIZE (2048 + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
>  #define NB_MBUF   512
>  
> +
> +static int
> +test_ring_ethdev_create(void)
> +{
> +	int retval;
> +	printf("Testing ring pmd create\n");
> +
> +	retval = rte_eth_from_rings(NULL, 0, NULL, 0, SOCKET0);
> +	if (retval < 0) {
> +		printf("Failure, failed to create zero-sized RXTX ring pmd\n");
> +		return -1;
> +	}
> +
> +	retval = rte_eth_from_rings(NULL, 0, NULL, 0, RTE_MAX_NUMA_NODES);
> +	if (retval >= 0) {
> +		printf("Failure, can create ring pmd on socket %d\n", RTE_MAX_NUMA_NODES);
> +		return -1;
> +	}
> +
> +	retval = rte_eth_from_rings(NULL, 1, &r2, 1, SOCKET0);
> +	if (retval >= 0) {
> +		printf("Failure, can create pmd with null rx rings\n");
> +		return -1;
> +	}
> +
> +	retval = rte_eth_from_rings(r1, 1, NULL, 1, SOCKET0);
> +	if (retval >= 0) {
> +		printf("Failure, can create pmd with null tx rings\n");
> +		return -1;
> +	}
> +
> +	retval = rte_eth_from_rings(&nullring, 1, r1, 2, SOCKET0);
> +	if (retval < 0) {
> +		printf("Failure, failed to create TX-only ring pmd\n");
> +		return -1;
> +	}
> +
> +	retval = rte_eth_from_rings(r1, 1, &nullring, 1, SOCKET0);
> +	if (retval < 0) {
> +		printf("Failure, failed to create RX-only ring pmd\n");
> +		return -1;
> +	}
> +
> +	retval = rte_eth_from_rings(&r2, 1, &r2, 1, SOCKET0);
> +	if (retval < 0) {
> +		printf("Failure, failed to create RXTX ring pmd\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  test_ethdev_configure(void)
>  {
> @@ -252,12 +303,26 @@ test_stats_reset(void)
>  static int
>  test_pmd_ring_init(void)
>  {
> +	const char * name1 = "R3";
> +	const char * name2 = "R4";
> +	const char * params_null = NULL;
> +	const char * params = "PARAMS";
>  	struct rte_eth_stats stats;
>  	struct rte_mbuf buf, *pbuf = &buf;
>  	struct rte_eth_conf null_conf;
>  
>  	printf("Testing ring pmd init\n");
>  
> +	if (rte_pmd_ring_devinit(name1, params_null) < 0) {
> +		printf("Testing ring pmd init fail\n");
> +		return -1;
> +	}
> +
> +	if (rte_pmd_ring_devinit(name2, params) < 0) {
> +		printf("Testing ring pmd init fail\n");
> +		return -1;
> +	}
> +
>  	if (RXTX_PORT2 >= RTE_MAX_ETHPORTS) {
>  		printf(" TX/RX port exceed max eth ports\n");
>  		return -1;
> @@ -304,144 +369,8 @@ test_pmd_ring_init(void)
>  
>  	rte_eth_dev_stop(RXTX_PORT2);
>  
> -	return 0;
> -}
> -
> -static int
> -test_pmd_ring_pair_create(void)
> -{
> -	struct rte_eth_stats stats, stats2;
> -	struct rte_mbuf buf, *pbuf = &buf;
> -	struct rte_eth_conf null_conf;
> -
> -	if ((RXTX_PORT4 >= RTE_MAX_ETHPORTS) || (RXTX_PORT5 >= RTE_MAX_ETHPORTS)) {
> -		printf(" TX/RX port exceed max eth ports\n");
> -		return -1;
> -	}
> -	if ((rte_eth_dev_configure(RXTX_PORT4, 1, 1, &null_conf) < 0)
> -		|| (rte_eth_dev_configure(RXTX_PORT5, 1, 1, &null_conf) < 0)) {
> -		printf("Configure failed for RXTX port\n");
> -		return -1;
> -	}
> -
> -	if ((rte_eth_tx_queue_setup(RXTX_PORT4, 0, RING_SIZE, SOCKET0, NULL) < 0)
> -		|| (rte_eth_tx_queue_setup(RXTX_PORT5, 0, RING_SIZE, SOCKET0, NULL) < 0)) {
> -		printf("TX queue setup failed\n");
> -		return -1;
> -	}
> -
> -	if ((rte_eth_rx_queue_setup(RXTX_PORT4, 0, RING_SIZE, SOCKET0, NULL, mp) < 0)
> -		|| (rte_eth_rx_queue_setup(RXTX_PORT5, 0, RING_SIZE, SOCKET0, NULL, mp) < 0)) {
> -		printf("RX queue setup failed\n");
> -		return -1;
> -	}
> -
> -	if ((rte_eth_dev_start(RXTX_PORT4) < 0)
> -		|| (rte_eth_dev_start(RXTX_PORT5) < 0)) {
> -		printf("Error starting RXTX port\n");
> -		return -1;
> -	}
> -
> -	/* send and receive 1 packet and check for stats update */
> -	if (rte_eth_tx_burst(RXTX_PORT4, 0, &pbuf, 1) != 1) {
> -		printf("Error sending packet to RXTX port\n");
> -		return -1;
> -	}
> -
> -	if (rte_eth_rx_burst(RXTX_PORT5, 0, &pbuf, 1) != 1) {
> -		printf("Error receiving packet from RXTX port\n");
> -		return -1;
> -	}
> -
> -	rte_eth_stats_get(RXTX_PORT4, &stats);
> -	rte_eth_stats_get(RXTX_PORT5, &stats2);
> -	if (stats.ipackets != 0 || stats.opackets != 1 ||
> -			stats.ibytes != 0 || stats.obytes != 0 ||
> -			stats.ierrors != 0 || stats.oerrors != 0) {
> -		printf("Error: RXTX port stats are not as expected\n");
> -		return -1;
> -	}
> -
> -	if (stats2.ipackets != 1 || stats2.opackets != 0 ||
> -			stats2.ibytes != 0 || stats2.obytes != 0 ||
> -			stats2.ierrors != 0 || stats2.oerrors != 0) {
> -		printf("Error: RXTX port stats are not as expected\n");
> -		return -1;
> -	}
> -
> -	rte_eth_dev_stop(RXTX_PORT4);
> -	rte_eth_dev_stop(RXTX_PORT5);
> -
> -	return 0;
> -}
> -
> -static int
> -test_pmd_ring_pair_attach(void)
> -{
> -	struct rte_eth_stats stats, stats2;
> -	struct rte_mbuf buf, *pbuf = &buf;
> -	struct rte_eth_conf null_conf;
> -
> -	if ((RXTX_PORT4 >= RTE_MAX_ETHPORTS) || (RXTX_PORT5 >= RTE_MAX_ETHPORTS)) {
> -		printf(" TX/RX port exceed max eth ports\n");
> -		return -1;
> -	}
> -	if ((rte_eth_dev_configure(RXTX_PORT4, 1, 1, &null_conf) < 0)
> -		|| (rte_eth_dev_configure(RXTX_PORT5, 1, 1, &null_conf) < 0)) {
> -		printf("Configure failed for RXTX port\n");
> -		return -1;
> -	}
> -
> -	if ((rte_eth_tx_queue_setup(RXTX_PORT4, 0, RING_SIZE, SOCKET0, NULL) < 0)
> -		|| (rte_eth_tx_queue_setup(RXTX_PORT5, 0, RING_SIZE, SOCKET0, NULL) < 0)) {
> -		printf("TX queue setup failed\n");
> -		return -1;
> -	}
> -
> -	if ((rte_eth_rx_queue_setup(RXTX_PORT4, 0, RING_SIZE, SOCKET0, NULL, mp) < 0)
> -		|| (rte_eth_rx_queue_setup(RXTX_PORT5, 0, RING_SIZE, SOCKET0, NULL, mp) < 0)) {
> -		printf("RX queue setup failed\n");
> -		return -1;
> -	}
> -
> -	if ((rte_eth_dev_start(RXTX_PORT4) < 0)
> -		|| (rte_eth_dev_start(RXTX_PORT5) < 0)) {
> -		printf("Error starting RXTX port\n");
> -		return -1;
> -	}
> -
> -	rte_eth_stats_reset(RXTX_PORT4);
> -	rte_eth_stats_reset(RXTX_PORT5);
> -
> -	/* send and receive 1 packet and check for stats update */
> -	if (rte_eth_tx_burst(RXTX_PORT4, 0, &pbuf, 1) != 1) {
> -		printf("Error sending packet to RXTX port\n");
> -		return -1;
> -	}
> -	if (rte_eth_rx_burst(RXTX_PORT5, 0, &pbuf, 1) != 1) {
> -		printf("Error receiving packet from RXTX port\n");
> -		return -1;
> -	}
> -
> -	rte_eth_stats_get(RXTX_PORT4, &stats);
> -	rte_eth_stats_get(RXTX_PORT5, &stats2);
> -	if (stats.ipackets != 0 || stats.opackets != 1 ||
> -			stats.ibytes != 0 || stats.obytes != 0 ||
> -			stats.ierrors != 0 || stats.oerrors != 0) {
> -		printf("Error: RXTX port stats are not as expected\n");
> -		return -1;
> -	}
> -
> -	if (stats2.ipackets != 1 || stats2.opackets != 0 ||
> -			stats2.ibytes != 0 || stats2.obytes != 0 ||
> -			stats2.ierrors != 0 || stats2.oerrors != 0) {
> -		printf("Error: RXTX port stats are not as expected\n");
> -		return -1;
> -	}
> -
> -	rte_eth_dev_stop(RXTX_PORT4);
> -	rte_eth_dev_stop(RXTX_PORT5);
> -
> +	/* Test init same name pmd ring */
> +	rte_pmd_ring_devinit(name1, params_null);
>  	return 0;
>  }
>  
> @@ -476,6 +405,9 @@ test_pmd_ring(void)
>  		return -1;
>  	}
>  
> +	if (test_ring_ethdev_create() < 0)
> +		return -1;
> +
>  	if (test_ethdev_configure() < 0)
>  		return -1;
>  
> @@ -495,11 +427,6 @@ test_pmd_ring(void)
>  	if (test_pmd_ring_init() < 0)
>  		return -1;
>  
> -	if (test_pmd_ring_pair_create() < 0)
> -		return -1;
> -
> -	if (test_pmd_ring_pair_attach() < 0)
> -		return -1;
>  	return 0;
>  }
>  
> -- 
> 1.7.0.7
> 
> 

Re: [dpdk-dev] [PATCH] test: Ring PMD unit test rollback

[De Lara Guarch, Pablo]

Hi Neil,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Friday, June 27, 2014 4:03 PM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] test: Ring PMD unit test rollback
> 
> On Fri, Jun 27, 2014 at 03:46:39PM +0100, Pablo de Lara wrote:
> > From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> >
> > New ring PMD unit test requires extra EAL option (vdev),
> > in order to pass, which I believe is incorrect, as this
> > test should be focused on testing the API, without needing
> > to pass any argument.
> >
> > Added again all functions that create all the ring devices
> > necessary for the test, and remove the last two tests
> > (test_pmd_ring_pair_create and test_pmd_ring_pair_attach),
> > as they call functions that are deprecated.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> Nak, this also re-adds the need to link the pmd to the application, either
> statically or at runtime (i.e. this implicitly obviates the -d option in
> rte_eal_parse_args).  If you really want to re-add these api calls, I
> recommend
> that you split them into a separate ring library and ring_pmd library.
> 
> Neil
> 
> P.S. you did remember to use the -d option in your testing, right?  Without it
> the pmd won't load and the vdev option won't find the proper pmd to parse
> it.
> 

Why does it need the -d? I could use the vdev option without loading any dynamic pmds, and
it created the devices, but it is just difficult to run a simple unit test, by having to know which
ethdevs you have to create, instead of letting the test itself to create the ones that it needs,
if you know what I mean. So, if this solution is not good enough, another idea could be to let 
the unit test make calls to the application, creating the virtual devices needed. What I don't 
like is the idea of having to pass the vdev to the generic test application, as it is intended to 
run other tests and not just the ring pmd (we could do something like in the eal_flags test,
where several test app instances are run, with different parameters, although I am not sure,
if this is possible for this kind of test).

Thanks,
Pablo

Re: [dpdk-dev] [PATCH] test: Ring PMD unit test rollback

[Neil Horman]

On Fri, Jun 27, 2014 at 03:47:07PM +0000, De Lara Guarch, Pablo wrote:
> Hi Neil,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > Sent: Friday, June 27, 2014 4:03 PM
> > To: De Lara Guarch, Pablo
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] test: Ring PMD unit test rollback
> > 
> > On Fri, Jun 27, 2014 at 03:46:39PM +0100, Pablo de Lara wrote:
> > > From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > >
> > > New ring PMD unit test requires extra EAL option (vdev),
> > > in order to pass, which I believe is incorrect, as this
> > > test should be focused on testing the API, without needing
> > > to pass any argument.
> > >
> > > Added again all functions that create all the ring devices
> > > necessary for the test, and remove the last two tests
> > > (test_pmd_ring_pair_create and test_pmd_ring_pair_attach),
> > > as they call functions that are deprecated.
> > >
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > 
> > Nak, this also re-adds the need to link the pmd to the application, either
> > statically or at runtime (i.e. this implicitly obviates the -d option in
> > rte_eal_parse_args).  If you really want to re-add these api calls, I
> > recommend
> > that you split them into a separate ring library and ring_pmd library.
> > 
> > Neil
> > 
> > P.S. you did remember to use the -d option in your testing, right?  Without it
> > the pmd won't load and the vdev option won't find the proper pmd to parse
> > it.
> > 
> 
> Why does it need the -d? I could use the vdev option without loading any dynamic pmds, and
You don't need it only if you build the application statically.  If you build it
as shared objects you need to specify which pmds to load at run time using the
-d option.  If you're building statically then no, you don't need it.

> it created the devices, but it is just difficult to run a simple unit test, by having to know which
> ethdevs you have to create, instead of letting the test itself to create the ones that it needs,
> if you know what I mean. So, if this solution is not good enough, another idea could be to let 
> the unit test make calls to the application, creating the virtual devices needed. What I don't 
> like is the idea of having to pass the vdev to the generic test application, as it is intended to 
> run other tests and not just the ring pmd (we could do something like in the eal_flags test,
> where several test app instances are run, with different parameters, although I am not sure,
> if this is possible for this kind of test).
> 


Alternatively to restoring the API, you can hardcode the command line strings
into the test pmd, so you don't have to specify them at run time, and just pass
them directly to rte_eal_init.

The problem with just blindly adding back the API is that it requires any
application that might want to use the ring pmd to load it at run time
unilaterally.

Regards
Neil

> Thanks,
> Pablo
>