similar to: [PATCH for-2.6.35] virtio_net: fix oom handling on tx

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 | 57 +++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff -r a936d320f6d5 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 | 57 +++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff -r a936d320f6d5 drivers/net/virtio_net.c ---

virtio net used to unlink skbs from send queues on error, but ever since 48925e372f04f5e35fec6269127c62b2c71ab794 we do not do this. This causes guest data corruption and crashes with vhost since net core can requeue the skb or free it without it being taken off the list. This patch fixes this by queueing the skb after successfull transmit. Signed-off-by: Michael S. Tsirkin <mst at

virtio net used to unlink skbs from send queues on error, but ever since 48925e372f04f5e35fec6269127c62b2c71ab794 we do not do this. This causes guest data corruption and crashes with vhost since net core can requeue the skb or free it without it being taken off the list. This patch fixes this by queueing the skb after successfull transmit. Signed-off-by: Michael S. Tsirkin <mst at

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 ---

On Sun, Mar 15, 2015 at 02:50:27PM +0800, Linhaifeng wrote: > Hi,Michael > > I had tested the start_xmit function by the follow code found that the tx queue's state is stopped and can't send any packets anymore. Why don't you Cc all maintainers on this email? Pls check the file MAINTAINERS for the full list. I added Cc for now. > > static netdev_tx_t

On Sun, Mar 15, 2015 at 02:50:27PM +0800, Linhaifeng wrote: > Hi,Michael > > I had tested the start_xmit function by the follow code found that the tx queue's state is stopped and can't send any packets anymore. Why don't you Cc all maintainers on this email? Pls check the file MAINTAINERS for the full list. I added Cc for now. > > static netdev_tx_t

This effectively reverts 99ffc696d10b28580fe93441d627cf290ac4484c "virtio: wean net driver off NETDEV_TX_BUSY". The complexity of queuing an skb (setting a tasklet to re-xmit) is questionable, especially once we get rid of the other reason for the tasklet in the next patch. If the skb won't fit in the tx queue, just return NETDEV_TX_BUSY. It might be frowned upon, but it's

This effectively reverts 99ffc696d10b28580fe93441d627cf290ac4484c "virtio: wean net driver off NETDEV_TX_BUSY". The complexity of queuing an skb (setting a tasklet to re-xmit) is questionable, especially once we get rid of the other reason for the tasklet in the next patch. If the skb won't fit in the tx queue, just return NETDEV_TX_BUSY. It might be frowned upon, but it's

OK, here's a new attempt to use the new capacity api. I also added more comments to clarify the logic. Hope this is more readable. Let me know pls. This is on top of the patches applied by Rusty. Note: there are now actually 2 calls to fee_old_xmit_skbs on data path so instead of passing flag '2' to the second one I thought we can just make each call free up at least 1 skb. This

OK, here's a new attempt to use the new capacity api. I also added more comments to clarify the logic. Hope this is more readable. Let me know pls. This is on top of the patches applied by Rusty. Note: there are now actually 2 calls to fee_old_xmit_skbs on data path so instead of passing flag '2' to the second one I thought we can just make each call free up at least 1 skb. This

OK, here's a new attempt to use the new capacity api. I also added more comments to clarify the logic. Hope this is more readable. Let me know pls. This is on top of the patches applied by Rusty. Warning: untested. Posting now to give people chance to comment on the API. Changes from v1: - fix comment in patch 2 to correct confusion noted by Rusty - rewrite patch 3 along the lines

OK, here's a new attempt to use the new capacity api. I also added more comments to clarify the logic. Hope this is more readable. Let me know pls. This is on top of the patches applied by Rusty. Warning: untested. Posting now to give people chance to comment on the API. Changes from v1: - fix comment in patch 2 to correct confusion noted by Rusty - rewrite patch 3 along the lines

virtqueue ops were introduced in the hope that we'll have multiple implementations besides virtio_ring, but none have surfaced so far, and given that existing virtio ring is deployed in production we are likely stuck with it now, so this layer just adds complexity and overhead. Further, the need to pass vq twice to each call (as in dev->vq->vq_ops->kick(dev->vq) ) adds potential

virtqueue ops were introduced in the hope that we'll have multiple implementations besides virtio_ring, but none have surfaced so far, and given that existing virtio ring is deployed in production we are likely stuck with it now, so this layer just adds complexity and overhead. Further, the need to pass vq twice to each call (as in dev->vq->vq_ops->kick(dev->vq) ) adds potential

On 2015/3/15 16:40, Michael S. Tsirkin wrote: > On Sun, Mar 15, 2015 at 02:50:27PM +0800, Linhaifeng wrote: >> Hi,Michael >> >> I had tested the start_xmit function by the follow code found that the tx queue's state is stopped and can't send any packets anymore. > > Why don't you Cc all maintainers on this email? > Pls check the file MAINTAINERS for the

On 2015/3/15 16:40, Michael S. Tsirkin wrote: > On Sun, Mar 15, 2015 at 02:50:27PM +0800, Linhaifeng wrote: >> Hi,Michael >> >> I had tested the start_xmit function by the follow code found that the tx queue's state is stopped and can't send any packets anymore. > > Why don't you Cc all maintainers on this email? > Pls check the file MAINTAINERS for the

On Mon, Mar 16, 2015 at 05:24:07PM +0800, Linhaifeng wrote: > > > On 2015/3/15 16:40, Michael S. Tsirkin wrote: > > On Sun, Mar 15, 2015 at 02:50:27PM +0800, Linhaifeng wrote: > >> Hi,Michael > >> > >> I had tested the start_xmit function by the follow code found that the tx queue's state is stopped and can't send any packets anymore. > >

On Mon, Mar 16, 2015 at 05:24:07PM +0800, Linhaifeng wrote: > > > On 2015/3/15 16:40, Michael S. Tsirkin wrote: > > On Sun, Mar 15, 2015 at 02:50:27PM +0800, Linhaifeng wrote: > >> Hi,Michael > >> > >> I had tested the start_xmit function by the follow code found that the tx queue's state is stopped and can't send any packets anymore. > >