All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 00/13] port: added port statistics
@ 2015-06-19  9:41 Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 01/13] port: added structures for port stats and config option Maciej Gajdzica
                   ` (13 more replies)
  0 siblings, 14 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for every type of port. By default all port statistics
are disabled, user must activate them in config file.

Changes in v2:
	- added missing signoffs

Changes in v3:
	- removed new config options to enable/disable stats
	- using RTE_LOG_LEVEL instead

Changes in v4:
	- created single config option for all port statistics

Changes in v5:
	- added missing CONFIG_ prefix to defines in config files

Maciej Gajdzica (13):
  port: added structures for port stats and config option
  port: added port_ethdev_reader stats
  port: added port_ethdev_writer stats
  port: added port_ethdev_writer_nodrop stats
  port: added port_frag stats
  port: added port_ras stats
  port: added port_ring_reader stats
  port: added port_ring_writer stats
  port: added port_ring_writer_nodrop stats
  port: added port_sched_reader stats
  port: added port_sched_writer stats
  port: added port_source stats
  port: added port_sink stats

 config/common_bsdapp                   |    1 +
 config/common_linuxapp                 |    1 +
 lib/librte_port/rte_port.h             |   60 +++++++++++++++--
 lib/librte_port/rte_port_ethdev.c      |  110 +++++++++++++++++++++++++++++-
 lib/librte_port/rte_port_frag.c        |   36 ++++++++++
 lib/librte_port/rte_port_ras.c         |   38 +++++++++++
 lib/librte_port/rte_port_ring.c        |  114 +++++++++++++++++++++++++++++++-
 lib/librte_port/rte_port_sched.c       |   96 +++++++++++++++++++++++++--
 lib/librte_port/rte_port_source_sink.c |   98 +++++++++++++++++++++++++--
 9 files changed, 537 insertions(+), 17 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v5 01/13] port: added structures for port stats and config option
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-23 10:26   ` Dumitrescu, Cristian
  2015-06-23 13:55   ` Thomas Monjalon
  2015-06-19  9:41 ` [PATCH v5 02/13] port: added port_ethdev_reader stats Maciej Gajdzica
                   ` (12 subsequent siblings)
  13 siblings, 2 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added common data structures for port statistics. Added config option to
enable stats collecting.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 config/common_bsdapp       |    1 +
 config/common_linuxapp     |    1 +
 lib/librte_port/rte_port.h |   60 ++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/config/common_bsdapp b/config/common_bsdapp
index c2374c0..c5036a4 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -383,6 +383,7 @@ CONFIG_RTE_LIBRTE_REORDER=y
 # Compile librte_port
 #
 CONFIG_RTE_LIBRTE_PORT=y
+CONFIG_RTE_PORT_STATS_COLLECT=n
 
 #
 # Compile librte_table
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 0078dc9..1fc5176 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -390,6 +390,7 @@ CONFIG_RTE_LIBRTE_REORDER=y
 # Compile librte_port
 #
 CONFIG_RTE_LIBRTE_PORT=y
+CONFIG_RTE_PORT_STATS_COLLECT=n
 
 #
 # Compile librte_table
diff --git a/lib/librte_port/rte_port.h b/lib/librte_port/rte_port.h
index d84e5a1..ab433e5 100644
--- a/lib/librte_port/rte_port.h
+++ b/lib/librte_port/rte_port.h
@@ -81,6 +81,12 @@ extern "C" {
 Cannot be changed. */
 #define RTE_PORT_IN_BURST_SIZE_MAX                         64
 
+/** Input port statistics */
+struct rte_port_in_stats {
+	uint64_t n_pkts_in;
+	uint64_t n_pkts_drop;
+};
+
 /**
  * Input port create
  *
@@ -120,17 +126,42 @@ typedef int (*rte_port_in_op_rx)(
 	struct rte_mbuf **pkts,
 	uint32_t n_pkts);
 
+/**
+ * Input port stats get
+ *
+ * @param port
+ *   Handle to output port instance
+ * @param stats
+ *   Handle to port_in stats struct to copy data
+ * @param clear
+ *   Flag indicating that stats should be cleared after read
+ *
+ * @return
+ *   Error code or 0 on success.
+ */
+typedef int (*rte_port_in_op_stats_read)(
+		void *port,
+		struct rte_port_in_stats *stats,
+		int clear);
+
 /** Input port interface defining the input port operation */
 struct rte_port_in_ops {
 	rte_port_in_op_create f_create; /**< Create */
 	rte_port_in_op_free f_free;     /**< Free */
 	rte_port_in_op_rx f_rx;         /**< Packet RX (packet burst) */
+	rte_port_in_op_stats_read f_stats;	/**< Stats */
 };
 
 /*
  * Port OUT
  *
  */
+/** Output port statistics */
+struct rte_port_out_stats {
+	uint64_t n_pkts_in;
+	uint64_t n_pkts_drop;
+};
+
 /**
  * Output port create
  *
@@ -197,13 +228,32 @@ typedef int (*rte_port_out_op_tx_bulk)(
  */
 typedef int (*rte_port_out_op_flush)(void *port);
 
+/**
+ * Output port stats read
+ *
+ * @param port
+ *   Handle to output port instance
+ * @param stats
+ *   Handle to port_out stats struct to copy data
+ * @param clear
+ *   Flag indicating that stats should be cleared after read
+ *
+ * @return
+ *   Error code or 0 on success.
+ */
+typedef int (*rte_port_out_op_stats_read)(
+		void *port,
+		struct rte_port_out_stats *stats,
+		int clear);
+
 /** Output port interface defining the output port operation */
 struct rte_port_out_ops {
-	rte_port_out_op_create f_create;   /**< Create */
-	rte_port_out_op_free f_free;       /**< Free */
-	rte_port_out_op_tx f_tx;           /**< Packet TX (single packet) */
-	rte_port_out_op_tx_bulk f_tx_bulk; /**< Packet TX (packet burst) */
-	rte_port_out_op_flush f_flush;     /**< Flush */
+	rte_port_out_op_create f_create;		/**< Create */
+	rte_port_out_op_free f_free;			/**< Free */
+	rte_port_out_op_tx f_tx;				/**< Packet TX (single packet) */
+	rte_port_out_op_tx_bulk f_tx_bulk;		/**< Packet TX (packet burst) */
+	rte_port_out_op_flush f_flush;			/**< Flush */
+	rte_port_out_op_stats_read f_stats;     /**< Stats */
 };
 
 #ifdef __cplusplus
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 02/13] port: added port_ethdev_reader stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 01/13] port: added structures for port stats and config option Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 03/13] port: added port_ethdev_writer stats Maciej Gajdzica
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for ethdev reader port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_ethdev.c |   37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/lib/librte_port/rte_port_ethdev.c b/lib/librte_port/rte_port_ethdev.c
index 39ed72d..da1af08 100644
--- a/lib/librte_port/rte_port_ethdev.c
+++ b/lib/librte_port/rte_port_ethdev.c
@@ -42,7 +42,23 @@
 /*
  * Port ETHDEV Reader
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_ETHDEV_READER_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_ETHDEV_READER_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_ETHDEV_READER_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_ETHDEV_READER_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_ethdev_reader {
+	struct rte_port_in_stats stats;
+
 	uint16_t queue_id;
 	uint8_t port_id;
 };
@@ -80,8 +96,11 @@ rte_port_ethdev_reader_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
 {
 	struct rte_port_ethdev_reader *p =
 		(struct rte_port_ethdev_reader *) port;
+	uint16_t rx_pkt_cnt;
 
-	return rte_eth_rx_burst(p->port_id, p->queue_id, pkts, n_pkts);
+	rx_pkt_cnt = rte_eth_rx_burst(p->port_id, p->queue_id, pkts, n_pkts);
+	RTE_PORT_ETHDEV_READER_STATS_PKTS_IN_ADD(p, rx_pkt_cnt);
+	return rx_pkt_cnt;
 }
 
 static int
@@ -97,6 +116,21 @@ rte_port_ethdev_reader_free(void *port)
 	return 0;
 }
 
+static int rte_port_ethdev_reader_stats_read(void *port,
+		struct rte_port_in_stats * stats, int clear)
+{
+	struct rte_port_ethdev_reader *p =
+			(struct rte_port_ethdev_reader *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Port ETHDEV Writer
  */
@@ -422,6 +456,7 @@ struct rte_port_in_ops rte_port_ethdev_reader_ops = {
 	.f_create = rte_port_ethdev_reader_create,
 	.f_free = rte_port_ethdev_reader_free,
 	.f_rx = rte_port_ethdev_reader_rx,
+	.f_stats = rte_port_ethdev_reader_stats_read,
 };
 
 struct rte_port_out_ops rte_port_ethdev_writer_ops = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 03/13] port: added port_ethdev_writer stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 01/13] port: added structures for port stats and config option Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 02/13] port: added port_ethdev_reader stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 04/13] port: added port_ethdev_writer_nodrop stats Maciej Gajdzica
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for ethdev writer port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_ethdev.c |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/lib/librte_port/rte_port_ethdev.c b/lib/librte_port/rte_port_ethdev.c
index da1af08..b5b39f8 100644
--- a/lib/librte_port/rte_port_ethdev.c
+++ b/lib/librte_port/rte_port_ethdev.c
@@ -134,7 +134,23 @@ static int rte_port_ethdev_reader_stats_read(void *port,
 /*
  * Port ETHDEV Writer
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_ETHDEV_WRITER_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_ETHDEV_WRITER_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_ETHDEV_WRITER_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_ETHDEV_WRITER_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_ethdev_writer {
+	struct rte_port_out_stats stats;
+
 	struct rte_mbuf *tx_buf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
 	uint32_t tx_burst_sz;
 	uint16_t tx_buf_count;
@@ -185,6 +201,7 @@ send_burst(struct rte_port_ethdev_writer *p)
 	nb_tx = rte_eth_tx_burst(p->port_id, p->queue_id,
 			 p->tx_buf, p->tx_buf_count);
 
+	RTE_PORT_ETHDEV_WRITER_STATS_PKTS_DROP_ADD(p, p->tx_buf_count - nb_tx);
 	for ( ; nb_tx < p->tx_buf_count; nb_tx++)
 		rte_pktmbuf_free(p->tx_buf[nb_tx]);
 
@@ -198,6 +215,7 @@ rte_port_ethdev_writer_tx(void *port, struct rte_mbuf *pkt)
 		(struct rte_port_ethdev_writer *) port;
 
 	p->tx_buf[p->tx_buf_count++] = pkt;
+	RTE_PORT_ETHDEV_WRITER_STATS_PKTS_IN_ADD(p, 1);
 	if (p->tx_buf_count >= p->tx_burst_sz)
 		send_burst(p);
 
@@ -223,9 +241,11 @@ rte_port_ethdev_writer_tx_bulk(void *port,
 		if (tx_buf_count)
 			send_burst(p);
 
+		RTE_PORT_ETHDEV_WRITER_STATS_PKTS_IN_ADD(p, n_pkts);
 		n_pkts_ok = rte_eth_tx_burst(p->port_id, p->queue_id, pkts,
 			n_pkts);
 
+		RTE_PORT_ETHDEV_WRITER_STATS_PKTS_DROP_ADD(p, n_pkts - n_pkts_ok);
 		for ( ; n_pkts_ok < n_pkts; n_pkts_ok++) {
 			struct rte_mbuf *pkt = pkts[n_pkts_ok];
 
@@ -238,6 +258,7 @@ rte_port_ethdev_writer_tx_bulk(void *port,
 			struct rte_mbuf *pkt = pkts[pkt_index];
 
 			p->tx_buf[tx_buf_count++] = pkt;
+			RTE_PORT_ETHDEV_WRITER_STATS_PKTS_IN_ADD(p, 1);
 			pkts_mask &= ~pkt_mask;
 		}
 
@@ -275,6 +296,21 @@ rte_port_ethdev_writer_free(void *port)
 	return 0;
 }
 
+static int rte_port_ethdev_writer_stats_read(void *port,
+		struct rte_port_out_stats *stats, int clear)
+{
+	struct rte_port_ethdev_writer *p =
+		(struct rte_port_ethdev_writer *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Port ETHDEV Writer Nodrop
  */
@@ -465,6 +501,7 @@ struct rte_port_out_ops rte_port_ethdev_writer_ops = {
 	.f_tx = rte_port_ethdev_writer_tx,
 	.f_tx_bulk = rte_port_ethdev_writer_tx_bulk,
 	.f_flush = rte_port_ethdev_writer_flush,
+	.f_stats = rte_port_ethdev_writer_stats_read,
 };
 
 struct rte_port_out_ops rte_port_ethdev_writer_nodrop_ops = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 04/13] port: added port_ethdev_writer_nodrop stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (2 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 03/13] port: added port_ethdev_writer stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 05/13] port: added port_frag stats Maciej Gajdzica
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for ethdev writer nodrop port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_ethdev.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/lib/librte_port/rte_port_ethdev.c b/lib/librte_port/rte_port_ethdev.c
index b5b39f8..cee1b33 100644
--- a/lib/librte_port/rte_port_ethdev.c
+++ b/lib/librte_port/rte_port_ethdev.c
@@ -314,7 +314,23 @@ static int rte_port_ethdev_writer_stats_read(void *port,
 /*
  * Port ETHDEV Writer Nodrop
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_ethdev_writer_nodrop {
+	struct rte_port_out_stats stats;
+
 	struct rte_mbuf *tx_buf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
 	uint32_t tx_burst_sz;
 	uint16_t tx_buf_count;
@@ -387,6 +403,7 @@ send_burst_nodrop(struct rte_port_ethdev_writer_nodrop *p)
 	}
 
 	/* We didn't send the packets in maximum allowed attempts */
+	RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_DROP_ADD(p, p->tx_buf_count - nb_tx);
 	for ( ; nb_tx < p->tx_buf_count; nb_tx++)
 		rte_pktmbuf_free(p->tx_buf[nb_tx]);
 
@@ -400,6 +417,7 @@ rte_port_ethdev_writer_nodrop_tx(void *port, struct rte_mbuf *pkt)
 		(struct rte_port_ethdev_writer_nodrop *) port;
 
 	p->tx_buf[p->tx_buf_count++] = pkt;
+	RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_IN_ADD(p, 1);
 	if (p->tx_buf_count >= p->tx_burst_sz)
 		send_burst_nodrop(p);
 
@@ -426,6 +444,7 @@ rte_port_ethdev_writer_nodrop_tx_bulk(void *port,
 		if (tx_buf_count)
 			send_burst_nodrop(p);
 
+		RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_IN_ADD(p, n_pkts);
 		n_pkts_ok = rte_eth_tx_burst(p->port_id, p->queue_id, pkts,
 			n_pkts);
 
@@ -448,6 +467,7 @@ rte_port_ethdev_writer_nodrop_tx_bulk(void *port,
 			struct rte_mbuf *pkt = pkts[pkt_index];
 
 			p->tx_buf[tx_buf_count++] = pkt;
+			RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_IN_ADD(p, 1);
 			pkts_mask &= ~pkt_mask;
 		}
 
@@ -485,6 +505,21 @@ rte_port_ethdev_writer_nodrop_free(void *port)
 	return 0;
 }
 
+static int rte_port_ethdev_writer_nodrop_stats_read(void *port,
+		struct rte_port_out_stats *stats, int clear)
+{
+	struct rte_port_ethdev_writer_nodrop *p =
+		(struct rte_port_ethdev_writer_nodrop *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Summary of port operations
  */
@@ -510,4 +545,5 @@ struct rte_port_out_ops rte_port_ethdev_writer_nodrop_ops = {
 	.f_tx = rte_port_ethdev_writer_nodrop_tx,
 	.f_tx_bulk = rte_port_ethdev_writer_nodrop_tx_bulk,
 	.f_flush = rte_port_ethdev_writer_nodrop_flush,
+	.f_stats = rte_port_ethdev_writer_nodrop_stats_read,
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 05/13] port: added port_frag stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (3 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 04/13] port: added port_ethdev_writer_nodrop stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 06/13] port: added port_ras stats Maciej Gajdzica
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for IPv4 and IPv6 fragmentation ports.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_frag.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/lib/librte_port/rte_port_frag.c b/lib/librte_port/rte_port_frag.c
index c4c05dc..3720d5d 100644
--- a/lib/librte_port/rte_port_frag.c
+++ b/lib/librte_port/rte_port_frag.c
@@ -41,6 +41,20 @@
 /* Max number of fragments per packet allowed */
 #define	RTE_PORT_FRAG_MAX_FRAGS_PER_PACKET 0x80
 
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_RING_READER_FRAG_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_RING_READER_FRAG_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_RING_READER_FRAG_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_RING_READER_FRAG_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 typedef int32_t
 		(*frag_op)(struct rte_mbuf *pkt_in,
 			struct rte_mbuf **pkts_out,
@@ -50,6 +64,8 @@ typedef int32_t
 			struct rte_mempool *pool_indirect);
 
 struct rte_port_ring_reader_frag {
+	struct rte_port_in_stats stats;
+
 	/* Input parameters */
 	struct rte_ring *ring;
 	uint32_t mtu;
@@ -171,6 +187,7 @@ rte_port_ring_reader_frag_rx(void *port,
 		if (p->n_pkts == 0) {
 			p->n_pkts = rte_ring_sc_dequeue_burst(p->ring,
 				(void **) p->pkts, RTE_PORT_IN_BURST_SIZE_MAX);
+			RTE_PORT_RING_READER_FRAG_STATS_PKTS_IN_ADD(p, p->n_pkts);
 			if (p->n_pkts == 0)
 				return n_pkts_out;
 			p->pos_pkts = 0;
@@ -203,6 +220,7 @@ rte_port_ring_reader_frag_rx(void *port,
 
 		if (status < 0) {
 			rte_pktmbuf_free(pkt);
+			RTE_PORT_RING_READER_FRAG_STATS_PKTS_DROP_ADD(p, 1);
 			continue;
 		}
 
@@ -252,6 +270,22 @@ rte_port_ring_reader_frag_free(void *port)
 	return 0;
 }
 
+static int
+rte_port_frag_reader_stats_read(void *port,
+		struct rte_port_in_stats *stats, int clear)
+{
+	struct rte_port_ring_reader_frag *p =
+		(struct rte_port_ring_reader_frag *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Summary of port operations
  */
@@ -259,10 +293,12 @@ struct rte_port_in_ops rte_port_ring_reader_ipv4_frag_ops = {
 	.f_create = rte_port_ring_reader_ipv4_frag_create,
 	.f_free = rte_port_ring_reader_frag_free,
 	.f_rx = rte_port_ring_reader_frag_rx,
+	.f_stats = rte_port_frag_reader_stats_read,
 };
 
 struct rte_port_in_ops rte_port_ring_reader_ipv6_frag_ops = {
 	.f_create = rte_port_ring_reader_ipv6_frag_create,
 	.f_free = rte_port_ring_reader_frag_free,
 	.f_rx = rte_port_ring_reader_frag_rx,
+	.f_stats = rte_port_frag_reader_stats_read,
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 06/13] port: added port_ras stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (4 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 05/13] port: added port_frag stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 07/13] port: added port_ring_reader stats Maciej Gajdzica
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for IPv4 and IPv6 reassembly ports.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_ras.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/lib/librte_port/rte_port_ras.c b/lib/librte_port/rte_port_ras.c
index 5eb627a..2c1822a 100644
--- a/lib/librte_port/rte_port_ras.c
+++ b/lib/librte_port/rte_port_ras.c
@@ -51,6 +51,20 @@
 #define RTE_PORT_RAS_N_ENTRIES (RTE_PORT_RAS_N_BUCKETS * RTE_PORT_RAS_N_ENTRIES_PER_BUCKET)
 #endif
 
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_RING_WRITER_RAS_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_RING_WRITER_RAS_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_RING_WRITER_RAS_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_RING_WRITER_RAS_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_ring_writer_ras;
 
 typedef void (*ras_op)(
@@ -63,6 +77,8 @@ static void
 process_ipv6(struct rte_port_ring_writer_ras *p, struct rte_mbuf *pkt);
 
 struct rte_port_ring_writer_ras {
+	struct rte_port_out_stats stats;
+
 	struct rte_mbuf *tx_buf[RTE_PORT_IN_BURST_SIZE_MAX];
 	struct rte_ring *ring;
 	uint32_t tx_burst_sz;
@@ -153,6 +169,7 @@ send_burst(struct rte_port_ring_writer_ras *p)
 	nb_tx = rte_ring_sp_enqueue_burst(p->ring, (void **)p->tx_buf,
 			p->tx_buf_count);
 
+	RTE_PORT_RING_WRITER_RAS_STATS_PKTS_DROP_ADD(p, p->tx_buf_count - nb_tx);
 	for ( ; nb_tx < p->tx_buf_count; nb_tx++)
 		rte_pktmbuf_free(p->tx_buf[nb_tx]);
 
@@ -225,6 +242,7 @@ rte_port_ring_writer_ras_tx(void *port, struct rte_mbuf *pkt)
 	struct rte_port_ring_writer_ras *p =
 			(struct rte_port_ring_writer_ras *) port;
 
+	RTE_PORT_RING_WRITER_RAS_STATS_PKTS_IN_ADD(p, 1);
 	p->f_ras(p, pkt);
 	if (p->tx_buf_count >= p->tx_burst_sz)
 		send_burst(p);
@@ -247,6 +265,7 @@ rte_port_ring_writer_ras_tx_bulk(void *port,
 		for (i = 0; i < n_pkts; i++) {
 			struct rte_mbuf *pkt = pkts[i];
 
+			RTE_PORT_RING_WRITER_RAS_STATS_PKTS_IN_ADD(p, 1);
 			p->f_ras(p, pkt);
 			if (p->tx_buf_count >= p->tx_burst_sz)
 				send_burst(p);
@@ -257,6 +276,7 @@ rte_port_ring_writer_ras_tx_bulk(void *port,
 			uint64_t pkt_mask = 1LLU << pkt_index;
 			struct rte_mbuf *pkt = pkts[pkt_index];
 
+			RTE_PORT_RING_WRITER_RAS_STATS_PKTS_IN_ADD(p, 1);
 			p->f_ras(p, pkt);
 			if (p->tx_buf_count >= p->tx_burst_sz)
 				send_burst(p);
@@ -298,6 +318,22 @@ rte_port_ring_writer_ras_free(void *port)
 	return 0;
 }
 
+static int
+rte_port_ras_writer_stats_read(void *port,
+		struct rte_port_out_stats *stats, int clear)
+{
+	struct rte_port_ring_writer_ras *p =
+		(struct rte_port_ring_writer_ras *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Summary of port operations
  */
@@ -307,6 +343,7 @@ struct rte_port_out_ops rte_port_ring_writer_ipv4_ras_ops = {
 	.f_tx = rte_port_ring_writer_ras_tx,
 	.f_tx_bulk = rte_port_ring_writer_ras_tx_bulk,
 	.f_flush = rte_port_ring_writer_ras_flush,
+	.f_stats = rte_port_ras_writer_stats_read,
 };
 
 struct rte_port_out_ops rte_port_ring_writer_ipv6_ras_ops = {
@@ -315,4 +352,5 @@ struct rte_port_out_ops rte_port_ring_writer_ipv6_ras_ops = {
 	.f_tx = rte_port_ring_writer_ras_tx,
 	.f_tx_bulk = rte_port_ring_writer_ras_tx_bulk,
 	.f_flush = rte_port_ring_writer_ras_flush,
+	.f_stats = rte_port_ras_writer_stats_read,
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 07/13] port: added port_ring_reader stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (5 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 06/13] port: added port_ras stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 08/13] port: added port_ring_writer stats Maciej Gajdzica
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for ring reader port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_ring.c |   39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/lib/librte_port/rte_port_ring.c b/lib/librte_port/rte_port_ring.c
index 89b9641..091b052 100644
--- a/lib/librte_port/rte_port_ring.c
+++ b/lib/librte_port/rte_port_ring.c
@@ -42,7 +42,23 @@
 /*
  * Port RING Reader
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_RING_READER_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_RING_READER_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_RING_READER_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_RING_READER_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_ring_reader {
+	struct rte_port_in_stats stats;
+
 	struct rte_ring *ring;
 };
 
@@ -77,8 +93,12 @@ static int
 rte_port_ring_reader_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
 {
 	struct rte_port_ring_reader *p = (struct rte_port_ring_reader *) port;
+	uint32_t nb_rx;
 
-	return rte_ring_sc_dequeue_burst(p->ring, (void **) pkts, n_pkts);
+	nb_rx = rte_ring_sc_dequeue_burst(p->ring, (void **) pkts, n_pkts);
+	RTE_PORT_RING_READER_STATS_PKTS_IN_ADD(p, nb_rx);
+
+	return nb_rx;
 }
 
 static int
@@ -94,6 +114,22 @@ rte_port_ring_reader_free(void *port)
 	return 0;
 }
 
+static int
+rte_port_ring_reader_stats_read(void *port,
+		struct rte_port_in_stats *stats, int clear)
+{
+	struct rte_port_ring_reader *p =
+		(struct rte_port_ring_reader *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Port RING Writer
  */
@@ -410,6 +446,7 @@ struct rte_port_in_ops rte_port_ring_reader_ops = {
 	.f_create = rte_port_ring_reader_create,
 	.f_free = rte_port_ring_reader_free,
 	.f_rx = rte_port_ring_reader_rx,
+	.f_stats = rte_port_ring_reader_stats_read,
 };
 
 struct rte_port_out_ops rte_port_ring_writer_ops = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 08/13] port: added port_ring_writer stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (6 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 07/13] port: added port_ring_reader stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 09/13] port: added port_ring_writer_nodrop stats Maciej Gajdzica
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for port writer port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_ring.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/lib/librte_port/rte_port_ring.c b/lib/librte_port/rte_port_ring.c
index 091b052..ff58009 100644
--- a/lib/librte_port/rte_port_ring.c
+++ b/lib/librte_port/rte_port_ring.c
@@ -133,7 +133,23 @@ rte_port_ring_reader_stats_read(void *port,
 /*
  * Port RING Writer
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_RING_WRITER_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_RING_WRITER_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_RING_WRITER_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_RING_WRITER_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_ring_writer {
+	struct rte_port_out_stats stats;
+
 	struct rte_mbuf *tx_buf[RTE_PORT_IN_BURST_SIZE_MAX];
 	struct rte_ring *ring;
 	uint32_t tx_burst_sz;
@@ -181,6 +197,7 @@ send_burst(struct rte_port_ring_writer *p)
 	nb_tx = rte_ring_sp_enqueue_burst(p->ring, (void **)p->tx_buf,
 			p->tx_buf_count);
 
+	RTE_PORT_RING_WRITER_STATS_PKTS_DROP_ADD(p, p->tx_buf_count - nb_tx);
 	for ( ; nb_tx < p->tx_buf_count; nb_tx++)
 		rte_pktmbuf_free(p->tx_buf[nb_tx]);
 
@@ -193,6 +210,7 @@ rte_port_ring_writer_tx(void *port, struct rte_mbuf *pkt)
 	struct rte_port_ring_writer *p = (struct rte_port_ring_writer *) port;
 
 	p->tx_buf[p->tx_buf_count++] = pkt;
+	RTE_PORT_RING_WRITER_STATS_PKTS_IN_ADD(p, 1);
 	if (p->tx_buf_count >= p->tx_burst_sz)
 		send_burst(p);
 
@@ -219,8 +237,10 @@ rte_port_ring_writer_tx_bulk(void *port,
 		if (tx_buf_count)
 			send_burst(p);
 
+		RTE_PORT_RING_WRITER_STATS_PKTS_IN_ADD(p, n_pkts);
 		n_pkts_ok = rte_ring_sp_enqueue_burst(p->ring, (void **)pkts, n_pkts);
 
+		RTE_PORT_RING_WRITER_STATS_PKTS_DROP_ADD(p, n_pkts - n_pkts_ok);
 		for ( ; n_pkts_ok < n_pkts; n_pkts_ok++) {
 			struct rte_mbuf *pkt = pkts[n_pkts_ok];
 
@@ -233,6 +253,7 @@ rte_port_ring_writer_tx_bulk(void *port,
 			struct rte_mbuf *pkt = pkts[pkt_index];
 
 			p->tx_buf[tx_buf_count++] = pkt;
+			RTE_PORT_RING_WRITER_STATS_PKTS_IN_ADD(p, 1);
 			pkts_mask &= ~pkt_mask;
 		}
 
@@ -269,6 +290,22 @@ rte_port_ring_writer_free(void *port)
 	return 0;
 }
 
+static int
+rte_port_ring_writer_stats_read(void *port,
+		struct rte_port_out_stats *stats, int clear)
+{
+	struct rte_port_ring_writer *p =
+		(struct rte_port_ring_writer *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Port RING Writer Nodrop
  */
@@ -455,6 +492,7 @@ struct rte_port_out_ops rte_port_ring_writer_ops = {
 	.f_tx = rte_port_ring_writer_tx,
 	.f_tx_bulk = rte_port_ring_writer_tx_bulk,
 	.f_flush = rte_port_ring_writer_flush,
+	.f_stats = rte_port_ring_writer_stats_read,
 };
 
 struct rte_port_out_ops rte_port_ring_writer_nodrop_ops = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 09/13] port: added port_ring_writer_nodrop stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (7 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 08/13] port: added port_ring_writer stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 10/13] port: added port_sched_reader stats Maciej Gajdzica
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for ring writer nodrop port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_ring.c |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/lib/librte_port/rte_port_ring.c b/lib/librte_port/rte_port_ring.c
index ff58009..9461c05 100644
--- a/lib/librte_port/rte_port_ring.c
+++ b/lib/librte_port/rte_port_ring.c
@@ -309,7 +309,23 @@ rte_port_ring_writer_stats_read(void *port,
 /*
  * Port RING Writer Nodrop
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_RING_WRITER_NODROP_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_RING_WRITER_NODROP_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_RING_WRITER_NODROP_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_RING_WRITER_NODROP_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_ring_writer_nodrop {
+	struct rte_port_out_stats stats;
+
 	struct rte_mbuf *tx_buf[RTE_PORT_IN_BURST_SIZE_MAX];
 	struct rte_ring *ring;
 	uint32_t tx_burst_sz;
@@ -379,6 +395,7 @@ send_burst_nodrop(struct rte_port_ring_writer_nodrop *p)
 	}
 
 	/* We didn't send the packets in maximum allowed attempts */
+	RTE_PORT_RING_WRITER_NODROP_STATS_PKTS_DROP_ADD(p, p->tx_buf_count - nb_tx);
 	for ( ; nb_tx < p->tx_buf_count; nb_tx++)
 		rte_pktmbuf_free(p->tx_buf[nb_tx]);
 
@@ -392,6 +409,7 @@ rte_port_ring_writer_nodrop_tx(void *port, struct rte_mbuf *pkt)
 			(struct rte_port_ring_writer_nodrop *) port;
 
 	p->tx_buf[p->tx_buf_count++] = pkt;
+	RTE_PORT_RING_WRITER_NODROP_STATS_PKTS_IN_ADD(p, 1);
 	if (p->tx_buf_count >= p->tx_burst_sz)
 		send_burst_nodrop(p);
 
@@ -418,6 +436,7 @@ rte_port_ring_writer_nodrop_tx_bulk(void *port,
 		if (tx_buf_count)
 			send_burst_nodrop(p);
 
+		RTE_PORT_RING_WRITER_NODROP_STATS_PKTS_IN_ADD(p, n_pkts);
 		n_pkts_ok = rte_ring_sp_enqueue_burst(p->ring, (void **)pkts, n_pkts);
 
 		if (n_pkts_ok >= n_pkts)
@@ -439,6 +458,7 @@ rte_port_ring_writer_nodrop_tx_bulk(void *port,
 			struct rte_mbuf *pkt = pkts[pkt_index];
 
 			p->tx_buf[tx_buf_count++] = pkt;
+			RTE_PORT_RING_WRITER_NODROP_STATS_PKTS_IN_ADD(p, 1);
 			pkts_mask &= ~pkt_mask;
 		}
 
@@ -476,6 +496,22 @@ rte_port_ring_writer_nodrop_free(void *port)
 	return 0;
 }
 
+static int
+rte_port_ring_writer_nodrop_stats_read(void *port,
+		struct rte_port_out_stats *stats, int clear)
+{
+	struct rte_port_ring_writer_nodrop *p =
+		(struct rte_port_ring_writer_nodrop *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Summary of port operations
  */
@@ -501,4 +537,5 @@ struct rte_port_out_ops rte_port_ring_writer_nodrop_ops = {
 	.f_tx = rte_port_ring_writer_nodrop_tx,
 	.f_tx_bulk = rte_port_ring_writer_nodrop_tx_bulk,
 	.f_flush = rte_port_ring_writer_nodrop_flush,
+	.f_stats = rte_port_ring_writer_nodrop_stats_read,
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 10/13] port: added port_sched_reader stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (8 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 09/13] port: added port_ring_writer_nodrop stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 11/13] port: added port_sched_writer stats Maciej Gajdzica
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for sched reader port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_sched.c |   39 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/lib/librte_port/rte_port_sched.c b/lib/librte_port/rte_port_sched.c
index 2107f4c..a82e4fa 100644
--- a/lib/librte_port/rte_port_sched.c
+++ b/lib/librte_port/rte_port_sched.c
@@ -40,7 +40,23 @@
 /*
  * Reader
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_SCHED_READER_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_SCHED_READER_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_SCHED_READER_PKTS_IN_ADD(port, val)
+#define RTE_PORT_SCHED_READER_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_sched_reader {
+	struct rte_port_in_stats stats;
+
 	struct rte_sched_port *sched;
 };
 
@@ -76,8 +92,12 @@ static int
 rte_port_sched_reader_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
 {
 	struct rte_port_sched_reader *p = (struct rte_port_sched_reader *) port;
+	uint32_t nb_rx;
 
-	return rte_sched_port_dequeue(p->sched, pkts, n_pkts);
+	nb_rx = rte_sched_port_dequeue(p->sched, pkts, n_pkts);
+	RTE_PORT_SCHED_READER_PKTS_IN_ADD(p, nb_rx);
+
+	return nb_rx;
 }
 
 static int
@@ -93,6 +113,22 @@ rte_port_sched_reader_free(void *port)
 	return 0;
 }
 
+static int
+rte_port_sched_reader_stats_read(void *port,
+		struct rte_port_in_stats *stats, int clear)
+{
+	struct rte_port_sched_reader *p =
+		(struct rte_port_sched_reader *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Writer
  */
@@ -228,6 +264,7 @@ struct rte_port_in_ops rte_port_sched_reader_ops = {
 	.f_create = rte_port_sched_reader_create,
 	.f_free = rte_port_sched_reader_free,
 	.f_rx = rte_port_sched_reader_rx,
+	.f_stats = rte_port_sched_reader_stats_read,
 };
 
 struct rte_port_out_ops rte_port_sched_writer_ops = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 11/13] port: added port_sched_writer stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (9 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 10/13] port: added port_sched_reader stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 12/13] port: added port_source stats Maciej Gajdzica
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for sched writer port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_sched.c |   57 ++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/lib/librte_port/rte_port_sched.c b/lib/librte_port/rte_port_sched.c
index a82e4fa..c5ff8ab 100644
--- a/lib/librte_port/rte_port_sched.c
+++ b/lib/librte_port/rte_port_sched.c
@@ -132,7 +132,23 @@ rte_port_sched_reader_stats_read(void *port,
 /*
  * Writer
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_SCHED_WRITER_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_SCHED_WRITER_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_SCHED_WRITER_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_SCHED_WRITER_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_sched_writer {
+	struct rte_port_out_stats stats;
+
 	struct rte_mbuf *tx_buf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
 	struct rte_sched_port *sched;
 	uint32_t tx_burst_sz;
@@ -180,8 +196,12 @@ rte_port_sched_writer_tx(void *port, struct rte_mbuf *pkt)
 	struct rte_port_sched_writer *p = (struct rte_port_sched_writer *) port;
 
 	p->tx_buf[p->tx_buf_count++] = pkt;
+	RTE_PORT_SCHED_WRITER_STATS_PKTS_IN_ADD(p, 1);
 	if (p->tx_buf_count >= p->tx_burst_sz) {
-		rte_sched_port_enqueue(p->sched, p->tx_buf, p->tx_buf_count);
+		__rte_unused uint32_t nb_tx;
+
+		nb_tx = rte_sched_port_enqueue(p->sched, p->tx_buf, p->tx_buf_count);
+		RTE_PORT_SCHED_WRITER_STATS_PKTS_DROP_ADD(p, p->tx_buf_count - nb_tx);
 		p->tx_buf_count = 0;
 	}
 
@@ -200,15 +220,18 @@ rte_port_sched_writer_tx_bulk(void *port,
 			((pkts_mask & bsz_mask) ^ bsz_mask);
 
 	if (expr == 0) {
+		__rte_unused uint32_t nb_tx;
 		uint64_t n_pkts = __builtin_popcountll(pkts_mask);
 
 		if (tx_buf_count) {
-			rte_sched_port_enqueue(p->sched, p->tx_buf,
+			nb_tx = rte_sched_port_enqueue(p->sched, p->tx_buf,
 				tx_buf_count);
+			RTE_PORT_SCHED_WRITER_STATS_PKTS_DROP_ADD(p, tx_buf_count - nb_tx);
 			p->tx_buf_count = 0;
 		}
 
-		rte_sched_port_enqueue(p->sched, pkts, n_pkts);
+		nb_tx = rte_sched_port_enqueue(p->sched, pkts, n_pkts);
+		RTE_PORT_SCHED_WRITER_STATS_PKTS_DROP_ADD(p, n_pkts - nb_tx);
 	} else {
 		for ( ; pkts_mask; ) {
 			uint32_t pkt_index = __builtin_ctzll(pkts_mask);
@@ -216,13 +239,17 @@ rte_port_sched_writer_tx_bulk(void *port,
 			struct rte_mbuf *pkt = pkts[pkt_index];
 
 			p->tx_buf[tx_buf_count++] = pkt;
+			RTE_PORT_SCHED_WRITER_STATS_PKTS_IN_ADD(p, 1);
 			pkts_mask &= ~pkt_mask;
 		}
 		p->tx_buf_count = tx_buf_count;
 
 		if (tx_buf_count >= p->tx_burst_sz) {
-			rte_sched_port_enqueue(p->sched, p->tx_buf,
+			__rte_unused uint32_t nb_tx;
+
+			nb_tx = rte_sched_port_enqueue(p->sched, p->tx_buf,
 				tx_buf_count);
+			RTE_PORT_SCHED_WRITER_STATS_PKTS_DROP_ADD(p, tx_buf_count - nb_tx);
 			p->tx_buf_count = 0;
 		}
 	}
@@ -236,7 +263,10 @@ rte_port_sched_writer_flush(void *port)
 	struct rte_port_sched_writer *p = (struct rte_port_sched_writer *) port;
 
 	if (p->tx_buf_count) {
-		rte_sched_port_enqueue(p->sched, p->tx_buf, p->tx_buf_count);
+		__rte_unused uint32_t nb_tx;
+
+		nb_tx = rte_sched_port_enqueue(p->sched, p->tx_buf, p->tx_buf_count);
+		RTE_PORT_SCHED_WRITER_STATS_PKTS_DROP_ADD(p, p->tx_buf_count - nb_tx);
 		p->tx_buf_count = 0;
 	}
 
@@ -257,6 +287,22 @@ rte_port_sched_writer_free(void *port)
 	return 0;
 }
 
+static int
+rte_port_sched_writer_stats_read(void *port,
+		struct rte_port_out_stats *stats, int clear)
+{
+	struct rte_port_sched_writer *p =
+		(struct rte_port_sched_writer *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Summary of port operations
  */
@@ -273,4 +319,5 @@ struct rte_port_out_ops rte_port_sched_writer_ops = {
 	.f_tx = rte_port_sched_writer_tx,
 	.f_tx_bulk = rte_port_sched_writer_tx_bulk,
 	.f_flush = rte_port_sched_writer_flush,
+	.f_stats = rte_port_sched_writer_stats_read,
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 12/13] port: added port_source stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (10 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 11/13] port: added port_sched_writer stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-19  9:41 ` [PATCH v5 13/13] port: added port_sink stats Maciej Gajdzica
  2015-06-23 10:31 ` [PATCH v5 00/13] port: added port statistics Dumitrescu, Cristian
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for source port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_source_sink.c |   35 ++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/lib/librte_port/rte_port_source_sink.c b/lib/librte_port/rte_port_source_sink.c
index b9a25bb..234ab18 100644
--- a/lib/librte_port/rte_port_source_sink.c
+++ b/lib/librte_port/rte_port_source_sink.c
@@ -42,7 +42,23 @@
 /*
  * Port SOURCE
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_SOURCE_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_SOURCE_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_SOURCE_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_SOURCE_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
 struct rte_port_source {
+	struct rte_port_in_stats stats;
+
 	struct rte_mempool *mempool;
 };
 
@@ -93,9 +109,27 @@ rte_port_source_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
 	if (rte_mempool_get_bulk(p->mempool, (void **) pkts, n_pkts) != 0)
 		return 0;
 
+	RTE_PORT_SOURCE_STATS_PKTS_IN_ADD(p, n_pkts);
+
 	return n_pkts;
 }
 
+static int
+rte_port_source_stats_read(void *port,
+		struct rte_port_in_stats *stats, int clear)
+{
+	struct rte_port_source *p =
+		(struct rte_port_source *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Port SINK
  */
@@ -147,6 +181,7 @@ struct rte_port_in_ops rte_port_source_ops = {
 	.f_create = rte_port_source_create,
 	.f_free = rte_port_source_free,
 	.f_rx = rte_port_source_rx,
+	.f_stats = rte_port_source_stats_read,
 };
 
 struct rte_port_out_ops rte_port_sink_ops = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v5 13/13] port: added port_sink stats
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (11 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 12/13] port: added port_source stats Maciej Gajdzica
@ 2015-06-19  9:41 ` Maciej Gajdzica
  2015-06-23 10:31 ` [PATCH v5 00/13] port: added port statistics Dumitrescu, Cristian
  13 siblings, 0 replies; 22+ messages in thread
From: Maciej Gajdzica @ 2015-06-19  9:41 UTC (permalink / raw)
  To: dev

Added statistics for sink port.

Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
---
 lib/librte_port/rte_port_source_sink.c |   63 ++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 4 deletions(-)

diff --git a/lib/librte_port/rte_port_source_sink.c b/lib/librte_port/rte_port_source_sink.c
index 234ab18..0a70228 100644
--- a/lib/librte_port/rte_port_source_sink.c
+++ b/lib/librte_port/rte_port_source_sink.c
@@ -133,28 +133,64 @@ rte_port_source_stats_read(void *port,
 /*
  * Port SINK
  */
+#ifdef RTE_PORT_STATS_COLLECT
+
+#define RTE_PORT_SINK_STATS_PKTS_IN_ADD(port, val) \
+	port->stats.n_pkts_in += val
+#define RTE_PORT_SINK_STATS_PKTS_DROP_ADD(port, val) \
+	port->stats.n_pkts_drop += val
+
+#else
+
+#define RTE_PORT_SINK_STATS_PKTS_IN_ADD(port, val)
+#define RTE_PORT_SINK_STATS_PKTS_DROP_ADD(port, val)
+
+#endif
+
+struct rte_port_sink {
+	struct rte_port_out_stats stats;
+};
+
 static void *
-rte_port_sink_create(__rte_unused void *params, __rte_unused int socket_id)
+rte_port_sink_create(__rte_unused void *params, int socket_id)
 {
-	return (void *) 1;
+	struct rte_port_sink *port;
+
+	/* Memory allocation */
+	port = rte_zmalloc_socket("PORT", sizeof(*port),
+			RTE_CACHE_LINE_SIZE, socket_id);
+	if (port == NULL) {
+		RTE_LOG(ERR, PORT, "%s: Failed to allocate port\n", __func__);
+		return NULL;
+	}
+
+	return port;
 }
 
 static int
-rte_port_sink_tx(__rte_unused void *port, struct rte_mbuf *pkt)
+rte_port_sink_tx(void *port, struct rte_mbuf *pkt)
 {
+	__rte_unused struct rte_port_sink *p = (struct rte_port_sink *) port;
+
+	RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
 	rte_pktmbuf_free(pkt);
+	RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
 
 	return 0;
 }
 
 static int
-rte_port_sink_tx_bulk(__rte_unused void *port, struct rte_mbuf **pkts,
+rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
 	uint64_t pkts_mask)
 {
+	__rte_unused struct rte_port_sink *p = (struct rte_port_sink *) port;
+
 	if ((pkts_mask & (pkts_mask + 1)) == 0) {
 		uint64_t n_pkts = __builtin_popcountll(pkts_mask);
 		uint32_t i;
 
+		RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, n_pkts);
+		RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, n_pkts);
 		for (i = 0; i < n_pkts; i++) {
 			struct rte_mbuf *pkt = pkts[i];
 
@@ -166,6 +202,8 @@ rte_port_sink_tx_bulk(__rte_unused void *port, struct rte_mbuf **pkts,
 			uint64_t pkt_mask = 1LLU << pkt_index;
 			struct rte_mbuf *pkt = pkts[pkt_index];
 
+			RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
+			RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
 			rte_pktmbuf_free(pkt);
 			pkts_mask &= ~pkt_mask;
 		}
@@ -174,6 +212,22 @@ rte_port_sink_tx_bulk(__rte_unused void *port, struct rte_mbuf **pkts,
 	return 0;
 }
 
+static int
+rte_port_sink_stats_read(void *port, struct rte_port_out_stats *stats,
+		int clear)
+{
+	struct rte_port_sink *p =
+		(struct rte_port_sink *) port;
+
+	if (stats != NULL)
+		memcpy(stats, &p->stats, sizeof(p->stats));
+
+	if (clear)
+		memset(&p->stats, 0, sizeof(p->stats));
+
+	return 0;
+}
+
 /*
  * Summary of port operations
  */
@@ -190,4 +244,5 @@ struct rte_port_out_ops rte_port_sink_ops = {
 	.f_tx = rte_port_sink_tx,
 	.f_tx_bulk = rte_port_sink_tx_bulk,
 	.f_flush = NULL,
+	.f_stats = rte_port_sink_stats_read,
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v5 01/13] port: added structures for port stats and config option
  2015-06-19  9:41 ` [PATCH v5 01/13] port: added structures for port stats and config option Maciej Gajdzica
