[11/14] net/enic: add the simple version of Tx handler

Message ID 20180628031940.17397-11-johndale@cisco.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [01/14] net/enic: fix receive packet types |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

John Daley (johndale) June 28, 2018, 3:19 a.m. UTC
  From: Hyong Youb Kim <hyonkim@cisco.com>

Add a much-simplified handler that works when all offloads are
disabled, except mbuf fast free. When compared against the default
handler, under ideal conditions, cycles per packet drop by 60+%.

By default, the driver tries to use the simple handler. Add a new
devarg (disable-simple-tx) to allow the user to force-disable this new
handler.

The idea of using specialized/simplified handlers is from the Intel
and Mellanox drivers.

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic.h        |  3 ++
 drivers/net/enic/enic_compat.h |  5 +++
 drivers/net/enic/enic_ethdev.c | 31 ++++++++++-------
 drivers/net/enic/enic_main.c   | 37 ++++++++++++++++++++
 drivers/net/enic/enic_rxtx.c   | 77 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 141 insertions(+), 12 deletions(-)
  

Comments

Ferruh Yigit June 28, 2018, 4:05 p.m. UTC | #1
On 6/28/2018 4:19 AM, John Daley wrote:
> From: Hyong Youb Kim <hyonkim@cisco.com>
> 
> Add a much-simplified handler that works when all offloads are
> disabled, except mbuf fast free. When compared against the default
> handler, under ideal conditions, cycles per packet drop by 60+%.
> 
> By default, the driver tries to use the simple handler. Add a new
> devarg (disable-simple-tx) to allow the user to force-disable this new
> handler.
> 
> The idea of using specialized/simplified handlers is from the Intel
> and Mellanox drivers.

Existing usage does this automatically instead of having a devarg for it, the
logic is use the simple handler when possible, if a feature requested that
simple handler doesn't support switch to handler that supports it, you may
consider same logic.

<...>

> @@ -42,6 +38,7 @@ static const struct rte_pci_id pci_id_enic_map[] = {
>  
>  #define ENIC_DEVARG_DISABLE_OVERLAY "disable-overlay"
>  #define ENIC_DEVARG_IG_VLAN_REWRITE "ig-vlan-rewrite"
> +#define ENIC_DEVARG_DISABLE_SIMPLE_TX "disable-simple-tx"

Same comment with previous one, please document new devargs.
  

Patch

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index af790fc2e..e1d0ea552 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -126,6 +126,7 @@  struct enic {
 	u8 filter_actions; /* HW supported actions */
 	bool vxlan;
 	bool disable_overlay; /* devargs disable_overlay=1 */
+	bool disable_simple_tx; /* devargs disable-simple-tx=1 */
 	bool nic_cfg_chk;     /* NIC_CFG_CHK available */
 	bool udp_rss_weak;    /* Bodega style UDP RSS */
 	uint8_t ig_vlan_rewrite_mode; /* devargs ig-vlan-rewrite */
@@ -318,6 +319,8 @@  uint16_t enic_dummy_recv_pkts(void *rx_queue,
 			      uint16_t nb_pkts);
 uint16_t enic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			uint16_t nb_pkts);
+uint16_t enic_simple_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
+			       uint16_t nb_pkts);
 uint16_t enic_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			uint16_t nb_pkts);
 int enic_set_mtu(struct enic *enic, uint16_t new_mtu);
diff --git a/drivers/net/enic/enic_compat.h b/drivers/net/enic/enic_compat.h
index c0af1ed29..ceb1b0962 100644
--- a/drivers/net/enic/enic_compat.h
+++ b/drivers/net/enic/enic_compat.h
@@ -56,6 +56,11 @@ 
 #define dev_debug(x, args...) dev_printk(DEBUG, args)
 
 extern int enicpmd_logtype_flow;
+extern int enicpmd_logtype_init;
+
+#define PMD_INIT_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, enicpmd_logtype_init, \
+		"%s" fmt "\n", __func__, ##args)
 
 #define __le16 u16
 #define __le32 u32
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index ef18f8802..d013333f9 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -24,10 +24,6 @@ 
 int enicpmd_logtype_init;
 int enicpmd_logtype_flow;
 
