[dpdk-dev] [RFC PATCH 3/4] add support for a ring to be a pktdev
Hi Bruce,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson
> Sent: Friday, April 17, 2015 4:17 PM
> To: dev at dpdk.org; Wiles, Keith
> Subject: [dpdk-dev] [RFC PATCH 3/4] add support for a ring to be a pktdev
>
> 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
I don't think it is a good idea to make rte_ring dependent on rte_pktdev (or
rte_ethdev).
If we'd like to have pkt (or eth) device based on rte_ring, why not to create
librte_pmd_ring
and put all that stuff there?
Konstantin
>
> #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 = >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 = >ring;
> + p->dev_data.tx_queues = >ring;
> +
> + return (void *)p;
> +}
> +
> +
> /*
> * change the high water mark. If *count* is 0, water marking is
> * disabled
> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> index af6..c2f28be 100644
> --- a/lib/librte_ring/rte_ring.h
> +++ b/lib/librte_ring/rte_ring.h
> @@ -301,6 +302,10 @@ int rte_ring_init(struct rte_ring *r, const char *name,
> unsigned count,
> struct rte_ring *rte_ring_create(const char *name, unsigned count,
>int socket_id, unsigned flags);
>
> +struct rte_pkt_dev;
> +
> +struct rte_pkt_dev *rte_ring_get_dev(struct rte_ring *r);
> +
> /**
> * Change the high water mark.
> *
> --
> 2.1.0
[dpdk-dev] [RFC PATCH 3/4] add support for a ring to be a pktdev
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson
> Sent: Friday, April 17, 2015 11:17 PM
> To: dev at dpdk.org; Wiles, Keith
> Subject: [dpdk-dev] [RFC PATCH 3/4] add support for a ring to be a pktdev
>
> 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 = >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 = >ring;
> + p->dev_data.tx_queues = >ring;
Why rx and tx doesn't need different ring here?
> +
> + return (void *)p;
> +}
> +
> +
> /*
> * change the high water mark. If *count* is 0, water marking is
> * disabled
> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h index
> af6..c2f28be 100644
> --- a/lib/librte_ring/rte_ring.h
> +++ b/lib/librte_ring/rte_ring.h
> @@ -301,6 +302,10 @@ int rte_ring_init(struct rte_ring *r, const char *name,
> unsigned count, struct rte_ring *rte_ring_create(const char *name,
> unsigned count,
>int socket_id, unsigned flags);
>
> +struct rte_pkt_dev;
> +
> +struct rte_pkt_dev *rte_ring_get_dev(struct rte_ring *r);
> +
> /**
> * Change the high water mark.
> *
> --
> 2.1.0
[dpdk-dev] [RFC PATCH 3/4] add support for a ring to be a pktdev
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 = >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 = >ring;
+ p->dev_data.tx_queues = >ring;
+
+ return (void *)p;
+}
+
+
/*
* change the high water mark. If *count* is 0, water marking is
* disabled
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index af6..c2f28be 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -301,6 +302,10 @@ int rte_ring_init(struct rte_ring *r, const char *name,
unsigned count,
struct rte_ring *rte_ring_create(const char *name, unsigned count,
int socket_id, unsigned flags);
+struct rte_pkt_dev;
+
+struct rte_pkt_dev *rte_ring_get_dev(struct rte_ring *r);
+
/**
* Change the high water mark.
*
--
2.1.0
[dpdk-dev] [RFC PATCH 3/4] add support for a ring to be a pktdev
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 = >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 = >ring;
> + p->dev_data.tx_queues = >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
