[PATCH 0/3] ring: provide rte_ring_as_ethdev API

[PATCH 0/3] ring: provide rte_ring_as_ethdev API

[Bruce Richardson]

This patch set aims to provide a shorter simpler alternative the public API functions for using rings as ethdevs provided by the librte_pmd_ring library. This alternative just provides simple RX and TX burst functions and a conversion API, without any of the complexities present in the pmd_ring version. This replacement should allow the public APIs in the pmd_ring library to be deprecated in the future.

Bruce Richardson (3):
  ethdev: Remove ethdev.h dependency on mbuf +  mempool
  ring: add support for converting a ring to ethdev
  ring: autotest for using ring as ethdev

 app/test-pmd/cmdline.c                  |  1 +
 app/test/test_pmd_ring.c                |  1 +
 app/test/test_ring.c                    | 25 ++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h           |  4 +++-
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c |  1 +
 lib/librte_ring/Makefile                |  1 +
 lib/librte_ring/rte_ring.c              | 42 +++++++++++++++++++++++++++++++++
 lib/librte_ring/rte_ring.h              | 11 +++++++++
 8 files changed, 85 insertions(+), 1 deletion(-)

-- 
1.9.0

[PATCH 1/3] ethdev: Remove ethdev.h dependency on mbuf + mempool

[Bruce Richardson]

This allows us to get the ethdev structure definition without a full
set of additional headers from other libs being included.
To ensure compilation, add new includes to C files that needed mbuf
header without explicitly including it.

Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 app/test-pmd/cmdline.c                  | 1 +
 app/test/test_pmd_ring.c                | 1 +
 lib/librte_ether/rte_ethdev.h           | 4 +++-
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c | 1 +
 4 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b3824f9..2b6ffe4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -74,6 +74,7 @@
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #include <rte_devargs.h>
+#include <rte_mbuf.h>
 
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
diff --git a/app/test/test_pmd_ring.c b/app/test/test_pmd_ring.c
index 4d9c2ba..3bb98ee 100644
--- a/app/test/test_pmd_ring.c
+++ b/app/test/test_pmd_ring.c
@@ -36,6 +36,7 @@
 
 #include <stdio.h>
 
