Re: [PATCH 24/33] virtio_net: xsk: stop disable tx napi

On Fri, Feb 03, 2023 at 04:49:16PM +0800, Xuan Zhuo wrote:

On Fri, 3 Feb 2023 03:33:41 -0500, "Michael S. Tsirkin" [off-list ref] wrote:

quoted

On Fri, Feb 03, 2023 at 11:24:42AM +0800, Xuan Zhuo wrote:

quoted

On Thu, 2 Feb 2023 12:25:59 -0500, "Michael S. Tsirkin" [off-list ref] wrote:

quoted

On Thu, Feb 02, 2023 at 07:00:49PM +0800, Xuan Zhuo wrote:

quoted

Since xsk's TX queue is consumed by TX NAPI, if sq is bound to xsk, then
we must stop tx napi from being disabled.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio/main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
index ed79e750bc6c..232cf151abff 100644
--- a/drivers/net/virtio/main.c
+++ b/drivers/net/virtio/main.c

@@ -2728,8 +2728,15 @@ static int virtnet_set_coalesce(struct net_device *dev,
 		return ret;

 	if (update_napi) {
-		for (i = 0; i < vi->max_queue_pairs; i++)
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			/* xsk xmit depend on the tx napi. So if xsk is active,

depends.

quoted

+			 * prevent modifications to tx napi.
+			 */
+			if (rtnl_dereference(vi->sq[i].xsk.pool))
+				continue;
+
 			vi->sq[i].napi.weight = napi_weight;

I don't get it.
changing napi weight does not work then.
why is this ok?

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

	if (napi->weight)
		virtqueue_napi_schedule(napi, vq);
	else
		/* We were probably waiting for more output buffers. */
		netif_wake_subqueue(vi->dev, vq2txq(vq));
}


If the weight is 0, tx napi will not be triggered again.

Thanks.

This needs more thought then. First ignoring what user is requesting is
not nice.

Maybe we should return an error.

maybe

quoted

Second what if napi is first disabled and then xsk enabled?

static int virtnet_xsk_pool_enable(struct net_device *dev,
				   struct xsk_buff_pool *pool,
				   u16 qid)
{
	struct virtnet_info *vi = netdev_priv(dev);
	struct receive_queue *rq;
	struct send_queue *sq;
	int err;

	if (qid >= vi->curr_queue_pairs)
		return -EINVAL;

	sq = &vi->sq[qid];
	rq = &vi->rq[qid];

	/* xsk zerocopy depend on the tx napi.
	 *
	 * All xsk packets are actually consumed and sent out from the xsk tx
	 * queue under the tx napi mechanism.
	 */
->	if (!sq->napi.weight)
		return -EPERM;

Thanks.

quoted

quoted

quoted

quoted

+		}
 	}

 	return ret;
--
2.32.0.3.g01195cf9f

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization