[PATCH net-next v2 0/2] virtio_net: support getting initial value of irq coalesce

[PATCH net-next v2 0/2] virtio_net: support getting initial value of irq coalesce

[Heng Qi]

Patch 1 from Xuan: the virtnet cvq supports to get
result from the device.

Patch 2 reuses the VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET cmd to
get init coalesce value.

Changelog
==========
v1->v2:
  - Update patch1's commit log.

Heng Qi (1):
  virtio_net: get init coalesce value when probe

Xuan Zhuo (1):
  virtio_net: introduce ability to get reply info from device

 drivers/net/virtio_net.c | 92 ++++++++++++++++++++++++++++++++++------
 1 file changed, 78 insertions(+), 14 deletions(-)

-- 
2.32.0.3.g01195cf9f

[PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device

[Heng Qi]

From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82

Based on the description provided in the above specification, we have
enabled the virtio-net driver to support acquiring some response
information from the device via the CVQ (Control Virtqueue).

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7176b956460b..3bc9b1e621db 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2527,11 +2527,12 @@ static int virtnet_tx_resize(struct virtnet_info *vi,
  * supported by the hypervisor, as indicated by feature bits, should
  * never fail unless improperly formatted.
  */
-static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
-				 struct scatterlist *out)
+static bool virtnet_send_command_reply(struct virtnet_info *vi, u8 class, u8 cmd,
+				       struct scatterlist *out,
+				       struct scatterlist *in)
 {
-	struct scatterlist *sgs[4], hdr, stat;
-	unsigned out_num = 0, tmp;
+	struct scatterlist *sgs[5], hdr, stat;
+	u32 out_num = 0, tmp, in_num = 0;
 	int ret;
 
 	/* Caller should know better */
@@ -2549,10 +2550,13 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
 
 	/* Add return status. */
 	sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
-	sgs[out_num] = &stat;
+	sgs[out_num + in_num++] = &stat;
 
-	BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
-	ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
+	if (in)
+		sgs[out_num + in_num++] = in;
+
+	BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
+	ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC);
 	if (ret < 0) {
 		dev_warn(&vi->vdev->dev,
 			 "Failed to add sgs for command vq: %d\n.", ret);
@@ -2574,6 +2578,12 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
 	return vi->ctrl->status == VIRTIO_NET_OK;
 }
 
+static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
+				 struct scatterlist *out)
+{
+	return virtnet_send_command_reply(vi, class, cmd, out, NULL);
+}
+
 static int virtnet_set_mac_address(struct net_device *dev, void *p)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
-- 
2.32.0.3.g01195cf9f

[PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe

[Heng Qi]

Currently, virtio-net lacks a way to obtain the default coalesce
values of the device during the probe phase. That is, the device
may have default experience values, but the user uses "ethtool -c"
to query that the values are still 0.

Therefore, we reuse VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET to complete the goal.

Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3bc9b1e621db..fe0c15819dd3 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -4623,6 +4623,46 @@ static int virtnet_validate(struct virtio_device *vdev)
 	return 0;
 }
 
+static int virtnet_get_coal_init_value(struct virtnet_info *vi,
+				       u16 _vqn, bool is_tx)
+{
+	struct virtio_net_ctrl_coal *coal = &vi->ctrl->coal_vq.coal;
+	__le16 *vqn = &vi->ctrl->coal_vq.vqn;
+	struct scatterlist sgs_in, sgs_out;
+	u32 usecs, pkts, i;
+	bool ret;
+
+	*vqn = cpu_to_le16(_vqn);
+
+	sg_init_one(&sgs_out, vqn, sizeof(*vqn));
+	sg_init_one(&sgs_in, coal, sizeof(*coal));
+	ret = virtnet_send_command_reply(vi, VIRTIO_NET_CTRL_NOTF_COAL,
+					 VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET,
+					 &sgs_out, &sgs_in);
+	if (!ret)
+		return ret;
+
+	usecs = le32_to_cpu(coal->max_usecs);
+	pkts = le32_to_cpu(coal->max_packets);
+	if (is_tx) {
+		vi->intr_coal_tx.max_usecs = usecs;
+		vi->intr_coal_tx.max_packets = pkts;
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			vi->sq[i].intr_coal.max_usecs = usecs;
+			vi->sq[i].intr_coal.max_packets = pkts;
+		}
+	} else {
+		vi->intr_coal_rx.max_usecs = usecs;
+		vi->intr_coal_rx.max_packets = pkts;
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			vi->rq[i].intr_coal.max_usecs = usecs;
+			vi->rq[i].intr_coal.max_packets = pkts;
+		}
+	}
+
+	return 0;
+}
+
 static bool virtnet_check_guest_gso(const struct virtnet_info *vi)
 {
 	return virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
@@ -4885,13 +4925,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 			vi->intr_coal_tx.max_packets = 0;
 	}
 