@ 2015-06-23 10:26   ` Dumitrescu, Cristian
  2015-06-23 13:55   ` Thomas Monjalon
  1 sibling, 0 replies; 22+ messages in thread
From: Dumitrescu, Cristian @ 2015-06-23 10:26 UTC (permalink / raw)
  To: Gajdzica, MaciejX T, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maciej Gajdzica
> Sent: Friday, June 19, 2015 10:41 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v5 01/13] port: added structures for port stats
> and config option
> 
> Added common data structures for port statistics. Added config option to
> enable stats collecting.
> 
> Signed-off-by: Maciej Gajdzica <maciejx.t.gajdzica@intel.com>
> ---

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v5 00/13] port: added port statistics
  2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
                   ` (12 preceding siblings ...)
  2015-06-19  9:41 ` [PATCH v5 13/13] port: added port_sink stats Maciej Gajdzica
@ 2015-06-23 10:31 ` Dumitrescu, Cristian
  2015-06-23 21:05   ` Thomas Monjalon
  13 siblings, 1 reply; 22+ messages in thread
From: Dumitrescu, Cristian @ 2015-06-23 10:31 UTC (permalink / raw)
  To: Gajdzica, MaciejX T, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maciej Gajdzica
> Sent: Friday, June 19, 2015 10:41 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v5 00/13] port: added port statistics
> 
> Added statistics for every type of port. By default all port statistics
> are disabled, user must activate them in config file.
> 
> Changes in v5:
> 	- added missing CONFIG_ prefix to defines in config files
> 

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v5 01/13] port: added structures for port stats and config option
  2015-06-19  9:41 ` [PATCH v5 01/13] port: added structures for port stats and config option Maciej Gajdzica
  2015-06-23 10:26   ` Dumitrescu, Cristian
