Re: [PATCH net-next] virtio-net: switch napi_tx without downing nic

[Zhu Yanjun]

在 2023/12/21 13:20, Heng Qi 写道:


在 2023/12/21 上午11:02, Zhu Yanjun 写道:
在 2023/12/20 16:07, Heng Qi 写道:
virtio-net has two ways to switch napi_tx: one is through the
module parameter, and the other is through coalescing parameter
settings (provided that the nic status is down).

Sometimes we face performance regression caused by napi_tx,
then we need to switch napi_tx when debugging. However, the
existing methods are a bit troublesome, such as needing to
reload the driver or turn off the network card. So try to make
this update.

What scenario can trigger this? We want to make tests on our device.

Hi Zhu Yanjun, you can use the following cmds:

ethtool -C tx-frames 0, to disable napi_tx
ethtool -C tx-frames 1, to enable napi_tx


Thanks a lot. Just now I made tests on our device. I confirmed that 
virtion_net driver can work well after running "ethtool -C NIC tx-frames 
0 && sleep 3 && ethtool -C NIC tx-frames 1".

You can add "Reviewed-and-tested-by: Zhu Yanjun <yanjun....@linux.dev>"

Thanks,

Zhu Yanjun


Thanks.


Zhu Yanjun


Signed-off-by: Heng Qi <hen...@linux.alibaba.com>
Reviewed-by: Xuan Zhuo <xuanz...@linux.alibaba.com>
---
  drivers/net/virtio_net.c | 81 
++++++++++++++++++----------------------
  1 file changed, 37 insertions(+), 44 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 10614e9f7cad..12f8e1f9971c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3559,16 +3559,37 @@ static int 
virtnet_coal_params_supported(struct ethtool_coalesce *ec)
      return 0;
  }
  -static int virtnet_should_update_vq_weight(int dev_flags, int 
weight,
-                       int vq_weight, bool *should_update)
+static void virtnet_switch_napi_tx(struct virtnet_info *vi, u32 
qstart,
+                   u32 qend, u32 tx_frames)
  {
-    if (weight ^ vq_weight) {
-        if (dev_flags & IFF_UP)
-            return -EBUSY;
-        *should_update = true;
-    }
+    struct net_device *dev = vi->dev;
+    int new_weight, cur_weight;
+    struct netdev_queue *txq;
+    struct send_queue *sq;
  -    return 0;
+    new_weight = tx_frames ? NAPI_POLL_WEIGHT : 0;
+    for (; qstart < qend; qstart++) {
+        sq = &vi->sq[qstart];
+        cur_weight = sq->napi.weight;
+        if (!(new_weight ^ cur_weight))
+            continue;
+
+        if (!(dev->flags & IFF_UP)) {
+            sq->napi.weight = new_weight;
+            continue;
+        }
+
+        if (cur_weight)
+            virtnet_napi_tx_disable(&sq->napi);
+
+        txq = netdev_get_tx_queue(dev, qstart);
+        __netif_tx_lock_bh(txq);
+        sq->napi.weight = new_weight;
+        __netif_tx_unlock_bh(txq);
+
+        if (!cur_weight)
+            virtnet_napi_tx_enable(vi, sq->vq, &sq->napi);
+    }
  }
    static int virtnet_set_coalesce(struct net_device *dev,
@@ -3577,25 +3598,11 @@ static int virtnet_set_coalesce(struct 
net_device *dev,
                  struct netlink_ext_ack *extack)
  {
      struct virtnet_info *vi = netdev_priv(dev);
-    int ret, queue_number, napi_weight;
-    bool update_napi = false;
-
-    /* Can't change NAPI weight if the link is up */
-    napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
-    for (queue_number = 0; queue_number < vi->max_queue_pairs; 
queue_number++) {
-        ret = virtnet_should_update_vq_weight(dev->flags, napi_weight,
- vi->sq[queue_number].napi.weight,
-                              &update_napi);
-        if (ret)
-            return ret;
-
-        if (update_napi) {
-            /* All queues that belong to [queue_number, 
vi->max_queue_pairs] will be
-             * updated for the sake of simplicity, which might not 
be necessary
-             */
-            break;
-        }
-    }
+    int ret;
+
+    /* Param tx_frames can be used to switch napi_tx */
+    virtnet_switch_napi_tx(vi, 0, vi->max_queue_pairs,
+                   ec->tx_max_coalesced_frames);
        if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL))
          ret = virtnet_send_notf_coal_cmds(vi, ec);
@@ -3605,11 +3612,6 @@ static int virtnet_set_coalesce(struct 
net_device *dev,
      if (ret)
          return ret;
  -    if (update_napi) {
-        for (; queue_number < vi->max_queue_pairs; queue_number++)
-            vi->sq[queue_number].napi.weight = napi_weight;
-    }
-
      return ret;
  }
  @@ -3641,19 +3643,13 @@ static int 
virtnet_set_per_queue_coalesce(struct net_device *dev,
                        struct ethtool_coalesce *ec)
  {
      struct virtnet_info *vi = netdev_priv(dev);
-    int ret, napi_weight;
-    bool update_napi = false;
+    int ret;
        if (queue >= vi->max_queue_pairs)
          return -EINVAL;
  -    /* Can't change NAPI weight if the link is up */
-    napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
-    ret = virtnet_should_update_vq_weight(dev->flags, napi_weight,
-                          vi->sq[queue].napi.weight,
-                          &update_napi);
-    if (ret)
-        return ret;
+    /* Param tx_frames can be used to switch napi_tx */
+    virtnet_switch_napi_tx(vi, queue, queue, 
ec->tx_max_coalesced_frames);
        if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
          ret = virtnet_send_notf_coal_vq_cmds(vi, ec, queue);
@@ -3663,9 +3659,6 @@ static int 
virtnet_set_per_queue_coalesce(struct net_device *dev,
      if (ret)
          return ret;
  -    if (update_napi)
-        vi->sq[queue].napi.weight = napi_weight;
-
      return 0;
  }