[dpdk-dev] [PATCH 0/3] null driver improvements for testability

[dpdk-dev] [PATCH 0/3] null driver improvements for testability

[Paul Atkins]

This patchset adds functionality to the null driver help when testing
a dataplane that uses dpdk.  The idea is that the the dataplane can
have multiple null interfaces attached, and each of theses can be
assigned a mac address. Packets can then be injected into the null
drivers by adding them to a ring, giving the application complete
control of the packets that arrive.  Packets that are sent by a null
driver can be stored on a ring, where the application can pick them up
and verify it is what was expected.  To allow the application to know
when packets have been pulled of the rx ring, counters of the number of
times an rx poll has been made are kept, and these can be retrieved via
the existing APIs.

All of these enhancements are using the existing APIs, and do not
modify the existing behaviour of the null driver if these features
are not configured.


Paul Atkins (3):
  null: add a new arg to allow users to specify ether address
  null: add rings to allow user to provide the mbufs for rx/tx
  null: add xstats to provide the number of rx polls

 drivers/net/null/rte_eth_null.c |  214 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 203 insertions(+), 11 deletions(-)

-- 
1.7.10.4

[dpdk-dev] [PATCH 1/3] null: add a new arg to allow users to specify ether address

[Paul Atkins]

Add a new argument to the null driver to allow the user to
specify the ether address to be used instead of the default
which is all zeroes. This also allows the user to specify
an address per device instead of them all using the same
default one.

Signed-off-by: Paul Atkins <patkins@brocade.com>
---
 drivers/net/null/rte_eth_null.c |   59 +++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 77fc988..9483d6a 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -41,15 +41,21 @@
 
 #include "rte_eth_null.h"
 
+#include <cmdline_parse.h>
+#include <cmdline_parse_etheraddr.h>
+
 #define ETH_NULL_PACKET_SIZE_ARG	"size"
 #define ETH_NULL_PACKET_COPY_ARG	"copy"
+#define ETH_NULL_ETH_ADDR_ARG		"eth_addr"
 
 static unsigned default_packet_size = 64;
 static unsigned default_packet_copy;
+static struct ether_addr default_eth_addr = { .addr_bytes = {0} };
 
 static const char *valid_arguments[] = {
 	ETH_NULL_PACKET_SIZE_ARG,
 	ETH_NULL_PACKET_COPY_ARG,
+	ETH_NULL_ETH_ADDR_ARG,
 	NULL
 };
 