@ 2015-06-23 13:55   ` Thomas Monjalon
  2015-06-23 14:30     ` Dumitrescu, Cristian
  2015-06-23 15:16     ` Neil Horman
  1 sibling, 2 replies; 22+ messages in thread
From: Thomas Monjalon @ 2015-06-23 13:55 UTC (permalink / raw)
  To: Maciej Gajdzica; +Cc: dev

2015-06-19 11:41, Maciej Gajdzica:
>  /** Input port interface defining the input port operation */
>  struct rte_port_in_ops {
>  	rte_port_in_op_create f_create; /**< Create */
>  	rte_port_in_op_free f_free;     /**< Free */
>  	rte_port_in_op_rx f_rx;         /**< Packet RX (packet burst) */
> +	rte_port_in_op_stats_read f_stats;	/**< Stats */
>  };

Isn't it breaking an ABI?

>  struct rte_port_out_ops {
> -	rte_port_out_op_create f_create;   /**< Create */
> -	rte_port_out_op_free f_free;       /**< Free */
> -	rte_port_out_op_tx f_tx;           /**< Packet TX (single packet) */
> -	rte_port_out_op_tx_bulk f_tx_bulk; /**< Packet TX (packet burst) */
> -	rte_port_out_op_flush f_flush;     /**< Flush */
> +	rte_port_out_op_create f_create;		/**< Create */
> +	rte_port_out_op_free f_free;			/**< Free */
> +	rte_port_out_op_tx f_tx;				/**< Packet TX (single packet) */
> +	rte_port_out_op_tx_bulk f_tx_bulk;		/**< Packet TX (packet burst) */
> +	rte_port_out_op_flush f_flush;			/**< Flush */

What is the goal of this change? Breaking the alignment?

> +	rte_port_out_op_stats_read f_stats;     /**< Stats */

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v5 01/13] port: added structures for port stats and config option
  2015-06-23 13:55   ` Thomas Monjalon