+#include <rte_mbuf.h>
 #include <rte_eth_ring.h>
 #include <rte_ethdev.h>
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index dea7471..44f064e 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -175,9 +175,11 @@ extern "C" {
 #include <rte_log.h>
 #include <rte_interrupts.h>
 #include <rte_pci.h>
-#include <rte_mbuf.h>
 #include "rte_ether.h"
 
+struct rte_mbuf;
+struct rte_mempool;
+
 /**
  * A structure used to retrieve statistics for an Ethernet port.
  */
diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
index 8259cfe..6f244b6 100644
--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
@@ -55,6 +55,7 @@
 #include <rte_tailq.h>
 #include <rte_eal.h>
 #include <rte_alarm.h>
+#include <rte_mbuf.h>
 #include <rte_ether.h>
 #include <rte_ethdev.h>
 #include <rte_atomic.h>
-- 
1.9.0

[PATCH 2/3] ring: add support for converting a ring to ethdev

[Bruce Richardson]

Add in a pair of functions which meet the criteria for rx_burst and
tx_burst, which then allow a ring to be used as though it were an
ethdev, so that code can be written agnostically. Provide a convertion
function that takes a single ring and returns an index of the ethdev
corresponding to it.

Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_ring/Makefile   |  1 +
 lib/librte_ring/rte_ring.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 lib/librte_ring/rte_ring.h | 11 +++++++++++
 3 files changed, 54 insertions(+)

diff --git a/lib/librte_ring/Makefile b/lib/librte_ring/Makefile
index 64c3460..b1d35ac 100644
--- a/lib/librte_ring/Makefile
+++ b/lib/librte_ring/Makefile
@@ -34,6 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 # library name
 LIB = librte_ring.a
 
+CFLAGS += -I$(RTE_SDK)/lib/librte_ether
 CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
 
 # all source are stored in SRCS-y
diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
index 3a919b0..56a46a7 100644
--- a/lib/librte_ring/rte_ring.c
+++ b/lib/librte_ring/rte_ring.c
@@ -86,6 +86,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_spinlock.h>
+#include <rte_ethdev.h>
 
 #include "rte_ring.h"
 
@@ -136,6 +137,7 @@ rte_ring_init(struct rte_ring *r, const char *name, unsigned count,
 	/* init the ring structure */
 	memset(r, 0, sizeof(*r));
 	rte_snprintf(r->name, sizeof(r->name), "%s", name);
+	r->self = r;
 	r->flags = flags;
 	r->prod.watermark = count;
 	r->prod.sp_enqueue = !!(flags & RING_F_SP_ENQ);
@@ -319,3 +321,43 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static uint16_t
+ring_eth_rx_burst(void *rxq, struct rte_mbuf **rx_pkts,
+		uint16_t nb_pkts)
+{
+	struct rte_ring *r = rxq;
+	return rte_ring_dequeue_burst(r, (void *)rx_pkts, nb_pkts);
+}
+
+static uint16_t
+ring_eth_tx_burst(void *txq, struct rte_mbuf **tx_pkts,
+		uint16_t nb_pkts)
+{
+	struct rte_ring *r = txq;
+	return rte_ring_enqueue_burst(r, (void *)tx_pkts, nb_pkts);
+}
+
+int
+rte_ring_as_eth_dev(struct rte_ring *r)
+{
+	static struct eth_dev_ops ops = { NULL };
+
+	/* reserve an ethdev entry */
+	struct rte_eth_dev *eth_dev = rte_eth_dev_allocate();
+	if (eth_dev == NULL)
+		goto error;
+
+	eth_dev->dev_ops = &ops;
+	eth_dev->rx_pkt_burst = ring_eth_rx_burst;
+	eth_dev->tx_pkt_burst = ring_eth_tx_burst;
+	eth_dev->data->nb_rx_queues = 1;
+	eth_dev->data->rx_queues = &r->self;
+	eth_dev->data->nb_tx_queues = 1;
+	eth_dev->data->tx_queues = &r->self;
+
+	return eth_dev->data->port_id;
+
+error:
+	return -1;
+}
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index da54e34..be6bc08 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -141,6 +141,7 @@ struct rte_ring {
 	TAILQ_ENTRY(rte_ring) next;      /**< Next in list. */
 
 	char name[RTE_RING_NAMESIZE];    /**< Name of the ring. */
+	void *self;                      /**< Self pointer - used by ethdev fn */
 	int flags;                       /**< Flags supplied at creation. */
 
 	/** Ring producer status. */
@@ -296,6 +297,16 @@ struct rte_ring *rte_ring_create(const char *name, unsigned count,
 				 int socket_id, unsigned flags);
 
 /**
+ * Use a ring as though it were an ethernet port
+ *
+ * @param r
+ *  Pointer to the ring structure
+ * @return
+ *  The port number of the new ethdev to be used for rx/tx burst functions
+ */
+int rte_ring_as_eth_dev(struct rte_ring *r);
+
+/**
  * Change the high water mark.
  *
  * If *count* is 0, water marking is disabled. Otherwise, it is set to the
-- 
1.9.0

[PATCH 3/3] ring: autotest for using ring as ethdev

[Bruce Richardson]

An automated unit test for the new API to allow a ring to be used as an
ethdev. Verifies that expected enqueue/dequeue functionality still
works.

Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 app/test/test_ring.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/app/test/test_ring.c b/app/test/test_ring.c
index cfd907f..f2aac24 100644
--- a/app/test/test_ring.c
+++ b/app/test/test_ring.c
@@ -58,6 +58,7 @@
 #include <rte_common.h>
 #include <rte_errno.h>
 #include <rte_hexdump.h>
+#include <rte_ethdev.h>
 
 #include "test.h"
 
@@ -1322,6 +1323,26 @@ fail_test:
 	return ret;
 }
 
+static int
+test_ring_as_eth_dev(void)
+{
+	int ethnum = rte_ring_as_eth_dev(r);
+	struct rte_mbuf *buf = NULL;
+
+	printf("Testing ring as ethdev - port num: %d\n", ethnum);
+	if (rte_eth_tx_burst(ethnum, 0, &buf, 1) != 1)
+		return -1;
+	if (rte_ring_count(r) != 1)
+		return -1;
+	if (rte_eth_rx_burst(ethnum, 0, &buf, 1) != 1)
+		return -1;
+	if (buf != NULL)
+		return -1;
+	printf("Enqueue/dequeue tests ok\n");
+
+	return 0;
+}
+
 int
 test_ring(void)
 {
@@ -1379,6 +1400,10 @@ test_ring(void)
 			else
 				printf ( "Test detected NULL ring lookup \n");
 
+	/* test using the ring as an ethdev */
+	if (test_ring_as_eth_dev() < 0)
+		return -1;
+
 	/* test of creating ring with wrong size */
 	if (test_ring_creation_with_wrong_size() < 0)
 		return -1;
-- 
1.9.0

Re: [PATCH 0/3] ring: provide rte_ring_as_ethdev API

[Neil Horman]

On Fri, May 16, 2014 at 07:15:11PM +0100, Bruce Richardson wrote:
> This patch set aims to provide a shorter simpler alternative the public API functions for using rings as ethdevs provided by the librte_pmd_ring library. This alternative just provides simple RX and TX burst functions and a conversion API, without any of the complexities present in the pmd_ring version. This replacement should allow the public APIs in the pmd_ring library to be deprecated in the future.
> 
> Bruce Richardson (3):
>   ethdev: Remove ethdev.h dependency on mbuf +  mempool
>   ring: add support for converting a ring to ethdev
>   ring: autotest for using ring as ethdev
> 
>  app/test-pmd/cmdline.c                  |  1 +
>  app/test/test_pmd_ring.c                |  1 +
>  app/test/test_ring.c                    | 25 ++++++++++++++++++++
>  lib/librte_ether/rte_ethdev.h           |  4 +++-
>  lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c |  1 +
>  lib/librte_ring/Makefile                |  1 +
>  lib/librte_ring/rte_ring.c              | 42 +++++++++++++++++++++++++++++++++
>  lib/librte_ring/rte_ring.h              | 11 +++++++++
>  8 files changed, 85 insertions(+), 1 deletion(-)
> 
> -- 
> 1.9.0
> 
> 
NAK, I don't think this makes sense.  If you want to encapsulate a ring pair as
an ethdev, then write a pmd that does so.  That will give you a standardized
ethdev that you can create using the existing --vdev librte_eal command line
options without having to widen your API surface, or having to write
applications that specifically know about the fact that your ethdev is composed
of rings under the covers.

Neil

Re: [PATCH 0/3] ring: provide rte_ring_as_ethdev API

[Richardson, Bruce]

> -----Original Message-----
> From: Neil Horman [mailto:nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org]
> Sent: Friday, May 16, 2014 7:54 PM
> To: Richardson, Bruce
> Cc: dev-VfR2kkLFssw@public.gmane.org
> Subject: Re: [dpdk-dev] [PATCH 0/3] ring: provide rte_ring_as_ethdev API
> 
> On Fri, May 16, 2014 at 07:15:11PM +0100, Bruce Richardson wrote:
> >
> NAK, I don't think this makes sense.  If you want to encapsulate a ring pair as
> an ethdev, then write a pmd that does so.  That will give you a standardized
> ethdev that you can create using the existing --vdev librte_eal command line
> options without having to widen your API surface, or having to write
> applications that specifically know about the fact that your ethdev is composed
> of rings under the covers.
> 

The objective is not to "encapsulate a ring pair", but instead allow a ring to be "type-cast" to an ethdev for the purposes of rx and tx. 
If this is provided, we can provide standard functions which work to take packets in using rx_burst and which send packets out after processing using tx_burst. The same code can then be used unmodified without worrying about whether the packets come from/to a NIC or from another core (via ring).

Re: [PATCH 0/3] ring: provide rte_ring_as_ethdev API

[Neil Horman]

On Mon, May 19, 2014 at 10:59:18AM +0000, Richardson, Bruce wrote:
> 
> 
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org]
> > Sent: Friday, May 16, 2014 7:54 PM
> > To: Richardson, Bruce
> > Cc: dev-VfR2kkLFssw@public.gmane.org
> > Subject: Re: [dpdk-dev] [PATCH 0/3] ring: provide rte_ring_as_ethdev API
> > 
> > On Fri, May 16, 2014 at 07:15:11PM +0100, Bruce Richardson wrote:
> > >
> > NAK, I don't think this makes sense.  If you want to encapsulate a ring pair as
> > an ethdev, then write a pmd that does so.  That will give you a standardized
> > ethdev that you can create using the existing --vdev librte_eal command line
> > options without having to widen your API surface, or having to write
> > applications that specifically know about the fact that your ethdev is composed
> > of rings under the covers.
> > 
> 
> The objective is not to "encapsulate a ring pair", but instead allow a ring to be "type-cast" to an ethdev for the purposes of rx and tx. 
Thats semantics.  Whatever you want to call it, you're goal is to treat a ring
pair like an ethernet interface.  You already have a mechanism to do that,
librte_pmd_ring.

> If this is provided, we can provide standard functions which work to take packets in using rx_burst and which send packets out after processing using tx_burst. The same code can then be used unmodified without worrying about whether the packets come from/to a NIC or from another core (via ring).
Again, you can already do this, librte_pmd_ring.  You're re-inventing the wheel.

If what you want is the ability to dynamically take a ring that you've created
and use it interchangeably as a ring and an ethernet device, I would suggest one
of the following:

1) Create a loopback PMD that registers a port, where anything transmitted to it
is immediately recieved on it again.  This allows you to reuse the existing
rte_eth_* api entirely to accomplish what you need

2) Create a tap device library and PMD, akin to the linux Tun/Tap device driver.
This is some additional work of course, and still expands the api, but does so
in a controlled and generic manner, useful to other applications.  By that I
mean that other data sources besides librte_ring can be used to feed data into
the network stack (pcap files or port mirroring applications, etc).

Either way, what you have right now is doing little more than solving a generic
problem only for one data source, in a way that unnecessecarily expands the API
surface.

Neil