-	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
-		/* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
-		for (i = 0; i < vi->max_queue_pairs; i++)
-			if (vi->sq[i].napi.weight)
-				vi->sq[i].intr_coal.max_packets = 1;
-	}
-
 #ifdef CONFIG_SYSFS
 	if (vi->mergeable_rx_bufs)
 		dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
@@ -4926,6 +4959,27 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 	virtio_device_ready(vdev);
 
+	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
+		/* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
+		for (i = 0; i < vi->max_queue_pairs; i++)
+			if (vi->sq[i].napi.weight)
+				vi->sq[i].intr_coal.max_packets = 1;
+
+		/* The loop exits if the default value from any
+		 * queue is successfully read.
+		 */
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			err = virtnet_get_coal_init_value(vi, rxq2vq(i), false);
+			if (!err)
+				break;
+		}
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			err = virtnet_get_coal_init_value(vi, txq2vq(i), true);
+			if (!err)
+				break;
+		}
+	}
+
 	_virtnet_set_queues(vi, vi->curr_queue_pairs);
 
 	/* a random MAC address has been assigned, notify the device.
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe

[Jason Wang]

On Fri, Apr 26, 2024 at 2:54 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> Currently, virtio-net lacks a way to obtain the default coalesce
> values of the device during the probe phase. That is, the device
> may have default experience values, but the user uses "ethtool -c"
> to query that the values are still 0.
>
> Therefore, we reuse VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET to complete the goal.
>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> ---
>  drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 61 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 3bc9b1e621db..fe0c15819dd3 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4623,6 +4623,46 @@ static int virtnet_validate(struct virtio_device *vdev)
>         return 0;
>  }
>
> +static int virtnet_get_coal_init_value(struct virtnet_info *vi,
> +                                      u16 _vqn, bool is_tx)
> +{
> +       struct virtio_net_ctrl_coal *coal = &vi->ctrl->coal_vq.coal;
> +       __le16 *vqn = &vi->ctrl->coal_vq.vqn;
> +       struct scatterlist sgs_in, sgs_out;
> +       u32 usecs, pkts, i;
> +       bool ret;
> +
> +       *vqn = cpu_to_le16(_vqn);
> +
> +       sg_init_one(&sgs_out, vqn, sizeof(*vqn));
> +       sg_init_one(&sgs_in, coal, sizeof(*coal));
> +       ret = virtnet_send_command_reply(vi, VIRTIO_NET_CTRL_NOTF_COAL,
> +                                        VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET,
> +                                        &sgs_out, &sgs_in);
> +       if (!ret)
> +               return ret;
> +
> +       usecs = le32_to_cpu(coal->max_usecs);
> +       pkts = le32_to_cpu(coal->max_packets);
> +       if (is_tx) {
> +               vi->intr_coal_tx.max_usecs = usecs;
> +               vi->intr_coal_tx.max_packets = pkts;
> +               for (i = 0; i < vi->max_queue_pairs; i++) {
> +                       vi->sq[i].intr_coal.max_usecs = usecs;
> +                       vi->sq[i].intr_coal.max_packets = pkts;
> +               }
> +       } else {
> +               vi->intr_coal_rx.max_usecs = usecs;
> +               vi->intr_coal_rx.max_packets = pkts;
> +               for (i = 0; i < vi->max_queue_pairs; i++) {
> +                       vi->rq[i].intr_coal.max_usecs = usecs;
> +                       vi->rq[i].intr_coal.max_packets = pkts;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  static bool virtnet_check_guest_gso(const struct virtnet_info *vi)
>  {
>         return virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
> @@ -4885,13 +4925,6 @@ static int virtnet_probe(struct virtio_device *vdev)
>                         vi->intr_coal_tx.max_packets = 0;
>         }
>
> -       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> -               /* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
> -               for (i = 0; i < vi->max_queue_pairs; i++)
> -                       if (vi->sq[i].napi.weight)
> -                               vi->sq[i].intr_coal.max_packets = 1;
> -       }
> -
>  #ifdef CONFIG_SYSFS
>         if (vi->mergeable_rx_bufs)
>                 dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
> @@ -4926,6 +4959,27 @@ static int virtnet_probe(struct virtio_device *vdev)
>
>         virtio_device_ready(vdev);
>
> +       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> +               /* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
> +               for (i = 0; i < vi->max_queue_pairs; i++)
> +                       if (vi->sq[i].napi.weight)
> +                               vi->sq[i].intr_coal.max_packets = 1;
> +
> +               /* The loop exits if the default value from any
> +                * queue is successfully read.
> +                */

So this assumes the default values are the same. Is this something
required by the spec? If not, we probably need to iterate all the
queues.

Thanks