@ 2015-06-23 14:30     ` Dumitrescu, Cristian
  2015-06-23 14:54       ` Thomas Monjalon
  2015-06-23 15:16     ` Neil Horman
  1 sibling, 1 reply; 22+ messages in thread
From: Dumitrescu, Cristian @ 2015-06-23 14:30 UTC (permalink / raw)
  To: Thomas Monjalon, Gajdzica, MaciejX T; +Cc: dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, June 23, 2015 2:55 PM
> To: Gajdzica, MaciejX T
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 01/13] port: added structures for port
> stats and config option
> 
> 2015-06-19 11:41, Maciej Gajdzica:
> >  /** Input port interface defining the input port operation */
> >  struct rte_port_in_ops {
> >  	rte_port_in_op_create f_create; /**< Create */
> >  	rte_port_in_op_free f_free;     /**< Free */
> >  	rte_port_in_op_rx f_rx;         /**< Packet RX (packet burst) */
> > +	rte_port_in_op_stats_read f_stats;	/**< Stats */
> >  };
> 
> Isn't it breaking an ABI?

This is simply adding a field at the end of the API structure. This structure is instantiated per each port type  and its role is very similar to a driver ops structure, for example:

	in file "rte_port_ethdev.h": extern struct rte_port_out_ops rte_port_ethdev_writer_ops;
	in file "rte_port_ring.h": extern struct rte_port_out_ops rte_port_ring_writer_nodrop_ops;

Typically, instances of these structures are only referenced through pointers by application code (and other libraries, like librte_pipeline), so code that is not aware of this last field in the structure will still continue to work.

The only case I see possible when code will break is if somebody would create an array of such structures, but I think this is not a realistic scenario. Instances of this structure are infrequent: once per port type in librte_port, and new instances are only created when the user wants to create new port type. Basically, instances of this structure are created in isolation and not in bulk (arrays).

Due to this, I do not see this as breaking the API. I think this is the most it could be done to minimize the effect on the ABI will still adding new functionality. Please let me know what you think.

> 
> >  struct rte_port_out_ops {
> > -	rte_port_out_op_create f_create;   /**< Create */
> > -	rte_port_out_op_free f_free;       /**< Free */
> > -	rte_port_out_op_tx f_tx;           /**< Packet TX (single packet) */
> > -	rte_port_out_op_tx_bulk f_tx_bulk; /**< Packet TX (packet burst)
> */
> > -	rte_port_out_op_flush f_flush;     /**< Flush */
> > +	rte_port_out_op_create f_create;		/**< Create */
> > +	rte_port_out_op_free f_free;			/**< Free */
> > +	rte_port_out_op_tx f_tx;				/**< Packet
> TX (single packet) */
> > +	rte_port_out_op_tx_bulk f_tx_bulk;		/**< Packet TX
> (packet burst) */
> > +	rte_port_out_op_flush f_flush;			/**< Flush */
> 
> What is the goal of this change? Breaking the alignment?

Shall we submit a new patch revision to fix the alignment of the comments?

> 
> > +	rte_port_out_op_stats_read f_stats;     /**< Stats */

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v5 01/13] port: added structures for port stats and config option
  2015-06-23 14:30     ` Dumitrescu, Cristian
