[dpdk-dev] [PATCH 06/11] enic: Tx perf - improve processing of mbufs held by driver

John Daley johndale at cisco.com
Fri May 20 21:04:09 CEST 2016


The list of mbufs held by the driver on Tx was allocated in chunks
(a hold-over from the enic kernel mode driver). The structure used
next pointers across chunks which led to cache misses.

Allocate the array used to hold mbufs in flight on Tx with
rte_zmalloc_socket(). Remove unnecessary fields from the sturcture
and use head and tail pointers instead of next pointers.

Signed-off-by: John Daley <johndale at cisco.com>
---
 drivers/net/enic/base/enic_vnic_wq.h | 26 +++----------
 drivers/net/enic/base/vnic_wq.c      | 75 +++++++++---------------------------
 drivers/net/enic/base/vnic_wq.h      | 56 ++++++++++-----------------
 drivers/net/enic/enic.h              | 24 ++++++++++++
 drivers/net/enic/enic_main.c         |  4 +-
 drivers/net/enic/enic_rxtx.c         | 23 +++--------
 6 files changed, 75 insertions(+), 133 deletions(-)

diff --git a/drivers/net/enic/base/enic_vnic_wq.h b/drivers/net/enic/base/enic_vnic_wq.h
index b019109..55c5664 100644
--- a/drivers/net/enic/base/enic_vnic_wq.h
+++ b/drivers/net/enic/base/enic_vnic_wq.h
@@ -40,37 +40,21 @@
 
 static inline void enic_vnic_post_wq_index(struct vnic_wq *wq)
 {
-	struct vnic_wq_buf *buf = wq->to_use;
-
 	/* Adding write memory barrier prevents compiler and/or CPU
 	 * reordering, thus avoiding descriptor posting before
 	 * descriptor is initialized. Otherwise, hardware can read
 	 * stale descriptor fields.
 	*/
 	wmb();
-	iowrite32(buf->index, &wq->ctrl->posted_index);
+	iowrite32(wq->head_idx, &wq->ctrl->posted_index);
 }
 
 static inline void enic_vnic_post_wq(struct vnic_wq *wq,
-				     void *os_buf, dma_addr_t dma_addr,
-				     unsigned int len, int sop,
-				     uint8_t desc_skip_cnt, uint8_t cq_entry,
-				     uint8_t compressed_send, uint64_t wrid)
+				     void *os_buf, uint8_t cq_entry)
 {
-	struct vnic_wq_buf *buf = wq->to_use;
-
-	buf->sop = sop;
-	buf->cq_entry = cq_entry;
-	buf->compressed_send = compressed_send;
-	buf->desc_skip_cnt = desc_skip_cnt;
-	buf->os_buf = os_buf;
-	buf->dma_addr = dma_addr;
-	buf->len = len;
-	buf->wr_id = wrid;
-
-	buf = buf->next;
-	wq->ring.desc_avail -= desc_skip_cnt;
-	wq->to_use = buf;
+	struct vnic_wq_buf *buf = &wq->bufs[wq->head_idx];
+	buf->mb = os_buf;
+	wq->head_idx = enic_ring_incr(wq->ring.desc_count, wq->head_idx);
 
 	if (cq_entry)
 		enic_vnic_post_wq_index(wq);
diff --git a/drivers/net/enic/base/vnic_wq.c b/drivers/net/enic/base/vnic_wq.c
index a3ef417..ab81c7e 100644
--- a/drivers/net/enic/base/vnic_wq.c
+++ b/drivers/net/enic/base/vnic_wq.c
@@ -59,71 +59,30 @@ int vnic_wq_alloc_ring(struct vnic_dev *vdev, struct vnic_wq *wq,
 
 static int vnic_wq_alloc_bufs(struct vnic_wq *wq)
 {
-	struct vnic_wq_buf *buf;
-	unsigned int i, j, count = wq->ring.desc_count;
-	unsigned int blks = VNIC_WQ_BUF_BLKS_NEEDED(count);
-
-	for (i = 0; i < blks; i++) {
-		wq->bufs[i] = kzalloc(VNIC_WQ_BUF_BLK_SZ(count), GFP_ATOMIC);
-		if (!wq->bufs[i])
-			return -ENOMEM;
-	}
-
-	for (i = 0; i < blks; i++) {
-		buf = wq->bufs[i];
-		for (j = 0; j < VNIC_WQ_BUF_BLK_ENTRIES(count); j++) {
-			buf->index = i * VNIC_WQ_BUF_BLK_ENTRIES(count) + j;
-			buf->desc = (u8 *)wq->ring.descs +
-				wq->ring.desc_size * buf->index;
-			if (buf->index + 1 == count) {
-				buf->next = wq->bufs[0];
-				break;
-			} else if (j + 1 == VNIC_WQ_BUF_BLK_ENTRIES(count)) {
-				buf->next = wq->bufs[i + 1];
-			} else {
-				buf->next = buf + 1;
-				buf++;
-			}
-		}
-	}
-
-	wq->to_use = wq->to_clean = wq->bufs[0];
-
+	unsigned int count = wq->ring.desc_count;
+       /* Allocate the mbuf ring */
+	wq->bufs = (struct vnic_wq_buf *)rte_zmalloc_socket("wq->bufs",
+		    sizeof(struct vnic_wq_buf) * count,
+		    RTE_CACHE_LINE_SIZE, wq->socket_id);
+	wq->head_idx = 0;
+	wq->tail_idx = 0;
+	if (wq->bufs == NULL)
+		return -ENOMEM;
 	return 0;
 }
 
 void vnic_wq_free(struct vnic_wq *wq)
 {
 	struct vnic_dev *vdev;
-	unsigned int i;
 
 	vdev = wq->vdev;
 
 	vnic_dev_free_desc_ring(vdev, &wq->ring);
 
-	for (i = 0; i < VNIC_WQ_BUF_BLKS_MAX; i++) {
-		if (wq->bufs[i]) {
-			kfree(wq->bufs[i]);
-			wq->bufs[i] = NULL;
-		}
-	}
-
+	rte_free(wq->bufs);
 	wq->ctrl = NULL;
 }
 
-int vnic_wq_mem_size(struct vnic_wq *wq, unsigned int desc_count,
-	unsigned int desc_size)
-{
-	int mem_size = 0;
-
-	mem_size += vnic_dev_desc_ring_size(&wq->ring, desc_count, desc_size);
-
-	mem_size += VNIC_WQ_BUF_BLKS_NEEDED(wq->ring.desc_count) *
-		VNIC_WQ_BUF_BLK_SZ(wq->ring.desc_count);
-
-	return mem_size;
-}
-
 
 int vnic_wq_alloc(struct vnic_dev *vdev, struct vnic_wq *wq, unsigned int index,
 	unsigned int desc_count, unsigned int desc_size)
@@ -172,9 +131,8 @@ void vnic_wq_init_start(struct vnic_wq *wq, unsigned int cq_index,
 	iowrite32(error_interrupt_offset, &wq->ctrl->error_interrupt_offset);
 	iowrite32(0, &wq->ctrl->error_status);
 
-	wq->to_use = wq->to_clean =
-		&wq->bufs[fetch_index / VNIC_WQ_BUF_BLK_ENTRIES(count)]
-			[fetch_index % VNIC_WQ_BUF_BLK_ENTRIES(count)];
+	wq->head_idx = fetch_index;
+	wq->tail_idx = wq->head_idx;
 }
 
 void vnic_wq_init(struct vnic_wq *wq, unsigned int cq_index,
@@ -223,18 +181,21 @@ void vnic_wq_clean(struct vnic_wq *wq,
 	void (*buf_clean)(struct vnic_wq *wq, struct vnic_wq_buf *buf))
 {
 	struct vnic_wq_buf *buf;
+	unsigned int  to_clean = wq->tail_idx;
 
-	buf = wq->to_clean;
+	buf = &wq->bufs[to_clean];
 
 	while (vnic_wq_desc_used(wq) > 0) {
 
 		(*buf_clean)(wq, buf);
+		to_clean = buf_idx_incr(wq->ring.desc_count, to_clean);
 
-		buf = wq->to_clean = buf->next;
+		buf = &wq->bufs[to_clean];
 		wq->ring.desc_avail++;
 	}
 
-	wq->to_use = wq->to_clean = wq->bufs[0];
+	wq->head_idx = 0;
+	wq->tail_idx = 0;
 
 	iowrite32(0, &wq->ctrl->fetch_index);
 	iowrite32(0, &wq->ctrl->posted_index);
diff --git a/drivers/net/enic/base/vnic_wq.h b/drivers/net/enic/base/vnic_wq.h
index d8660e4..a6759f5 100644
--- a/drivers/net/enic/base/vnic_wq.h
+++ b/drivers/net/enic/base/vnic_wq.h
@@ -64,40 +64,19 @@ struct vnic_wq_ctrl {
 	u32 pad9;
 };
 
+/* 16 bytes */
 struct vnic_wq_buf {
-	struct vnic_wq_buf *next;
-	dma_addr_t dma_addr;
-	void *os_buf;
-	unsigned int len;
-	unsigned int index;
-	int sop;
-	void *desc;
-	uint64_t wr_id; /* Cookie */
-	uint8_t cq_entry; /* Gets completion event from hw */
-	uint8_t desc_skip_cnt; /* Num descs to occupy */
-	uint8_t compressed_send; /* Both hdr and payload in one desc */
+	void *mb;
 };
 
-/* Break the vnic_wq_buf allocations into blocks of 32/64 entries */
-#define VNIC_WQ_BUF_MIN_BLK_ENTRIES 32
-#define VNIC_WQ_BUF_DFLT_BLK_ENTRIES 64
-#define VNIC_WQ_BUF_BLK_ENTRIES(entries) \
-	((unsigned int)((entries < VNIC_WQ_BUF_DFLT_BLK_ENTRIES) ? \
-	VNIC_WQ_BUF_MIN_BLK_ENTRIES : VNIC_WQ_BUF_DFLT_BLK_ENTRIES))
-#define VNIC_WQ_BUF_BLK_SZ(entries) \
-	(VNIC_WQ_BUF_BLK_ENTRIES(entries) * sizeof(struct vnic_wq_buf))
-#define VNIC_WQ_BUF_BLKS_NEEDED(entries) \
-	DIV_ROUND_UP(entries, VNIC_WQ_BUF_BLK_ENTRIES(entries))
-#define VNIC_WQ_BUF_BLKS_MAX VNIC_WQ_BUF_BLKS_NEEDED(4096)
-
 struct vnic_wq {
 	unsigned int index;
 	struct vnic_dev *vdev;
 	struct vnic_wq_ctrl __iomem *ctrl;              /* memory-mapped */
 	struct vnic_dev_ring ring;
-	struct vnic_wq_buf *bufs[VNIC_WQ_BUF_BLKS_MAX];
-	struct vnic_wq_buf *to_use;
-	struct vnic_wq_buf *to_clean;
+	struct vnic_wq_buf *bufs;
+	unsigned int head_idx;
+	unsigned int tail_idx;
 	unsigned int pkts_outstanding;
 	unsigned int socket_id;
 };
@@ -114,11 +93,6 @@ static inline unsigned int vnic_wq_desc_used(struct vnic_wq *wq)
 	return wq->ring.desc_count - wq->ring.desc_avail - 1;
 }
 
-static inline void *vnic_wq_next_desc(struct vnic_wq *wq)
-{
-	return wq->to_use->desc;
-}
-
 #define PI_LOG2_CACHE_LINE_SIZE        5
 #define PI_INDEX_BITS            12
 #define PI_INDEX_MASK ((1U << PI_INDEX_BITS) - 1)
@@ -191,6 +165,15 @@ static inline u64 vnic_cached_posted_index(dma_addr_t addr, unsigned int len,
 	PI_PREFETCH_ADDR_MASK) << PI_PREFETCH_ADDR_OFF);
 }
 
+static inline uint32_t
+buf_idx_incr(uint32_t n_descriptors, uint32_t idx)
+{
+	idx++;
+	if (unlikely(idx == n_descriptors))
+		idx = 0;
+	return idx;
+}
+
 static inline void vnic_wq_service(struct vnic_wq *wq,
 	struct cq_desc *cq_desc, u16 completed_index,
 	void (*buf_service)(struct vnic_wq *wq,
@@ -198,21 +181,24 @@ static inline void vnic_wq_service(struct vnic_wq *wq,
 	void *opaque)
 {
 	struct vnic_wq_buf *buf;
+	unsigned int to_clean = wq->tail_idx;
 
-	buf = wq->to_clean;
+	buf = &wq->bufs[to_clean];
 	while (1) {
 
 		(*buf_service)(wq, cq_desc, buf, opaque);
 
 		wq->ring.desc_avail++;
 
-		wq->to_clean = buf->next;
 
-		if (buf->index == completed_index)
+		to_clean = buf_idx_incr(wq->ring.desc_count, to_clean);
+
+		if (to_clean == completed_index)
 			break;
 
-		buf = wq->to_clean;
+		buf = &wq->bufs[to_clean];
 	}
+	wq->tail_idx = to_clean;
 }
 
 void vnic_wq_free(struct vnic_wq *wq);
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 5b58a65..fe186de 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -163,6 +163,30 @@ static inline struct enic *pmd_priv(struct rte_eth_dev *eth_dev)
 	return (struct enic *)eth_dev->data->dev_private;
 }
 
+static inline uint32_t
+enic_ring_add(uint32_t n_descriptors, uint32_t i0, uint32_t i1)
+{
+	uint32_t d = i0 + i1;
+	d -= (d >= n_descriptors) ? n_descriptors : 0;
+	return d;
+}
+
+static inline uint32_t
+enic_ring_sub(uint32_t n_descriptors, uint32_t i0, uint32_t i1)
+{
+	int32_t d = i1 - i0;
+	return (uint32_t)((d < 0) ? ((int32_t)n_descriptors + d) : d);
+}
+
+static inline uint32_t
+enic_ring_incr(uint32_t n_descriptors, uint32_t idx)
+{
+	idx++;
+	if (unlikely(idx == n_descriptors))
+		idx = 0;
+	return idx;
+}
+
 extern void enic_fdir_stats_get(struct enic *enic,
 	struct rte_eth_fdir_stats *stats);
 extern int enic_fdir_add_fltr(struct enic *enic,
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index f41ef86..5bf5fcf 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -105,10 +105,10 @@ void enic_set_hdr_split_size(struct enic *enic, u16 split_hdr_size)
 
 void enic_free_wq_buf(__rte_unused struct vnic_wq *wq, struct vnic_wq_buf *buf)
 {
-	struct rte_mbuf *mbuf = (struct rte_mbuf *)buf->os_buf;
+	struct rte_mbuf *mbuf = (struct rte_mbuf *)buf->mb;
 
 	rte_mempool_put(mbuf->pool, mbuf);
-	buf->os_buf = NULL;
+	buf->mb = NULL;
 }
 
 static void enic_log_q_error(struct enic *enic)
diff --git a/drivers/net/enic/enic_rxtx.c b/drivers/net/enic/enic_rxtx.c
index 26a7b6d..e63f2af 100644
--- a/drivers/net/enic/enic_rxtx.c
+++ b/drivers/net/enic/enic_rxtx.c
@@ -220,17 +220,6 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf)
 	mbuf->ol_flags = pkt_flags;
 }
 
-static inline uint32_t
-enic_ring_add(uint32_t n_descriptors, uint32_t i0, uint32_t i1)
-{
-	uint32_t d = i0 + i1;
-	RTE_ASSERT(i0 < n_descriptors);
-	RTE_ASSERT(i1 < n_descriptors);
-	d -= (d >= n_descriptors) ? n_descriptors : 0;
-	return d;
-}
-
-
 uint16_t
 enic_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	       uint16_t nb_pkts)
