netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.)
@ 2016-01-15 14:55 David Vrabel
  2016-01-15 14:55 ` [PATCHv2 1/3] xen-netback: use skb to determine number of required guest Rx requests David Vrabel
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: David Vrabel @ 2016-01-15 14:55 UTC (permalink / raw)
  To: netdev; +Cc: xen-devel, Wei Liu, David Vrabel, Ian Campbell

"xen-netback: use skb to determine number of required" plus two other
minor fixes I found down the back of the sofa.

David

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCHv2 1/3] xen-netback: use skb to determine number of required guest Rx requests
  2016-01-15 14:55 [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.) David Vrabel
@ 2016-01-15 14:55 ` David Vrabel
  2016-01-15 14:55 ` [PATCHv2 2/3] xen-netback: delete NAPI instance when queue fails to initialize David Vrabel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: David Vrabel @ 2016-01-15 14:55 UTC (permalink / raw)
  To: netdev; +Cc: David Vrabel, xen-devel, Ian Campbell, Wei Liu

Using the MTU or GSO size to determine the number of required guest Rx
requests for an skb was subtly broken since these value may change at
runtime.

After 1650d5455bd2dc6b5ee134bd6fc1a3236c266b5b (xen-netback: always
fully coalesce guest Rx packets) we always fully pack a packet into
its guest Rx slots.  Calculating the number of required slots from the
packet length is then easy.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/net/xen-netback/netback.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 1049c34..61b97c3 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -149,20 +149,19 @@ static inline pending_ring_idx_t pending_index(unsigned i)
 	return i & (MAX_PENDING_REQS-1);
 }
 
-static int xenvif_rx_ring_slots_needed(struct xenvif *vif)
-{
-	if (vif->gso_mask)
-		return DIV_ROUND_UP(vif->dev->gso_max_size, XEN_PAGE_SIZE) + 1;
-	else
-		return DIV_ROUND_UP(vif->dev->mtu, XEN_PAGE_SIZE);
-}
-
 static bool xenvif_rx_ring_slots_available(struct xenvif_queue *queue)
 {
 	RING_IDX prod, cons;
+	struct sk_buff *skb;
 	int needed;
 
-	needed = xenvif_rx_ring_slots_needed(queue->vif);
+	skb = skb_peek(&queue->rx_queue);
+	if (!skb)
+		return false;
+
+	needed = DIV_ROUND_UP(skb->len, XEN_PAGE_SIZE);
+	if (skb_is_gso(skb))
+		needed++;
 
 	do {
 		prod = queue->rx.sring->req_prod;
@@ -2005,8 +2004,7 @@ static bool xenvif_rx_queue_ready(struct xenvif_queue *queue)
 
 static bool xenvif_have_rx_work(struct xenvif_queue *queue)
 {
-	return (!skb_queue_empty(&queue->rx_queue)
-		&& xenvif_rx_ring_slots_available(queue))
+	return xenvif_rx_ring_slots_available(queue)
 		|| (queue->vif->stall_timeout &&
 		    (xenvif_rx_queue_stalled(queue)
 		     || xenvif_rx_queue_ready(queue)))
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCHv2 2/3] xen-netback: delete NAPI instance when queue fails to initialize
  2016-01-15 14:55 [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.) David Vrabel
  2016-01-15 14:55 ` [PATCHv2 1/3] xen-netback: use skb to determine number of required guest Rx requests David Vrabel
@ 2016-01-15 14:55 ` David Vrabel
  2016-01-15 14:55 ` [PATCHv2 3/3] xen-netback: free queues after freeing the net device David Vrabel
  2016-01-15 20:13 ` [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.) David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: David Vrabel @ 2016-01-15 14:55 UTC (permalink / raw)
  To: netdev; +Cc: xen-devel, Wei Liu, David Vrabel, Ian Campbell

When xenvif_connect() fails it may leave a stale NAPI instance added to
the device.  Make sure we delete it in the error path.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/net/xen-netback/interface.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index e7bd63e..3bba6ce 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -615,6 +615,7 @@ err_tx_unbind:
 	queue->tx_irq = 0;
 err_unmap:
 	xenvif_unmap_frontend_rings(queue);
+	netif_napi_del(&queue->napi);
 err:
 	module_put(THIS_MODULE);
 	return err;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCHv2 3/3] xen-netback: free queues after freeing the net device
  2016-01-15 14:55 [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.) David Vrabel
  2016-01-15 14:55 ` [PATCHv2 1/3] xen-netback: use skb to determine number of required guest Rx requests David Vrabel
  2016-01-15 14:55 ` [PATCHv2 2/3] xen-netback: delete NAPI instance when queue fails to initialize David Vrabel
@ 2016-01-15 14:55 ` David Vrabel
  2016-01-15 20:13 ` [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.) David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: David Vrabel @ 2016-01-15 14:55 UTC (permalink / raw)
  To: netdev; +Cc: xen-devel, Wei Liu, David Vrabel, Ian Campbell

If a queue still has a NAPI instance added to the net device, freeing
the queues early results in a use-after-free.

The shouldn't ever happen because we disconnect and tear down all queues
before freeing the net device, but doing this makes it obviously safe.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/net/xen-netback/interface.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 3bba6ce..f5231a2 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -685,22 +685,16 @@ void xenvif_deinit_queue(struct xenvif_queue *queue)
 
 void xenvif_free(struct xenvif *vif)
 {
-	struct xenvif_queue *queue = NULL;
+	struct xenvif_queue *queues = vif->queues;
 	unsigned int num_queues = vif->num_queues;
 	unsigned int queue_index;
 
 	unregister_netdev(vif->dev);
-
-	for (queue_index = 0; queue_index < num_queues; ++queue_index) {
-		queue = &vif->queues[queue_index];
-		xenvif_deinit_queue(queue);
-	}
-
-	vfree(vif->queues);
-	vif->queues = NULL;
-	vif->num_queues = 0;
-
 	free_netdev(vif->dev);
 
+	for (queue_index = 0; queue_index < num_queues; ++queue_index)
+		xenvif_deinit_queue(&queues[queue_index]);
+	vfree(queues);
+
 	module_put(THIS_MODULE);
 }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.)
  2016-01-15 14:55 [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.) David Vrabel
                   ` (2 preceding siblings ...)
  2016-01-15 14:55 ` [PATCHv2 3/3] xen-netback: free queues after freeing the net device David Vrabel
@ 2016-01-15 20:13 ` David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2016-01-15 20:13 UTC (permalink / raw)
  To: david.vrabel; +Cc: netdev, xen-devel, ian.campbell, wei.liu2

From: David Vrabel <david.vrabel@citrix.com>
Date: Fri, 15 Jan 2016 14:55:33 +0000

> "xen-netback: use skb to determine number of required" plus two other
> minor fixes I found down the back of the sofa.

Series applied, thanks David.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-01-15 20:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-15 14:55 [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.) David Vrabel
2016-01-15 14:55 ` [PATCHv2 1/3] xen-netback: use skb to determine number of required guest Rx requests David Vrabel
2016-01-15 14:55 ` [PATCHv2 2/3] xen-netback: delete NAPI instance when queue fails to initialize David Vrabel
2016-01-15 14:55 ` [PATCHv2 3/3] xen-netback: free queues after freeing the net device David Vrabel
2016-01-15 20:13 ` [PATCHv2 0/3 net] xen-netback: use skb to determine number of required (etc.) David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).