@ 2015-06-23 14:54       ` Thomas Monjalon
  2015-06-23 15:21         ` Dumitrescu, Cristian
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2015-06-23 14:54 UTC (permalink / raw)
  To: Dumitrescu, Cristian; +Cc: dev

2015-06-23 14:30, Dumitrescu, Cristian:
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > 2015-06-19 11:41, Maciej Gajdzica:
> > >  /** Input port interface defining the input port operation */
> > >  struct rte_port_in_ops {
> > >  	rte_port_in_op_create f_create; /**< Create */
> > >  	rte_port_in_op_free f_free;     /**< Free */
> > >  	rte_port_in_op_rx f_rx;         /**< Packet RX (packet burst) */
> > > +	rte_port_in_op_stats_read f_stats;	/**< Stats */
> > >  };
> > 
> > Isn't it breaking an ABI?
> 
> This is simply adding a field at the end of the API structure. This structure is instantiated per each port type  and its role is very similar to a driver ops structure, for example:
> 
> 	in file "rte_port_ethdev.h": extern struct rte_port_out_ops rte_port_ethdev_writer_ops;
> 	in file "rte_port_ring.h": extern struct rte_port_out_ops rte_port_ring_writer_nodrop_ops;
> 
> Typically, instances of these structures are only referenced through pointers by application code (and other libraries, like librte_pipeline), so code that is not aware of this last field in the structure will still continue to work.
> 
> The only case I see possible when code will break is if somebody would create an array of such structures, but I think this is not a realistic scenario. Instances of this structure are infrequent: once per port type in librte_port, and new instances are only created when the user wants to create new port type. Basically, instances of this structure are created in isolation and not in bulk (arrays).

