[PATCH net-next RFC 2/4] virtio-net: transmit napi

[PATCH net-next RFC 2/4] virtio-net: transmit napi

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Convert virtio-net to a standard napi tx completion path. This enables
better TCP pacing using TCP small queues and increases single stream
throughput.

The virtio-net driver currently cleans tx descriptors on transmission
of new packets in ndo_start_xmit. Latency depends on new traffic, so
is unbounded. To avoid deadlock when a socket reaches its snd limit,
packets are orphaned on tranmission. This breaks socket backpressure,
including TSQ.

Napi increases the number of interrupts generated compared to the
current model, which keeps interrupts disabled as long as the ring
has enough free descriptors. Keep tx napi optional for now. Follow-on
patches will reduce the interrupt cost.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/virtio_net.c | 73 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8c21e9a4adc7..9a9031640179 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -33,6 +33,8 @@
 static int napi_weight = NAPI_POLL_WEIGHT;
 module_param(napi_weight, int, 0444);
 
+static int napi_tx_weight = NAPI_POLL_WEIGHT;
+
 static bool csum = true, gso = true;
 module_param(csum, bool, 0444);
 module_param(gso, bool, 0444);
@@ -86,6 +88,8 @@ struct send_queue {
 
 	/* Name of the send queue: output.$index */
 	char name[40];
+
+	struct napi_struct napi;
 };
 
 /* Internal representation of a receive virtqueue */
@@ -262,12 +266,16 @@ static void virtqueue_napi_complete(struct napi_struct *napi,
 static void skb_xmit_done(struct virtqueue *vq)
 {
 	struct virtnet_info *vi = vq->vdev->priv;
+	struct napi_struct *napi = &vi->sq[vq2txq(vq)].napi;
 
 	/* Suppress further interrupts. */
 	virtqueue_disable_cb(vq);
 
-	/* We were probably waiting for more output buffers. */
-	netif_wake_subqueue(vi->dev, vq2txq(vq));
+	if (napi->weight)
+		virtqueue_napi_schedule(napi, vq);
+	else
+		/* We were probably waiting for more output buffers. */
+		netif_wake_subqueue(vi->dev, vq2txq(vq));
 }
 
 static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx)
@@ -961,6 +969,9 @@ static void skb_recv_done(struct virtqueue *rvq)
 
 static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
 {
+	if (!napi->weight)
+		return;
+
 	napi_enable(napi);
 
 	/* If all buffers were filled by other side before we napi_enabled, we
@@ -1046,12 +1057,13 @@ static int virtnet_open(struct net_device *dev)
 			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
 				schedule_delayed_work(&vi->refill, 0);
 		virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+		virtnet_napi_enable(vi->sq[i].vq, &vi->sq[i].napi);
 	}
 
 	return 0;
 }
 
-static void free_old_xmit_skbs(struct send_queue *sq)
+static unsigned int free_old_xmit_skbs(struct send_queue *sq, int budget)
 {
 	struct sk_buff *skb;
 	unsigned int len;
@@ -1060,7 +1072,8 @@ static void free_old_xmit_skbs(struct send_queue *sq)
 	unsigned int packets = 0;
 	unsigned int bytes = 0;
 
-	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
+	while (packets < budget &&
+	       (skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 
 		bytes += skb->len;
@@ -1073,12 +1086,35 @@ static void free_old_xmit_skbs(struct send_queue *sq)
 	 * happens when called speculatively from start_xmit.
 	 */
 	if (!packets)
-		return;
+		return 0;
 
 	u64_stats_update_begin(&stats->tx_syncp);
 	stats->tx_bytes += bytes;
 	stats->tx_packets += packets;
 	u64_stats_update_end(&stats->tx_syncp);
+
+	return packets;
+}
+
+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));
+	bool complete = false;
+
+	__netif_tx_lock(txq, smp_processor_id());
+	if (free_old_xmit_skbs(sq, budget) < budget)
+		complete = true;
+	__netif_tx_unlock(txq);
+
+	if (complete)
+		virtqueue_napi_complete(napi, sq->vq, 0);
+
+	if (sq->vq->num_free >= 2 + MAX_SKB_FRAGS)
+		netif_wake_subqueue(vi->dev, vq2txq(sq->vq));
+
+	return complete ? 0 : budget;
 }
 
 static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
@@ -1130,9 +1166,11 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	int err;
 	struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
 	bool kick = !skb->xmit_more;
