search for: virtnet_poll_tx

...* Free up any pending old buffers before queueing new ones. */ >> - free_old_xmit_skbs(sq); >> + if (!use_napi) >> + free_old_xmit_skbs(sq); > > > I'm not sure this is best or even correct. Consider we clean xmit packets > speculatively in virtnet_poll_tx(), we need call free_old_xmit_skbs() > unconditionally. This can also help to reduce the possible of napi > rescheduling in virtnet_poll_tx(). Because of the use of trylock there. Absolutely, thanks! Perhaps I should only use trylock in the opportunistic clean path from the rx softirq and fu...

...* Free up any pending old buffers before queueing new ones. */ >> - free_old_xmit_skbs(sq); >> + if (!use_napi) >> + free_old_xmit_skbs(sq); > > > I'm not sure this is best or even correct. Consider we clean xmit packets > speculatively in virtnet_poll_tx(), we need call free_old_xmit_skbs() > unconditionally. This can also help to reduce the possible of napi > rescheduling in virtnet_poll_tx(). Because of the use of trylock there. Absolutely, thanks! Perhaps I should only use trylock in the opportunistic clean path from the rx softirq and fu...

...gt;rq[i].napi); > + virtnet_napi_tx_enable(vi, vi->sq[i].vq, &vi->sq[i].napi); > } > > return 0; > @@ -1081,6 +1104,25 @@ static void free_old_xmit_skbs(struct send_queue *sq) > u64_stats_update_end(&stats->tx_syncp); > } > > +static int virtnet_poll_tx(struct napi_struct *napi, int budget) > +{ > + struct send_queue *sq = container_of(napi, struct send_queue, napi); > + struct virtnet_info *vi = sq->vq->vdev->priv; > + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); > + > + if (__netif_tx_...

...; virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi); + virtnet_napi_tx_enable(vi, vi->sq[i].vq, &vi->sq[i].napi); } return 0; @@ -1081,6 +1104,25 @@ static void free_old_xmit_skbs(struct send_queue *sq) u64_stats_update_end(&stats->tx_syncp); } +static int virtnet_poll_tx(struct napi_struct *napi, int budget) +{ + struct send_queue *sq = container_of(napi, struct send_queue, napi); + struct virtnet_info *vi = sq->vq->vdev->priv; + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); + + if (__netif_tx_trylock(txq)) { + free_old_xm...

...re output buffers. */ - netif_wake_subqueue(vi->dev, vq2txq(vq)); + if (napi_schedule_prep(&sq->napi)) { + __napi_schedule(&sq->napi); + } } static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) @@ -766,6 +791,37 @@ again: return received; } +static int virtnet_poll_tx(struct napi_struct *napi, int budget) +{ + struct send_queue *sq = + container_of(napi, struct send_queue, napi); + struct virtnet_info *vi = sq->vq->vdev->priv; + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); + unsigned int r, sent = 0; + +again: + __neti...

...re output buffers. */ - netif_wake_subqueue(vi->dev, vq2txq(vq)); + if (napi_schedule_prep(&sq->napi)) { + __napi_schedule(&sq->napi); + } } static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) @@ -766,6 +791,37 @@ again: return received; } +static int virtnet_poll_tx(struct napi_struct *napi, int budget) +{ + struct send_queue *sq = + container_of(napi, struct send_queue, napi); + struct virtnet_info *vi = sq->vq->vdev->priv; + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); + unsigned int r, sent = 0; + +again: + __neti...

...7 +1368,7 @@ static void virtnet_poll_cleantx(struct receive_queue *rq) > return; > > if (__netif_tx_trylock(txq)) { > - free_old_xmit_skbs(sq); > + free_old_xmit_skbs(sq, txq, true); > __netif_tx_unlock(txq); > } > > @@ -1440,7 +1444,7 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget) > struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); > > __netif_tx_lock(txq, raw_smp_processor_id()); > - free_old_xmit_skbs(sq); > + free_old_xmit_skbs(sq, txq, true); > __netif_tx_unlock(txq); >...

...7 +1368,7 @@ static void virtnet_poll_cleantx(struct receive_queue *rq) > return; > > if (__netif_tx_trylock(txq)) { > - free_old_xmit_skbs(sq); > + free_old_xmit_skbs(sq, txq, true); > __netif_tx_unlock(txq); > } > > @@ -1440,7 +1444,7 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget) > struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); > > __netif_tx_lock(txq, raw_smp_processor_id()); > - free_old_xmit_skbs(sq); > + free_old_xmit_skbs(sq, txq, true); > __netif_tx_unlock(txq); >...

...y vring size is accepted. This is problematic because it may result in attempting to transmit a packet with more fragments than there are descriptors in the ring. Furthermore, it leads to an immediate bug: The condition: (sq->vq->num_free >= 2 + MAX_SKB_FRAGS) in virtnet_poll_cleantx and virtnet_poll_tx always evaluates to false, so netif_tx_wake_queue is not called, leading to TX timeouts. Signed-off-by: Alvaro Karsz <alvaro.karsz at solid-run.com> --- drivers/net/virtio_net.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/drivers/net/virtio_net.c b/drivers/...