@@ -382,12 +371,15 @@ void enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
 		   uint8_t sop, uint8_t eop, uint8_t cq_entry,
 		   uint16_t ol_flags, uint16_t vlan_tag)
 {
-	struct wq_enet_desc *desc = vnic_wq_next_desc(wq);
+	struct wq_enet_desc *desc, *descs;
 	uint16_t mss = 0;
 	uint8_t vlan_tag_insert = 0;
 	uint64_t bus_addr = (dma_addr_t)
 	    (tx_pkt->buf_physaddr + tx_pkt->data_off);
 
+	descs = (struct wq_enet_desc *)wq->ring.descs;
+	desc = descs + wq->head_idx;
+
 	if (sop) {
 		if (ol_flags & PKT_TX_VLAN_PKT)
 			vlan_tag_insert = 1;
@@ -414,12 +406,7 @@ void enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
 		vlan_tag,
 		0 /* loopback */);
 
-	enic_vnic_post_wq(wq, (void *)tx_pkt, bus_addr, len,
-			  sop,
-			  1 /*desc_skip_cnt*/,
-			  cq_entry,
-			  0 /*compressed send*/,
-			  0 /*wrid*/);
+	enic_vnic_post_wq(wq, (void *)tx_pkt, cq_entry);
 }
 
 uint16_t enic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
-- 
2.7.0



More information about the dev mailing list