Why wouldn't it be a problem even for single instance?
If the application allocates one with old sizeof and the lib is trying to write
in the new field, there can be a problem, no?

Maybe Neil has an opinion?

> Due to this, I do not see this as breaking the API. I think this is the most it could be done to minimize the effect on the ABI will still adding new functionality. Please let me know what you think.
> 
> > 
> > >  struct rte_port_out_ops {
> > > -	rte_port_out_op_create f_create;   /**< Create */
> > > -	rte_port_out_op_free f_free;       /**< Free */
> > > -	rte_port_out_op_tx f_tx;           /**< Packet TX (single packet) */
> > > -	rte_port_out_op_tx_bulk f_tx_bulk; /**< Packet TX (packet burst)
> > */
> > > -	rte_port_out_op_flush f_flush;     /**< Flush */
> > > +	rte_port_out_op_create f_create;		/**< Create */
> > > +	rte_port_out_op_free f_free;			/**< Free */
> > > +	rte_port_out_op_tx f_tx;				/**< Packet
> > TX (single packet) */
> > > +	rte_port_out_op_tx_bulk f_tx_bulk;		/**< Packet TX
> > (packet burst) */
> > > +	rte_port_out_op_flush f_flush;			/**< Flush */
> > 
> > What is the goal of this change? Breaking the alignment?
> 
> Shall we submit a new patch revision to fix the alignment of the comments?

Yes using spaces for alignment would be better.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v5 01/13] port: added structures for port stats and config option
  2015-06-23 13:55   ` Thomas Monjalon
  2015-06-23 14:30     ` Dumitrescu, Cristian