-#define PMD_INIT_LOG(level, fmt, args...) \
-	rte_log(RTE_LOG_ ## level, enicpmd_logtype_init, \
-		"%s" fmt "\n", __func__, ##args)
-
 #define ENICPMD_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
 
 /*
@@ -42,6 +38,7 @@  static const struct rte_pci_id pci_id_enic_map[] = {
 
 #define ENIC_DEVARG_DISABLE_OVERLAY "disable-overlay"
 #define ENIC_DEVARG_IG_VLAN_REWRITE "ig-vlan-rewrite"
+#define ENIC_DEVARG_DISABLE_SIMPLE_TX "disable-simple-tx"
 
 RTE_INIT(enicpmd_init_log);
 static void
@@ -920,22 +917,27 @@  static const struct eth_dev_ops enicpmd_eth_dev_ops = {
 	.udp_tunnel_port_del  = enicpmd_dev_udp_tunnel_port_del,
 };
 
-static int enic_parse_disable_overlay(__rte_unused const char *key,
-				      const char *value,
-				      void *opaque)
+static int enic_parse_zero_one(const char *key,
+			       const char *value,
+			       void *opaque)
 {
 	struct enic *enic;
+	bool b;
 
 	enic = (struct enic *)opaque;
 	if (strcmp(value, "0") == 0) {
-		enic->disable_overlay = false;
+		b = false;
 	} else if (strcmp(value, "1") == 0) {
-		enic->disable_overlay = true;
+		b = true;
 	} else {
-		dev_err(enic, "Invalid value for " ENIC_DEVARG_DISABLE_OVERLAY
-			": expected=0|1 given=%s\n", value);
+		dev_err(enic, "Invalid value for %s"
+			": expected=0|1 given=%s\n", key, value);
 		return -EINVAL;
 	}
+	if (strcmp(key, ENIC_DEVARG_DISABLE_OVERLAY) == 0)
+		enic->disable_overlay = b;
+	if (strcmp(key, ENIC_DEVARG_DISABLE_SIMPLE_TX) == 0)
+		enic->disable_simple_tx = b;
 	return 0;
 }
 
@@ -976,6 +978,7 @@  static int enic_check_devargs(struct rte_eth_dev *dev)
 {
 	static const char *const valid_keys[] = {
 		ENIC_DEVARG_DISABLE_OVERLAY,
+		ENIC_DEVARG_DISABLE_SIMPLE_TX,
 		ENIC_DEVARG_IG_VLAN_REWRITE,
 		NULL};
 	struct enic *enic = pmd_priv(dev);
@@ -984,6 +987,7 @@  static int enic_check_devargs(struct rte_eth_dev *dev)
 	ENICPMD_FUNC_TRACE();
 
 	enic->disable_overlay = false;
+	enic->disable_simple_tx = false;
 	enic->ig_vlan_rewrite_mode = IG_VLAN_REWRITE_MODE_PASS_THRU;
 	if (!dev->device->devargs)
 		return 0;
@@ -991,7 +995,9 @@  static int enic_check_devargs(struct rte_eth_dev *dev)
 	if (!kvlist)
 		return -EINVAL;
 	if (rte_kvargs_process(kvlist, ENIC_DEVARG_DISABLE_OVERLAY,
-			       enic_parse_disable_overlay, enic) < 0 ||
+			       enic_parse_zero_one, enic) < 0 ||
+	    rte_kvargs_process(kvlist, ENIC_DEVARG_DISABLE_SIMPLE_TX,
+			       enic_parse_zero_one, enic) < 0 ||
 	    rte_kvargs_process(kvlist, ENIC_DEVARG_IG_VLAN_REWRITE,
 			       enic_parse_ig_vlan_rewrite, enic) < 0) {
 		rte_kvargs_free(kvlist);
@@ -1059,4 +1065,5 @@  RTE_PMD_REGISTER_PCI_TABLE(net_enic, pci_id_enic_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_enic, "* igb_uio | uio_pci_generic | vfio-pci");
 RTE_PMD_REGISTER_PARAM_STRING(net_enic,
 	ENIC_DEVARG_DISABLE_OVERLAY "=0|1 "
+	ENIC_DEVARG_DISABLE_SIMPLE_TX "=0|1 "
 	ENIC_DEVARG_IG_VLAN_REWRITE "=trunk|untag|priority|pass");
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index c03ec247a..66706448c 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -493,6 +493,27 @@  static void enic_rxq_intr_deinit(struct enic *enic)
 	}
 }
 
+static void enic_prep_wq_for_simple_tx(struct enic *enic, uint16_t queue_idx)
+{
+	struct wq_enet_desc *desc;
+	struct vnic_wq *wq;
+	unsigned int i;
+
+	/*
+	 * Fill WQ descriptor fields that never change. Every descriptor is
+	 * one packet, so set EOP. Also set CQ_ENTRY every ENIC_WQ_CQ_THRESH
+	 * descriptors (i.e. request one completion update every 32 packets).
+	 */
+	wq = &enic->wq[queue_idx];
+	desc = (struct wq_enet_desc *)wq->ring.descs;
+	for (i = 0; i < wq->ring.desc_count; i++, desc++) {
+		desc->header_length_flags = 1 << WQ_ENET_FLAGS_EOP_SHIFT;
+		if (i % ENIC_WQ_CQ_THRESH == ENIC_WQ_CQ_THRESH - 1)
+			desc->header_length_flags |=
+				(1 << WQ_ENET_FLAGS_CQ_ENTRY_SHIFT);
+	}
+}
+
 int enic_enable(struct enic *enic)
 {
 	unsigned int index;
@@ -535,6 +556,22 @@  int enic_enable(struct enic *enic)
 		}
 	}
 
