From 2af5adf962d4611a576061501faa8fb39590407e Mon Sep 17 00:00:00 2001 From: Joe Damato Date: Fri, 7 Mar 2025 01:12:09 +0000 Subject: [PATCH 1/4] virtio-net: Refactor napi_enable paths Refactor virtnet_napi_enable and virtnet_napi_tx_enable to take a struct receive_queue. Create a helper, virtnet_napi_do_enable, which contains the logic to enable a NAPI. Signed-off-by: Joe Damato Acked-by: Michael S. Tsirkin Acked-by: Jason Wang Tested-by: Lei Yang Reviewed-by: Xuan Zhuo Link: https://patch.msgid.link/20250307011215.266806-2-jdamato@fastly.com Signed-off-by: Jakub Kicinski --- drivers/net/virtio_net.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index ac26a6201c44..133b004c7a9a 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2807,7 +2807,8 @@ static void skb_recv_done(struct virtqueue *rvq) virtqueue_napi_schedule(&rq->napi, rvq); } -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi) +static void virtnet_napi_do_enable(struct virtqueue *vq, + struct napi_struct *napi) { napi_enable(napi); @@ -2820,10 +2821,16 @@ static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi) local_bh_enable(); } -static void virtnet_napi_tx_enable(struct virtnet_info *vi, - struct virtqueue *vq, - struct napi_struct *napi) +static void virtnet_napi_enable(struct receive_queue *rq) { + virtnet_napi_do_enable(rq->vq, &rq->napi); +} + +static void virtnet_napi_tx_enable(struct send_queue *sq) +{ + struct virtnet_info *vi = sq->vq->vdev->priv; + struct napi_struct *napi = &sq->napi; + if (!napi->weight) return; @@ -2835,7 +2842,7 @@ static void virtnet_napi_tx_enable(struct virtnet_info *vi, return; } - return virtnet_napi_enable(vq, napi); + virtnet_napi_do_enable(sq->vq, napi); } static void virtnet_napi_tx_disable(struct napi_struct *napi) @@ -2856,7 +2863,7 @@ static void refill_work(struct work_struct *work) napi_disable(&rq->napi); still_empty = !try_fill_recv(vi, rq, GFP_KERNEL); - virtnet_napi_enable(rq->vq, &rq->napi); + virtnet_napi_enable(rq); /* In theory, this can happen: if we don't get any buffers in * we will *never* try to fill again. @@ -3073,8 +3080,8 @@ static int virtnet_enable_queue_pair(struct virtnet_info *vi, int qp_index) if (err < 0) goto err_xdp_reg_mem_model; - virtnet_napi_enable(vi->rq[qp_index].vq, &vi->rq[qp_index].napi); - virtnet_napi_tx_enable(vi, vi->sq[qp_index].vq, &vi->sq[qp_index].napi); + virtnet_napi_enable(&vi->rq[qp_index]); + virtnet_napi_tx_enable(&vi->sq[qp_index]); return 0; @@ -3339,7 +3346,7 @@ static void virtnet_rx_resume(struct virtnet_info *vi, struct receive_queue *rq) schedule_delayed_work(&vi->refill, 0); if (running) - virtnet_napi_enable(rq->vq, &rq->napi); + virtnet_napi_enable(rq); } static int virtnet_rx_resize(struct virtnet_info *vi, @@ -3402,7 +3409,7 @@ static void virtnet_tx_resume(struct virtnet_info *vi, struct send_queue *sq) __netif_tx_unlock_bh(txq); if (running) - virtnet_napi_tx_enable(vi, sq->vq, &sq->napi); + virtnet_napi_tx_enable(sq); } static int virtnet_tx_resize(struct virtnet_info *vi, struct send_queue *sq, @@ -5983,9 +5990,8 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog, if (old_prog) bpf_prog_put(old_prog); if (netif_running(dev)) { - virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi); - virtnet_napi_tx_enable(vi, vi->sq[i].vq, - &vi->sq[i].napi); + virtnet_napi_enable(&vi->rq[i]); + virtnet_napi_tx_enable(&vi->sq[i]); } } @@ -6000,9 +6006,8 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog, if (netif_running(dev)) { for (i = 0; i < vi->max_queue_pairs; i++) { - virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi); - virtnet_napi_tx_enable(vi, vi->sq[i].vq, - &vi->sq[i].napi); + virtnet_napi_enable(&vi->rq[i]); + virtnet_napi_tx_enable(&vi->sq[i]); } } if (prog) From 986a93045183ae2f13e6d99d990ae8be36f6d6b0 Mon Sep 17 00:00:00 2001 From: Joe Damato Date: Fri, 7 Mar 2025 01:12:10 +0000 Subject: [PATCH 2/4] virtio-net: Refactor napi_disable paths Create virtnet_napi_disable helper and refactor virtnet_napi_tx_disable to take a struct send_queue. Signed-off-by: Joe Damato Acked-by: Michael S. Tsirkin Acked-by: Jason Wang Tested-by: Lei Yang Reviewed-by: Xuan Zhuo Link: https://patch.msgid.link/20250307011215.266806-3-jdamato@fastly.com Signed-off-by: Jakub Kicinski --- drivers/net/virtio_net.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 133b004c7a9a..e578885c1093 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2845,12 +2845,21 @@ static void virtnet_napi_tx_enable(struct send_queue *sq) virtnet_napi_do_enable(sq->vq, napi); } -static void virtnet_napi_tx_disable(struct napi_struct *napi) +static void virtnet_napi_tx_disable(struct send_queue *sq) { + struct napi_struct *napi = &sq->napi; + if (napi->weight) napi_disable(napi); } +static void virtnet_napi_disable(struct receive_queue *rq) +{ + struct napi_struct *napi = &rq->napi; + + napi_disable(napi); +} + static void refill_work(struct work_struct *work) { struct virtnet_info *vi = @@ -2861,7 +2870,7 @@ static void refill_work(struct work_struct *work) for (i = 0; i < vi->curr_queue_pairs; i++) { struct receive_queue *rq = &vi->rq[i]; - napi_disable(&rq->napi); + virtnet_napi_disable(rq); still_empty = !try_fill_recv(vi, rq, GFP_KERNEL); virtnet_napi_enable(rq); @@ -3060,8 +3069,8 @@ static int virtnet_poll(struct napi_struct *napi, int budget) static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index) { - virtnet_napi_tx_disable(&vi->sq[qp_index].napi); - napi_disable(&vi->rq[qp_index].napi); + virtnet_napi_tx_disable(&vi->sq[qp_index]); + virtnet_napi_disable(&vi->rq[qp_index]); xdp_rxq_info_unreg(&vi->rq[qp_index].xdp_rxq); } @@ -3333,7 +3342,7 @@ static void virtnet_rx_pause(struct virtnet_info *vi, struct receive_queue *rq) bool running = netif_running(vi->dev); if (running) { - napi_disable(&rq->napi); + virtnet_napi_disable(rq); virtnet_cancel_dim(vi, &rq->dim); } } @@ -3375,7 +3384,7 @@ static void virtnet_tx_pause(struct virtnet_info *vi, struct send_queue *sq) qindex = sq - vi->sq; if (running) - virtnet_napi_tx_disable(&sq->napi); + virtnet_napi_tx_disable(sq); txq = netdev_get_tx_queue(vi->dev, qindex); @@ -5952,8 +5961,8 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog, /* Make sure NAPI is not using any XDP TX queues for RX. */ if (netif_running(dev)) { for (i = 0; i < vi->max_queue_pairs; i++) { - napi_disable(&vi->rq[i].napi); - virtnet_napi_tx_disable(&vi->sq[i].napi); + virtnet_napi_disable(&vi->rq[i]); + virtnet_napi_tx_disable(&vi->sq[i]); } } From e7231f49d526823975bbfe0abde5c50b7e8dfe3a Mon Sep 17 00:00:00 2001 From: Joe Damato Date: Fri, 7 Mar 2025 01:12:11 +0000 Subject: [PATCH 3/4] virtio-net: Map NAPIs to queues Use netif_queue_set_napi to map NAPIs to queue IDs so that the mapping can be accessed by user apps. Note that the netif_queue_set_napi currently requires RTNL, so care must be taken to ensure RTNL is held on paths where this API might be reached. The paths in the driver where this API can be reached appear to be: - ndo_open, ndo_close, which hold RTNL so no driver change is needed. - rx_pause, rx_resume, tx_pause, tx_resume are reached either via an ethtool ioctl or via XSK - neither path requires a driver change. - power management paths (which call open and close), which have been updated to hold/release RTNL. $ ethtool -i ens4 | grep driver driver: virtio_net $ sudo ethtool -L ens4 combined 4 $ ./tools/net/ynl/pyynl/cli.py \ --spec Documentation/netlink/specs/netdev.yaml \ --dump queue-get --json='{"ifindex": 2}' [{'id': 0, 'ifindex': 2, 'napi-id': 8289, 'type': 'rx'}, {'id': 1, 'ifindex': 2, 'napi-id': 8290, 'type': 'rx'}, {'id': 2, 'ifindex': 2, 'napi-id': 8291, 'type': 'rx'}, {'id': 3, 'ifindex': 2, 'napi-id': 8292, 'type': 'rx'}, {'id': 0, 'ifindex': 2, 'type': 'tx'}, {'id': 1, 'ifindex': 2, 'type': 'tx'}, {'id': 2, 'ifindex': 2, 'type': 'tx'}, {'id': 3, 'ifindex': 2, 'type': 'tx'}] Note that virtio_net has TX-only NAPIs which do not have NAPI IDs, so the lack of 'napi-id' in the above output is expected. Signed-off-by: Joe Damato Acked-by: Jason Wang Reviewed-by: Xuan Zhuo Link: https://patch.msgid.link/20250307011215.266806-4-jdamato@fastly.com Signed-off-by: Jakub Kicinski --- drivers/net/virtio_net.c | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e578885c1093..7bd63a677123 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2823,13 +2823,18 @@ static void virtnet_napi_do_enable(struct virtqueue *vq, static void virtnet_napi_enable(struct receive_queue *rq) { + struct virtnet_info *vi = rq->vq->vdev->priv; + int qidx = vq2rxq(rq->vq); + virtnet_napi_do_enable(rq->vq, &rq->napi); + netif_queue_set_napi(vi->dev, qidx, NETDEV_QUEUE_TYPE_RX, &rq->napi); } static void virtnet_napi_tx_enable(struct send_queue *sq) { struct virtnet_info *vi = sq->vq->vdev->priv; struct napi_struct *napi = &sq->napi; + int qidx = vq2txq(sq->vq); if (!napi->weight) return; @@ -2843,20 +2848,28 @@ static void virtnet_napi_tx_enable(struct send_queue *sq) } virtnet_napi_do_enable(sq->vq, napi); + netif_queue_set_napi(vi->dev, qidx, NETDEV_QUEUE_TYPE_TX, napi); } static void virtnet_napi_tx_disable(struct send_queue *sq) { + struct virtnet_info *vi = sq->vq->vdev->priv; struct napi_struct *napi = &sq->napi; + int qidx = vq2txq(sq->vq); - if (napi->weight) + if (napi->weight) { + netif_queue_set_napi(vi->dev, qidx, NETDEV_QUEUE_TYPE_TX, NULL); napi_disable(napi); + } } static void virtnet_napi_disable(struct receive_queue *rq) { + struct virtnet_info *vi = rq->vq->vdev->priv; struct napi_struct *napi = &rq->napi; + int qidx = vq2rxq(rq->vq); + netif_queue_set_napi(vi->dev, qidx, NETDEV_QUEUE_TYPE_RX, NULL); napi_disable(napi); } @@ -2870,9 +2883,23 @@ static void refill_work(struct work_struct *work) for (i = 0; i < vi->curr_queue_pairs; i++) { struct receive_queue *rq = &vi->rq[i]; - virtnet_napi_disable(rq); + /* + * When queue API support is added in the future and the call + * below becomes napi_disable_locked, this driver will need to + * be refactored. + * + * One possible solution would be to: + * - cancel refill_work with cancel_delayed_work (note: + * non-sync) + * - cancel refill_work with cancel_delayed_work_sync in + * virtnet_remove after the netdev is unregistered + * - wrap all of the work in a lock (perhaps the netdev + * instance lock) + * - check netif_running() and return early to avoid a race + */ + napi_disable(&rq->napi); still_empty = !try_fill_recv(vi, rq, GFP_KERNEL); - virtnet_napi_enable(rq); + virtnet_napi_do_enable(rq->vq, &rq->napi); /* In theory, this can happen: if we don't get any buffers in * we will *never* try to fill again. @@ -5650,8 +5677,11 @@ static void virtnet_freeze_down(struct virtio_device *vdev) netif_tx_lock_bh(vi->dev); netif_device_detach(vi->dev); netif_tx_unlock_bh(vi->dev); - if (netif_running(vi->dev)) + if (netif_running(vi->dev)) { + rtnl_lock(); virtnet_close(vi->dev); + rtnl_unlock(); + } } static int init_vqs(struct virtnet_info *vi); @@ -5671,7 +5701,9 @@ static int virtnet_restore_up(struct virtio_device *vdev) enable_rx_mode_work(vi); if (netif_running(vi->dev)) { + rtnl_lock(); err = virtnet_open(vi->dev); + rtnl_unlock(); if (err) return err; } From d5d715207e2911b84b92f10420d4a8d7653aa98d Mon Sep 17 00:00:00 2001 From: Joe Damato Date: Fri, 7 Mar 2025 01:12:12 +0000 Subject: [PATCH 4/4] virtio_net: Use persistent NAPI config Use persistent NAPI config so that NAPI IDs are not renumbered as queue counts change. $ sudo ethtool -l ens4 | tail -5 | egrep -i '(current|combined)' Current hardware settings: Combined: 4 $ ./tools/net/ynl/pyynl/cli.py \ --spec Documentation/netlink/specs/netdev.yaml \ --dump queue-get --json='{"ifindex": 2}' [{'id': 0, 'ifindex': 2, 'napi-id': 8193, 'type': 'rx'}, {'id': 1, 'ifindex': 2, 'napi-id': 8194, 'type': 'rx'}, {'id': 2, 'ifindex': 2, 'napi-id': 8195, 'type': 'rx'}, {'id': 3, 'ifindex': 2, 'napi-id': 8196, 'type': 'rx'}, {'id': 0, 'ifindex': 2, 'type': 'tx'}, {'id': 1, 'ifindex': 2, 'type': 'tx'}, {'id': 2, 'ifindex': 2, 'type': 'tx'}, {'id': 3, 'ifindex': 2, 'type': 'tx'}] Now adjust the queue count, note that the NAPI IDs are not renumbered: $ sudo ethtool -L ens4 combined 1 $ ./tools/net/ynl/pyynl/cli.py \ --spec Documentation/netlink/specs/netdev.yaml \ --dump queue-get --json='{"ifindex": 2}' [{'id': 0, 'ifindex': 2, 'napi-id': 8193, 'type': 'rx'}, {'id': 0, 'ifindex': 2, 'type': 'tx'}] $ sudo ethtool -L ens4 combined 8 $ ./tools/net/ynl/pyynl/cli.py \ --spec Documentation/netlink/specs/netdev.yaml \ --dump queue-get --json='{"ifindex": 2}' [{'id': 0, 'ifindex': 2, 'napi-id': 8193, 'type': 'rx'}, {'id': 1, 'ifindex': 2, 'napi-id': 8194, 'type': 'rx'}, {'id': 2, 'ifindex': 2, 'napi-id': 8195, 'type': 'rx'}, {'id': 3, 'ifindex': 2, 'napi-id': 8196, 'type': 'rx'}, {'id': 4, 'ifindex': 2, 'napi-id': 8197, 'type': 'rx'}, {'id': 5, 'ifindex': 2, 'napi-id': 8198, 'type': 'rx'}, {'id': 6, 'ifindex': 2, 'napi-id': 8199, 'type': 'rx'}, {'id': 7, 'ifindex': 2, 'napi-id': 8200, 'type': 'rx'}, [...] Signed-off-by: Joe Damato Reviewed-by: Gerhard Engleder Acked-by: Jason Wang Acked-by: Michael S. Tsirkin Tested-by: Lei Yang Reviewed-by: Xuan Zhuo Link: https://patch.msgid.link/20250307011215.266806-5-jdamato@fastly.com Signed-off-by: Jakub Kicinski --- drivers/net/virtio_net.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 7bd63a677123..34cec2b11b74 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -6455,8 +6455,9 @@ static int virtnet_alloc_queues(struct virtnet_info *vi) INIT_DELAYED_WORK(&vi->refill, refill_work); for (i = 0; i < vi->max_queue_pairs; i++) { vi->rq[i].pages = NULL; - netif_napi_add_weight(vi->dev, &vi->rq[i].napi, virtnet_poll, - napi_weight); + netif_napi_add_config(vi->dev, &vi->rq[i].napi, virtnet_poll, + i); + vi->rq[i].napi.weight = napi_weight; netif_napi_add_tx_weight(vi->dev, &vi->sq[i].napi, virtnet_poll_tx, napi_tx ? napi_weight : 0);