> +               for (i = 0; i < vi->max_queue_pairs; i++) {
> +                       err = virtnet_get_coal_init_value(vi, rxq2vq(i), false);
> +                       if (!err)
> +                               break;
> +               }
> +               for (i = 0; i < vi->max_queue_pairs; i++) {
> +                       err = virtnet_get_coal_init_value(vi, txq2vq(i), true);
> +                       if (!err)
> +                               break;
> +               }
> +       }
> +
>         _virtnet_set_queues(vi, vi->curr_queue_pairs);
>
>         /* a random MAC address has been assigned, notify the device.
> --
> 2.32.0.3.g01195cf9f
>

Re: [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device

[Jason Wang]

On Fri, Apr 26, 2024 at 2:54 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
>
> Based on the description provided in the above specification, we have
> enabled the virtio-net driver to support acquiring some response
> information from the device via the CVQ (Control Virtqueue).
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

I wonder if we need to tweak the spec as it has:

"""
Upon disabling and re-enabling a transmit virtqueue, the device MUST
set the coalescing parameters of the virtqueue
to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET
command, or, if the driver did not set any TX coalescing parameters,
to 0.
"""

So for reset, this patch tells us the device would have a non-zero
default value.

But spec tolds us after vq reset, it has a zero default value ...

Thanks


> ---
>  drivers/net/virtio_net.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7176b956460b..3bc9b1e621db 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2527,11 +2527,12 @@ static int virtnet_tx_resize(struct virtnet_info *vi,
>   * supported by the hypervisor, as indicated by feature bits, should
>   * never fail unless improperly formatted.
>   */
> -static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> -                                struct scatterlist *out)
> +static bool virtnet_send_command_reply(struct virtnet_info *vi, u8 class, u8 cmd,
> +                                      struct scatterlist *out,
> +                                      struct scatterlist *in)
>  {
> -       struct scatterlist *sgs[4], hdr, stat;
> -       unsigned out_num = 0, tmp;
> +       struct scatterlist *sgs[5], hdr, stat;
> +       u32 out_num = 0, tmp, in_num = 0;
>         int ret;
>
>         /* Caller should know better */
> @@ -2549,10 +2550,13 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
>
>         /* Add return status. */
>         sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
> -       sgs[out_num] = &stat;
> +       sgs[out_num + in_num++] = &stat;
>
> -       BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
> -       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
> +       if (in)
> +               sgs[out_num + in_num++] = in;
> +
> +       BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
> +       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC);
>         if (ret < 0) {
>                 dev_warn(&vi->vdev->dev,
>                          "Failed to add sgs for command vq: %d\n.", ret);
> @@ -2574,6 +2578,12 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
>         return vi->ctrl->status == VIRTIO_NET_OK;
>  }
>
> +static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> +                                struct scatterlist *out)
> +{
> +       return virtnet_send_command_reply(vi, class, cmd, out, NULL);
> +}
> +
>  static int virtnet_set_mac_address(struct net_device *dev, void *p)
>  {
>         struct virtnet_info *vi = netdev_priv(dev);
> --
> 2.32.0.3.g01195cf9f
>

Re: [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device

[Heng Qi]

On Tue, 7 May 2024 14:24:19 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Fri, Apr 26, 2024 at 2:54 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >
> > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >
> > As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> >
> > Based on the description provided in the above specification, we have
> > enabled the virtio-net driver to support acquiring some response
> > information from the device via the CVQ (Control Virtqueue).
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> 
> I wonder if we need to tweak the spec as it has:
> 
> """
> Upon disabling and re-enabling a transmit virtqueue, the device MUST
> set the coalescing parameters of the virtqueue
> to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET
> command, or, if the driver did not set any TX coalescing parameters,
> to 0.
> """
> 
> So for reset, this patch tells us the device would have a non-zero
> default value.
> 
> But spec tolds us after vq reset, it has a zero default value ...

Maybe we add a bool or flag for driver to mark whether the user has actively
configured interrupt coalescing parameters. Then we can take actions when
vq reset occurs?

Thanks.

> 
> Thanks
> 
> 
> > ---
> >  drivers/net/virtio_net.c | 24 +++++++++++++++++-------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 7176b956460b..3bc9b1e621db 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2527,11 +2527,12 @@ static int virtnet_tx_resize(struct virtnet_info *vi,
> >   * supported by the hypervisor, as indicated by feature bits, should
> >   * never fail unless improperly formatted.
> >   */
> > -static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > -                                struct scatterlist *out)
> > +static bool virtnet_send_command_reply(struct virtnet_info *vi, u8 class, u8 cmd,
> > +                                      struct scatterlist *out,
> > +                                      struct scatterlist *in)
> >  {
> > -       struct scatterlist *sgs[4], hdr, stat;
> > -       unsigned out_num = 0, tmp;
> > +       struct scatterlist *sgs[5], hdr, stat;
> > +       u32 out_num = 0, tmp, in_num = 0;
> >         int ret;
> >
> >         /* Caller should know better */
> > @@ -2549,10 +2550,13 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> >
> >         /* Add return status. */
> >         sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
> > -       sgs[out_num] = &stat;
> > +       sgs[out_num + in_num++] = &stat;
> >
> > -       BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
> > -       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
> > +       if (in)
> > +               sgs[out_num + in_num++] = in;
> > +
> > +       BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
> > +       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC);
> >         if (ret < 0) {
> >                 dev_warn(&vi->vdev->dev,
> >                          "Failed to add sgs for command vq: %d\n.", ret);
> > @@ -2574,6 +2578,12 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> >         return vi->ctrl->status == VIRTIO_NET_OK;
> >  }
> >
> > +static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > +                                struct scatterlist *out)
> > +{
> > +       return virtnet_send_command_reply(vi, class, cmd, out, NULL);
> > +}
> > +
> >  static int virtnet_set_mac_address(struct net_device *dev, void *p)
> >  {
> >         struct virtnet_info *vi = netdev_priv(dev);
> > --
> > 2.32.0.3.g01195cf9f
> >
> 

Re: [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe

[Heng Qi]

On Tue, 7 May 2024 14:24:12 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Fri, Apr 26, 2024 at 2:54 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >
> > Currently, virtio-net lacks a way to obtain the default coalesce
> > values of the device during the probe phase. That is, the device
> > may have default experience values, but the user uses "ethtool -c"
> > to query that the values are still 0.
> >
> > Therefore, we reuse VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET to complete the goal.
> >
> > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > ---
> >  drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 61 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 3bc9b1e621db..fe0c15819dd3 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -4623,6 +4623,46 @@ static int virtnet_validate(struct virtio_device *vdev)
> >         return 0;
> >  }
> >
> > +static int virtnet_get_coal_init_value(struct virtnet_info *vi,
> > +                                      u16 _vqn, bool is_tx)
> > +{
> > +       struct virtio_net_ctrl_coal *coal = &vi->ctrl->coal_vq.coal;
> > +       __le16 *vqn = &vi->ctrl->coal_vq.vqn;
> > +       struct scatterlist sgs_in, sgs_out;
> > +       u32 usecs, pkts, i;
> > +       bool ret;
> > +
> > +       *vqn = cpu_to_le16(_vqn);
> > +
> > +       sg_init_one(&sgs_out, vqn, sizeof(*vqn));
> > +       sg_init_one(&sgs_in, coal, sizeof(*coal));
> > +       ret = virtnet_send_command_reply(vi, VIRTIO_NET_CTRL_NOTF_COAL,
> > +                                        VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET,
> > +                                        &sgs_out, &sgs_in);
> > +       if (!ret)
> > +               return ret;
> > +
> > +       usecs = le32_to_cpu(coal->max_usecs);
> > +       pkts = le32_to_cpu(coal->max_packets);
> > +       if (is_tx) {
> > +               vi->intr_coal_tx.max_usecs = usecs;
> > +               vi->intr_coal_tx.max_packets = pkts;
> > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > +                       vi->sq[i].intr_coal.max_usecs = usecs;
> > +                       vi->sq[i].intr_coal.max_packets = pkts;
> > +               }
> > +       } else {
> > +               vi->intr_coal_rx.max_usecs = usecs;
> > +               vi->intr_coal_rx.max_packets = pkts;
> > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > +                       vi->rq[i].intr_coal.max_usecs = usecs;
> > +                       vi->rq[i].intr_coal.max_packets = pkts;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  static bool virtnet_check_guest_gso(const struct virtnet_info *vi)
> >  {
> >         return virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
> > @@ -4885,13 +4925,6 @@ static int virtnet_probe(struct virtio_device *vdev)
> >                         vi->intr_coal_tx.max_packets = 0;
> >         }
> >
> > -       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> > -               /* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
> > -               for (i = 0; i < vi->max_queue_pairs; i++)
> > -                       if (vi->sq[i].napi.weight)
> > -                               vi->sq[i].intr_coal.max_packets = 1;
> > -       }
> > -
> >  #ifdef CONFIG_SYSFS
> >         if (vi->mergeable_rx_bufs)
> >                 dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
> > @@ -4926,6 +4959,27 @@ static int virtnet_probe(struct virtio_device *vdev)
> >
> >         virtio_device_ready(vdev);
> >
> > +       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> > +               /* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
> > +               for (i = 0; i < vi->max_queue_pairs; i++)
> > +                       if (vi->sq[i].napi.weight)
> > +                               vi->sq[i].intr_coal.max_packets = 1;
> > +
> > +               /* The loop exits if the default value from any
> > +                * queue is successfully read.
> > +                */
> 
> So this assumes the default values are the same. Is this something
> required by the spec? If not, we probably need to iterate all the
> queues.
> 

From internal practice, and from the default behavior of other existing drivers,
the queues all have the same value at the beginning, so here it seems feasible
that we get the value of queue 0 to represent the global value instead of using
a loop.

Moreover, obtaining the value once for each queue initially does not seem to be
very friendly for devices with a large number of queues.

Thanks.

> Thanks
> 
> 
> > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > +                       err = virtnet_get_coal_init_value(vi, rxq2vq(i), false);
> > +                       if (!err)
> > +                               break;
> > +               }
> > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > +                       err = virtnet_get_coal_init_value(vi, txq2vq(i), true);
> > +                       if (!err)
> > +                               break;
> > +               }
> > +       }
> > +
> >         _virtnet_set_queues(vi, vi->curr_queue_pairs);
> >
> >         /* a random MAC address has been assigned, notify the device.
> > --
> > 2.32.0.3.g01195cf9f
> >
> 

Re: [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device

[Jason Wang]

On Tue, May 7, 2024 at 2:56 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> On Tue, 7 May 2024 14:24:19 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Fri, Apr 26, 2024 at 2:54 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> > >
> > > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > >
> > > As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> > >
> > > Based on the description provided in the above specification, we have
> > > enabled the virtio-net driver to support acquiring some response
> > > information from the device via the CVQ (Control Virtqueue).
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >
> > I wonder if we need to tweak the spec as it has:
> >
> > """
> > Upon disabling and re-enabling a transmit virtqueue, the device MUST
> > set the coalescing parameters of the virtqueue
> > to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET
> > command, or, if the driver did not set any TX coalescing parameters,
> > to 0.
> > """
> >
> > So for reset, this patch tells us the device would have a non-zero
> > default value.
> >
> > But spec tolds us after vq reset, it has a zero default value ...
>
> Maybe we add a bool or flag for driver to mark whether the user has actively
> configured interrupt coalescing parameters. Then we can take actions when
> vq reset occurs?

I basically mean we probably need to tweak the spec. For example say
the device may have a default value for coalescing so driver need to
read them.

Thanks

>
> Thanks.
>
> >
> > Thanks
> >
> >
> > > ---
> > >  drivers/net/virtio_net.c | 24 +++++++++++++++++-------
> > >  1 file changed, 17 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 7176b956460b..3bc9b1e621db 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -2527,11 +2527,12 @@ static int virtnet_tx_resize(struct virtnet_info *vi,
> > >   * supported by the hypervisor, as indicated by feature bits, should
> > >   * never fail unless improperly formatted.
> > >   */
> > > -static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > > -                                struct scatterlist *out)
> > > +static bool virtnet_send_command_reply(struct virtnet_info *vi, u8 class, u8 cmd,
> > > +                                      struct scatterlist *out,
> > > +                                      struct scatterlist *in)
> > >  {
> > > -       struct scatterlist *sgs[4], hdr, stat;
> > > -       unsigned out_num = 0, tmp;
> > > +       struct scatterlist *sgs[5], hdr, stat;
> > > +       u32 out_num = 0, tmp, in_num = 0;
> > >         int ret;
> > >
> > >         /* Caller should know better */
> > > @@ -2549,10 +2550,13 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > >
> > >         /* Add return status. */
> > >         sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
> > > -       sgs[out_num] = &stat;
> > > +       sgs[out_num + in_num++] = &stat;
> > >
> > > -       BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
> > > -       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
> > > +       if (in)
> > > +               sgs[out_num + in_num++] = in;
> > > +
> > > +       BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
> > > +       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC);
> > >         if (ret < 0) {
> > >                 dev_warn(&vi->vdev->dev,
> > >                          "Failed to add sgs for command vq: %d\n.", ret);
> > > @@ -2574,6 +2578,12 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > >         return vi->ctrl->status == VIRTIO_NET_OK;
> > >  }
> > >
> > > +static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > > +                                struct scatterlist *out)
> > > +{
> > > +       return virtnet_send_command_reply(vi, class, cmd, out, NULL);
> > > +}
> > > +
> > >  static int virtnet_set_mac_address(struct net_device *dev, void *p)
> > >  {
> > >         struct virtnet_info *vi = netdev_priv(dev);
> > > --
> > > 2.32.0.3.g01195cf9f
> > >
> >
>

Re: [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe

[Jason Wang]

On Tue, May 7, 2024 at 3:01 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> On Tue, 7 May 2024 14:24:12 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Fri, Apr 26, 2024 at 2:54 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> > >
> > > Currently, virtio-net lacks a way to obtain the default coalesce
> > > values of the device during the probe phase. That is, the device
> > > may have default experience values, but the user uses "ethtool -c"
> > > to query that the values are still 0.
> > >
> > > Therefore, we reuse VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET to complete the goal.
> > >
> > > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > > ---
> > >  drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++++++++++-----
> > >  1 file changed, 61 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 3bc9b1e621db..fe0c15819dd3 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -4623,6 +4623,46 @@ static int virtnet_validate(struct virtio_device *vdev)
> > >         return 0;
> > >  }
> > >
> > > +static int virtnet_get_coal_init_value(struct virtnet_info *vi,
> > > +                                      u16 _vqn, bool is_tx)
> > > +{
> > > +       struct virtio_net_ctrl_coal *coal = &vi->ctrl->coal_vq.coal;
> > > +       __le16 *vqn = &vi->ctrl->coal_vq.vqn;
> > > +       struct scatterlist sgs_in, sgs_out;
> > > +       u32 usecs, pkts, i;
> > > +       bool ret;
> > > +
> > > +       *vqn = cpu_to_le16(_vqn);
> > > +
> > > +       sg_init_one(&sgs_out, vqn, sizeof(*vqn));
> > > +       sg_init_one(&sgs_in, coal, sizeof(*coal));
> > > +       ret = virtnet_send_command_reply(vi, VIRTIO_NET_CTRL_NOTF_COAL,
> > > +                                        VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET,
> > > +                                        &sgs_out, &sgs_in);
> > > +       if (!ret)
> > > +               return ret;
> > > +
> > > +       usecs = le32_to_cpu(coal->max_usecs);
> > > +       pkts = le32_to_cpu(coal->max_packets);
> > > +       if (is_tx) {
> > > +               vi->intr_coal_tx.max_usecs = usecs;
> > > +               vi->intr_coal_tx.max_packets = pkts;
> > > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > > +                       vi->sq[i].intr_coal.max_usecs = usecs;
> > > +                       vi->sq[i].intr_coal.max_packets = pkts;
> > > +               }
> > > +       } else {
> > > +               vi->intr_coal_rx.max_usecs = usecs;
> > > +               vi->intr_coal_rx.max_packets = pkts;
> > > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > > +                       vi->rq[i].intr_coal.max_usecs = usecs;
> > > +                       vi->rq[i].intr_coal.max_packets = pkts;
> > > +               }
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static bool virtnet_check_guest_gso(const struct virtnet_info *vi)
> > >  {
> > >         return virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
> > > @@ -4885,13 +4925,6 @@ static int virtnet_probe(struct virtio_device *vdev)
> > >                         vi->intr_coal_tx.max_packets = 0;
> > >         }
> > >
> > > -       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> > > -               /* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
> > > -               for (i = 0; i < vi->max_queue_pairs; i++)
> > > -                       if (vi->sq[i].napi.weight)
> > > -                               vi->sq[i].intr_coal.max_packets = 1;
> > > -       }
> > > -
> > >  #ifdef CONFIG_SYSFS
> > >         if (vi->mergeable_rx_bufs)
> > >                 dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
> > > @@ -4926,6 +4959,27 @@ static int virtnet_probe(struct virtio_device *vdev)
> > >
> > >         virtio_device_ready(vdev);
> > >
> > > +       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> > > +               /* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
> > > +               for (i = 0; i < vi->max_queue_pairs; i++)
> > > +                       if (vi->sq[i].napi.weight)
> > > +                               vi->sq[i].intr_coal.max_packets = 1;
> > > +
> > > +               /* The loop exits if the default value from any
> > > +                * queue is successfully read.
> > > +                */
> >
> > So this assumes the default values are the same. Is this something
> > required by the spec? If not, we probably need to iterate all the
> > queues.
> >
>
> From internal practice, and from the default behavior of other existing drivers,
> the queues all have the same value at the beginning, so here it seems feasible
> that we get the value of queue 0 to represent the global value instead of using
> a loop.

Well, unless the spec says the values are equal, the driver needs to iterate.

>
> Moreover, obtaining the value once for each queue initially does not seem to be
> very friendly for devices with a large number of queues.

We probably don't care too much about the time spent on the probe. For
example, there would be a lot of registers read/write as well.

Thanks

>
> Thanks.
>
> > Thanks
> >
> >
> > > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > > +                       err = virtnet_get_coal_init_value(vi, rxq2vq(i), false);
> > > +                       if (!err)
> > > +                               break;
> > > +               }
> > > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > > +                       err = virtnet_get_coal_init_value(vi, txq2vq(i), true);
> > > +                       if (!err)
> > > +                               break;
> > > +               }
> > > +       }
> > > +
> > >         _virtnet_set_queues(vi, vi->curr_queue_pairs);
> > >
> > >         /* a random MAC address has been assigned, notify the device.
> > > --
> > > 2.32.0.3.g01195cf9f
> > >
> >
>

Re: [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe

[Heng Qi]

On Wed, 8 May 2024 10:22:04 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Tue, May 7, 2024 at 3:01 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >
> > On Tue, 7 May 2024 14:24:12 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > On Fri, Apr 26, 2024 at 2:54 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> > > >
> > > > Currently, virtio-net lacks a way to obtain the default coalesce
> > > > values of the device during the probe phase. That is, the device
> > > > may have default experience values, but the user uses "ethtool -c"
> > > > to query that the values are still 0.
> > > >
> > > > Therefore, we reuse VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET to complete the goal.
> > > >
> > > > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > > > ---
> > > >  drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++++++++++-----
> > > >  1 file changed, 61 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 3bc9b1e621db..fe0c15819dd3 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -4623,6 +4623,46 @@ static int virtnet_validate(struct virtio_device *vdev)
> > > >         return 0;
> > > >  }
> > > >
> > > > +static int virtnet_get_coal_init_value(struct virtnet_info *vi,
> > > > +                                      u16 _vqn, bool is_tx)
> > > > +{
> > > > +       struct virtio_net_ctrl_coal *coal = &vi->ctrl->coal_vq.coal;
> > > > +       __le16 *vqn = &vi->ctrl->coal_vq.vqn;
> > > > +       struct scatterlist sgs_in, sgs_out;
> > > > +       u32 usecs, pkts, i;
> > > > +       bool ret;
> > > > +
> > > > +       *vqn = cpu_to_le16(_vqn);
> > > > +
> > > > +       sg_init_one(&sgs_out, vqn, sizeof(*vqn));
> > > > +       sg_init_one(&sgs_in, coal, sizeof(*coal));
> > > > +       ret = virtnet_send_command_reply(vi, VIRTIO_NET_CTRL_NOTF_COAL,
> > > > +                                        VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET,
> > > > +                                        &sgs_out, &sgs_in);
> > > > +       if (!ret)
> > > > +               return ret;
> > > > +
> > > > +       usecs = le32_to_cpu(coal->max_usecs);
> > > > +       pkts = le32_to_cpu(coal->max_packets);
> > > > +       if (is_tx) {
> > > > +               vi->intr_coal_tx.max_usecs = usecs;
> > > > +               vi->intr_coal_tx.max_packets = pkts;
> > > > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > > > +                       vi->sq[i].intr_coal.max_usecs = usecs;
> > > > +                       vi->sq[i].intr_coal.max_packets = pkts;
> > > > +               }
> > > > +       } else {
> > > > +               vi->intr_coal_rx.max_usecs = usecs;
> > > > +               vi->intr_coal_rx.max_packets = pkts;
> > > > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > > > +                       vi->rq[i].intr_coal.max_usecs = usecs;
> > > > +                       vi->rq[i].intr_coal.max_packets = pkts;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > >  static bool virtnet_check_guest_gso(const struct virtnet_info *vi)
> > > >  {
> > > >         return virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
> > > > @@ -4885,13 +4925,6 @@ static int virtnet_probe(struct virtio_device *vdev)
> > > >                         vi->intr_coal_tx.max_packets = 0;
> > > >         }
> > > >
> > > > -       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> > > > -               /* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
> > > > -               for (i = 0; i < vi->max_queue_pairs; i++)
> > > > -                       if (vi->sq[i].napi.weight)
> > > > -                               vi->sq[i].intr_coal.max_packets = 1;
> > > > -       }
> > > > -
> > > >  #ifdef CONFIG_SYSFS
> > > >         if (vi->mergeable_rx_bufs)
> > > >                 dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
> > > > @@ -4926,6 +4959,27 @@ static int virtnet_probe(struct virtio_device *vdev)
> > > >
> > > >         virtio_device_ready(vdev);
> > > >
> > > > +       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> > > > +               /* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
> > > > +               for (i = 0; i < vi->max_queue_pairs; i++)
> > > > +                       if (vi->sq[i].napi.weight)
> > > > +                               vi->sq[i].intr_coal.max_packets = 1;
> > > > +
> > > > +               /* The loop exits if the default value from any
> > > > +                * queue is successfully read.
> > > > +                */
> > >
> > > So this assumes the default values are the same. Is this something
> > > required by the spec? If not, we probably need to iterate all the
> > > queues.
> > >
> >
> > From internal practice, and from the default behavior of other existing drivers,
> > the queues all have the same value at the beginning, so here it seems feasible
> > that we get the value of queue 0 to represent the global value instead of using
> > a loop.
> 
> Well, unless the spec says the values are equal, the driver needs to iterate.