@@ -69,6 +75,7 @@ struct null_queue {
 struct pmd_internals {
 	unsigned packet_size;
 	unsigned packet_copy;
+	struct ether_addr eth_addr;
 	unsigned numa_node;
 
 	unsigned nb_rx_queues;
@@ -89,8 +96,6 @@ struct pmd_internals {
 	uint8_t rss_key[40];                /**< 40-byte hash key. */
 };
 
-
-static struct ether_addr eth_addr = { .addr_bytes = {0} };
 static const char *drivername = "Null PMD";
 static struct rte_eth_link pmd_link = {
 	.link_speed = 10000,
@@ -485,11 +490,12 @@ static const struct eth_dev_ops ops = {
 	.rss_hash_conf_get = eth_rss_hash_conf_get
 };
 
-int
-eth_dev_null_create(const char *name,
-		const unsigned numa_node,
-		unsigned packet_size,
-		unsigned packet_copy)
+static int
+eth_dev_null_create_internal(const char *name,
+			     const unsigned numa_node,
+			     unsigned packet_size,
+			     unsigned packet_copy,
+			     struct ether_addr eth_addr)
 {
 	const unsigned nb_rx_queues = 1;
 	const unsigned nb_tx_queues = 1;
@@ -539,6 +545,7 @@ eth_dev_null_create(const char *name,
 	internals->nb_tx_queues = nb_tx_queues;
 	internals->packet_size = packet_size;
 	internals->packet_copy = packet_copy;
+	internals->eth_addr = eth_addr;
 	internals->numa_node = numa_node;
 
 	internals->flow_type_rss_offloads =  ETH_RSS_PROTO_MASK;
@@ -551,7 +558,7 @@ eth_dev_null_create(const char *name,
 	data->nb_rx_queues = (uint16_t)nb_rx_queues;
 	data->nb_tx_queues = (uint16_t)nb_tx_queues;
 	data->dev_link = pmd_link;
-	data->mac_addrs = &eth_addr;
+	data->mac_addrs = &internals->eth_addr;
 	strncpy(data->name, eth_dev->data->name, strlen(eth_dev->data->name));
 
 	eth_dev->data = data;
@@ -583,6 +590,16 @@ error:
 	return -1;
 }
 
+int
+eth_dev_null_create(const char *name,
+		const unsigned numa_node,
+		unsigned packet_size,
+		unsigned packet_copy)
+{
+	return eth_dev_null_create_internal(name, numa_node, packet_size,
+					    packet_copy, default_eth_addr);
+}
+
 static inline int
 get_packet_size_arg(const char *key __rte_unused,
 		const char *value, void *extra_args)
@@ -617,12 +634,24 @@ get_packet_copy_arg(const char *key __rte_unused,
 	return 0;
 }
 
+static inline int
+get_eth_addr_arg(const char *key __rte_unused,
+		 const char *value, void *extra_args)
+{
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
+	return cmdline_parse_etheraddr(NULL, value, extra_args,
+				       sizeof(struct ether_addr));
+}
+
 static int
 rte_pmd_null_devinit(const char *name, const char *params)
 {
 	unsigned numa_node;
 	unsigned packet_size = default_packet_size;
 	unsigned packet_copy = default_packet_copy;
+	struct ether_addr eth_addr = default_eth_addr;
 	struct rte_kvargs *kvlist = NULL;
 	int ret;
 
@@ -639,7 +668,6 @@ rte_pmd_null_devinit(const char *name, const char *params)
 			return -1;
 
 		if (rte_kvargs_count(kvlist, ETH_NULL_PACKET_SIZE_ARG) == 1) {
-
 			ret = rte_kvargs_process(kvlist,
 					ETH_NULL_PACKET_SIZE_ARG,
 					&get_packet_size_arg, &packet_size);
@@ -648,20 +676,29 @@ rte_pmd_null_devinit(const char *name, const char *params)
 		}
 
 		if (rte_kvargs_count(kvlist, ETH_NULL_PACKET_COPY_ARG) == 1) {
-
 			ret = rte_kvargs_process(kvlist,
 					ETH_NULL_PACKET_COPY_ARG,
 					&get_packet_copy_arg, &packet_copy);
 			if (ret < 0)
 				goto free_kvlist;
 		}
+
+		if (rte_kvargs_count(kvlist, ETH_NULL_ETH_ADDR_ARG) == 1) {
+			ret = rte_kvargs_process(kvlist,
+						 ETH_NULL_ETH_ADDR_ARG,
+						 &get_eth_addr_arg, &eth_addr);
+			if (ret < 0)
+				goto free_kvlist;
+		}
+
 	}
 
 	RTE_LOG(INFO, PMD, "Configure pmd_null: packet size is %d, "
 			"packet copy is %s\n", packet_size,
 			packet_copy ? "enabled" : "disabled");
 
-	ret = eth_dev_null_create(name, numa_node, packet_size, packet_copy);
+	ret = eth_dev_null_create_internal(name, numa_node, packet_size,
+					   packet_copy, eth_addr);
 
 free_kvlist:
 	if (kvlist)
-- 
1.7.10.4

[dpdk-dev] [PATCH 2/3] null: add rings to allow user to provide the mbufs for rx/tx

[Paul Atkins]

When using the null driver it is useful to be able to
provide a set of mbufs to be received on the interface.
Add an option to specify a ring that the driver can poll to
provide the set of packets that have been received.  Add a
similar ring for the tx side where the packets that are being
transmitted can be stored, so the user can see what was sent.

Signed-off-by: Paul Atkins <patkins@brocade.com>
---
 drivers/net/null/rte_eth_null.c |   91 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 3 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 9483d6a..176f477 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -47,6 +47,8 @@
 #define ETH_NULL_PACKET_SIZE_ARG	"size"
 #define ETH_NULL_PACKET_COPY_ARG	"copy"
 #define ETH_NULL_ETH_ADDR_ARG		"eth_addr"
+#define ETH_NULL_RX_RING		"rx_ring"
+#define ETH_NULL_TX_RING		"tx_ring"
 
 static unsigned default_packet_size = 64;
 static unsigned default_packet_copy;
@@ -56,6 +58,8 @@ static const char *valid_arguments[] = {
 	ETH_NULL_PACKET_SIZE_ARG,
 	ETH_NULL_PACKET_COPY_ARG,
 	ETH_NULL_ETH_ADDR_ARG,
+	ETH_NULL_RX_RING,
+	ETH_NULL_TX_RING,
 	NULL
 };
 
@@ -76,6 +80,8 @@ struct pmd_internals {
 	unsigned packet_size;
 	unsigned packet_copy;
 	struct ether_addr eth_addr;
+	struct rte_ring *rx_ring;
+	struct rte_ring *tx_ring;
 	unsigned numa_node;
 
 	unsigned nb_rx_queues;
@@ -158,6 +164,22 @@ eth_null_copy_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 }
 
 static uint16_t
+eth_null_copy_rx_from_ring(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
+{
+	int i;
+	struct null_queue *h = q;
+
+	if ((q == NULL) || (bufs == NULL) || (nb_bufs == 0))
+		return 0;
+
+	i = rte_ring_mc_dequeue_burst(h->internals->rx_ring, (void **)bufs,
+				      nb_bufs);
+	rte_atomic64_add(&h->rx_pkts, 1);
+
+	return i;
+}
+
+static uint16_t
 eth_null_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 {
 	int i;
@@ -196,6 +218,23 @@ eth_null_copy_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	return i;
 }
 
+static uint16_t
+eth_null_copy_tx_to_ring(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
+{
+	int i;
+	struct null_queue *h = q;
+
+	if ((q == NULL) || (bufs == NULL) || (nb_bufs == 0))
+		return 0;
+
+	i = rte_ring_enqueue_burst(h->internals->tx_ring, (void **)bufs,
+				   nb_bufs);
+
+	rte_atomic64_add(&h->tx_pkts, 1);
+
+	return i;
+}
+
 static int
 eth_dev_configure(struct rte_eth_dev *dev) {
 	struct pmd_internals *internals;
@@ -495,7 +534,11 @@ eth_dev_null_create_internal(const char *name,
 			     const unsigned numa_node,
 			     unsigned packet_size,
 			     unsigned packet_copy,
-			     struct ether_addr eth_addr)
+			     struct ether_addr eth_addr,
+			     struct rte_ring *rx_ring,
+			     struct rte_ring *tx_ring
+
+	)
 {
 	const unsigned nb_rx_queues = 1;
 	const unsigned nb_tx_queues = 1;
@@ -546,6 +589,8 @@ eth_dev_null_create_internal(const char *name,
 	internals->packet_size = packet_size;
 	internals->packet_copy = packet_copy;
 	internals->eth_addr = eth_addr;
+	internals->rx_ring = rx_ring;
+	internals->tx_ring = tx_ring;
 	internals->numa_node = numa_node;
 
 	internals->flow_type_rss_offloads =  ETH_RSS_PROTO_MASK;
@@ -580,6 +625,10 @@ eth_dev_null_create_internal(const char *name,
 		eth_dev->rx_pkt_burst = eth_null_rx;
 		eth_dev->tx_pkt_burst = eth_null_tx;
 	}
+	if (rx_ring)
+		eth_dev->rx_pkt_burst = eth_null_copy_rx_from_ring;
+	if (tx_ring)
+		eth_dev->tx_pkt_burst = eth_null_copy_tx_to_ring;
 
 	return 0;
 
@@ -597,7 +646,8 @@ eth_dev_null_create(const char *name,
 		unsigned packet_copy)
 {
 	return eth_dev_null_create_internal(name, numa_node, packet_size,
-					    packet_copy, default_eth_addr);
+					    packet_copy, default_eth_addr,
+					    NULL, NULL);
 }
 
 static inline int
@@ -645,6 +695,23 @@ get_eth_addr_arg(const char *key __rte_unused,
 				       sizeof(struct ether_addr));
 }
 
+static inline int
+get_ring_arg(const char *key __rte_unused,
+	     const char *value, void *extra_args)
+{
+	const char *a = value;
+	struct rte_ring **ring = extra_args;
+
+	if ((value == NULL) || (extra_args == NULL))
+		return -EINVAL;
+
+	*ring = rte_ring_create(a, 64, SOCKET_ID_ANY, 0);
+	if (*ring == NULL)
+		return -1;
+
+	return 0;
+}
+
 static int
 rte_pmd_null_devinit(const char *name, const char *params)
 {
@@ -652,6 +719,8 @@ rte_pmd_null_devinit(const char *name, const char *params)
 	unsigned packet_size = default_packet_size;
 	unsigned packet_copy = default_packet_copy;
 	struct ether_addr eth_addr = default_eth_addr;
+	struct rte_ring *rx_ring = NULL;
+	struct rte_ring *tx_ring = NULL;
 	struct rte_kvargs *kvlist = NULL;
 	int ret;
 
@@ -691,6 +760,21 @@ rte_pmd_null_devinit(const char *name, const char *params)
 				goto free_kvlist;
 		}
 
+		if (rte_kvargs_count(kvlist, ETH_NULL_RX_RING) == 1) {
+			ret = rte_kvargs_process(kvlist,
+						 ETH_NULL_RX_RING,
+						 &get_ring_arg, &rx_ring);
+			if (ret < 0)
+				goto free_kvlist;
+		}
+
+		if (rte_kvargs_count(kvlist, ETH_NULL_TX_RING) == 1) {
+			ret = rte_kvargs_process(kvlist,
+						 ETH_NULL_TX_RING,
+						 &get_ring_arg, &tx_ring);
+			if (ret < 0)
+				goto free_kvlist;
+		}
 	}
 
 	RTE_LOG(INFO, PMD, "Configure pmd_null: packet size is %d, "
@@ -698,7 +782,8 @@ rte_pmd_null_devinit(const char *name, const char *params)
 			packet_copy ? "enabled" : "disabled");
 
 	ret = eth_dev_null_create_internal(name, numa_node, packet_size,
-					   packet_copy, eth_addr);
+					   packet_copy, eth_addr, rx_ring,
+					   tx_ring);
 
 free_kvlist:
 	if (kvlist)
-- 
1.7.10.4

[dpdk-dev] [PATCH 3/3] null: add xstats to provide the number of rx polls

[Paul Atkins]

When using the null driver and passing in packets via
the rx ring, it is useful to know that the driver has
polled the ring. Add a count of rx polls to the xstats
to provide this information.

Signed-off-by: Paul Atkins <patkins@brocade.com>
---
 drivers/net/null/rte_eth_null.c |   70 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 176f477..3feab44 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -65,6 +65,15 @@ static const char *valid_arguments[] = {
 
 struct pmd_internals;
 
+struct eth_null_xstats {
+	rte_atomic64_t rx_polls;
+	rte_atomic64_t tx_polls;
+};
+
+struct eth_null_hw_stats {
+	uint64_t rx_polls;
+};
+
 struct null_queue {
 	struct pmd_internals *internals;
 
@@ -74,6 +83,7 @@ struct null_queue {
 	rte_atomic64_t rx_pkts;
 	rte_atomic64_t tx_pkts;
 	rte_atomic64_t err_pkts;
+	struct eth_null_xstats xstats;
 };
 
 struct pmd_internals {
@@ -109,6 +119,19 @@ static struct rte_eth_link pmd_link = {
 	.link_status = 0
 };
 
+/* store statistics names and its offset in stats structure */
+struct eth_null_xstats_name_off {
+	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	unsigned offset;
+};
+
+static const struct eth_null_xstats_name_off eth_null_stats_strings[] = {
+	{"rx_polls", offsetof(struct eth_null_xstats, rx_polls)},
+};
+
+#define ETH_NULL_NB_XSTATS_PER_Q (sizeof(eth_null_stats_strings) /	\
+				  sizeof(eth_null_stats_strings[0]))
+
 static uint16_t
 eth_null_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 {
@@ -175,6 +198,7 @@ eth_null_copy_rx_from_ring(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	i = rte_ring_mc_dequeue_burst(h->internals->rx_ring, (void **)bufs,
 				      nb_bufs);
 	rte_atomic64_add(&h->rx_pkts, 1);
+	rte_atomic64_add(&h->xstats.rx_polls, 1);
 
 	return i;
 }
@@ -231,6 +255,7 @@ eth_null_copy_tx_to_ring(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 				   nb_bufs);
 
 	rte_atomic64_add(&h->tx_pkts, 1);
+	rte_atomic64_add(&h->tx_pkts, i);
 
 	return i;
 }
@@ -410,6 +435,49 @@ eth_stats_reset(struct rte_eth_dev *dev)
 	}
 }
 
+static int
+eth_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats *xstats,
+	       unsigned n)
+{
+	struct pmd_internals *internal;
+	unsigned count = ETH_NULL_NB_XSTATS_PER_Q * dev->data->nb_rx_queues;
+	unsigned q, i = 0;
+	uint64_t val, *stats_ptr;
+
+	if (n < count)
+		return count;
+
+	internal = dev->data->dev_private;
+
+	/* Extended stats */
+	count = 0;
+	for (q = 0; q < ETH_NULL_NB_XSTATS_PER_Q; q++) {
+		for (i = 0; i < dev->data->nb_rx_queues; i++) {
+			stats_ptr = RTE_PTR_ADD(
+				&internal->rx_null_queues[q].xstats,
+				eth_null_stats_strings[i].offset +
+				q * sizeof(uint64_t));
+			val = *stats_ptr;
+			snprintf(xstats[count].name, sizeof(xstats[count].name),
+				 "rx_queue_%u_%s", q,
+				 eth_null_stats_strings[count].name);
+			xstats[count++].value = val;
+		}
+	}
+	return count;
+}
+
+static void
+eth_xstats_reset(struct rte_eth_dev *dev)
+{
+	struct pmd_internals *internal;
+	unsigned i;
+
+	internal = dev->data->dev_private;
+	for (i = 0; i < dev->data->nb_rx_queues; i++)
+		internal->rx_null_queues[i].xstats.rx_polls.cnt = 0;
+}
+
 static void
 eth_queue_release(void *q)
 {
@@ -523,6 +591,8 @@ static const struct eth_dev_ops ops = {
 	.link_update = eth_link_update,
 	.stats_get = eth_stats_get,
 	.stats_reset = eth_stats_reset,
+	.xstats_get = eth_xstats_get,
+	.xstats_reset = eth_xstats_reset,
 	.reta_update = eth_rss_reta_update,
 	.reta_query = eth_rss_reta_query,
 	.rss_hash_update = eth_rss_hash_update,
-- 
1.7.10.4

Re: [dpdk-dev] [PATCH 0/3] null driver improvements for testability

[Thomas Monjalon]

Hi Paul,

2016-01-29 16:18, Paul Atkins:
> This patchset adds functionality to the null driver help when testing
> a dataplane that uses dpdk.  The idea is that the the dataplane can
> have multiple null interfaces attached, and each of theses can be
> assigned a mac address. Packets can then be injected into the null
> drivers by adding them to a ring, giving the application complete
> control of the packets that arrive.  Packets that are sent by a null
> driver can be stored on a ring, where the application can pick them up
> and verify it is what was expected.  To allow the application to know
> when packets have been pulled of the rx ring, counters of the number of
> times an rx poll has been made are kept, and these can be retrieved via
> the existing APIs.

I have not read your code, just read this description.
It sounds like being a ring PMD. Have you already checked it?
http://dpdk.org/browse/dpdk/tree/drivers/net/ring/rte_eth_ring.c

Re: [dpdk-dev] [PATCH 0/3] null driver improvements for testability

[Paul Atkins]

Hi Thomas,

On 29/01/16 16:31, Thomas Monjalon wrote:
> Hi Paul,
>
> 2016-01-29 16:18, Paul Atkins:
>> This patchset adds functionality to the null driver help when testing
>> a dataplane that uses dpdk.  The idea is that the the dataplane can
>> have multiple null interfaces attached, and each of theses can be
>> assigned a mac address. Packets can then be injected into the null
>> drivers by adding them to a ring, giving the application complete
>> control of the packets that arrive.  Packets that are sent by a null
>> driver can be stored on a ring, where the application can pick them up
>> and verify it is what was expected.  To allow the application to know
>> when packets have been pulled of the rx ring, counters of the number of
>> times an rx poll has been made are kept, and these can be retrieved via
>> the existing APIs.
> I have not read your code, just read this description.
> It sounds like being a ring PMD. Have you already checked it?
> https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_browse_dpdk_tree_drivers_net_ring_rte-5Feth-5Fring.c&d=CwICAg&c=IL_XqQWOjubgfqINi2jTzg&r=45ezphVDEm8OnEpH-fLWdXvR3RneLhhNZRDLQRgR6LY&m=wJLO24XFe_B0nZve6mkvocCt7fQWo3PULCTWxrC8rZk&s=bIWycJrY-PYgzkQsBeRfkl8JCHFcxRAHhHDrqRSzHYs&e=

I hadn't seen the ring PMD. I will have a look at it and see if I can 
make it do what i need.

thanks,
Paul

Re: [dpdk-dev] [PATCH 0/3] null driver improvements for testability

[Bruce Richardson]

On Fri, Jan 29, 2016 at 04:47:58PM +0000, Paul Atkins wrote:
> Hi Thomas,
> 
> On 29/01/16 16:31, Thomas Monjalon wrote:
> >Hi Paul,
> >
> >2016-01-29 16:18, Paul Atkins:
> >>This patchset adds functionality to the null driver help when testing
> >>a dataplane that uses dpdk.  The idea is that the the dataplane can
> >>have multiple null interfaces attached, and each of theses can be
> >>assigned a mac address. Packets can then be injected into the null
> >>drivers by adding them to a ring, giving the application complete
> >>control of the packets that arrive.  Packets that are sent by a null
> >>driver can be stored on a ring, where the application can pick them up
> >>and verify it is what was expected.  To allow the application to know
> >>when packets have been pulled of the rx ring, counters of the number of
> >>times an rx poll has been made are kept, and these can be retrieved via
> >>the existing APIs.
> >I have not read your code, just read this description.
> >It sounds like being a ring PMD. Have you already checked it?
> >https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_browse_dpdk_tree_drivers_net_ring_rte-5Feth-5Fring.c&d=CwICAg&c=IL_XqQWOjubgfqINi2jTzg&r=45ezphVDEm8OnEpH-fLWdXvR3RneLhhNZRDLQRgR6LY&m=wJLO24XFe_B0nZve6mkvocCt7fQWo3PULCTWxrC8rZk&s=bIWycJrY-PYgzkQsBeRfkl8JCHFcxRAHhHDrqRSzHYs&e=
> 
> I hadn't seen the ring PMD. I will have a look at it and see if I can make
> it do what i need.
> 
> thanks,
> Paul

Hi Paul,

any update on this. Your patches are still showing as pending in patchwork, but
if ring pmd is more what need, we can set these patches aside as unneeded, and
remove them from the patchwork merge backlog.

thanks,
/Bruce

Re: [dpdk-dev] [PATCH 0/3] null driver improvements for testability

[Paul Atkins]

On 17/02/16 17:23, Bruce Richardson wrote:
> On Fri, Jan 29, 2016 at 04:47:58PM +0000, Paul Atkins wrote:
>> Hi Thomas,
>>
>> On 29/01/16 16:31, Thomas Monjalon wrote:
>>> Hi Paul,
>>>
>>> 2016-01-29 16:18, Paul Atkins:
>>>> This patchset adds functionality to the null driver help when testing
>>>> a dataplane that uses dpdk.  The idea is that the the dataplane can
>>>> have multiple null interfaces attached, and each of theses can be
>>>> assigned a mac address. Packets can then be injected into the null
>>>> drivers by adding them to a ring, giving the application complete
>>>> control of the packets that arrive.  Packets that are sent by a null
>>>> driver can be stored on a ring, where the application can pick them up
>>>> and verify it is what was expected.  To allow the application to know
>>>> when packets have been pulled of the rx ring, counters of the number of
>>>> times an rx poll has been made are kept, and these can be retrieved via
>>>> the existing APIs.
>>> I have not read your code, just read this description.
>>> It sounds like being a ring PMD. Have you already checked it?
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_browse_dpdk_tree_drivers_net_ring_rte-5Feth-5Fring.c&d=CwICAg&c=IL_XqQWOjubgfqINi2jTzg&r=45ezphVDEm8OnEpH-fLWdXvR3RneLhhNZRDLQRgR6LY&m=wJLO24XFe_B0nZve6mkvocCt7fQWo3PULCTWxrC8rZk&s=bIWycJrY-PYgzkQsBeRfkl8JCHFcxRAHhHDrqRSzHYs&e=
>> I hadn't seen the ring PMD. I will have a look at it and see if I can make
>> it do what i need.
>>
>> thanks,
>> Paul
> Hi Paul,
>
> any update on this. Your patches are still showing as pending in patchwork, but
> if ring pmd is more what need, we can set these patches aside as unneeded, and
> remove them from the patchwork merge backlog.

Hi Bruce,

Sorry for the delay.  The patchset adds 3 things: assigning a mac addr 
to the null pmd, adding the rings to the null pmd and adding xstats for 
how many times the null pmd has been polled.  I could move to using the 
ring pmd, but I would still need the other 2 parts (mac addr and 
stats).  It seems like the ring pmd shouldn't really have these two 
extra things added, but i could do that if it that is preferred over 
what is in the current patchset.

thanks,
Paul
>
> thanks,
> /Bruce

Re: [dpdk-dev] [PATCH 0/3] null driver improvements for testability

[Bruce Richardson]

On Thu, Feb 18, 2016 at 12:19:58PM +0000, Paul Atkins wrote:
> 
> 
> On 17/02/16 17:23, Bruce Richardson wrote:
> >On Fri, Jan 29, 2016 at 04:47:58PM +0000, Paul Atkins wrote:
> >>Hi Thomas,
> >>
> >>On 29/01/16 16:31, Thomas Monjalon wrote:
> >>>Hi Paul,
> >>>
> >>>2016-01-29 16:18, Paul Atkins:
> >>>>This patchset adds functionality to the null driver help when testing
> >>>>a dataplane that uses dpdk.  The idea is that the the dataplane can
> >>>>have multiple null interfaces attached, and each of theses can be
> >>>>assigned a mac address. Packets can then be injected into the null
> >>>>drivers by adding them to a ring, giving the application complete
> >>>>control of the packets that arrive.  Packets that are sent by a null
> >>>>driver can be stored on a ring, where the application can pick them up
> >>>>and verify it is what was expected.  To allow the application to know
> >>>>when packets have been pulled of the rx ring, counters of the number of
> >>>>times an rx poll has been made are kept, and these can be retrieved via
> >>>>the existing APIs.
> >>>I have not read your code, just read this description.
> >>>It sounds like being a ring PMD. Have you already checked it?
> >>>https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_browse_dpdk_tree_drivers_net_ring_rte-5Feth-5Fring.c&d=CwICAg&c=IL_XqQWOjubgfqINi2jTzg&r=45ezphVDEm8OnEpH-fLWdXvR3RneLhhNZRDLQRgR6LY&m=wJLO24XFe_B0nZve6mkvocCt7fQWo3PULCTWxrC8rZk&s=bIWycJrY-PYgzkQsBeRfkl8JCHFcxRAHhHDrqRSzHYs&e=
> >>I hadn't seen the ring PMD. I will have a look at it and see if I can make
> >>it do what i need.
> >>
> >>thanks,
> >>Paul
> >Hi Paul,
> >
> >any update on this. Your patches are still showing as pending in patchwork, but
> >if ring pmd is more what need, we can set these patches aside as unneeded, and
> >remove them from the patchwork merge backlog.
> 
> Hi Bruce,
> 
> Sorry for the delay.  The patchset adds 3 things: assigning a mac addr to
> the null pmd, adding the rings to the null pmd and adding xstats for how
> many times the null pmd has been polled.  I could move to using the ring
> pmd, but I would still need the other 2 parts (mac addr and stats).  It
> seems like the ring pmd shouldn't really have these two extra things added,
> but i could do that if it that is preferred over what is in the current
> patchset.
> 
Adding a mac address to be reported by the ring PMD should not be a problem.
Having a stat that tracks polls might be depending on how it is done - if it
uses an atomic, as in this patchset, it would problematic as it would add a
severe performance hit for the SP/SC ring case. However, you could get around
that by copying what is already done in the PMD for tracking packet counts.

Overall, though, it seems that it might be worthwhile doing the work to extend
the ring pmd rather than turning the null pmd into a second ring one.

Regards,
/Bruce

Re: [dpdk-dev] [PATCH 0/3] null driver improvements for testability

[Paul Atkins]

On 23/02/16 15:21, Bruce Richardson wrote:
> On Thu, Feb 18, 2016 at 12:19:58PM +0000, Paul Atkins wrote:
>>
>> On 17/02/16 17:23, Bruce Richardson wrote:
>>> On Fri, Jan 29, 2016 at 04:47:58PM +0000, Paul Atkins wrote:
>>>> Hi Thomas,
>>>>
>>>> On 29/01/16 16:31, Thomas Monjalon wrote:
>>>>> Hi Paul,
>>>>>
>>>>> 2016-01-29 16:18, Paul Atkins:
>>>>>> This patchset adds functionality to the null driver help when testing
>>>>>> a dataplane that uses dpdk.  The idea is that the the dataplane can
>>>>>> have multiple null interfaces attached, and each of theses can be
>>>>>> assigned a mac address. Packets can then be injected into the null
>>>>>> drivers by adding them to a ring, giving the application complete
>>>>>> control of the packets that arrive.  Packets that are sent by a null
>>>>>> driver can be stored on a ring, where the application can pick them up
>>>>>> and verify it is what was expected.  To allow the application to know
>>>>>> when packets have been pulled of the rx ring, counters of the number of
>>>>>> times an rx poll has been made are kept, and these can be retrieved via
>>>>>> the existing APIs.
>>>>> I have not read your code, just read this description.
>>>>> It sounds like being a ring PMD. Have you already checked it?
>>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_browse_dpdk_tree_drivers_net_ring_rte-5Feth-5Fring.c&d=CwICAg&c=IL_XqQWOjubgfqINi2jTzg&r=45ezphVDEm8OnEpH-fLWdXvR3RneLhhNZRDLQRgR6LY&m=wJLO24XFe_B0nZve6mkvocCt7fQWo3PULCTWxrC8rZk&s=bIWycJrY-PYgzkQsBeRfkl8JCHFcxRAHhHDrqRSzHYs&e=
>>>> I hadn't seen the ring PMD. I will have a look at it and see if I can make
>>>> it do what i need.
>>>>
>>>> thanks,
>>>> Paul
>>> Hi Paul,
>>>
>>> any update on this. Your patches are still showing as pending in patchwork, but
>>> if ring pmd is more what need, we can set these patches aside as unneeded, and
>>> remove them from the patchwork merge backlog.
>> Hi Bruce,
>>
>> Sorry for the delay.  The patchset adds 3 things: assigning a mac addr to
>> the null pmd, adding the rings to the null pmd and adding xstats for how
>> many times the null pmd has been polled.  I could move to using the ring
>> pmd, but I would still need the other 2 parts (mac addr and stats).  It
>> seems like the ring pmd shouldn't really have these two extra things added,
>> but i could do that if it that is preferred over what is in the current
>> patchset.
>>
> Adding a mac address to be reported by the ring PMD should not be a problem.
> Having a stat that tracks polls might be depending on how it is done - if it
> uses an atomic, as in this patchset, it would problematic as it would add a
> severe performance hit for the SP/SC ring case. However, you could get around
> that by copying what is already done in the PMD for tracking packet counts.
>
> Overall, though, it seems that it might be worthwhile doing the work to extend
> the ring pmd rather than turning the null pmd into a second ring one.

Ok, I will make those changes and resubmit - you can mark the current 
patches as unneeded.

thanks,
Paul

>
> Regards,
> /Bruce