+	bool use_napi = sq->napi.weight;
 
 	/* Free up any pending old buffers before queueing new ones. */
-	free_old_xmit_skbs(sq);
+	if (!use_napi)
+		free_old_xmit_skbs(sq, napi_weight);
 
 	/* timestamp packet in software */
 	skb_tx_timestamp(skb);
@@ -1152,8 +1190,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	/* Don't wait up for transmitted skbs to be freed. */
-	skb_orphan(skb);
-	nf_reset(skb);
+	if (!use_napi) {
+		skb_orphan(skb);
+		nf_reset(skb);
+	}
 
 	/* If running out of space, stop queue to avoid getting packets that we
 	 * are then unable to transmit.
@@ -1167,9 +1207,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 */
 	if (sq->vq->num_free < 2+MAX_SKB_FRAGS) {
 		netif_stop_subqueue(dev, qnum);
-		if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
+		if (!use_napi &&
+		    unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
 			/* More just got used, free them then recheck. */
-			free_old_xmit_skbs(sq);
+			free_old_xmit_skbs(sq, virtqueue_get_vring_size(sq->vq));
 			if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
 				netif_start_subqueue(dev, qnum);
 				virtqueue_disable_cb(sq->vq);
@@ -1371,8 +1412,10 @@ 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++)
+	for (i = 0; i < vi->max_queue_pairs; i++) {
 		napi_disable(&vi->rq[i].napi);
+		napi_disable(&vi->sq[i].napi);
+	}
 
 	return 0;
 }
@@ -1719,6 +1762,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
 	if (netif_running(vi->dev)) {
 		for (i = 0; i < vi->max_queue_pairs; i++)
 			napi_disable(&vi->rq[i].napi);
+			napi_disable(&vi->sq[i].napi);
 	}
 }
 
@@ -1741,8 +1785,10 @@ static int virtnet_restore_up(struct virtio_device *vdev)
 			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
 				schedule_delayed_work(&vi->refill, 0);
 
-		for (i = 0; i < vi->max_queue_pairs; i++)
+		for (i = 0; i < vi->max_queue_pairs; i++) {
 			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+			virtnet_napi_enable(vi->sq[i].vq, &vi->sq[i].napi);
+		}
 	}
 
 	netif_device_attach(vi->dev);
@@ -1947,6 +1993,7 @@ static void virtnet_free_queues(struct virtnet_info *vi)
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		napi_hash_del(&vi->rq[i].napi);
 		netif_napi_del(&vi->rq[i].napi);
+		netif_napi_del(&vi->sq[i].napi);
 	}
 
 	/* We called napi_hash_del() before netif_napi_del(),
@@ -2132,6 +2179,8 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
 		vi->rq[i].pages = NULL;
 		netif_napi_add(vi->dev, &vi->rq[i].napi, virtnet_poll,
 			       napi_weight);
+		netif_napi_add(vi->dev, &vi->sq[i].napi, virtnet_poll_tx,
+			       napi_tx_weight);
 
 		sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
 		ewma_pkt_len_init(&vi->rq[i].mrg_avg_pkt_len);
-- 
2.12.0.rc1.440.g5b76565f74-goog

[PATCH net-next RFC 2/4] virtio-net: transmit napi

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Convert virtio-net to a standard napi tx completion path. This enables
better TCP pacing using TCP small queues and increases single stream
throughput.

The virtio-net driver currently cleans tx descriptors on transmission
of new packets in ndo_start_xmit. Latency depends on new traffic, so
is unbounded. To avoid deadlock when a socket reaches its snd limit,
packets are orphaned on tranmission. This breaks socket backpressure,
including TSQ.

Napi increases the number of interrupts generated compared to the
current model, which keeps interrupts disabled as long as the ring
has enough free descriptors. Keep tx napi optional for now. Follow-on
patches will reduce the interrupt cost.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/virtio_net.c | 73 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8c21e9a4adc7..9a9031640179 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -33,6 +33,8 @@
 static int napi_weight = NAPI_POLL_WEIGHT;
 module_param(napi_weight, int, 0444);
 
+static int napi_tx_weight = NAPI_POLL_WEIGHT;
+
 static bool csum = true, gso = true;
 module_param(csum, bool, 0444);
 module_param(gso, bool, 0444);
@@ -86,6 +88,8 @@ struct send_queue {
 
 	/* Name of the send queue: output.$index */
 	char name[40];
