netvsc: performance regressions in net-next

PATCH In review Stephen Hemminger 2 participants 6 messages Mar 20, 2017 – Mar 22, 2017

Subsystems (3)

Networking Network drivers Hyper-V/Azure core and drivers

lore

Share & tools

lore

Stephen Hemminger

author

2017-03-20 20:28 UTC #0 lore raw

cover netvsc: performance regressions in net-next

Fix for performance regression introduced with NAPI change; and followup cleanup of vmbus core code.

Stephen Hemminger (2):
netvsc: fix NAPI performance regression
vmbus: no longer expose iterator internals

 drivers/hv/ring_buffer.c        | 51 +++++++++++++++++++++--------------------
 drivers/net/hyperv/hyperv_net.h |  1 +
 drivers/net/hyperv/netvsc.c     | 41 +++++++++++++++------------------
 include/linux/hyperv.h          | 22 +-----------------
 4 files changed, 46 insertions(+), 69 deletions(-)

Signature · 1 line

2.11.0

Stephen Hemminger

author

2017-03-20 20:28 UTC #1 lore raw

PATCH 1/2 netvsc: fix NAPI performance regression

When using NAPI, the single stream performance declined signifcantly because the poll routine was updating host after every burst of packets. This excess signalling caused host throttling.

This fix restores the old behavior. Host is only signalled after the ring has been emptied.

Signed-off-by Stephen Hemminger <[email protected]>

---
 drivers/net/hyperv/hyperv_net.h |  1 +
 drivers/net/hyperv/netvsc.c     | 41 ++++++++++++++++++-----------------------
 2 files changed, 19 insertions(+), 23 deletions(-)

drivers/net/hyperv/hyperv_net.h +1 -0

index 6b5f75217694..a33f2ee86044 100644
@@ -723,6 +723,7 @@ struct net_device_context {
 /* Per channel data */
 struct netvsc_channel {
 	struct vmbus_channel *channel;
+	const struct vmpacket_descriptor *desc;
 	struct napi_struct napi;
 	struct multi_send_data msd;
 	struct multi_recv_comp mrc;

drivers/net/hyperv/netvsc.c +18 ~~-23~~

77 lines not on this page · raw message

Signature · 1 line

2.11.0

Stephen Hemminger

author

2017-03-20 20:28 UTC #2 lore raw

PATCH 2/2 vmbus: no longer expose iterator internals

In reply to Stephen Hemminger

Since NAPI no longer needs to jump out of iterator early, no need to expose the internal iterator steps.

Signed-off-by Stephen Hemminger <[email protected]>

---
 drivers/hv/ring_buffer.c | 51 ++++++++++++++++++++++++------------------------
 include/linux/hyperv.h   | 22 +--------------------
 2 files changed, 27 insertions(+), 46 deletions(-)

drivers/hv/ring_buffer.c +26 ~~-25~~

index c3f1a9e33cef..280e2010913f 100644
@@ -458,13 +458,31 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct vmbus_channel *channel)
 EXPORT_SYMBOL_GPL(hv_pkt_iter_first);
 
 /*
+ * Update host ring buffer after iterating over packets.
+ */
+static void hv_pkt_iter_close(struct vmbus_channel *channel)
+{
+	struct hv_ring_buffer_info *rbi = &channel->inbound;
+
+	/*
+	 * Make sure all reads are done before we update the read index since
+	 * the writer may start writing to the read area once the read index
+	 * is updated.
+	 */
+	virt_rmb();
+	rbi->ring_buffer->read_index = rbi->priv_read_index;
+
+	hv_signal_on_read(channel);
+}
+
+/*
  * Get next vmbus packet from ring buffer.
  *
  * Advances the current location (priv_read_index) and checks for more
- * data. If the end of the ring buffer is reached, then return NULL.
+ * data. If at end of list, return NULL and update host.
  */
 struct vmpacket_descriptor *
-__hv_pkt_iter_next(struct vmbus_channel *channel,
+hv_pkt_iter_next(struct vmbus_channel *channel,
 		   const struct vmpacket_descriptor *desc)
 {
 	struct hv_ring_buffer_info *rbi = &channel->inbound;

@@ -476,29 +494,12 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
 	if (rbi->priv_read_index >= dsize)
 		rbi->priv_read_index -= dsize;
 
-	/* more data? */
-	if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
+	/* if no more data? */
+	if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor)) {
+		hv_pkt_iter_close(channel);
 		return NULL;
-	else
-		return hv_get_ring_buffer(rbi) + rbi->priv_read_index;
-}
-EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
-
-/*
- * Update host ring buffer after iterating over packets.
- */
-void hv_pkt_iter_close(struct vmbus_channel *channel)
-{
-	struct hv_ring_buffer_info *rbi = &channel->inbound;
-
-	/*
-	 * Make sure all reads are done before we update the read index since
-	 * the writer may start writing to the read area once the read index
-	 * is updated.
-	 */
-	virt_rmb();
-	rbi->ring_buffer->read_index = rbi->priv_read_index;
+	}
 
-	hv_signal_on_read(channel);
+	return hv_get_ring_buffer(rbi) + rbi->priv_read_index;
 }
