similar to: [PATCH 5/5] virtio_net: Recycle unused recv buffer pages for large skbs in net driver

On Thursday 09 October 2008 06:34:58 Mark McLoughlin wrote: > Each time we re-fill the recv queue with buffers, we allocate > one too many skbs and free it again when adding fails. We should > recycle the pages allocated in this case. > > A previous version of this patch made trim_pages() trim trailing > unused pages from skbs with some paged data, but this actually > caused a

On Thursday 09 October 2008 06:34:59 Mark McLoughlin wrote: > From: Herbert Xu <herbert.xu at redhat.com> > > If segmentation offload is enabled by the host, we currently allocate > maximum sized packet buffers and pass them to the host. This uses up > 20 ring entries, allowing us to supply only 20 packet buffers to the > host with a 256 entry ring. This is a huge overhead

On Thursday 09 October 2008 06:34:59 Mark McLoughlin wrote: > From: Herbert Xu <herbert.xu at redhat.com> > > If segmentation offload is enabled by the host, we currently allocate > maximum sized packet buffers and pass them to the host. This uses up > 20 ring entries, allowing us to supply only 20 packet buffers to the > host with a 256 entry ring. This is a huge overhead

On Thursday 01 May 2008 00:31:46 Mark McLoughlin wrote: > virtio_net currently only frees old transmit skbs just > before queueing new ones. If the queue is full, it then > enables interrupts and waits for notification that more > work has been performed. Hi Mark, This patch is fine, but it's better to do it from skb_xmit_done(). Of course, this is usually called from an

On Thursday 01 May 2008 00:31:46 Mark McLoughlin wrote: > virtio_net currently only frees old transmit skbs just > before queueing new ones. If the queue is full, it then > enables interrupts and waits for notification that more > work has been performed. Hi Mark, This patch is fine, but it's better to do it from skb_xmit_done(). Of course, this is usually called from an

The virtio_net driver is complicated by the two methods of freeing old xmit buffers (in addition to freeing old ones at the start of the xmit path). The original code used a 1/10 second timer attached to xmit_free(), reset on every xmit. Before we orphaned skbs on xmit, the transmitting userspace could block with a full socket until the timer fired, the skb destructor was called, and they were

The virtio_net driver is complicated by the two methods of freeing old xmit buffers (in addition to freeing old ones at the start of the xmit path). The original code used a 1/10 second timer attached to xmit_free(), reset on every xmit. Before we orphaned skbs on xmit, the transmitting userspace could block with a full socket until the timer fired, the skb destructor was called, and they were

We check for finished xmit skbs on every xmit, or on a timer (unless the host promises to force an interrupt when the xmit ring is empty). This can penalize userspace tasks which fill their sockbuf. Not much difference with TSO, but measurable with large numbers of packets. There are a finite number of packets which can be in the transmission queue. We could fire the timer more than every

We check for finished xmit skbs on every xmit, or on a timer (unless the host promises to force an interrupt when the xmit ring is empty). This can penalize userspace tasks which fill their sockbuf. Not much difference with TSO, but measurable with large numbers of packets. There are a finite number of packets which can be in the transmission queue. We could fire the timer more than every

From: Mark McLoughlin <markmc at redhat.com> hdr->csum_start is the offset from the start of the ethernet header to the transport layer checksum field. skb->csum_start is the offset from skb->head. skb_partial_csum_set() assumes that skb->data points to the ethernet header - i.e. it computes skb->csum_start by adding the headroom to hdr->csum_start. Since

Since we want to reset the device to remove them, this is simpler (device is reset for us on driver remove). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- drivers/net/virtio_net.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff -r 7e5b3ff06f60 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Wed Jan 23 22:53:14

Since we want to reset the device to remove them, this is simpler (device is reset for us on driver remove). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- drivers/net/virtio_net.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff -r 7e5b3ff06f60 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Wed Jan 23 22:53:14

skb is only NULL the first time around: it's more correct to test for being under-budget. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff -r 2a94425ac7d5 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Thu Nov 15 13:47:28 2007 +1100 +++ b/drivers/net/virtio_net.c Thu Nov 15 23:10:44 2007 +1100 @@ -198,8 +198,8 @@ again: if (vi->num < vi->max / 2)

skb is only NULL the first time around: it's more correct to test for being under-budget. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff -r 2a94425ac7d5 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Thu Nov 15 13:47:28 2007 +1100 +++ b/drivers/net/virtio_net.c Thu Nov 15 23:10:44 2007 +1100 @@ -198,8 +198,8 @@ again: if (vi->num < vi->max / 2)

On 2014/12/1 18:17, Jason Wang wrote: > On newer hosts that support delayed tx interrupts, > we probably don't have much to gain from orphaning > packets early. > > Note: this might degrade performance for > hosts without event idx support. > Should be addressed by the next patch. > > Cc: Rusty Russell <rusty at rustcorp.com.au> > Cc: Michael S. Tsirkin

On 2014/12/1 18:17, Jason Wang wrote: > On newer hosts that support delayed tx interrupts, > we probably don't have much to gain from orphaning > packets early. > > Note: this might degrade performance for > hosts without event idx support. > Should be addressed by the next patch. > > Cc: Rusty Russell <rusty at rustcorp.com.au> > Cc: Michael S. Tsirkin

If we fail to transmit a packet, we assume the queue is full and put the skb into last_xmit_skb. However, if more space frees up before we xmit it, we loop, and the result can be transmitting the same skb twice. Fix is simple: set skb to NULL if we've used it in some way, and check before sending. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c |

If we fail to transmit a packet, we assume the queue is full and put the skb into last_xmit_skb. However, if more space frees up before we xmit it, we loop, and the result can be transmitting the same skb twice. Fix is simple: set skb to NULL if we've used it in some way, and check before sending. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c |

Herbert tells me that returning NETDEV_TX_BUSY from hard_start_xmit is seen as a poor thing to do; we should cache the packet and stop the queue. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c | 58 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 18 deletions(-) diff -r 26ba2dd67f46 drivers/net/virtio_net.c ---

Herbert tells me that returning NETDEV_TX_BUSY from hard_start_xmit is seen as a poor thing to do; we should cache the packet and stop the queue. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c | 58 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 18 deletions(-) diff -r 26ba2dd67f46 drivers/net/virtio_net.c ---