@ 2015-06-23 15:16     ` Neil Horman
  1 sibling, 0 replies; 22+ messages in thread
From: Neil Horman @ 2015-06-23 15:16 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Tue, Jun 23, 2015 at 03:55:25PM +0200, Thomas Monjalon wrote:
> 2015-06-19 11:41, Maciej Gajdzica:
> >  /** Input port interface defining the input port operation */
> >  struct rte_port_in_ops {
> >  	rte_port_in_op_create f_create; /**< Create */
> >  	rte_port_in_op_free f_free;     /**< Free */
> >  	rte_port_in_op_rx f_rx;         /**< Packet RX (packet burst) */
> > +	rte_port_in_op_stats_read f_stats;	/**< Stats */
> >  };
> 
> Isn't it breaking an ABI?
> 
This is an interesting question.  Strictly speaking, yes it breaks ABI because
we're adding space, and if older applications statically allocate this
structure, it will be smaller than the port library expects, potentially
scribbling over someone elses memory.  That said, I'm not sure this structure is
meant to be accessed outside of the library.  If it isn't, then we can feel
comfortable that no one is going to access the data structure from code that was
compiled out of sync with the defining library.

The implication if thats true of course is that we should make this structure
opaque outside of the library with a structure prototype and move its definition
into the library private namespace, but I'm fine with doing that at a later date
if the intention is not to have applications touch this structure.

Regards
Neil

> >  struct rte_port_out_ops {
> > -	rte_port_out_op_create f_create;   /**< Create */
> > -	rte_port_out_op_free f_free;       /**< Free */
> > -	rte_port_out_op_tx f_tx;           /**< Packet TX (single packet) */
> > -	rte_port_out_op_tx_bulk f_tx_bulk; /**< Packet TX (packet burst) */
> > -	rte_port_out_op_flush f_flush;     /**< Flush */
> > +	rte_port_out_op_create f_create;		/**< Create */
> > +	rte_port_out_op_free f_free;			/**< Free */
> > +	rte_port_out_op_tx f_tx;				/**< Packet TX (single packet) */
> > +	rte_port_out_op_tx_bulk f_tx_bulk;		/**< Packet TX (packet burst) */
> > +	rte_port_out_op_flush f_flush;			/**< Flush */
> 
> What is the goal of this change? Breaking the alignment?
> 
> > +	rte_port_out_op_stats_read f_stats;     /**< Stats */
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v5 01/13] port: added structures for port stats and config option
  2015-06-23 14:54       ` Thomas Monjalon
