[PATCH 0/4] netvsc performance enhancements

[PATCH 0/4] netvsc performance enhancements

[Stephen Hemminger]

Some more netvsc performance related changes.
The biggest benefit comes from lowering the hypervisor polling rate
from 100us to 50us.  Supporting tx_done_cleanup can also help some
applications get through the send completions faster.

Stephen Hemminger (4):
  netvsc: chimney buffer size error handling
  bus/vmbus: add host latency tuning function
  netvsc: set lower host latency
  netvsc: implement tx_done_cleanup

 drivers/bus/vmbus/rte_bus_vmbus.h           | 15 +++++++++++
 drivers/bus/vmbus/rte_bus_vmbus_version.map |  1 +
 drivers/bus/vmbus/vmbus_channel.c           | 30 +++++++++++++++++++++
 drivers/net/netvsc/hn_ethdev.c              |  7 +++++
 drivers/net/netvsc/hn_nvs.c                 | 13 +++------
 drivers/net/netvsc/hn_rndis.c               |  2 +-
 drivers/net/netvsc/hn_rxtx.c                | 26 ++++++++++++++----
 drivers/net/netvsc/hn_var.h                 |  7 ++++-
 8 files changed, 85 insertions(+), 16 deletions(-)

-- 
2.18.0


Stephen Hemminger (4):
  netvsc: chimney buffer size error handling
  bus/vmbus: add host latency tuning function
  netvsc: set lower host latency
  netvsc: implement tx_done_cleanup

 drivers/bus/vmbus/rte_bus_vmbus.h           | 15 ++++++++++++
 drivers/bus/vmbus/rte_bus_vmbus_version.map |  1 +
 drivers/bus/vmbus/vmbus_channel.c           | 26 +++++++++++++++++++++
 drivers/net/netvsc/hn_ethdev.c              |  7 ++++++
 drivers/net/netvsc/hn_nvs.c                 | 13 ++++-------
 drivers/net/netvsc/hn_rndis.c               |  2 +-
 drivers/net/netvsc/hn_rxtx.c                | 26 +++++++++++++++++----
 drivers/net/netvsc/hn_var.h                 |  7 +++++-
 8 files changed, 81 insertions(+), 16 deletions(-)

-- 
2.18.0

[PATCH 1/4] netvsc: chimney buffer size error handling

[Stephen Hemminger]

Fix the error handling in setting up transmit buffer.
If setting up chimney buffer fails, then it is not connected so
no need to send disconnect.

Allow for some unused area if full area is not used.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/netvsc/hn_nvs.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/netvsc/hn_nvs.c b/drivers/net/netvsc/hn_nvs.c
index 77d3b839fd45..a458bb720f82 100644
--- a/drivers/net/netvsc/hn_nvs.c
+++ b/drivers/net/netvsc/hn_nvs.c
@@ -279,14 +279,13 @@ hn_nvs_conn_chim(struct hn_data *hv)
 			       NVS_TYPE_CHIM_CONNRESP);
 	if (error) {
 		PMD_DRV_LOG(ERR, "exec nvs chim conn failed");
-		goto cleanup;
+		return error;
 	}
 
 	if (resp.status != NVS_STATUS_OK) {
 		PMD_DRV_LOG(ERR, "nvs chim conn failed: %x",
 			    resp.status);
-		error = -EIO;
-		goto cleanup;
+		return -EIO;
 	}
 
 	sectsz = resp.sectsz;
@@ -295,7 +294,8 @@ hn_nvs_conn_chim(struct hn_data *hv)
 		PMD_DRV_LOG(NOTICE,
 			    "invalid chimney sending buffer section size: %u",
 			    sectsz);
-		return 0;
+		error = -EINVAL;
+		goto cleanup;
 	}
 
 	hv->chim_szmax = sectsz;
@@ -304,11 +304,6 @@ hn_nvs_conn_chim(struct hn_data *hv)
 	PMD_DRV_LOG(INFO, "send buffer %lu section size:%u, count:%u",
 		    len, hv->chim_szmax, hv->chim_cnt);
 
-	if (len % hv->chim_szmax != 0) {
-		PMD_DRV_LOG(NOTICE,
-			    "chimney sending sections are not properly aligned");
-	}
-
 	/* Done! */
 	return 0;
 
-- 
2.18.0