+	/*
+	 * Use the simple TX handler if possible. All offloads must be disabled
+	 * except mbuf fast free.
+	 */
+	if (!enic->disable_simple_tx &&
+	    (eth_dev->data->dev_conf.txmode.offloads &
+	     ~DEV_TX_OFFLOAD_MBUF_FAST_FREE) == 0) {
+		PMD_INIT_LOG(DEBUG, " use the simple tx handler");
+		eth_dev->tx_pkt_burst = &enic_simple_xmit_pkts;
+		for (index = 0; index < enic->wq_count; index++)
+			enic_prep_wq_for_simple_tx(enic, index);
+	} else {
+		PMD_INIT_LOG(DEBUG, " use the default tx handler");
+		eth_dev->tx_pkt_burst = &enic_xmit_pkts;
+	}
+
 	for (index = 0; index < enic->wq_count; index++)
 		enic_start_wq(enic, index);
 	for (index = 0; index < enic->rq_count; index++)
diff --git a/drivers/net/enic/enic_rxtx.c b/drivers/net/enic/enic_rxtx.c
index 7cddb53d9..7dec486fe 100644
--- a/drivers/net/enic/enic_rxtx.c
+++ b/drivers/net/enic/enic_rxtx.c
@@ -741,4 +741,81 @@  uint16_t enic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	return index;
 }
 
+static void enqueue_simple_pkts(struct rte_mbuf **pkts,
+				struct wq_enet_desc *desc,
+				uint16_t n,
+				struct enic *enic)
+{
+	struct rte_mbuf *p;
+
+	while (n) {
+		n--;
+		p = *pkts++;
+		desc->address = p->buf_iova + p->data_off;
+		desc->length = p->pkt_len;
+		/*
+		 * The app should not send oversized
+		 * packets. tx_pkt_prepare includes a check as
+		 * well. But some apps ignore the device max size and
+		 * tx_pkt_prepare. Oversized packets cause WQ errrors
+		 * and the NIC ends up disabling the whole WQ. So
+		 * truncate packets..
+		 */
+		if (unlikely(p->pkt_len > ENIC_TX_MAX_PKT_SIZE)) {
+			desc->length = ENIC_TX_MAX_PKT_SIZE;
+			rte_atomic64_inc(&enic->soft_stats.tx_oversized);
+		}
+		desc++;
+	}
+}
+
+uint16_t enic_simple_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
+			       uint16_t nb_pkts)
+{
+	unsigned int head_idx, desc_count;
+	struct wq_enet_desc *desc;
+	struct vnic_wq *wq;
+	struct enic *enic;
+	uint16_t rem, n;
+
+	wq = (struct vnic_wq *)tx_queue;
+	enic = vnic_dev_priv(wq->vdev);
+	enic_cleanup_wq(enic, wq);
+	/* Will enqueue this many packets in this call */
+	nb_pkts = RTE_MIN(nb_pkts, wq->ring.desc_avail);
+	if (nb_pkts == 0)
+		return 0;
+
+	head_idx = wq->head_idx;
+	desc_count = wq->ring.desc_count;
+
+	/* Descriptors until the end of the ring */
+	n = desc_count - head_idx;
+	n = RTE_MIN(nb_pkts, n);
 
+	/* Save mbuf pointers to free later */
+	memcpy(wq->bufs + head_idx, tx_pkts, sizeof(struct rte_mbuf *) * n);
+
+	/* Enqueue until the ring end */
+	rem = nb_pkts - n;
+	desc = ((struct wq_enet_desc *)wq->ring.descs) + head_idx;
+	enqueue_simple_pkts(tx_pkts, desc, n, enic);
+
+	/* Wrap to the start of the ring */
+	if (rem) {
+		tx_pkts += n;
+		memcpy(wq->bufs, tx_pkts, sizeof(struct rte_mbuf *) * rem);
+		desc = (struct wq_enet_desc *)wq->ring.descs;
+		enqueue_simple_pkts(tx_pkts, desc, rem, enic);
+	}
+	rte_wmb();
+
+	/* Update head_idx and desc_avail */
+	wq->ring.desc_avail -= nb_pkts;
+	head_idx += nb_pkts;
+	if (head_idx >= desc_count)
+		head_idx -= desc_count;
+	wq->head_idx = head_idx;
+	iowrite32_relaxed(head_idx, &wq->ctrl->posted_index);
+	return nb_pkts;
+}