-EXPORT_SYMBOL_GPL(hv_pkt_iter_close);
+EXPORT_SYMBOL_GPL(hv_pkt_iter_next);

include/linux/hyperv.h +1 ~~-21~~

index 36162485d663..7df6ab5b3067 100644
@@ -1550,32 +1550,12 @@ static inline u32 hv_pkt_datalen(const struct vmpacket_descriptor *desc)
 	return (desc->len8 << 3) - (desc->offset8 << 3);
 }
 
-
 struct vmpacket_descriptor *
 hv_pkt_iter_first(struct vmbus_channel *channel);
 
 struct vmpacket_descriptor *
-__hv_pkt_iter_next(struct vmbus_channel *channel,
-		   const struct vmpacket_descriptor *pkt);
-
-void hv_pkt_iter_close(struct vmbus_channel *channel);
-
-/*
- * Get next packet descriptor from iterator
- * If at end of list, return NULL and update host.
- */
-static inline struct vmpacket_descriptor *
 hv_pkt_iter_next(struct vmbus_channel *channel,
-		 const struct vmpacket_descriptor *pkt)
-{
-	struct vmpacket_descriptor *nxt;
-
-	nxt = __hv_pkt_iter_next(channel, pkt);
-	if (!nxt)
-		hv_pkt_iter_close(channel);
-
-	return nxt;
-}
+		   const struct vmpacket_descriptor *pkt);
 
 #define foreach_vmbus_pkt(pkt, channel) \
 	for (pkt = hv_pkt_iter_first(channel); pkt; \

Signature · 1 line

2.11.0

Stephen Hemminger

author

2017-03-20 20:32 UTC #3 lore raw

In reply to Stephen Hemminger

Quoted 7 lines · On Mon, 20 Mar 2017 13:28:03 -0700 Stephen Hemminger…

Fix for performance regression introduced with NAPI change; and followup cleanup of vmbus core code.
Stephen Hemminger (2):
netvsc: fix NAPI performance regression
vmbus: no longer expose iterator internals

Quoted 6 lines

 drivers/hv/ring_buffer.c        | 51 +++++++++++++++++++++--------------------
 drivers/net/hyperv/hyperv_net.h |  1 +
 drivers/net/hyperv/netvsc.c     | 41 +++++++++++++++------------------
 include/linux/hyperv.h          | 22 +-----------------
 4 files changed, 46 insertions(+), 69 deletions(-)

Drop this version. The code is correct the second patch causes clashes with some later patches. Will resend.

David Miller

maintainer

2017-03-22 18:23 UTC #4 lore raw

From: Stephen Hemminger <[email protected]>
Date: Mon, 20 Mar 2017 13:31:52 -0700

Quoted 17 lines

Quoted 7 lines · On Mon, 20 Mar 2017 13:28:03 -0700 Stephen Hemminger…
Fix for performance regression introduced with NAPI change; and followup cleanup of vmbus core code.
Stephen Hemminger (2):
netvsc: fix NAPI performance regression
vmbus: no longer expose iterator internals
Quoted 6 lines
 drivers/hv/ring_buffer.c        | 51 +++++++++++++++++++++--------------------
 drivers/net/hyperv/hyperv_net.h |  1 +
 drivers/net/hyperv/netvsc.c     | 41 +++++++++++++++------------------
 include/linux/hyperv.h          | 22 +-----------------
 4 files changed, 46 insertions(+), 69 deletions(-)

Quoted 2 lines

Drop this version. The code is correct the second patch causes clashes with some later patches. Will resend.

Oh, ok, good thing I saw this before pushing out.

Reverted.

David Miller

maintainer

2017-03-22 18:22 UTC #5 lore raw

In reply to Stephen Hemminger

From: Stephen Hemminger <[email protected]>
Date: Mon, 20 Mar 2017 13:28:03 -0700

Quoted 2 lines

Fix for performance regression introduced with NAPI change; and followup cleanup of vmbus core code.

Series applied, thanks Stephen.