...for more output buffers. */ - netif_wake_subqueue(vi->dev, vq2txq(vq)); + if (napi_schedule_prep(&sq->napi)) + __napi_schedule(&sq->napi); } static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) @@ -774,6 +798,37 @@ again: return received; } +static int virtnet_poll_tx(struct napi_struct *napi, int budget) +{ + struct send_queue *sq = + container_of(napi, struct send_queue, napi); + struct virtnet_info *vi = sq->vq->vdev->priv; + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); + unsigned int sent = 0; + bool enable_done; +...

...for more output buffers. */ - netif_wake_subqueue(vi->dev, vq2txq(vq)); + if (napi_schedule_prep(&sq->napi)) + __napi_schedule(&sq->napi); } static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) @@ -774,6 +798,37 @@ again: return received; } +static int virtnet_poll_tx(struct napi_struct *napi, int budget) +{ + struct send_queue *sq = + container_of(napi, struct send_queue, napi); + struct virtnet_info *vi = sq->vq->vdev->priv; + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); + unsigned int sent = 0; + bool enable_done; +...

...ore and a new helper to publish current avail >> idx as used event. >> >> This series fixes several issues of original rfc pointed out by Michael. > Could you list the issues, for ease of review? Probably just one: - Move the virtqueue_disable_cb() from skb_xmit_done() into virtnet_poll_tx() under tx lock.

...ore and a new helper to publish current avail >> idx as used event. >> >> This series fixes several issues of original rfc pointed out by Michael. > Could you list the issues, for ease of review? Probably just one: - Move the virtqueue_disable_cb() from skb_xmit_done() into virtnet_poll_tx() under tx lock.

...x_enable(vi, vi->sq[i].vq, &vi->sq[i].napi); } return 0; + + /* error unwinding of xdp init */ +err_xdp_reg_mem_model: + xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq); +err_xdp_info_reg: + for (i = i - 1; i >= 0; i--) + virtnet_disable_qp(vi, i); + + return err; } static int virtnet_poll_tx(struct napi_struct *napi, int budget) -- 2.37.1 (Apple Git-137.1)

...ng xmit_more (new patch 5/5) this improves cycles especially for multi UDP_STREAM, which does not benefit from cleaning tx completions on rx napi. - move free_old_xmit_skbs (new patch 3/5) to avoid forward declaration not changed: - deduplicate virnet_poll_tx and virtnet_poll_txclean they look similar, but have differ too much to make it worthwhile. - delay netif_wake_subqueue for more than 2 + MAX_SKB_FRAGS evaluated, but made no difference - patch 1/5 RFC -> v1: - dropped vhost interrupt moderation patch: not needed and...

...ng xmit_more (new patch 5/5) this improves cycles especially for multi UDP_STREAM, which does not benefit from cleaning tx completions on rx napi. - move free_old_xmit_skbs (new patch 3/5) to avoid forward declaration not changed: - deduplicate virnet_poll_tx and virtnet_poll_txclean they look similar, but have differ too much to make it worthwhile. - delay netif_wake_subqueue for more than 2 + MAX_SKB_FRAGS evaluated, but made no difference - patch 1/5 RFC -> v1: - dropped vhost interrupt moderation patch: not needed and...

...t_napi_tx_enable(vi, vi->sq[i].vq, &vi->sq[i].napi); + goto err_enable_qp; } return 0; + +err_enable_qp: + disable_delayed_refill(vi); + cancel_delayed_work_sync(&vi->refill); + + for (i--; i >= 0; i--) + virtnet_disable_queue_pair(vi, i); + return err; } static int virtnet_poll_tx(struct napi_struct *napi, int budget) @@ -2305,11 +2335,8 @@ static int virtnet_close(struct net_device *dev) /* Make sure refill_work doesn't re-enable napi! */ cancel_delayed_work_sync(&vi->refill); - for (i = 0; i < vi->max_queue_pairs; i++) { - virtnet_napi_tx_disable(&a...

...ke_subqueue(vi->dev, vq2txq(vq)); > + virtqueue_disable_cb(sq->vq); > + napi_schedule(&sq->napi); > } > > static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) > @@ -777,6 +808,32 @@ again: > return received; > } > > +static int virtnet_poll_tx(struct napi_struct *napi, int budget) > +{ > + struct send_queue *sq = > + container_of(napi, struct send_queue, napi); > + struct virtnet_info *vi = sq->vq->vdev->priv; > + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); > + u32 limit = vi...

...ke_subqueue(vi->dev, vq2txq(vq)); > + virtqueue_disable_cb(sq->vq); > + napi_schedule(&sq->napi); > } > > static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) > @@ -777,6 +808,32 @@ again: > return received; > } > > +static int virtnet_poll_tx(struct napi_struct *napi, int budget) > +{ > + struct send_queue *sq = > + container_of(napi, struct send_queue, napi); > + struct virtnet_info *vi = sq->vq->vdev->priv; > + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); > + u32 limit = vi...

With the implementation of napi-tx in virtio driver, we clean tx descriptors from rx napi handler, for the purpose of reducing tx complete interrupts. But this introduces a race where tx complete interrupt has been raised, but the handler finds there is no work to do because we have done the work in the previous rx interrupt handler. A similar issue exists with polling from start_xmit, it is