Re: [dpdk-dev] [RFC PATCH 3/4] add support for a ring to be a pktdev

[Neil Horman <nhorman@tuxdriver.com>]

On Fri, Apr 17, 2015 at 04:16:43PM +0100, Bruce Richardson wrote:
> Add a new public API function, and two internal wrapper functions so we
> can use ring as a pktdev.
> ---
>  lib/librte_ring/rte_ring.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  lib/librte_ring/rte_ring.h |  3 +++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/lib/librte_ring/Makefile b/lib/librte_ring/Makefile
> index 84ad3d3..bc5dd09 100644
> --- a/lib/librte_ring/Makefile
> +++ b/lib/librte_ring/Makefile
> @@ -47,6 +47,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_RING) := rte_ring.c
>  SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h
> 
>  # this lib needs eal and rte_malloc
> -DEPDIRS-$(CONFIG_RTE_LIBRTE_RING) += lib/librte_eal lib/librte_malloc
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_RING) += lib/librte_eal lib/librte_malloc lib/librte_pktdev
> 
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
> index c9e59d4..424da20 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_pktdev.h>
> 
>  #include "rte_ring.h"
> 
> @@ -208,6 +208,47 @@ rte_ring_create(const char *name, unsigned count, int socket_id,
>  	return r;
>  }
>  
> +static uint16_t
> +dev_rx(void *r, struct rte_mbuf **pkts, uint16_t max_pkts)
> +{
> +	return rte_ring_dequeue_burst(r, (void *)pkts, max_pkts);
> +}
> +
> +static uint16_t
> +dev_tx(void *r, struct rte_mbuf **pkts, uint16_t max_pkts)
> +{
> +	return rte_ring_enqueue_burst(r, (void *)pkts, max_pkts);
> +}
> +
> +#define rte_ring_dev_data rte_pkt_dev_data
> +
> +struct rte_pkt_dev *
> +rte_ring_get_dev(struct rte_ring *r)
> +{
> +	struct ring_as_pktdev {
> +		RTE_PKT_DEV_HDR(rte_ring_dev);
> +		struct rte_ring_dev_data dev_data;
> +		void *ring;
> +	} *p;
> +	if (r == NULL ||
> +			(p = rte_zmalloc(NULL, sizeof(*p), 0)) == NULL)
> +		return NULL;
> +
> +	p->ring = r;
> +	p->rx_pkt_burst = dev_rx;
> +	p->tx_pkt_burst = dev_tx;
> +	p->data = &p->dev_data;
> +
> +	snprintf(p->dev_data.name, sizeof(p->dev_data.name), "%s", r->name);
> +	p->dev_data.nb_rx_queues = 1;
> +	p->dev_data.nb_tx_queues = 1;
> +	p->dev_data.rx_queues = &p->ring;
> +	p->dev_data.tx_queues = &p->ring;
> +
> +	return (void *)p;
> +}
> +
> +

So, yeah, you have the ability to get a new ethdev out of the ring api here,
thats great.  But to do it, you've created an additional API call point.  I
don't think thats a reasonable trade off.  If you had just registered a PMD,
with minimal methods (rx and tx), you might have had a little more code
involved, but for that additional code you would have bought yourself all the
infrastructure that ethdevs provide, like the ability to allocate and register
them at exeution time administratively (with the --vdev option).  You saved
yourself a bit of code here, but lost the ability to do all the other things
that you might want to do with ring ethdevs.

Neil