Re: [PATCH 1/4] netvsc: chimney buffer size error handling

[Ferruh Yigit]

On 8/9/2018 6:50 PM, Stephen Hemminger wrote:
> Fix the error handling in setting up transmit buffer.
> If setting up chimney buffer fails, then it is not connected so
> no need to send disconnect.
> 
> Allow for some unused area if full area is not used.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>

subsytem is net/netvsc, so title should be:
net/netvsc: fix chimney buffer size error handling

Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
Cc: stable@dpdk.org

Assuming you want patch to be backported.

I will update as above while merging.

Re: [PATCH 1/4] netvsc: chimney buffer size error handling

[Stephen Hemminger]

On Thu, 23 Aug 2018 15:45:17 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 8/9/2018 6:50 PM, Stephen Hemminger wrote:
> > Fix the error handling in setting up transmit buffer.
> > If setting up chimney buffer fails, then it is not connected so
> > no need to send disconnect.
> > 
> > Allow for some unused area if full area is not used.
> > 
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>  
> 
> subsytem is net/netvsc, so title should be:
> net/netvsc: fix chimney buffer size error handling
> 
> Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
> Cc: stable@dpdk.org
> 
> Assuming you want patch to be backported.
> 
> I will update as above while merging.

There is not a high urgency to back port this. If chimney buffer setup
fails the driver is unusable anyway. It is more of a theoretical error condition.

[PATCH 2/4] bus/vmbus: add host latency tuning function

[Stephen Hemminger]

Add vmbus API to allow tuning the scan interval on the host side.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/bus/vmbus/rte_bus_vmbus.h           | 15 ++++++++++++
 drivers/bus/vmbus/rte_bus_vmbus_version.map |  1 +
 drivers/bus/vmbus/vmbus_channel.c           | 26 +++++++++++++++++++++
 3 files changed, 42 insertions(+)

diff --git a/drivers/bus/vmbus/rte_bus_vmbus.h b/drivers/bus/vmbus/rte_bus_vmbus.h
index 4a2c1f6fd918..2839fef5b27a 100644
--- a/drivers/bus/vmbus/rte_bus_vmbus.h
+++ b/drivers/bus/vmbus/rte_bus_vmbus.h
@@ -364,6 +364,21 @@ void rte_vmbus_chan_signal_read(struct vmbus_channel *chan, uint32_t bytes_read)
  */
 uint16_t rte_vmbus_sub_channel_index(const struct vmbus_channel *chan);
 
