dev Digest, Vol 348, Issue 163

From: Chengwen Feng <[email protected]>

This patch adds checking for rte_kni_init() result.

Fixes 75e2bc54c018 ("net/kni: add KNI PMD")

Cc [email protected]

Signed-off-by Chengwen Feng <[email protected]>

Signed-off-by Min Hu (Connor) <[email protected]>

Hi Dharmik,

Please see some comments below.

Quoted 86 lines · Dharmik Thakkar

From: Joyce Kong <[email protected]>

If cache is enabled, objects will be retrieved/put from/to cache, subsequently from/to the common pool. Now the debug stats calculate the objects retrieved/put from/to cache and pool together, it is better to distinguish them.

Signed-off-by Joyce Kong <[email protected]>

Signed-off-by Dharmik Thakkar <[email protected]>

Reviewed-by Ruifeng Wang <[email protected]>

Reviewed-by Honnappa Nagarahalli <[email protected]>

---
 lib/librte_mempool/rte_mempool.c | 24 ++++++++++++++++  
lib/librte_mempool/rte_mempool.h | 47 
++++++++++++++++++++++----------
 2 files changed, 57 insertions(+), 14 deletions(-)

lib/librte_mempool/rte_mempool.c +24 -0

index afb1239c8d48..339f14455624 100644
@@ -1244,6 +1244,18 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		sum.put_bulk += mp->stats[lcore_id].put_bulk;
 		sum.put_objs += mp->stats[lcore_id].put_objs;
+		sum.put_common_pool_bulk +=
+			mp->stats[lcore_id].put_common_pool_bulk;
+		sum.put_common_pool_objs +=
+			mp->stats[lcore_id].put_common_pool_objs;
+		sum.put_cache_bulk += mp->stats[lcore_id].put_cache_bulk;
+		sum.put_cache_objs += mp->stats[lcore_id].put_cache_objs;
+		sum.get_common_pool_bulk +=
+			mp->stats[lcore_id].get_common_pool_bulk;
+		sum.get_common_pool_objs +=
+			mp->stats[lcore_id].get_common_pool_objs;
+		sum.get_cache_bulk += mp->stats[lcore_id].get_cache_bulk;
+		sum.get_cache_objs += mp->stats[lcore_id].get_cache_objs;
 		sum.get_success_bulk += mp->stats[lcore_id].get_success_bulk;
 		sum.get_success_objs += mp->stats[lcore_id].get_success_objs;
 		sum.get_fail_bulk += mp->stats[lcore_id].get_fail_bulk;

@@ -1254,6 +1266,18 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
 	fprintf(f, "  stats:\n");
 	fprintf(f, "    put_bulk=%"PRIu64"\n", sum.put_bulk);
 	fprintf(f, "    put_objs=%"PRIu64"\n", sum.put_objs);
+	fprintf(f, "    put_common_pool_bulk=%"PRIu64"\n",
+						sum.put_common_pool_bulk);
+	fprintf(f, "    put_common_pool_objs=%"PRIu64"\n",
+						sum.put_common_pool_objs);
+	fprintf(f, "    put_cache_bulk=%"PRIu64"\n", sum.put_cache_bulk);
+	fprintf(f, "    put_cache_objs=%"PRIu64"\n", sum.put_cache_objs);
+	fprintf(f, "    get_common_pool_bulk=%"PRIu64"\n",
+						sum.get_common_pool_bulk);
+	fprintf(f, "    get_common_pool_objs=%"PRIu64"\n",
+						sum.get_common_pool_objs);
+	fprintf(f, "    get_cache_bulk=%"PRIu64"\n", sum.get_cache_bulk);
+	fprintf(f, "    get_cache_objs=%"PRIu64"\n", sum.get_cache_objs);
 	fprintf(f, "    get_success_bulk=%"PRIu64"\n", sum.get_success_bulk);
 	fprintf(f, "    get_success_objs=%"PRIu64"\n", sum.get_success_objs);
 	fprintf(f, "    get_fail_bulk=%"PRIu64"\n", sum.get_fail_bulk);

lib/librte_mempool/rte_mempool.h +11 -6

