From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 7F5AD5A40 for ; Fri, 17 Apr 2015 19:31:35 +0200 (CEST) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1YjA6y-0003xc-GO; Fri, 17 Apr 2015 13:31:34 -0400 Date: Fri, 17 Apr 2015 13:31:26 -0400 From: Neil Horman To: Bruce Richardson Message-ID: <20150417173126.GD26164@hmsreliant.think-freely.org> References: <1428954274-26944-1-git-send-email-keith.wiles@intel.com> <1429283804-28087-1-git-send-email-bruce.richardson@intel.com> <1429283804-28087-4-git-send-email-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1429283804-28087-4-git-send-email-bruce.richardson@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [RFC PATCH 3/4] add support for a ring to be a pktdev X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Apr 2015 17:31:35 -0000 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 > #include > #include > +#include > > #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