+/**
+ * Set the host monitor latency hint
+ *
+ * @param dev
+ *    VMBUS device
+ * @param chan
+ *	Pointer to vmbus_channel structure.
+ * @param latency
+ *	Approximate wait period between hypervisor examinations of
+ *	the trigger page (in nanoseconds).
+ */
+void rte_vmbus_set_latency(const struct rte_vmbus_device *dev,
+			   const struct vmbus_channel *chan,
+			   uint32_t latency);
+
 /**
  * Register a VMBUS driver.
  *
diff --git a/drivers/bus/vmbus/rte_bus_vmbus_version.map b/drivers/bus/vmbus/rte_bus_vmbus_version.map
index dabb9203104b..5b01ff30d7b0 100644
--- a/drivers/bus/vmbus/rte_bus_vmbus_version.map
+++ b/drivers/bus/vmbus/rte_bus_vmbus_version.map
@@ -20,6 +20,7 @@ DPDK_18.08 {
 	rte_vmbus_probe;
 	rte_vmbus_register;
 	rte_vmbus_scan;
+	rte_vmbus_set_latency;
 	rte_vmbus_sub_channel_index;
 	rte_vmbus_subchan_open;
 	rte_vmbus_unmap_device;
diff --git a/drivers/bus/vmbus/vmbus_channel.c b/drivers/bus/vmbus/vmbus_channel.c
index cc5f3e8379a5..bd14c0662b46 100644
--- a/drivers/bus/vmbus/vmbus_channel.c
+++ b/drivers/bus/vmbus/vmbus_channel.c
@@ -59,6 +59,32 @@ vmbus_set_event(const struct rte_vmbus_device *dev,
 	vmbus_set_monitor(dev, chan->monitor_id);
 }
 
+/*
+ * Set the wait between when hypervisor examines the trigger.
+ */
+void
+rte_vmbus_set_latency(const struct rte_vmbus_device *dev,
+		      const struct vmbus_channel *chan,
+		      uint32_t latency)
+{
+	uint32_t trig_idx = chan->monitor_id / VMBUS_MONTRIG_LEN;
+	uint32_t trig_offs = chan->monitor_id % VMBUS_MONTRIG_LEN;
+
+	if (latency >= UINT16_MAX * 100) {
+		VMBUS_LOG(ERR, "invalid latency value %u", latency);
+		return;
+	}
+
+	if (trig_idx >= VMBUS_MONTRIGS_MAX) {
+		VMBUS_LOG(ERR, "invalid monitor trigger %u",
+			  trig_idx);
+		return;
+	}
+
+	/* Host value is expressed in 100 nanosecond units */
+	dev->monitor_page->lat[trig_idx][trig_offs] = latency / 100;
+}
+
 /*
  * Notify host that there are data pending on our TX bufring.
  *
-- 
2.18.0

Re: [PATCH 2/4] bus/vmbus: add host latency tuning function

[Ferruh Yigit]

On 8/9/2018 6:50 PM, Stephen Hemminger wrote:
> Add vmbus API to allow tuning the scan interval on the host side.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>

<...>

> @@ -20,6 +20,7 @@ DPDK_18.08 {
>  	rte_vmbus_probe;
>  	rte_vmbus_register;
>  	rte_vmbus_scan;
> +	rte_vmbus_set_latency;
>  	rte_vmbus_sub_channel_index;
>  	rte_vmbus_subchan_open;
>  	rte_vmbus_unmap_device;

Needs to be on DPDK_18.11 now, will fix while merging.

(Since this is not really a public API, I think no need to mark as experimental
as process and checkpatches requires.)

Re: [PATCH 2/4] bus/vmbus: add host latency tuning function

[Stephen Hemminger]

On Thu, 23 Aug 2018 15:45:44 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 8/9/2018 6:50 PM, Stephen Hemminger wrote:
> > Add vmbus API to allow tuning the scan interval on the host side.
> > 
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>  
> 
> <...>
> 
> > @@ -20,6 +20,7 @@ DPDK_18.08 {
> >  	rte_vmbus_probe;
> >  	rte_vmbus_register;
> >  	rte_vmbus_scan;
> > +	rte_vmbus_set_latency;
> >  	rte_vmbus_sub_channel_index;
> >  	rte_vmbus_subchan_open;
> >  	rte_vmbus_unmap_device;  
> 
> Needs to be on DPDK_18.11 now, will fix while merging.
> 
> (Since this is not really a public API, I think no need to mark as experimental
> as process and checkpatches requires.)

Sure, thanks. I was testing on 18.08.  We need to be able to figure
out internal versus public API symbols at some point.  It doesn't matter
if a bus API changes if there is no userspace API that can directly access it.

[PATCH 3/4] netvsc: set lower host latency

[Stephen Hemminger]

Tune the vmbus connection so the host scans faster. This improves
transmit performance. The host default value is 100us but setting
to 50us reduces packet loss significantly.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/netvsc/hn_ethdev.c | 6 ++++++
 drivers/net/netvsc/hn_var.h    | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 78b842ba2d68..148e6a33d682 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -264,6 +264,9 @@ static int hn_subchan_configure(struct hn_data *hv,
 			return err;
 		}
 
+		rte_vmbus_set_latency(hv->vmbus, new_sc,
+				      HN_CHAN_LATENCY_NS);
+
 		retry = 0;
 		chn_index = rte_vmbus_sub_channel_index(new_sc);
 		if (chn_index == 0 || chn_index > hv->max_queues) {
@@ -629,6 +632,9 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	if (err)
 		return err;
 
+	rte_vmbus_set_latency(hv->vmbus, hv->channels[0],
+			      HN_CHAN_LATENCY_NS);
+
 	hv->primary = hn_rx_queue_alloc(hv, 0,
 					eth_dev->device->numa_node);
 
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index f7ff8585bc1c..b3e0a93d45df 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -20,6 +20,9 @@
 /* Retry interval */
 #define HN_CHAN_INTERVAL_US	100
 
+/* Host monitor interval */
+#define HN_CHAN_LATENCY_NS	50000
+
 /* Buffers need to be aligned */
 #ifndef PAGE_SIZE
 #define PAGE_SIZE 4096