+
+	struct napi_struct napi;
 };
 
 /* Internal representation of a receive virtqueue */
@@ -262,12 +266,16 @@ static void virtqueue_napi_complete(struct napi_struct *napi,
 static void skb_xmit_done(struct virtqueue *vq)
 {
 	struct virtnet_info *vi = vq->vdev->priv;
+	struct napi_struct *napi = &vi->sq[vq2txq(vq)].napi;
 
 	/* Suppress further interrupts. */
 	virtqueue_disable_cb(vq);
 
-	/* We were probably waiting for more output buffers. */
-	netif_wake_subqueue(vi->dev, vq2txq(vq));
+	if (napi->weight)
+		virtqueue_napi_schedule(napi, vq);
+	else
+		/* We were probably waiting for more output buffers. */
+		netif_wake_subqueue(vi->dev, vq2txq(vq));
 }
 
 static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx)
@@ -961,6 +969,9 @@ static void skb_recv_done(struct virtqueue *rvq)
 
 static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
 {
+	if (!napi->weight)
+		return;
+
 	napi_enable(napi);
 
 	/* If all buffers were filled by other side before we napi_enabled, we
@@ -1046,12 +1057,13 @@ static int virtnet_open(struct net_device *dev)
 			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
 				schedule_delayed_work(&vi->refill, 0);
 		virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+		virtnet_napi_enable(vi->sq[i].vq, &vi->sq[i].napi);
 	}
 
 	return 0;
 }
 
-static void free_old_xmit_skbs(struct send_queue *sq)
+static unsigned int free_old_xmit_skbs(struct send_queue *sq, int budget)
 {
 	struct sk_buff *skb;
 	unsigned int len;
@@ -1060,7 +1072,8 @@ static void free_old_xmit_skbs(struct send_queue *sq)
 	unsigned int packets = 0;
 	unsigned int bytes = 0;
 
-	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
+	while (packets < budget &&
+	       (skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 
 		bytes += skb->len;
@@ -1073,12 +1086,35 @@ static void free_old_xmit_skbs(struct send_queue *sq)
 	 * happens when called speculatively from start_xmit.
 	 */
 	if (!packets)
-		return;
+		return 0;
 
 	u64_stats_update_begin(&stats->tx_syncp);
 	stats->tx_bytes += bytes;
 	stats->tx_packets += packets;
 	u64_stats_update_end(&stats->tx_syncp);
+
+	return packets;
+}
+
+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));
+	bool complete = false;
+
+	__netif_tx_lock(txq, smp_processor_id());
+	if (free_old_xmit_skbs(sq, budget) < budget)
+		complete = true;
+	__netif_tx_unlock(txq);
+
+	if (complete)
+		virtqueue_napi_complete(napi, sq->vq, 0);
+
+	if (sq->vq->num_free >= 2 + MAX_SKB_FRAGS)
+		netif_wake_subqueue(vi->dev, vq2txq(sq->vq));
+
+	return complete ? 0 : budget;
 }
 
 static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
@@ -1130,9 +1166,11 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	int err;
 	struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
 	bool kick = !skb->xmit_more;
+	bool use_napi = sq->napi.weight;
 
 	/* Free up any pending old buffers before queueing new ones. */
-	free_old_xmit_skbs(sq);
+	if (!use_napi)
+		free_old_xmit_skbs(sq, napi_weight);
 
 	/* timestamp packet in software */
 	skb_tx_timestamp(skb);
@@ -1152,8 +1190,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	/* Don't wait up for transmitted skbs to be freed. */
-	skb_orphan(skb);
-	nf_reset(skb);
+	if (!use_napi) {
+		skb_orphan(skb);
+		nf_reset(skb);
+	}
 
 	/* If running out of space, stop queue to avoid getting packets that we
 	 * are then unable to transmit.
@@ -1167,9 +1207,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 */
 	if (sq->vq->num_free < 2+MAX_SKB_FRAGS) {
 		netif_stop_subqueue(dev, qnum);
-		if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
+		if (!use_napi &&
+		    unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
 			/* More just got used, free them then recheck. */
-			free_old_xmit_skbs(sq);
+			free_old_xmit_skbs(sq, virtqueue_get_vring_size(sq->vq));
 			if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
 				netif_start_subqueue(dev, qnum);
 				virtqueue_disable_cb(sq->vq);
@@ -1371,8 +1412,10 @@ 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++)
+	for (i = 0; i < vi->max_queue_pairs; i++) {
 		napi_disable(&vi->rq[i].napi);
+		napi_disable(&vi->sq[i].napi);
+	}
 
 	return 0;
 }