Ok. Will update this in the next version.

Thanks.

> 
> >
> > Moreover, obtaining the value once for each queue initially does not seem to be
> > very friendly for devices with a large number of queues.
> 
> We probably don't care too much about the time spent on the probe. For
> example, there would be a lot of registers read/write as well.
> 
> Thanks
> 
> >
> > Thanks.
> >
> > > Thanks
> > >
> > >
> > > > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > > > +                       err = virtnet_get_coal_init_value(vi, rxq2vq(i), false);
> > > > +                       if (!err)
> > > > +                               break;
> > > > +               }
> > > > +               for (i = 0; i < vi->max_queue_pairs; i++) {
> > > > +                       err = virtnet_get_coal_init_value(vi, txq2vq(i), true);
> > > > +                       if (!err)
> > > > +                               break;
> > > > +               }
> > > > +       }
> > > > +
> > > >         _virtnet_set_queues(vi, vi->curr_queue_pairs);
> > > >
> > > >         /* a random MAC address has been assigned, notify the device.
> > > > --
> > > > 2.32.0.3.g01195cf9f
> > > >
> > >
> >
> 

Re: [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device

[Heng Qi]

On Wed, 8 May 2024 10:20:05 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Tue, May 7, 2024 at 2:56 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >
> > On Tue, 7 May 2024 14:24:19 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > On Fri, Apr 26, 2024 at 2:54 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> > > >
> > > > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > >
> > > > As the spec https://github.com/oasis-tcs/virtio-spec/commit/42f389989823039724f95bbbd243291ab0064f82
> > > >
> > > > Based on the description provided in the above specification, we have
> > > > enabled the virtio-net driver to support acquiring some response
> > > > information from the device via the CVQ (Control Virtqueue).
> > > >
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > >
> > > I wonder if we need to tweak the spec as it has:
> > >
> > > """
> > > Upon disabling and re-enabling a transmit virtqueue, the device MUST
> > > set the coalescing parameters of the virtqueue
> > > to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET
> > > command, or, if the driver did not set any TX coalescing parameters,
> > > to 0.
> > > """
> > >
> > > So for reset, this patch tells us the device would have a non-zero
> > > default value.
> > >
> > > But spec tolds us after vq reset, it has a zero default value ...
> >
> > Maybe we add a bool or flag for driver to mark whether the user has actively
> > configured interrupt coalescing parameters. Then we can take actions when
> > vq reset occurs?
> 
> I basically mean we probably need to tweak the spec. For example say
> the device may have a default value for coalescing so driver need to
> read them.

Well, I'll post a tweak patch, and since the current virtio spec mailing list
is still not ready, I'll Cc people who were previously involved in the
discussion.

Thanks.

> 
> Thanks
> 
> >
> > Thanks.
> >
> > >
> > > Thanks
> > >
> > >
> > > > ---
> > > >  drivers/net/virtio_net.c | 24 +++++++++++++++++-------
> > > >  1 file changed, 17 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 7176b956460b..3bc9b1e621db 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -2527,11 +2527,12 @@ static int virtnet_tx_resize(struct virtnet_info *vi,
> > > >   * supported by the hypervisor, as indicated by feature bits, should
> > > >   * never fail unless improperly formatted.
> > > >   */
> > > > -static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > > > -                                struct scatterlist *out)
> > > > +static bool virtnet_send_command_reply(struct virtnet_info *vi, u8 class, u8 cmd,
> > > > +                                      struct scatterlist *out,
> > > > +                                      struct scatterlist *in)
> > > >  {
> > > > -       struct scatterlist *sgs[4], hdr, stat;
> > > > -       unsigned out_num = 0, tmp;
> > > > +       struct scatterlist *sgs[5], hdr, stat;
> > > > +       u32 out_num = 0, tmp, in_num = 0;
> > > >         int ret;
> > > >
> > > >         /* Caller should know better */
> > > > @@ -2549,10 +2550,13 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > > >
> > > >         /* Add return status. */
> > > >         sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
> > > > -       sgs[out_num] = &stat;
> > > > +       sgs[out_num + in_num++] = &stat;
> > > >
> > > > -       BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
> > > > -       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
> > > > +       if (in)
> > > > +               sgs[out_num + in_num++] = in;
> > > > +
> > > > +       BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
> > > > +       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC);
> > > >         if (ret < 0) {
> > > >                 dev_warn(&vi->vdev->dev,
> > > >                          "Failed to add sgs for command vq: %d\n.", ret);
> > > > @@ -2574,6 +2578,12 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > > >         return vi->ctrl->status == VIRTIO_NET_OK;
> > > >  }
> > > >
> > > > +static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> > > > +                                struct scatterlist *out)
> > > > +{
> > > > +       return virtnet_send_command_reply(vi, class, cmd, out, NULL);
> > > > +}
> > > > +
> > > >  static int virtnet_set_mac_address(struct net_device *dev, void *p)
> > > >  {
> > > >         struct virtnet_info *vi = netdev_priv(dev);
> > > > --
> > > > 2.32.0.3.g01195cf9f
> > > >
> > >
> >
>