-- 
2.18.0

[PATCH 4/4] netvsc: implement tx_done_cleanup

[Stephen Hemminger]

Add tx_done_cleanup ethdev hook to allow application to
control if/when it wants completions to be handled.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/netvsc/hn_ethdev.c |  1 +
 drivers/net/netvsc/hn_rndis.c  |  2 +-
 drivers/net/netvsc/hn_rxtx.c   | 26 +++++++++++++++++++++-----
 drivers/net/netvsc/hn_var.h    |  4 +++-
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 148e6a33d682..2200ee319f98 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -547,6 +547,7 @@ static const struct eth_dev_ops hn_eth_dev_ops = {
 	.allmulticast_disable   = hn_dev_allmulticast_disable,
 	.tx_queue_setup		= hn_dev_tx_queue_setup,
 	.tx_queue_release	= hn_dev_tx_queue_release,
+	.tx_done_cleanup        = hn_dev_tx_done_cleanup,
 	.rx_queue_setup		= hn_dev_rx_queue_setup,
 	.rx_queue_release	= hn_dev_rx_queue_release,
 	.link_update		= hn_dev_link_update,
diff --git a/drivers/net/netvsc/hn_rndis.c b/drivers/net/netvsc/hn_rndis.c
index bde33969331e..f44add726b91 100644
--- a/drivers/net/netvsc/hn_rndis.c
+++ b/drivers/net/netvsc/hn_rndis.c
@@ -382,7 +382,7 @@ static int hn_rndis_exec1(struct hn_data *hv,
 	if (comp) {
 		/* Poll primary channel until response received */
 		while (hv->rndis_pending == rid)
-			hn_process_events(hv, 0);
+			hn_process_events(hv, 0, 1);
 
 		memcpy(comp, hv->rndis_resp, comp_len);
 	}
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index 02ef27e363cc..24abc2a91cea 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -801,6 +801,14 @@ hn_dev_rx_queue_release(void *arg)
 	}
 }
 
+int
+hn_dev_tx_done_cleanup(void *arg, uint32_t free_cnt)
+{
+	struct hn_tx_queue *txq = arg;
+
+	return hn_process_events(txq->hv, txq->queue_id, free_cnt);
+}
+
 void
 hn_dev_rx_queue_info(struct rte_eth_dev *dev, uint16_t queue_idx,
 		     struct rte_eth_rxq_info *qinfo)