@ 2015-06-23 15:21         ` Dumitrescu, Cristian
  0 siblings, 0 replies; 22+ messages in thread
From: Dumitrescu, Cristian @ 2015-06-23 15:21 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, June 23, 2015 3:55 PM
> To: Dumitrescu, Cristian
> Cc: Gajdzica, MaciejX T; dev@dpdk.org; nhorman@tuxdriver.com
> Subject: Re: [dpdk-dev] [PATCH v5 01/13] port: added structures for port
> stats and config option
> 
> 2015-06-23 14:30, Dumitrescu, Cristian:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > > 2015-06-19 11:41, Maciej Gajdzica:
> > > >  /** Input port interface defining the input port operation */
> > > >  struct rte_port_in_ops {
> > > >  	rte_port_in_op_create f_create; /**< Create */
> > > >  	rte_port_in_op_free f_free;     /**< Free */
> > > >  	rte_port_in_op_rx f_rx;         /**< Packet RX (packet burst) */
> > > > +	rte_port_in_op_stats_read f_stats;	/**< Stats */
> > > >  };
> > >
> > > Isn't it breaking an ABI?
> >
> > This is simply adding a field at the end of the API structure. This structure is
> instantiated per each port type  and its role is very similar to a driver ops
> structure, for example:
> >
> > 	in file "rte_port_ethdev.h": extern struct rte_port_out_ops
> rte_port_ethdev_writer_ops;
> > 	in file "rte_port_ring.h": extern struct rte_port_out_ops
> rte_port_ring_writer_nodrop_ops;
> >
> > Typically, instances of these structures are only referenced through
> pointers by application code (and other libraries, like librte_pipeline), so code
> that is not aware of this last field in the structure will still continue to work.
> >
> > The only case I see possible when code will break is if somebody would
> create an array of such structures, but I think this is not a realistic scenario.
> Instances of this structure are infrequent: once per port type in librte_port,
> and new instances are only created when the user wants to create new port
> type. Basically, instances of this structure are created in isolation and not in
> bulk (arrays).
> 
> Why wouldn't it be a problem even for single instance?
> If the application allocates one with old sizeof and the lib is trying to write
> in the new field, there can be a problem, no?
> 

The only case when the application is required to create a new instance of this structure is when the application is defining a new port type (unlikely). In this case, the application using the old structure layout is not aware about the statistics functionality, so it will not invoke it, so the library will not attempt to read the f_stats structure field. Since this field is immediately after the old structure layout, the other fields in the structure are not disturbed, so the application works just fine.

The only case when the application using the old structure layout is impacted is when the position of the old structure fields changes, and this can only happen when an array of such structures is created. To my earlier point, this is not realistic, as instances of this structure are created in isolation (single instance, not array of instances) and are accessed through pointers.

> Maybe Neil has an opinion?
> 
> > Due to this, I do not see this as breaking the API. I think this is the most it
> could be done to minimize the effect on the ABI will still adding new
> functionality. Please let me know what you think.
> >
> > >
> > > >  struct rte_port_out_ops {
> > > > -	rte_port_out_op_create f_create;   /**< Create */
> > > > -	rte_port_out_op_free f_free;       /**< Free */
> > > > -	rte_port_out_op_tx f_tx;           /**< Packet TX (single packet) */
> > > > -	rte_port_out_op_tx_bulk f_tx_bulk; /**< Packet TX (packet burst)
> > > */
> > > > -	rte_port_out_op_flush f_flush;     /**< Flush */
> > > > +	rte_port_out_op_create f_create;		/**< Create */
> > > > +	rte_port_out_op_free f_free;			/**< Free */
> > > > +	rte_port_out_op_tx f_tx;				/**< Packet
> > > TX (single packet) */
> > > > +	rte_port_out_op_tx_bulk f_tx_bulk;		/**< Packet TX
> > > (packet burst) */
> > > > +	rte_port_out_op_flush f_flush;			/**< Flush */
> > >
> > > What is the goal of this change? Breaking the alignment?
> >
> > Shall we submit a new patch revision to fix the alignment of the
> comments?
> 
> Yes using spaces for alignment would be better.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v5 00/13] port: added port statistics
  2015-06-23 10:31 ` [PATCH v5 00/13] port: added port statistics Dumitrescu, Cristian
@ 2015-06-23 21:05   ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2015-06-23 21:05 UTC (permalink / raw)
  To: Gajdzica, MaciejX T; +Cc: dev

> > Added statistics for every type of port. By default all port statistics
> > are disabled, user must activate them in config file.
> > 
> > Changes in v5:
> > 	- added missing CONFIG_ prefix to defines in config files
> 
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-06-23 21:06 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-19  9:41 [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 01/13] port: added structures for port stats and config option Maciej Gajdzica
2015-06-23 10:26   ` Dumitrescu, Cristian
2015-06-23 13:55   ` Thomas Monjalon
2015-06-23 14:30     ` Dumitrescu, Cristian
2015-06-23 14:54       ` Thomas Monjalon
2015-06-23 15:21         ` Dumitrescu, Cristian
2015-06-23 15:16     ` Neil Horman
2015-06-19  9:41 ` [PATCH v5 02/13] port: added port_ethdev_reader stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 03/13] port: added port_ethdev_writer stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 04/13] port: added port_ethdev_writer_nodrop stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 05/13] port: added port_frag stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 06/13] port: added port_ras stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 07/13] port: added port_ring_reader stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 08/13] port: added port_ring_writer stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 09/13] port: added port_ring_writer_nodrop stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 10/13] port: added port_sched_reader stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 11/13] port: added port_sched_writer stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 12/13] port: added port_source stats Maciej Gajdzica
2015-06-19  9:41 ` [PATCH v5 13/13] port: added port_sink stats Maciej Gajdzica
2015-06-23 10:31 ` [PATCH v5 00/13] port: added port statistics Dumitrescu, Cristian
2015-06-23 21:05   ` Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.