DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/7] rte_sched: make RED optional at runtime
@ 2015-02-01 10:03 Stephen Hemminger
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's Stephen Hemminger
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Stephen Hemminger @ 2015-02-01 10:03 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

From: Stephen Hemminger <shemming@brocade.com>

Want to be able to build with RTE_SCHED_RED enabled but
allow disabling RED on a per-queue basis at runtime.

RED is disabled unless min/max thresholds set.

Signed-off-by: Stephen Hemmminger <stephen@networkplumber.org>
---
 lib/librte_sched/rte_sched.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index 95dee27..6928c98 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -636,6 +636,12 @@ rte_sched_port_config(struct rte_sched_port_params *params)
 		uint32_t j;
 
 		for (j = 0; j < e_RTE_METER_COLORS; j++) {
+			/* if min/max are both zero, then RED is disabled */
+			if ((params->red_params[i][j].min_th |
+			     params->red_params[i][j].max_th) == 0) {
+				continue;
+			}
+
 			if (rte_red_config_init(&port->red_config[i][j],
 				params->red_params[i][j].wq_log2,
 				params->red_params[i][j].min_th,
@@ -1069,6 +1075,9 @@ rte_sched_port_red_drop(struct rte_sched_port *port, struct rte_mbuf *pkt, uint3
 	color = rte_sched_port_pkt_read_color(pkt);
 	red_cfg = &port->red_config[tc_index][color];
 
+	if ( (red_cfg->min_th | red_cfg->max_th) == 0)
+		return 0;
+
 	qe = port->queue_extra + qindex;
 	red = &qe->red;
 
-- 
2.1.4

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

* [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's
  2015-02-01 10:03 [dpdk-dev] [PATCH 1/7] rte_sched: make RED optional at runtime Stephen Hemminger
@ 2015-02-01 10:03 ` Stephen Hemminger
       [not found]   ` <2601191342CEEE43887BDE71AB977258213E2822@irsmsx105.ger.corp.intel.com>
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 3/7] rte_sched: keep track of RED drops Stephen Hemminger
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2015-02-01 10:03 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

From: Stephen Hemminger <shemming@brocade.com>

The QoS subport is limited to 8 bits in original code.
But customers demanded ability to support full number of VLAN's (4096)
therefore use reserved field of mbuf for this field instead
of packing inside other classify portions.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_mbuf/rte_mbuf.h   |  2 +-
 lib/librte_sched/rte_sched.h | 31 ++++++++++++++++++++-----------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 16059c6..b6b08f4 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -242,7 +242,7 @@ struct rte_mbuf {
 	uint16_t data_len;        /**< Amount of data in segment buffer. */
 	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
 	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
-	uint16_t reserved;
+	uint16_t subport;	  /**< SCHED Subport ID */
 	union {
 		uint32_t rss;     /**< RSS hash result if RSS enabled */
 		struct {
diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
index e6bba22..0688422 100644
--- a/lib/librte_sched/rte_sched.h
+++ b/lib/librte_sched/rte_sched.h
@@ -195,15 +195,18 @@ struct rte_sched_port_params {
 #endif
 };
 
-/** Path through the scheduler hierarchy used by the scheduler enqueue operation to
-identify the destination queue for the current packet. Stored in the field hash.sched
-of struct rte_mbuf of each packet, typically written by the classification stage and read by
-scheduler enqueue.*/
+/*
+ * Path through the scheduler hierarchy used by the scheduler enqueue
+ * operation to identify the destination queue for the current
+ * packet. Stored in the field pkt.hash.sched of struct rte_mbuf of
+ * each packet, typically written by the classification stage and read
+ * by scheduler enqueue.
+ */
 struct rte_sched_port_hierarchy {
 	uint32_t queue:2;                /**< Queue ID (0 .. 3) */
 	uint32_t traffic_class:2;        /**< Traffic class ID (0 .. 3)*/
 	uint32_t pipe:20;                /**< Pipe ID */
-	uint32_t subport:6;              /**< Subport ID */
+	uint32_t extra:6;		 /**< currently unused */
 	uint32_t color:2;                /**< Color */
 };
 
@@ -350,12 +353,15 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
  */
 static inline void
 rte_sched_port_pkt_write(struct rte_mbuf *pkt,
-	uint32_t subport, uint32_t pipe, uint32_t traffic_class, uint32_t queue, enum rte_meter_color color)
+			 uint32_t subport, uint32_t pipe,
+			 uint32_t traffic_class,
+			 uint32_t queue, enum rte_meter_color color)
 {
-	struct rte_sched_port_hierarchy *sched = (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
+	struct rte_sched_port_hierarchy *sched
+		= (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
 
+	pkt->subport = subport;
 	sched->color = (uint32_t) color;
-	sched->subport = subport;
 	sched->pipe = pipe;
 	sched->traffic_class = traffic_class;
 	sched->queue = queue;
@@ -379,11 +385,14 @@ rte_sched_port_pkt_write(struct rte_mbuf *pkt,
  *
  */
 static inline void
-rte_sched_port_pkt_read_tree_path(struct rte_mbuf *pkt, uint32_t *subport, uint32_t *pipe, uint32_t *traffic_class, uint32_t *queue)
+rte_sched_port_pkt_read_tree_path(struct rte_mbuf *pkt, uint32_t *subport,
+				  uint32_t *pipe, uint32_t *traffic_class,
+				  uint32_t *queue)
 {
-	struct rte_sched_port_hierarchy *sched = (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
+	struct rte_sched_port_hierarchy *sched
+		= (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
 
-	*subport = sched->subport;
+	*subport = pkt->subport;
 	*pipe = sched->pipe;
 	*traffic_class = sched->traffic_class;
 	*queue = sched->queue;
-- 
2.1.4

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

* [dpdk-dev] [PATCH 3/7] rte_sched: keep track of RED drops
  2015-02-01 10:03 [dpdk-dev] [PATCH 1/7] rte_sched: make RED optional at runtime Stephen Hemminger
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's Stephen Hemminger
@ 2015-02-01 10:03 ` Stephen Hemminger
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 4/7] rte_sched: don't clear statistics when read Stephen Hemminger
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2015-02-01 10:03 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

From: Stephen Hemminger <shemming@brocade.com>

Add new statistic to keep track of drops due to RED.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_sched/rte_sched.c | 28 +++++++++++++++++++++++-----
 lib/librte_sched/rte_sched.h |  6 ++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index 6928c98..8cb8bf1 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -1028,7 +1028,9 @@ rte_sched_port_update_subport_stats(struct rte_sched_port *port, uint32_t qindex
 }
 
 static inline void
-rte_sched_port_update_subport_stats_on_drop(struct rte_sched_port *port, uint32_t qindex, struct rte_mbuf *pkt)
+rte_sched_port_update_subport_stats_on_drop(struct rte_sched_port *port,
+					    uint32_t qindex,
+					    struct rte_mbuf *pkt, uint32_t red)
 {
 	struct rte_sched_subport *s = port->subport + (qindex / rte_sched_port_queues_per_subport(port));
 	uint32_t tc_index = (qindex >> 2) & 0x3;
@@ -1036,6 +1038,9 @@ rte_sched_port_update_subport_stats_on_drop(struct rte_sched_port *port, uint32_
 
 	s->stats.n_pkts_tc_dropped[tc_index] += 1;
 	s->stats.n_bytes_tc_dropped[tc_index] += pkt_len;
+#ifdef RTE_SCHED_RED
+	s->stats.n_pkts_red_dropped[tc_index] += red;
+#endif
 }
 
 static inline void
@@ -1049,13 +1054,18 @@ rte_sched_port_update_queue_stats(struct rte_sched_port *port, uint32_t qindex,
 }
 
 static inline void
-rte_sched_port_update_queue_stats_on_drop(struct rte_sched_port *port, uint32_t qindex, struct rte_mbuf *pkt)
+rte_sched_port_update_queue_stats_on_drop(struct rte_sched_port *port,
+					  uint32_t qindex,
+					  struct rte_mbuf *pkt, uint32_t red)
 {
 	struct rte_sched_queue_extra *qe = port->queue_extra + qindex;
 	uint32_t pkt_len = pkt->pkt_len;
 
 	qe->stats.n_pkts_dropped += 1;
 	qe->stats.n_bytes_dropped += pkt_len;
+#ifdef RTE_SCHED_RED
+	qe->stats.n_pkts_red_dropped += red;
+#endif
 }
 
 #endif /* RTE_SCHED_COLLECT_STATS */
@@ -1206,12 +1216,20 @@ rte_sched_port_enqueue_qwa(struct rte_sched_port *port, uint32_t qindex, struct
 	qlen = q->qw - q->qr;
 
 	/* Drop the packet (and update drop stats) when queue is full */
-	if (unlikely(rte_sched_port_red_drop(port, pkt, qindex, qlen) || (qlen >= qsize))) {
+	if (unlikely(rte_sched_port_red_drop(port, pkt, qindex, qlen))) {
+#ifdef RTE_SCHED_COLLECT_STATS
+		rte_sched_port_update_subport_stats_on_drop(port, qindex, pkt, 1);
+		rte_sched_port_update_queue_stats_on_drop(port, qindex, pkt, 1);
+#endif
 		rte_pktmbuf_free(pkt);
+	}
+
+	if (qlen >= qsize) {
 #ifdef RTE_SCHED_COLLECT_STATS
-		rte_sched_port_update_subport_stats_on_drop(port, qindex, pkt);
-		rte_sched_port_update_queue_stats_on_drop(port, qindex, pkt);
+		rte_sched_port_update_subport_stats_on_drop(port, qindex, pkt, 0);
+		rte_sched_port_update_queue_stats_on_drop(port, qindex, pkt, 0);
 #endif
+		rte_pktmbuf_free(pkt);
 		return 0;
 	}
 
diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
index 0688422..d5a1d5b 100644
--- a/lib/librte_sched/rte_sched.h
+++ b/lib/librte_sched/rte_sched.h
@@ -140,6 +140,9 @@ struct rte_sched_subport_stats {
 	                                      subport for each traffic class*/
 	uint32_t n_bytes_tc_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE]; /**< Number of bytes dropped by the current
                                           subport for each traffic class due to subport queues being full or congested */
+#ifdef RTE_SCHED_RED
+	uint32_t n_pkts_red_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE]; /**< Number of packets dropped by red */
+#endif
 };
 
 /** Pipe configuration parameters. The period and credits_per_period parameters are measured
@@ -168,6 +171,9 @@ struct rte_sched_queue_stats {
 	/* Packets */
 	uint32_t n_pkts;                 /**< Number of packets successfully written to current queue */
 	uint32_t n_pkts_dropped;         /**< Number of packets dropped due to current queue being full or congested */
+#ifdef RTE_SCHED_RED
+	uint32_t n_pkts_red_dropped;
+#endif
 
 	/* Bytes */
 	uint32_t n_bytes;                /**< Number of bytes successfully written to current queue */
-- 
2.1.4

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

* [dpdk-dev] [PATCH 4/7] rte_sched: don't clear statistics when read
  2015-02-01 10:03 [dpdk-dev] [PATCH 1/7] rte_sched: make RED optional at runtime Stephen Hemminger
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's Stephen Hemminger
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 3/7] rte_sched: keep track of RED drops Stephen Hemminger
@ 2015-02-01 10:03 ` Stephen Hemminger
  2015-02-01 14:25   ` Neil Horman
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 5/7] rte_sched: don't put tabs in log messages Stephen Hemminger
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2015-02-01 10:03 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

From: Stephen Hemminger <shemming@brocade.com>

Make rte_sched statistics API work like the ethernet statistics API.
Don't auto-clear statistics.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_sched/rte_sched.c | 30 ++++++++++++++++++++++++++++++
 lib/librte_sched/rte_sched.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index 8cb8bf1..d891e50 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -935,6 +935,21 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
 }
 
 int
+rte_sched_subport_stats_reset(struct rte_sched_port *port,
+			      uint32_t subport_id)
+{
+	struct rte_sched_subport *s;
+
+	/* Check user parameters */
+	if (port == NULL || subport_id >= port->n_subports_per_port)
+		return -1;
+
+	s = port->subport + subport_id;
+	memset(&s->stats, 0, sizeof(struct rte_sched_subport_stats));
+	return 0;
+}
+
+int
 rte_sched_queue_read_stats(struct rte_sched_port *port,
 	uint32_t queue_id,
 	struct rte_sched_queue_stats *stats,
@@ -963,6 +978,21 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
 	return 0;
 }
 
+int
+rte_sched_queue_stats_reset(struct rte_sched_port *port,
+			    uint32_t queue_id)
+{
+	struct rte_sched_queue_extra *qe;
+
+	/* Check user parameters */
+	if (port == NULL || queue_id >= rte_sched_port_queues_per_port(port))
+		return -1;
+
+	qe = port->queue_extra + queue_id;
+	memset(&qe->stats, 0, sizeof(struct rte_sched_queue_stats));
+	return 0;
+}
+
 static inline uint32_t
 rte_sched_port_qindex(struct rte_sched_port *port, uint32_t subport, uint32_t pipe, uint32_t traffic_class, uint32_t queue)
 {
diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
index d5a1d5b..64b4dd6 100644
--- a/lib/librte_sched/rte_sched.h
+++ b/lib/librte_sched/rte_sched.h
@@ -316,6 +316,21 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
 	struct rte_sched_subport_stats *stats,
 	uint32_t *tc_ov);
 
+
+/**
+ * Hierarchical scheduler subport statistics reset
+ *
+ * @param port
+ *   Handle to port scheduler instance
+ * @param subport_id
+ *   Subport ID
+ * @return
+ *   0 upon success, error code otherwise
+ */
+int
+rte_sched_subport_stats_reset(struct rte_sched_port *port,
+			      uint32_t subport_id);
+
 /**
  * Hierarchical scheduler queue statistics read
  *
@@ -337,6 +352,20 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
 	struct rte_sched_queue_stats *stats,
 	uint16_t *qlen);
 
+/**
+ * Hierarchical scheduler queue statistics reset
+ *
+ * @param port
+ *   Handle to port scheduler instance
+ * @param queue_id
+ *   Queue ID within port scheduler
+ * @return
+ *   0 upon success, error code otherwise
+ */
+int
+rte_sched_queue_stats_reset(struct rte_sched_port *port,
+			    uint32_t queue_id);
+
 /*
  * Run-time
  *
-- 
2.1.4

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

* [dpdk-dev] [PATCH 5/7] rte_sched: don't put tabs in log messages
  2015-02-01 10:03 [dpdk-dev] [PATCH 1/7] rte_sched: make RED optional at runtime Stephen Hemminger
                   ` (2 preceding siblings ...)
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 4/7] rte_sched: don't clear statistics when read Stephen Hemminger
@ 2015-02-01 10:03 ` Stephen Hemminger
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 6/7] rte_sched: eliminate floating point in calculating byte clock Stephen Hemminger
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 7/7] rte_sched: rearrange data structures Stephen Hemminger
  5 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2015-02-01 10:03 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

From: Stephen Hemminger <shemming@brocade.com>

syslog does not like tabs in log messages; tab gets translated to #011

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_sched/rte_sched.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index d891e50..55fbc14 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -495,10 +495,10 @@ rte_sched_port_log_pipe_profile(struct rte_sched_port *port, uint32_t i)
 	struct rte_sched_pipe_profile *p = port->pipe_profiles + i;
 
 	RTE_LOG(INFO, SCHED, "Low level config for pipe profile %u:\n"
-		"\tToken bucket: period = %u, credits per period = %u, size = %u\n"
-		"\tTraffic classes: period = %u, credits per period = [%u, %u, %u, %u]\n"
-		"\tTraffic class 3 oversubscription: weight = %hhu\n"
-		"\tWRR cost: [%hhu, %hhu, %hhu, %hhu], [%hhu, %hhu, %hhu, %hhu], [%hhu, %hhu, %hhu, %hhu], [%hhu, %hhu, %hhu, %hhu]\n",
+		"    Token bucket: period = %u, credits per period = %u, size = %u\n"
+		"    Traffic classes: period = %u, credits per period = [%u, %u, %u, %u]\n"
+		"    Traffic class 3 oversubscription: weight = %hhu\n"
+		"    WRR cost: [%hhu, %hhu, %hhu, %hhu], [%hhu, %hhu, %hhu, %hhu], [%hhu, %hhu, %hhu, %hhu], [%hhu, %hhu, %hhu, %hhu]\n",
 		i,
 
 		/* Token bucket */
@@ -716,9 +716,9 @@ rte_sched_port_log_subport_config(struct rte_sched_port *port, uint32_t i)
 	struct rte_sched_subport *s = port->subport + i;
 
 	RTE_LOG(INFO, SCHED, "Low level config for subport %u:\n"
-		"\tToken bucket: period = %u, credits per period = %u, size = %u\n"
-		"\tTraffic classes: period = %u, credits per period = [%u, %u, %u, %u]\n"
-		"\tTraffic class 3 oversubscription: wm min = %u, wm max = %u\n",
+		"    Token bucket: period = %u, credits per period = %u, size = %u\n"
+		"    Traffic classes: period = %u, credits per period = [%u, %u, %u, %u]\n"
+		"    Traffic class 3 oversubscription: wm min = %u, wm max = %u\n",
 		i,
 
 		/* Token bucket */
-- 
2.1.4

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

* [dpdk-dev] [PATCH 6/7] rte_sched: eliminate floating point in calculating byte clock
  2015-02-01 10:03 [dpdk-dev] [PATCH 1/7] rte_sched: make RED optional at runtime Stephen Hemminger
                   ` (3 preceding siblings ...)
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 5/7] rte_sched: don't put tabs in log messages Stephen Hemminger
@ 2015-02-01 10:03 ` Stephen Hemminger
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 7/7] rte_sched: rearrange data structures Stephen Hemminger
  5 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2015-02-01 10:03 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

From: Stephen Hemminger <shemming@brocade.com>

The old code was doing a floating point divide for each rte_dequeue()
which is very expensive. Change to using fixed point scaled math instead.
This improved performance from 5Gbit/sec to 10 Gbit/sec

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_sched/rte_sched.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index 55fbc14..3023457 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -102,6 +102,9 @@
 
 #define RTE_SCHED_BMP_POS_INVALID             UINT32_MAX
 
+/* For cycles_per_byte calculation */
+#define RTE_SCHED_TIME_SHIFT		      20
+
 struct rte_sched_subport {
 	/* Token bucket (TB) */
 	uint64_t tb_time; /* time of last update */
@@ -239,7 +242,7 @@ struct rte_sched_port {
 	uint64_t time_cpu_cycles;     /* Current CPU time measured in CPU cyles */
 	uint64_t time_cpu_bytes;      /* Current CPU time measured in bytes */
 	uint64_t time;                /* Current NIC TX time measured in bytes */
-	double cycles_per_byte;       /* CPU cycles per byte */
+	uint32_t cycles_per_byte;       /* CPU cycles per byte (scaled) */
 
 	/* Scheduling loop detection */
 	uint32_t pipe_loop;
@@ -657,7 +660,9 @@ rte_sched_port_config(struct rte_sched_port_params *params)
 	port->time_cpu_cycles = rte_get_tsc_cycles();
 	port->time_cpu_bytes = 0;
 	port->time = 0;
-	port->cycles_per_byte = ((double) rte_get_tsc_hz()) / ((double) params->rate);
+
+	port->cycles_per_byte = (rte_get_tsc_hz() << RTE_SCHED_TIME_SHIFT)
+		/ params->rate;
 
 	/* Scheduling loop detection */
 	port->pipe_loop = RTE_SCHED_PIPE_INVALID;
@@ -2156,11 +2161,12 @@ rte_sched_port_time_resync(struct rte_sched_port *port)
 {
 	uint64_t cycles = rte_get_tsc_cycles();
 	uint64_t cycles_diff = cycles - port->time_cpu_cycles;
-	double bytes_diff = ((double) cycles_diff) / port->cycles_per_byte;
+	uint64_t bytes_diff = (cycles_diff << RTE_SCHED_TIME_SHIFT)
+		/ port->cycles_per_byte;
 
 	/* Advance port time */
 	port->time_cpu_cycles = cycles;
-	port->time_cpu_bytes += (uint64_t) bytes_diff;
+	port->time_cpu_bytes += bytes_diff;
 	if (port->time < port->time_cpu_bytes) {
 		port->time = port->time_cpu_bytes;
 	}
-- 
2.1.4

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

* [dpdk-dev] [PATCH 7/7] rte_sched: rearrange data structures
  2015-02-01 10:03 [dpdk-dev] [PATCH 1/7] rte_sched: make RED optional at runtime Stephen Hemminger
                   ` (4 preceding siblings ...)
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 6/7] rte_sched: eliminate floating point in calculating byte clock Stephen Hemminger
@ 2015-02-01 10:03 ` Stephen Hemminger
  5 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2015-02-01 10:03 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

From: Stephen Hemminger <shemming@brocade.com>

Rearrange internal data structures to eliminate holes.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_sched/rte_sched.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index 3023457..7de1395 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -198,6 +198,8 @@ struct rte_sched_grinder {
 	enum grinder_state state;
 	uint32_t productive;
 	uint32_t pindex;
+	uint32_t qpos;
+
 	struct rte_sched_subport *subport;
 	struct rte_sched_pipe *pipe;
 	struct rte_sched_pipe_profile *pipe_params;
@@ -212,11 +214,10 @@ struct rte_sched_grinder {
 	uint32_t tc_index;
 	struct rte_sched_queue *queue[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
 	struct rte_mbuf **qbase[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
+	struct rte_mbuf *pkt;
 	uint32_t qindex[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
-	uint16_t qsize;
 	uint32_t qmask;
-	uint32_t qpos;
-	struct rte_mbuf *pkt;
+	uint16_t qsize;
 
 	/* WRR */
 	uint16_t wrr_tokens[RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS];
@@ -234,9 +235,7 @@ struct rte_sched_port {
 	uint16_t qsize[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
 	uint32_t n_pipe_profiles;
 	uint32_t pipe_tc3_rate_max;
-#ifdef RTE_SCHED_RED
-	struct rte_red_config red_config[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][e_RTE_METER_COLORS];
-#endif
+	uint32_t busy_grinders;
 
 	/* Timing */
 	uint64_t time_cpu_cycles;     /* Current CPU time measured in CPU cyles */
@@ -247,16 +246,15 @@ struct rte_sched_port {
 	/* Scheduling loop detection */
 	uint32_t pipe_loop;
 	uint32_t pipe_exhaustion;
+	uint32_t n_pkts_out;
 
 	/* Bitmap */
 	struct rte_bitmap *bmp;
+	struct rte_mbuf **pkts_out;
 	uint32_t grinder_base_bmp_pos[RTE_SCHED_PORT_N_GRINDERS] __rte_aligned_16;
 
 	/* Grinders */
 	struct rte_sched_grinder grinder[RTE_SCHED_PORT_N_GRINDERS];
-	uint32_t busy_grinders;
-	struct rte_mbuf **pkts_out;
-	uint32_t n_pkts_out;
 
 	/* Queue base calculation */
 	uint32_t qsize_add[RTE_SCHED_QUEUES_PER_PIPE];
@@ -270,6 +268,9 @@ struct rte_sched_port {
 	struct rte_sched_pipe_profile *pipe_profiles;
 	uint8_t *bmp_array;
 	struct rte_mbuf **queue_array;
+#ifdef RTE_SCHED_RED
+	struct rte_red_config red_config[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][e_RTE_METER_COLORS];
+#endif
 	uint8_t memory[0] __rte_cache_aligned;
 } __rte_cache_aligned;
 
-- 
2.1.4

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

* Re: [dpdk-dev] [PATCH 4/7] rte_sched: don't clear statistics when read
  2015-02-01 10:03 ` [dpdk-dev] [PATCH 4/7] rte_sched: don't clear statistics when read Stephen Hemminger
@ 2015-02-01 14:25   ` Neil Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Neil Horman @ 2015-02-01 14:25 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Stephen Hemminger

On Sun, Feb 01, 2015 at 10:03:48AM +0000, Stephen Hemminger wrote:
> From: Stephen Hemminger <shemming@brocade.com>
> 
> Make rte_sched statistics API work like the ethernet statistics API.
> Don't auto-clear statistics.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_sched/rte_sched.c | 30 ++++++++++++++++++++++++++++++
>  lib/librte_sched/rte_sched.h | 29 +++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index 8cb8bf1..d891e50 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -935,6 +935,21 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
>  }
>  
>  int
> +rte_sched_subport_stats_reset(struct rte_sched_port *port,
> +			      uint32_t subport_id)
> +{
> +	struct rte_sched_subport *s;
> +
> +	/* Check user parameters */
> +	if (port == NULL || subport_id >= port->n_subports_per_port)
> +		return -1;
> +
> +	s = port->subport + subport_id;
> +	memset(&s->stats, 0, sizeof(struct rte_sched_subport_stats));
Its like this in the current implementation as well, but isn't this a bit racy?
Like if we're clearning the stats while another thread is polling the interface?

Would it be worth implementing a toggle mechanism whereby a reset does an atomic
cmpxch on the stats pointer between two stats copies, then zeroing the exchanged
copy?
Neil

> +	return 0;
> +}
> +
> +int
>  rte_sched_queue_read_stats(struct rte_sched_port *port,
>  	uint32_t queue_id,
>  	struct rte_sched_queue_stats *stats,
> @@ -963,6 +978,21 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
>  	return 0;
>  }
>  
> +int
> +rte_sched_queue_stats_reset(struct rte_sched_port *port,
> +			    uint32_t queue_id)
> +{
> +	struct rte_sched_queue_extra *qe;
> +
> +	/* Check user parameters */
> +	if (port == NULL || queue_id >= rte_sched_port_queues_per_port(port))
> +		return -1;
> +
> +	qe = port->queue_extra + queue_id;
> +	memset(&qe->stats, 0, sizeof(struct rte_sched_queue_stats));
> +	return 0;
> +}
> +
>  static inline uint32_t
>  rte_sched_port_qindex(struct rte_sched_port *port, uint32_t subport, uint32_t pipe, uint32_t traffic_class, uint32_t queue)
>  {
> diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
> index d5a1d5b..64b4dd6 100644
> --- a/lib/librte_sched/rte_sched.h
> +++ b/lib/librte_sched/rte_sched.h
> @@ -316,6 +316,21 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
>  	struct rte_sched_subport_stats *stats,
>  	uint32_t *tc_ov);
>  
> +
> +/**
> + * Hierarchical scheduler subport statistics reset
> + *
> + * @param port
> + *   Handle to port scheduler instance
> + * @param subport_id
> + *   Subport ID
> + * @return
> + *   0 upon success, error code otherwise
> + */
> +int
> +rte_sched_subport_stats_reset(struct rte_sched_port *port,
> +			      uint32_t subport_id);
> +
>  /**
>   * Hierarchical scheduler queue statistics read
>   *
> @@ -337,6 +352,20 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
>  	struct rte_sched_queue_stats *stats,
>  	uint16_t *qlen);
>  
> +/**
> + * Hierarchical scheduler queue statistics reset
> + *
> + * @param port
> + *   Handle to port scheduler instance
> + * @param queue_id
> + *   Queue ID within port scheduler
> + * @return
> + *   0 upon success, error code otherwise
> + */
> +int
> +rte_sched_queue_stats_reset(struct rte_sched_port *port,
> +			    uint32_t queue_id);
> +
>  /*
>   * Run-time
>   *
> -- 
> 2.1.4
> 
> 

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

* Re: [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's
       [not found]   ` <2601191342CEEE43887BDE71AB977258213E2822@irsmsx105.ger.corp.intel.com>
@ 2015-02-02 22:31     ` Stephen Hemminger
  2015-02-03  0:07       ` Ananyev, Konstantin
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2015-02-02 22:31 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: dev, Stephen Hemminger

On Mon, 2 Feb 2015 14:21:58 +0000
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:

> Hi Stephen,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > Sent: Sunday, February 01, 2015 10:04 AM
> > To: dev@dpdk.org
> > Cc: Stephen Hemminger
> > Subject: [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's
> > 
> > From: Stephen Hemminger <shemming@brocade.com>
> > 
> > The QoS subport is limited to 8 bits in original code.
> > But customers demanded ability to support full number of VLAN's (4096)
> > therefore use reserved field of mbuf for this field instead
> > of packing inside other classify portions.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/librte_mbuf/rte_mbuf.h   |  2 +-
> >  lib/librte_sched/rte_sched.h | 31 ++++++++++++++++++++-----------
> >  2 files changed, 21 insertions(+), 12 deletions(-)
> > 
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index 16059c6..b6b08f4 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -242,7 +242,7 @@ struct rte_mbuf {
> >  	uint16_t data_len;        /**< Amount of data in segment buffer. */
> >  	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
> >  	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
> > -	uint16_t reserved;
> > +	uint16_t subport;	  /**< SCHED Subport ID */
> 
> As I remember, we keep these reserved 2 bytes for RX 2 double vlan tag offload.
> So probably not a good idea to use it for something that is rte_sched specific.
> If you really need extra space fo rte_sched fields inside mbuf, can't you move it into second cache line?
> Or might be you can use userdata, to either store sched information directly, or as a pointer to some external memory  location? 
> Another possibility - union mbuf.hash is 64bit now, while sched uses only 32bits.
> So might be you can rearrange it to make sched 64bits too?
> Something like:
> 
> union {
>                 uint32_t rss;     /**< RSS hash result if RSS enabled */
>                 struct {
>                         union {
>                                 struct {
>                                         uint16_t hash;
>                                         uint16_t id;
>                                 };
>                                 uint32_t lo;
>                                 /**< Second 4 flexible bytes */
>                         };
>                         uint32_t hi;
>                         /**< First 4 flexible bytes or FD ID, dependent on
>                              PKT_RX_FDIR_* flag in ol_flags. */
>                 } fdir;           /**< Filter identifier if FDIR enabled */
> -                uint32_t sched;   /**< Hierarchical scheduler */
> +               uint64_t sched;   /**< Hierarchical scheduler */
>                 uint32_t usr;     /**< User defined tags. See @rte_distributor_p
> rocess */
> } hash;                   /**< hash information */

Increasing the size of that union totally breaks other alignment and is a not starter.

The reserved field is not use upstream merged code and therefore is fair game.
First to claim it wins.

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

* Re: [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's
  2015-02-02 22:31     ` Stephen Hemminger
@ 2015-02-03  0:07       ` Ananyev, Konstantin
  0 siblings, 0 replies; 10+ messages in thread
From: Ananyev, Konstantin @ 2015-02-03  0:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Stephen Hemminger



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, February 02, 2015 10:32 PM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org; Stephen Hemminger
> Subject: Re: [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's
> 
> On Mon, 2 Feb 2015 14:21:58 +0000
> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> 
> > Hi Stephen,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > > Sent: Sunday, February 01, 2015 10:04 AM
> > > To: dev@dpdk.org
> > > Cc: Stephen Hemminger
> > > Subject: [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's
> > >
> > > From: Stephen Hemminger <shemming@brocade.com>
> > >
> > > The QoS subport is limited to 8 bits in original code.
> > > But customers demanded ability to support full number of VLAN's (4096)
> > > therefore use reserved field of mbuf for this field instead
> > > of packing inside other classify portions.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > >  lib/librte_mbuf/rte_mbuf.h   |  2 +-
> > >  lib/librte_sched/rte_sched.h | 31 ++++++++++++++++++++-----------
> > >  2 files changed, 21 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > > index 16059c6..b6b08f4 100644
> > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > @@ -242,7 +242,7 @@ struct rte_mbuf {
> > >  	uint16_t data_len;        /**< Amount of data in segment buffer. */
> > >  	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
> > >  	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
> > > -	uint16_t reserved;
> > > +	uint16_t subport;	  /**< SCHED Subport ID */
> >
> > As I remember, we keep these reserved 2 bytes for RX 2 double vlan tag offload.
> > So probably not a good idea to use it for something that is rte_sched specific.
> > If you really need extra space fo rte_sched fields inside mbuf, can't you move it into second cache line?
> > Or might be you can use userdata, to either store sched information directly, or as a pointer to some external memory  location?
> > Another possibility - union mbuf.hash is 64bit now, while sched uses only 32bits.
> > So might be you can rearrange it to make sched 64bits too?
> > Something like:
> >
> > union {
> >                 uint32_t rss;     /**< RSS hash result if RSS enabled */
> >                 struct {
> >                         union {
> >                                 struct {
> >                                         uint16_t hash;
> >                                         uint16_t id;
> >                                 };
> >                                 uint32_t lo;
> >                                 /**< Second 4 flexible bytes */
> >                         };
> >                         uint32_t hi;
> >                         /**< First 4 flexible bytes or FD ID, dependent on
> >                              PKT_RX_FDIR_* flag in ol_flags. */
> >                 } fdir;           /**< Filter identifier if FDIR enabled */
> > -                uint32_t sched;   /**< Hierarchical scheduler */
> > +               uint64_t sched;   /**< Hierarchical scheduler */
> >                 uint32_t usr;     /**< User defined tags. See @rte_distributor_p
> > rocess */
> > } hash;                   /**< hash information */
> 
> Increasing the size of that union totally breaks other alignment and is a not starter.

struct fdir already is 64bit width.
Though yes, we can't use uint64_t directly, as it would break alignment - totally forgot about it.
But nothing prevents you from doing:

struct { uint32_t lo, hi;} sched;

 right?

> 
> The reserved field is not use upstream merged code and therefore is fair game.

As you can see that reserved field lies inside first 16B from rx_descriptor_fields1;
So hopefully we will be able to load it from RX descriptors in one SSE load/store together with 
other RXD fields.
Anyway these 16B are supposed to contain fields that are filled by RXD (as the name suggests).

> First to claim it wins.

Wins what?
Sorry, but you can't pollute mbuf structure with whatever you like.
So NACK for now.

Konstantin

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

end of thread, other threads:[~2015-02-03  0:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-01 10:03 [dpdk-dev] [PATCH 1/7] rte_sched: make RED optional at runtime Stephen Hemminger
2015-02-01 10:03 ` [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's Stephen Hemminger
     [not found]   ` <2601191342CEEE43887BDE71AB977258213E2822@irsmsx105.ger.corp.intel.com>
2015-02-02 22:31     ` Stephen Hemminger
2015-02-03  0:07       ` Ananyev, Konstantin
2015-02-01 10:03 ` [dpdk-dev] [PATCH 3/7] rte_sched: keep track of RED drops Stephen Hemminger
2015-02-01 10:03 ` [dpdk-dev] [PATCH 4/7] rte_sched: don't clear statistics when read Stephen Hemminger
2015-02-01 14:25   ` Neil Horman
2015-02-01 10:03 ` [dpdk-dev] [PATCH 5/7] rte_sched: don't put tabs in log messages Stephen Hemminger
2015-02-01 10:03 ` [dpdk-dev] [PATCH 6/7] rte_sched: eliminate floating point in calculating byte clock Stephen Hemminger
2015-02-01 10:03 ` [dpdk-dev] [PATCH 7/7] rte_sched: rearrange data structures Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).