@@ -831,25 +839,27 @@ hn_nvs_handle_notify(const struct vmbus_chanpkt_hdr *pkthdr,
  * Process pending events on the channel.
  * Called from both Rx queue poll and Tx cleanup
  */
-void hn_process_events(struct hn_data *hv, uint16_t queue_id)
+uint32_t hn_process_events(struct hn_data *hv, uint16_t queue_id,
+			   uint32_t tx_limit)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[hv->port_id];
 	struct hn_rx_queue *rxq;
 	uint32_t bytes_read = 0;
+	uint32_t tx_done = 0;
 	int ret = 0;
 
 	rxq = queue_id == 0 ? hv->primary : dev->data->rx_queues[queue_id];
 
 	/* If no pending data then nothing to do */
 	if (rte_vmbus_chan_rx_empty(rxq->chan))
-		return;
+		return 0;
 
 	/*
 	 * Since channel is shared between Rx and TX queue need to have a lock
 	 * since DPDK does not force same CPU to be used for Rx/Tx.
 	 */
 	if (unlikely(!rte_spinlock_trylock(&rxq->ring_lock)))
-		return;
+		return 0;
 
 	for (;;) {
 		const struct vmbus_chanpkt_hdr *pkt;
@@ -873,6 +883,7 @@ void hn_process_events(struct hn_data *hv, uint16_t queue_id)
 
 		switch (pkt->type) {
 		case VMBUS_CHANPKT_TYPE_COMP:
+			++tx_done;
 			hn_nvs_handle_comp(dev, queue_id, pkt, data);
 			break;
 
@@ -889,6 +900,9 @@ void hn_process_events(struct hn_data *hv, uint16_t queue_id)
 			break;
 		}
 
+		if (tx_limit && tx_done >= tx_limit)
+			break;
+
 		if (rxq->rx_ring && rte_ring_full(rxq->rx_ring))
 			break;
 	}
@@ -897,6 +911,8 @@ void hn_process_events(struct hn_data *hv, uint16_t queue_id)
 		rte_vmbus_chan_signal_read(rxq->chan, bytes_read);
 
 	rte_spinlock_unlock(&rxq->ring_lock);
+
+	return tx_done;
 }
 
 static void hn_append_to_chim(struct hn_tx_queue *txq,
@@ -1244,7 +1260,7 @@ hn_xmit_pkts(void *ptxq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		return 0;
 
 	if (rte_mempool_avail_count(hv->tx_pool) <= txq->free_thresh)
-		hn_process_events(hv, txq->queue_id);
+		hn_process_events(hv, txq->queue_id, 0);
 
 	for (nb_tx = 0; nb_tx < nb_pkts; nb_tx++) {
 		struct rte_mbuf *m = tx_pkts[nb_tx];
@@ -1326,7 +1342,7 @@ hn_recv_pkts(void *prxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 
 	/* If ring is empty then process more */
 	if (rte_ring_count(rxq->rx_ring) < nb_pkts)
-		hn_process_events(hv, rxq->queue_id);
+		hn_process_events(hv, rxq->queue_id, 0);
 
 	/* Get mbufs off staging ring */
 	return rte_ring_sc_dequeue_burst(rxq->rx_ring, (void **)rx_pkts,
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index b3e0a93d45df..fec8d7c402a5 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -133,7 +133,8 @@ hn_primary_chan(const struct hn_data *hv)
 	return hv->channels[0];
 }
 
-void hn_process_events(struct hn_data *hv, uint16_t queue_id);
+uint32_t hn_process_events(struct hn_data *hv, uint16_t queue_id,
+		       uint32_t tx_limit);
 
 uint16_t hn_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		      uint16_t nb_pkts);
@@ -147,6 +148,7 @@ int	hn_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 void	hn_dev_tx_queue_release(void *arg);
 void	hn_dev_tx_queue_info(struct rte_eth_dev *dev, uint16_t queue_idx,
 			     struct rte_eth_txq_info *qinfo);
+int	hn_dev_tx_done_cleanup(void *arg, uint32_t free_cnt);
 
 struct hn_rx_queue *hn_rx_queue_alloc(struct hn_data *hv,
 				      uint16_t queue_id,
-- 
2.18.0

Re: [PATCH 4/4] netvsc: implement tx_done_cleanup

[Ferruh Yigit]

On 8/9/2018 6:50 PM, Stephen Hemminger wrote:
> Add tx_done_cleanup ethdev hook to allow application to
> control if/when it wants completions to be handled.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  drivers/net/netvsc/hn_ethdev.c |  1 +
>  drivers/net/netvsc/hn_rndis.c  |  2 +-
>  drivers/net/netvsc/hn_rxtx.c   | 26 +++++++++++++++++++++-----
>  drivers/net/netvsc/hn_var.h    |  4 +++-
>  4 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
> index 148e6a33d682..2200ee319f98 100644
> --- a/drivers/net/netvsc/hn_ethdev.c
> +++ b/drivers/net/netvsc/hn_ethdev.c
> @@ -547,6 +547,7 @@ static const struct eth_dev_ops hn_eth_dev_ops = {
>  	.allmulticast_disable   = hn_dev_allmulticast_disable,
>  	.tx_queue_setup		= hn_dev_tx_queue_setup,
>  	.tx_queue_release	= hn_dev_tx_queue_release,
> +	.tx_done_cleanup        = hn_dev_tx_done_cleanup,

This requires "Free Tx mbuf on demand" feature advertised as supported on
doc/guides/nics/features/netvsc.ini document. Will add while merging.

Re: [PATCH 0/4] netvsc performance enhancements

[Ferruh Yigit]

On 8/9/2018 6:50 PM, Stephen Hemminger wrote:
> Some more netvsc performance related changes.
> The biggest benefit comes from lowering the hypervisor polling rate
> from 100us to 50us.  Supporting tx_done_cleanup can also help some
> applications get through the send completions faster.
> 
> Stephen Hemminger (4):
>   netvsc: chimney buffer size error handling
>   bus/vmbus: add host latency tuning function
>   netvsc: set lower host latency
>   netvsc: implement tx_done_cleanup

Series applied to dpdk-next-net/master, thanks.

(Mentioned changes in individual patches applied while applying.)