index 848a19226149..0959f8a3f367 100644
@@ -66,12 +66,20 @@ extern "C" {
  * A structure that stores the mempool statistics (per-lcore).
  */
 struct rte_mempool_debug_stats {
-	uint64_t put_bulk;         /**< Number of puts. */
-	uint64_t put_objs;         /**< Number of objects successfully put. */
-	uint64_t get_success_bulk; /**< Successful allocation number. */
-	uint64_t get_success_objs; /**< Objects successfully allocated. */
-	uint64_t get_fail_bulk;    /**< Failed allocation number. */
-	uint64_t get_fail_objs;    /**< Objects that failed to be allocated. */
+	uint64_t put_bulk;		  /**< Number of puts. */
+	uint64_t put_objs;		  /**< Number of objects successfully put. */
+	uint64_t put_common_pool_bulk;	  /**< Number of bulks enqueued in common pool. */
+	uint64_t put_common_pool_objs;	  /**< Number of objects enqueued in common pool. */
+	uint64_t put_cache_bulk;	  /**< Number of bulks enqueued in cache. */
+	uint64_t put_cache_objs;	  /**< Number of objects enqueued in cache. */
+	uint64_t get_common_pool_bulk;    /**< Number of bulks dequeued from common pool. */
+	uint64_t get_common_pool_objs;	  /**< Number of objects dequeued from common pool. */
+	uint64_t get_cache_bulk;	  /**< Number of bulks dequeued from cache. */
+	uint64_t get_cache_objs;	  /**< Number of objects dequeued from cache. */
+	uint64_t get_success_bulk;	  /**< Successful allocation number. */

lib/librte_mempool/rte_mempool.h extern "C" { · line 82

Quoted 3 lines

lib/librte_mempool/rte_mempool.h +3 -0

+	uint64_t get_success_objs;	  /**< Objects successfully allocated. */
+	uint64_t get_fail_bulk;		  /**< Failed allocation number. */
+	uint64_t get_fail_objs;		  /**< Objects that failed to be allocated. */

I missed it the first time, but this changes the size of the rte_mempool_debug_stats structure. I think we don't care about this ABI breakage because this structure is only defined if RTE_LIBRTE_MEMPOOL_DEBUG is set. But just in case, adding Ray as Cc.

About the field themselves, I'm not certain that there is an added value to have stats for cache gets and puts. My feeling is that the important stat to monitor is the access to common pool, because it is the one that highlights a possible performance impact (contention). The cache stats are more or less equal to "success + fail - common". Moreover, it will simplify the patch and avoid risks of mistakes.

What do you think?

Quoted 42 lines

lib/librte_mempool/rte_mempool.h +12 -1

 	/** Successful allocation number of contiguous blocks. */
 	uint64_t get_success_blks;
 	/** Failed allocation number of contiguous blocks. */ @@ -699,10 
+707,18 @@ rte_mempool_ops_dequeue_bulk(struct rte_mempool *mp,
 		void **obj_table, unsigned n)
 {
 	struct rte_mempool_ops *ops;
+	int ret;
 
 	rte_mempool_trace_ops_dequeue_bulk(mp, obj_table, n);
 	ops = rte_mempool_get_ops(mp->ops_index);
-	return ops->dequeue(mp, obj_table, n);
+	ret = ops->dequeue(mp, obj_table, n);
+	if (ret == 0) {
+		__MEMPOOL_STAT_ADD(mp, get_common_pool_bulk, 1);
+		__MEMPOOL_STAT_ADD(mp, get_common_pool_objs, n);
+		__MEMPOOL_STAT_ADD(mp, get_success_bulk, 1);
+		__MEMPOOL_STAT_ADD(mp, get_success_objs, n);
+	}
+	return ret;
 }
 
 /**

@@ -749,6 +765,8 @@ rte_mempool_ops_enqueue_bulk(struct rte_mempool 
 *mp, void * const *obj_table,  {
 	struct rte_mempool_ops *ops;
 
+	__MEMPOOL_STAT_ADD(mp, put_common_pool_bulk, 1);
+	__MEMPOOL_STAT_ADD(mp, put_common_pool_objs, n);
 	rte_mempool_trace_ops_enqueue_bulk(mp, obj_table, n);
 	ops = rte_mempool_get_ops(mp->ops_index);
 	return ops->enqueue(mp, obj_table, n); @@ -1297,14 +1315,18 @@ 

__mempool_generic_put(struct rte_mempool *mp, void * const *obj_table,

lib/librte_mempool/rte_mempool.h +1 -1

 	/* Add elements back into the cache */
 	rte_memcpy(&cache_objs[0], obj_table, sizeof(void *) * n);
-
 	cache->len += n;
 
+	__MEMPOOL_STAT_ADD(mp, put_cache_bulk, 1);

lib/librte_mempool/rte_mempool.h

Quoted 10 lines

lib/librte_mempool/rte_mempool.h +5 -1

+
 	if (cache->len >= cache->flushthresh) {
+		__MEMPOOL_STAT_ADD(mp, put_cache_objs,
+				   n - (cache->len - cache->size));
 		rte_mempool_ops_enqueue_bulk(mp, &cache->objs[cache->size],
 				cache->len - cache->size);
 		cache->len = cache->size;
-	}
+	} else
+		__MEMPOOL_STAT_ADD(mp, put_cache_objs, n);

In case we keep cache stats, I'd add {} after the else to be consistent with the if().

lib/librte_mempool/rte_mempool.h __mempool_generic_get(struct rte_mempool *mp, · line 1465

Quoted 11 lines

lib/librte_mempool/rte_mempool.h +2 -2

 	return;

@@ -1438,8 +1460,8 @@ __mempool_generic_get(struct rte_mempool *mp, 
 void **obj_table,
 
 	cache->len -= n;
 
-	__MEMPOOL_STAT_ADD(mp, get_success_bulk, 1);
-	__MEMPOOL_STAT_ADD(mp, get_success_objs, n);
+	__MEMPOOL_STAT_ADD(mp, get_cache_bulk, 1);
+	__MEMPOOL_STAT_ADD(mp, get_cache_objs, n);

In case we keep cache stats, I don't think we should remove get_success stats increment. Else, the success stats will never be incremented when retrieving objects from the cache.

Quoted 12 lines

lib/librte_mempool/rte_mempool.h +0 -3

 	return 0;
 

@@ -1451,9 +1473,6 @@ __mempool_generic_get(struct rte_mempool *mp, void **obj_table,
 	if (ret < 0) {
 		__MEMPOOL_STAT_ADD(mp, get_fail_bulk, 1);
 		__MEMPOOL_STAT_ADD(mp, get_fail_objs, n);
-	} else {
-		__MEMPOOL_STAT_ADD(mp, get_success_bulk, 1);
-		__MEMPOOL_STAT_ADD(mp, get_success_objs, n);
 	}
 

Quoted 4 lines

lib/librte_mempool/rte_mempool.h +0 -1

 	return ret;
--

2.17.1

This adds the support to set 'Bus Master Enable' bit in the PCI command register.

Signed-off-by Haiyue Wang <[email protected]>

Tested-by Qi Zhang <[email protected]>

---
 drivers/bus/pci/pci_common.c  | 20 ++++++++++++++++++++
 drivers/bus/pci/rte_bus_pci.h | 12 ++++++++++++
 drivers/bus/pci/version.map   |  1 +
 lib/pci/rte_pci.h             |  4 ++++
 4 files changed, 37 insertions(+)

drivers/bus/pci/pci_common.c +20 -0

index ee7f96635..b631cb9c7 100644
@@ -746,6 +746,26 @@ rte_pci_find_ext_capability(struct rte_pci_device *dev, uint32_t cap)
 	return 0;
 }
 
+int
+rte_pci_enable_bus_master(struct rte_pci_device *dev)
+{
+	uint16_t cmd;
+
+	if (rte_pci_read_config(dev, &cmd, sizeof(cmd), RTE_PCI_COMMAND) < 0) {
+		RTE_LOG(ERR, EAL, "error in reading PCI command register\n");
+		return -1;
+	}
+
+	cmd |= RTE_PCI_COMMAND_MASTER;
+
+	if (rte_pci_write_config(dev, &cmd, sizeof(cmd), RTE_PCI_COMMAND) < 0) {
+		RTE_LOG(ERR, EAL, "error in writing PCI command register\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 struct rte_pci_bus rte_pci_bus = {
 	.bus = {
 		.scan = rte_pci_scan,

drivers/bus/pci/rte_bus_pci.h +12 -0

index 64886b473..83caf477b 100644
@@ -249,6 +249,18 @@ void rte_pci_dump(FILE *f);
 __rte_experimental
 off_t rte_pci_find_ext_capability(struct rte_pci_device *dev, uint32_t cap);
 
+/**
+ * Enables Bus Master for device's PCI command register.
+ *
+ *  @param dev
+ *    A pointer to rte_pci_device structure.
+ *
+ *  @return
+ *  0 on success, -1 on error in PCI config space read/write.
+ */
+__rte_experimental
+int rte_pci_enable_bus_master(struct rte_pci_device *dev);
+
 /**
  * Register a PCI driver.
  *

drivers/bus/pci/version.map +0 -0

index f33ed0abd..b271e48a8 100644

drivers/bus/pci/version.map +1 -0

+	rte_pci_enable_bus_master;
 	rte_pci_find_ext_capability;
 };

lib/pci/rte_pci.h +0 -0

index a8f8e404a..1f33d687f 100644

From: Akhil Goyal <[email protected]>

A new field ``ca_enqueue`` is added in ``rte_eventdev`` in the end to maintain ABI. It needs to be moved above in the structure to align with other enqueue callbacks.

Signed-off-by Akhil Goyal <[email protected]>

---
 doc/guides/rel_notes/deprecation.rst | 4 ++++
 1 file changed, 4 insertions(+)

doc/guides/rel_notes/deprecation.rst +0 -0

index 2afc84c39..a973de4a9 100644