@@ -1719,6 +1762,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
 	if (netif_running(vi->dev)) {
 		for (i = 0; i < vi->max_queue_pairs; i++)
 			napi_disable(&vi->rq[i].napi);
+			napi_disable(&vi->sq[i].napi);
 	}
 }
 
@@ -1741,8 +1785,10 @@ static int virtnet_restore_up(struct virtio_device *vdev)
 			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
 				schedule_delayed_work(&vi->refill, 0);
 
-		for (i = 0; i < vi->max_queue_pairs; i++)
+		for (i = 0; i < vi->max_queue_pairs; i++) {
 			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+			virtnet_napi_enable(vi->sq[i].vq, &vi->sq[i].napi);
+		}
 	}
 
 	netif_device_attach(vi->dev);
@@ -1947,6 +1993,7 @@ static void virtnet_free_queues(struct virtnet_info *vi)
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		napi_hash_del(&vi->rq[i].napi);
 		netif_napi_del(&vi->rq[i].napi);
+		netif_napi_del(&vi->sq[i].napi);
 	}
 
 	/* We called napi_hash_del() before netif_napi_del(),
@@ -2132,6 +2179,8 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
 		vi->rq[i].pages = NULL;
 		netif_napi_add(vi->dev, &vi->rq[i].napi, virtnet_poll,
 			       napi_weight);
+		netif_napi_add(vi->dev, &vi->sq[i].napi, virtnet_poll_tx,
+			       napi_tx_weight);
 
 		sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
 		ewma_pkt_len_init(&vi->rq[i].mrg_avg_pkt_len);
-- 
2.12.0.rc1.440.g5b76565f74-goog

Re: [PATCH net-next RFC 2/4] virtio-net: transmit napi

[Jason Wang]

On 2017年03月03日 22:39, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Convert virtio-net to a standard napi tx completion path. This enables
> better TCP pacing using TCP small queues and increases single stream
> throughput.
>
> The virtio-net driver currently cleans tx descriptors on transmission
> of new packets in ndo_start_xmit. Latency depends on new traffic, so
> is unbounded. To avoid deadlock when a socket reaches its snd limit,
> packets are orphaned on tranmission. This breaks socket backpressure,
> including TSQ.
>
> Napi increases the number of interrupts generated compared to the
> current model, which keeps interrupts disabled as long as the ring
> has enough free descriptors. Keep tx napi optional for now. Follow-on
> patches will reduce the interrupt cost.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>   drivers/net/virtio_net.c | 73 ++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 61 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 8c21e9a4adc7..9a9031640179 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -33,6 +33,8 @@
>   static int napi_weight = NAPI_POLL_WEIGHT;
>   module_param(napi_weight, int, 0444);
>   
> +static int napi_tx_weight = NAPI_POLL_WEIGHT;
> +

Maybe we should use module_param for this? Or in the future, use 
tx-frames-irq for a per-device configuration.

Thanks

Re: [PATCH net-next RFC 2/4] virtio-net: transmit napi

[Willem de Bruijn]

>>   drivers/net/virtio_net.c | 73
>> ++++++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 61 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 8c21e9a4adc7..9a9031640179 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -33,6 +33,8 @@
>>   static int napi_weight = NAPI_POLL_WEIGHT;
>>   module_param(napi_weight, int, 0444);
>>   +static int napi_tx_weight = NAPI_POLL_WEIGHT;
>> +
>
>
> Maybe we should use module_param for this? Or in the future, use
> tx-frames-irq for a per-device configuration.

This option should eventually just go away, and napi tx become the
standard mode.

In the short term, while we evaluate it on varied workloads, a
module_param sounds good to me. In general that is frowned
upon, as it leads to different configuration interfaces for each
device driver. But that should not be a concern in this limited
case.

Re: [PATCH net-next RFC 2/4] virtio-net: transmit napi

[David Miller]

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Mon, 6 Mar 2017 12:50:19 -0500

>>>   drivers/net/virtio_net.c | 73
>>> ++++++++++++++++++++++++++++++++++++++++--------
>>>   1 file changed, 61 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index 8c21e9a4adc7..9a9031640179 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -33,6 +33,8 @@
>>>   static int napi_weight = NAPI_POLL_WEIGHT;
>>>   module_param(napi_weight, int, 0444);
>>>   +static int napi_tx_weight = NAPI_POLL_WEIGHT;
>>> +
>>
>>
>> Maybe we should use module_param for this? Or in the future, use
>> tx-frames-irq for a per-device configuration.
> 
> This option should eventually just go away, and napi tx become the
> standard mode.
> 
> In the short term, while we evaluate it on varied workloads, a
> module_param sounds good to me. In general that is frowned
> upon, as it leads to different configuration interfaces for each
> device driver. But that should not be a concern in this limited
> case.

In any event, do we really need a TX weight at all?

I guess you tried this, but why doesn't it not work to just do
all TX work unconditionally in a NAPI poll pass?  This is how
we encourage all NIC drivers to handle this.

Re: [PATCH net-next RFC 2/4] virtio-net: transmit napi

[Michael S. Tsirkin]

On Mon, Mar 06, 2017 at 10:55:22AM -0800, David Miller wrote:
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Date: Mon, 6 Mar 2017 12:50:19 -0500
> 
> >>>   drivers/net/virtio_net.c | 73
> >>> ++++++++++++++++++++++++++++++++++++++++--------
> >>>   1 file changed, 61 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >>> index 8c21e9a4adc7..9a9031640179 100644
> >>> --- a/drivers/net/virtio_net.c
> >>> +++ b/drivers/net/virtio_net.c
> >>> @@ -33,6 +33,8 @@
> >>>   static int napi_weight = NAPI_POLL_WEIGHT;
> >>>   module_param(napi_weight, int, 0444);
> >>>   +static int napi_tx_weight = NAPI_POLL_WEIGHT;
> >>> +
> >>
> >>
> >> Maybe we should use module_param for this? Or in the future, use
> >> tx-frames-irq for a per-device configuration.
> > 
> > This option should eventually just go away, and napi tx become the
> > standard mode.
> > 
> > In the short term, while we evaluate it on varied workloads, a
> > module_param sounds good to me. In general that is frowned
> > upon, as it leads to different configuration interfaces for each
> > device driver. But that should not be a concern in this limited
> > case.
> 
> In any event, do we really need a TX weight at all?
> 
> I guess you tried this, but why doesn't it not work to just do
> all TX work unconditionally in a NAPI poll pass?  This is how
> we encourage all NIC drivers to handle this.

This seems to be more or less what this driver does already.
So I suspect it can just ignore the weight.

-- 
MST

Re: [PATCH net-next RFC 2/4] virtio-net: transmit napi

[Willem de Bruijn]

On Mon, Mar 6, 2017 at 2:33 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Mar 06, 2017 at 10:55:22AM -0800, David Miller wrote:
>> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
>> Date: Mon, 6 Mar 2017 12:50:19 -0500
>>
>> >>>   drivers/net/virtio_net.c | 73
>> >>> ++++++++++++++++++++++++++++++++++++++++--------
>> >>>   1 file changed, 61 insertions(+), 12 deletions(-)
>> >>>
>> >>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> >>> index 8c21e9a4adc7..9a9031640179 100644
>> >>> --- a/drivers/net/virtio_net.c
>> >>> +++ b/drivers/net/virtio_net.c
>> >>> @@ -33,6 +33,8 @@
>> >>>   static int napi_weight = NAPI_POLL_WEIGHT;
>> >>>   module_param(napi_weight, int, 0444);
>> >>>   +static int napi_tx_weight = NAPI_POLL_WEIGHT;
>> >>> +
>> >>
>> >>
>> >> Maybe we should use module_param for this? Or in the future, use
>> >> tx-frames-irq for a per-device configuration.
>> >
>> > This option should eventually just go away, and napi tx become the
>> > standard mode.
>> >
>> > In the short term, while we evaluate it on varied workloads, a
>> > module_param sounds good to me. In general that is frowned
>> > upon, as it leads to different configuration interfaces for each
>> > device driver. But that should not be a concern in this limited
>> > case.
>>
>> In any event, do we really need a TX weight at all?
>>
>> I guess you tried this, but why doesn't it not work to just do
>> all TX work unconditionally in a NAPI poll pass?  This is how
>> we encourage all NIC drivers to handle this.
>
> This seems to be more or less what this driver does already.
> So I suspect it can just ignore the weight.

Okay. Then we still need a boolean to toggle tx napi until we're
sure that the old path can be deprecated.