* [PATCH net 1/6] virtio-net: initially change the value of tx-frames
2023-09-19 7:49 [PATCH net 0/6] virtio-net: Fix and update interrupt moderation Heng Qi
@ 2023-09-19 7:49 ` Heng Qi
2023-09-21 7:03 ` Jason Wang
2023-09-19 7:49 ` [PATCH net 2/6] virtio-net: fix mismatch of getting tx-frames Heng Qi
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Heng Qi @ 2023-09-19 7:49 UTC (permalink / raw)
To: netdev, virtualization
Cc: Michael S . Tsirkin, Jason Wang, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo
Background:
1. Commit 0c465be183c7 ("virtio_net: ethtool tx napi configuration") uses
tx-frames to toggle napi_tx (0 off and 1 on) if notification coalescing
is not supported.
2. Commit 31c03aef9bc2 ("virtio_net: enable napi_tx by default") enables
napi_tx for all txqs by default.
Status:
When virtio-net supports notification coalescing, after initialization,
tx-frames is 0 and napi_tx is true.
Problem:
When the user only wants to set rx coalescing params using
ethtool -C eth0 rx-usecs 10, or
ethtool -Q eth0 queue_mask 0x1 -C rx-usecs 10,
these cmds will carry tx-frames as 0, causing the napi_tx switching condition
is satisfied. Then the user gets:
netlink error: Device or resource busy.
The same happens when trying to set rx-frames, adaptive_rx, adaptive_tx...
Result:
When notification coalescing feature is negotiated, initially make the
value of tx-frames to be consistent with napi_tx.
For compatibility with the past, it is still supported to use tx-frames
to toggle napi_tx.
Reported-by: Xiaoming Zhao <zxm377917@alibaba-inc.com>
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
drivers/net/virtio_net.c | 42 +++++++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index fe7f314d65c9..fd5bc8d59eda 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -4442,13 +4442,6 @@ static int virtnet_probe(struct virtio_device *vdev)
dev->xdp_features |= NETDEV_XDP_ACT_RX_SG;
}
- if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
- vi->intr_coal_rx.max_usecs = 0;
- vi->intr_coal_tx.max_usecs = 0;
- vi->intr_coal_tx.max_packets = 0;
- vi->intr_coal_rx.max_packets = 0;
- }
-
if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
vi->has_rss_hash_report = true;
@@ -4523,6 +4516,41 @@ static int virtnet_probe(struct virtio_device *vdev)
if (err)
goto free;
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
+ vi->intr_coal_rx.max_usecs = 0;
+ vi->intr_coal_tx.max_usecs = 0;
+ vi->intr_coal_rx.max_packets = 0;
+
+ /* Why is this needed?
+ * If without this setting, consider that when VIRTIO_NET_F_NOTF_COAL is
+ * negotiated and napi_tx is initially true: when the user sets non tx-frames
+ * parameters, such as the following cmd or others,
+ * ethtool -C eth0 rx-usecs 10.
+ * Then
+ * 1. ethtool_set_coalesce() first calls virtnet_get_coalesce() to get
+ * the last parameters except rx-usecs. If tx-frames has never been set before,
+ * virtnet_get_coalesce() returns with tx-frames=0 in the parameters.
+ * 2. virtnet_set_coalesce() is then called, according to 1:
+ * ec->tx_max_coalesced_frames=0. Now napi_tx switching condition is met.
+ * 3. If the device is up, the user setting fails:
+ * "netlink error: Device or resource busy"
+ * This is not intuitive. Therefore, we keep napi_tx state consistent with
+ * tx-frames when VIRTIO_NET_F_NOTF_COAL is negotiated. This behavior is
+ * compatible with before.
+ */
+ if (vi->sq[0].napi.weight)
+ vi->intr_coal_tx.max_packets = 1;
+ else
+ 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;
--
2.19.1.6.gb485710b
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net 1/6] virtio-net: initially change the value of tx-frames
2023-09-19 7:49 ` [PATCH net 1/6] virtio-net: initially change the value of tx-frames Heng Qi
@ 2023-09-21 7:03 ` Jason Wang
0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2023-09-21 7:03 UTC (permalink / raw)
To: Heng Qi
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo
On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> Background:
> 1. Commit 0c465be183c7 ("virtio_net: ethtool tx napi configuration") uses
> tx-frames to toggle napi_tx (0 off and 1 on) if notification coalescing
> is not supported.
> 2. Commit 31c03aef9bc2 ("virtio_net: enable napi_tx by default") enables
> napi_tx for all txqs by default.
>
> Status:
> When virtio-net supports notification coalescing, after initialization,
> tx-frames is 0 and napi_tx is true.
>
> Problem:
> When the user only wants to set rx coalescing params using
> ethtool -C eth0 rx-usecs 10, or
> ethtool -Q eth0 queue_mask 0x1 -C rx-usecs 10,
> these cmds will carry tx-frames as 0, causing the napi_tx switching condition
> is satisfied. Then the user gets:
> netlink error: Device or resource busy.
>
> The same happens when trying to set rx-frames, adaptive_rx, adaptive_tx...
>
> Result:
It's probably not the result but how to fix it?
> When notification coalescing feature is negotiated, initially make the
> value of tx-frames to be consistent with napi_tx.
>
> For compatibility with the past, it is still supported to use tx-frames
> to toggle napi_tx.
>
> Reported-by: Xiaoming Zhao <zxm377917@alibaba-inc.com>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> ---
> drivers/net/virtio_net.c | 42 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index fe7f314d65c9..fd5bc8d59eda 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4442,13 +4442,6 @@ static int virtnet_probe(struct virtio_device *vdev)
> dev->xdp_features |= NETDEV_XDP_ACT_RX_SG;
> }
>
> - if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
> - vi->intr_coal_rx.max_usecs = 0;
> - vi->intr_coal_tx.max_usecs = 0;
> - vi->intr_coal_tx.max_packets = 0;
> - vi->intr_coal_rx.max_packets = 0;
> - }
> -
> if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> vi->has_rss_hash_report = true;
>
> @@ -4523,6 +4516,41 @@ static int virtnet_probe(struct virtio_device *vdev)
> if (err)
> goto free;
>
> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
> + vi->intr_coal_rx.max_usecs = 0;
> + vi->intr_coal_tx.max_usecs = 0;
> + vi->intr_coal_rx.max_packets = 0;
> +
> + /* Why is this needed?
> + * If without this setting, consider that when VIRTIO_NET_F_NOTF_COAL is
> + * negotiated and napi_tx is initially true: when the user sets non tx-frames
> + * parameters, such as the following cmd or others,
> + * ethtool -C eth0 rx-usecs 10.
> + * Then
> + * 1. ethtool_set_coalesce() first calls virtnet_get_coalesce() to get
> + * the last parameters except rx-usecs. If tx-frames has never been set before,
> + * virtnet_get_coalesce() returns with tx-frames=0 in the parameters.
> + * 2. virtnet_set_coalesce() is then called, according to 1:
> + * ec->tx_max_coalesced_frames=0. Now napi_tx switching condition is met.
> + * 3. If the device is up, the user setting fails:
> + * "netlink error: Device or resource busy"
> + * This is not intuitive. Therefore, we keep napi_tx state consistent with
> + * tx-frames when VIRTIO_NET_F_NOTF_COAL is negotiated. This behavior is
> + * compatible with before.
> + */
Maybe it's sufficient to say it tries to align the behaviour with that
tx NAPI is enabled by default?
Other than these minor issues:
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
> + if (vi->sq[0].napi.weight)
> + vi->intr_coal_tx.max_packets = 1;
> + else
> + 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;
> --
> 2.19.1.6.gb485710b
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net 2/6] virtio-net: fix mismatch of getting tx-frames
2023-09-19 7:49 [PATCH net 0/6] virtio-net: Fix and update interrupt moderation Heng Qi
2023-09-19 7:49 ` [PATCH net 1/6] virtio-net: initially change the value of tx-frames Heng Qi
@ 2023-09-19 7:49 ` Heng Qi
2023-09-22 3:11 ` Jason Wang
2023-09-19 7:49 ` [PATCH net 3/6] virtio-net: consistently save parameters for per-queue Heng Qi
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Heng Qi @ 2023-09-19 7:49 UTC (permalink / raw)
To: netdev, virtualization
Cc: Michael S . Tsirkin, Jason Wang, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo, Gavin Li
Since virtio-net allows switching napi_tx for per txq, we have to
get the specific txq's result now.
Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
Cc: Gavin Li <gavinl@nvidia.com>
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index fd5bc8d59eda..80d35a864790 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3453,7 +3453,7 @@ static int virtnet_get_per_queue_coalesce(struct net_device *dev,
} else {
ec->rx_max_coalesced_frames = 1;
- if (vi->sq[0].napi.weight)
+ if (vi->sq[queue].napi.weight)
ec->tx_max_coalesced_frames = 1;
}
--
2.19.1.6.gb485710b
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net 2/6] virtio-net: fix mismatch of getting tx-frames
2023-09-19 7:49 ` [PATCH net 2/6] virtio-net: fix mismatch of getting tx-frames Heng Qi
@ 2023-09-22 3:11 ` Jason Wang
0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2023-09-22 3:11 UTC (permalink / raw)
To: Heng Qi
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> Since virtio-net allows switching napi_tx for per txq, we have to
> get the specific txq's result now.
>
> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
> Cc: Gavin Li <gavinl@nvidia.com>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
> ---
> drivers/net/virtio_net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index fd5bc8d59eda..80d35a864790 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3453,7 +3453,7 @@ static int virtnet_get_per_queue_coalesce(struct net_device *dev,
> } else {
> ec->rx_max_coalesced_frames = 1;
>
> - if (vi->sq[0].napi.weight)
> + if (vi->sq[queue].napi.weight)
> ec->tx_max_coalesced_frames = 1;
> }
>
> --
> 2.19.1.6.gb485710b
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net 3/6] virtio-net: consistently save parameters for per-queue
2023-09-19 7:49 [PATCH net 0/6] virtio-net: Fix and update interrupt moderation Heng Qi
2023-09-19 7:49 ` [PATCH net 1/6] virtio-net: initially change the value of tx-frames Heng Qi
2023-09-19 7:49 ` [PATCH net 2/6] virtio-net: fix mismatch of getting tx-frames Heng Qi
@ 2023-09-19 7:49 ` Heng Qi
2023-09-22 4:26 ` Jason Wang
2023-09-19 7:49 ` [PATCH net 4/6] virtio-net: fix per queue coalescing parameter setting Heng Qi
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Heng Qi @ 2023-09-19 7:49 UTC (permalink / raw)
To: netdev, virtualization
Cc: Michael S . Tsirkin, Jason Wang, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo, Gavin Li
When using .set_coalesce interface to set all queue coalescing
parameters, we need to update both per-queue and global save values.
Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
Cc: Gavin Li <gavinl@nvidia.com>
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
drivers/net/virtio_net.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 80d35a864790..ce60162d380a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3233,6 +3233,7 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
struct ethtool_coalesce *ec)
{
struct scatterlist sgs_tx, sgs_rx;
+ int i;
vi->ctrl->coal_tx.tx_usecs = cpu_to_le32(ec->tx_coalesce_usecs);
vi->ctrl->coal_tx.tx_max_packets = cpu_to_le32(ec->tx_max_coalesced_frames);
@@ -3246,6 +3247,10 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
/* Save parameters */
vi->intr_coal_tx.max_usecs = ec->tx_coalesce_usecs;
vi->intr_coal_tx.max_packets = ec->tx_max_coalesced_frames;
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ vi->sq[i].intr_coal.max_usecs = ec->tx_coalesce_usecs;
+ vi->sq[i].intr_coal.max_packets = ec->tx_max_coalesced_frames;
+ }
vi->ctrl->coal_rx.rx_usecs = cpu_to_le32(ec->rx_coalesce_usecs);
vi->ctrl->coal_rx.rx_max_packets = cpu_to_le32(ec->rx_max_coalesced_frames);
@@ -3259,6 +3264,10 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
/* Save parameters */
vi->intr_coal_rx.max_usecs = ec->rx_coalesce_usecs;
vi->intr_coal_rx.max_packets = ec->rx_max_coalesced_frames;
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ vi->rq[i].intr_coal.max_usecs = ec->rx_coalesce_usecs;
+ vi->rq[i].intr_coal.max_packets = ec->rx_max_coalesced_frames;
+ }
return 0;
}
--
2.19.1.6.gb485710b
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net 3/6] virtio-net: consistently save parameters for per-queue
2023-09-19 7:49 ` [PATCH net 3/6] virtio-net: consistently save parameters for per-queue Heng Qi
@ 2023-09-22 4:26 ` Jason Wang
0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2023-09-22 4:26 UTC (permalink / raw)
To: Heng Qi
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> When using .set_coalesce interface to set all queue coalescing
> parameters, we need to update both per-queue and global save values.
>
> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
> Cc: Gavin Li <gavinl@nvidia.com>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
> ---
> drivers/net/virtio_net.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 80d35a864790..ce60162d380a 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3233,6 +3233,7 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
> struct ethtool_coalesce *ec)
> {
> struct scatterlist sgs_tx, sgs_rx;
> + int i;
>
> vi->ctrl->coal_tx.tx_usecs = cpu_to_le32(ec->tx_coalesce_usecs);
> vi->ctrl->coal_tx.tx_max_packets = cpu_to_le32(ec->tx_max_coalesced_frames);
> @@ -3246,6 +3247,10 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
> /* Save parameters */
> vi->intr_coal_tx.max_usecs = ec->tx_coalesce_usecs;
> vi->intr_coal_tx.max_packets = ec->tx_max_coalesced_frames;
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> + vi->sq[i].intr_coal.max_usecs = ec->tx_coalesce_usecs;
> + vi->sq[i].intr_coal.max_packets = ec->tx_max_coalesced_frames;
> + }
>
> vi->ctrl->coal_rx.rx_usecs = cpu_to_le32(ec->rx_coalesce_usecs);
> vi->ctrl->coal_rx.rx_max_packets = cpu_to_le32(ec->rx_max_coalesced_frames);
> @@ -3259,6 +3264,10 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
> /* Save parameters */
> vi->intr_coal_rx.max_usecs = ec->rx_coalesce_usecs;
> vi->intr_coal_rx.max_packets = ec->rx_max_coalesced_frames;
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> + vi->rq[i].intr_coal.max_usecs = ec->rx_coalesce_usecs;
> + vi->rq[i].intr_coal.max_packets = ec->rx_max_coalesced_frames;
> + }
>
> return 0;
> }
> --
> 2.19.1.6.gb485710b
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net 4/6] virtio-net: fix per queue coalescing parameter setting
2023-09-19 7:49 [PATCH net 0/6] virtio-net: Fix and update interrupt moderation Heng Qi
` (2 preceding siblings ...)
2023-09-19 7:49 ` [PATCH net 3/6] virtio-net: consistently save parameters for per-queue Heng Qi
@ 2023-09-19 7:49 ` Heng Qi
2023-09-22 4:27 ` Jason Wang
2023-09-19 7:49 ` [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize Heng Qi
2023-09-19 7:49 ` [PATCH net 6/6] virtio-net: a tiny comment update Heng Qi
5 siblings, 1 reply; 21+ messages in thread
From: Heng Qi @ 2023-09-19 7:49 UTC (permalink / raw)
To: netdev, virtualization
Cc: Michael S . Tsirkin, Jason Wang, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo, Gavin Li
When the user sets a non-zero coalescing parameter to 0 for a specific
virtqueue, it does not work as expected, so let's fix this.
Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
Reported-by: Xiaoming Zhao <zxm377917@alibaba-inc.com>
Cc: Gavin Li <gavinl@nvidia.com>
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
drivers/net/virtio_net.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ce60162d380a..f9a7f6afd099 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3296,27 +3296,23 @@ static int virtnet_send_notf_coal_vq_cmds(struct virtnet_info *vi,
{
int err;
- if (ec->rx_coalesce_usecs || ec->rx_max_coalesced_frames) {
- err = virtnet_send_ctrl_coal_vq_cmd(vi, rxq2vq(queue),
- ec->rx_coalesce_usecs,
- ec->rx_max_coalesced_frames);
- if (err)
- return err;
- /* Save parameters */
- vi->rq[queue].intr_coal.max_usecs = ec->rx_coalesce_usecs;
- vi->rq[queue].intr_coal.max_packets = ec->rx_max_coalesced_frames;
- }
+ err = virtnet_send_ctrl_coal_vq_cmd(vi, rxq2vq(queue),
+ ec->rx_coalesce_usecs,
+ ec->rx_max_coalesced_frames);
+ if (err)
+ return err;
+ /* Save parameters */
+ vi->rq[queue].intr_coal.max_usecs = ec->rx_coalesce_usecs;
+ vi->rq[queue].intr_coal.max_packets = ec->rx_max_coalesced_frames;
- if (ec->tx_coalesce_usecs || ec->tx_max_coalesced_frames) {
- err = virtnet_send_ctrl_coal_vq_cmd(vi, txq2vq(queue),
- ec->tx_coalesce_usecs,
- ec->tx_max_coalesced_frames);
- if (err)
- return err;
- /* Save parameters */
- vi->sq[queue].intr_coal.max_usecs = ec->tx_coalesce_usecs;
- vi->sq[queue].intr_coal.max_packets = ec->tx_max_coalesced_frames;
- }
+ err = virtnet_send_ctrl_coal_vq_cmd(vi, txq2vq(queue),
+ ec->tx_coalesce_usecs,
+ ec->tx_max_coalesced_frames);
+ if (err)
+ return err;
+ /* Save parameters */
+ vi->sq[queue].intr_coal.max_usecs = ec->tx_coalesce_usecs;
+ vi->sq[queue].intr_coal.max_packets = ec->tx_max_coalesced_frames;
return 0;
}
--
2.19.1.6.gb485710b
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net 4/6] virtio-net: fix per queue coalescing parameter setting
2023-09-19 7:49 ` [PATCH net 4/6] virtio-net: fix per queue coalescing parameter setting Heng Qi
@ 2023-09-22 4:27 ` Jason Wang
0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2023-09-22 4:27 UTC (permalink / raw)
To: Heng Qi
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> When the user sets a non-zero coalescing parameter to 0 for a specific
> virtqueue, it does not work as expected, so let's fix this.
>
> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
> Reported-by: Xiaoming Zhao <zxm377917@alibaba-inc.com>
> Cc: Gavin Li <gavinl@nvidia.com>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> ---
> drivers/net/virtio_net.c | 36 ++++++++++++++++--------------------
> 1 file changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index ce60162d380a..f9a7f6afd099 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3296,27 +3296,23 @@ static int virtnet_send_notf_coal_vq_cmds(struct virtnet_info *vi,
> {
> int err;
>
> - if (ec->rx_coalesce_usecs || ec->rx_max_coalesced_frames) {
> - err = virtnet_send_ctrl_coal_vq_cmd(vi, rxq2vq(queue),
> - ec->rx_coalesce_usecs,
> - ec->rx_max_coalesced_frames);
> - if (err)
> - return err;
> - /* Save parameters */
> - vi->rq[queue].intr_coal.max_usecs = ec->rx_coalesce_usecs;
> - vi->rq[queue].intr_coal.max_packets = ec->rx_max_coalesced_frames;
> - }
> + err = virtnet_send_ctrl_coal_vq_cmd(vi, rxq2vq(queue),
> + ec->rx_coalesce_usecs,
> + ec->rx_max_coalesced_frames);
> + if (err)
> + return err;
> + /* Save parameters */
I think code explains itself, so we can remove this.
Others look good.
Thanks
> + vi->rq[queue].intr_coal.max_usecs = ec->rx_coalesce_usecs;
> + vi->rq[queue].intr_coal.max_packets = ec->rx_max_coalesced_frames;
>
> - if (ec->tx_coalesce_usecs || ec->tx_max_coalesced_frames) {
> - err = virtnet_send_ctrl_coal_vq_cmd(vi, txq2vq(queue),
> - ec->tx_coalesce_usecs,
> - ec->tx_max_coalesced_frames);
> - if (err)
> - return err;
> - /* Save parameters */
> - vi->sq[queue].intr_coal.max_usecs = ec->tx_coalesce_usecs;
> - vi->sq[queue].intr_coal.max_packets = ec->tx_max_coalesced_frames;
> - }
> + err = virtnet_send_ctrl_coal_vq_cmd(vi, txq2vq(queue),
> + ec->tx_coalesce_usecs,
> + ec->tx_max_coalesced_frames);
> + if (err)
> + return err;
> + /* Save parameters */
> + vi->sq[queue].intr_coal.max_usecs = ec->tx_coalesce_usecs;
> + vi->sq[queue].intr_coal.max_packets = ec->tx_max_coalesced_frames;
>
> return 0;
> }
> --
> 2.19.1.6.gb485710b
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-19 7:49 [PATCH net 0/6] virtio-net: Fix and update interrupt moderation Heng Qi
` (3 preceding siblings ...)
2023-09-19 7:49 ` [PATCH net 4/6] virtio-net: fix per queue coalescing parameter setting Heng Qi
@ 2023-09-19 7:49 ` Heng Qi
2023-09-22 4:29 ` Jason Wang
2023-09-19 7:49 ` [PATCH net 6/6] virtio-net: a tiny comment update Heng Qi
5 siblings, 1 reply; 21+ messages in thread
From: Heng Qi @ 2023-09-19 7:49 UTC (permalink / raw)
To: netdev, virtualization
Cc: Michael S . Tsirkin, Jason Wang, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo, Gavin Li
According to the definition of virtqueue coalescing spec[1]:
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.
Upon disabling and re-enabling a receive virtqueue, the device MUST set
the coalescing parameters of the virtqueue to those configured through the
VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
any RX coalescing parameters, to 0.
We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
[1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
Cc: Gavin Li <gavinl@nvidia.com>
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
drivers/net/virtio_net.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f9a7f6afd099..9888218abab4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2855,6 +2855,9 @@ static void virtnet_get_ringparam(struct net_device *dev,
ring->tx_pending = virtqueue_get_vring_size(vi->sq[0].vq);
}
+static int virtnet_send_ctrl_coal_vq_cmd(struct virtnet_info *vi,
+ u16 vqn, u32 max_usecs, u32 max_packets);
+
static int virtnet_set_ringparam(struct net_device *dev,
struct ethtool_ringparam *ring,
struct kernel_ethtool_ringparam *kernel_ring,
@@ -2890,12 +2893,36 @@ static int virtnet_set_ringparam(struct net_device *dev,
err = virtnet_tx_resize(vi, sq, ring->tx_pending);
if (err)
return err;
+
+ /* 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.
+ */
+ err = virtnet_send_ctrl_coal_vq_cmd(vi, txq2vq(i),
+ vi->intr_coal_tx.max_usecs,
+ vi->intr_coal_tx.max_packets);
+ if (err)
+ return err;
+ /* Save parameters */
+ vi->sq[i].intr_coal.max_usecs = vi->intr_coal_tx.max_usecs;
+ vi->sq[i].intr_coal.max_packets = vi->intr_coal_tx.max_packets;
}
if (ring->rx_pending != rx_pending) {
err = virtnet_rx_resize(vi, rq, ring->rx_pending);
if (err)
return err;
+
+ /* The reason is same as the transmit virtqueue reset */
+ err = virtnet_send_ctrl_coal_vq_cmd(vi, rxq2vq(i),
+ vi->intr_coal_rx.max_usecs,
+ vi->intr_coal_rx.max_packets);
+ if (err)
+ return err;
+ /* Save parameters */
+ vi->rq[i].intr_coal.max_usecs = vi->intr_coal_rx.max_usecs;
+ vi->rq[i].intr_coal.max_packets = vi->intr_coal_rx.max_packets;
}
}
--
2.19.1.6.gb485710b
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-19 7:49 ` [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize Heng Qi
@ 2023-09-22 4:29 ` Jason Wang
2023-09-22 5:02 ` Heng Qi
0 siblings, 1 reply; 21+ messages in thread
From: Jason Wang @ 2023-09-22 4:29 UTC (permalink / raw)
To: Heng Qi
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> According to the definition of virtqueue coalescing spec[1]:
>
> 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.
>
> Upon disabling and re-enabling a receive virtqueue, the device MUST set
> the coalescing parameters of the virtqueue to those configured through the
> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
> any RX coalescing parameters, to 0.
>
> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
>
> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
>
> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
I'm not sure this is a real fix as spec allows it to go zero?
Thanks
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-22 4:29 ` Jason Wang
@ 2023-09-22 5:02 ` Heng Qi
2023-09-22 7:32 ` Jason Wang
0 siblings, 1 reply; 21+ messages in thread
From: Heng Qi @ 2023-09-22 5:02 UTC (permalink / raw)
To: Jason Wang
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
在 2023/9/22 下午12:29, Jason Wang 写道:
> On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>> According to the definition of virtqueue coalescing spec[1]:
>>
>> 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.
>>
>> Upon disabling and re-enabling a receive virtqueue, the device MUST set
>> the coalescing parameters of the virtqueue to those configured through the
>> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
>> any RX coalescing parameters, to 0.
>>
>> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
>>
>> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
>>
>> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
> I'm not sure this is a real fix as spec allows it to go zero?
The spec says that if the user has configured interrupt coalescing
parameters,
parameters need to be restored after vq_reset, otherwise set to 0.
vi->intr_coal_tx and vi->intr_coal_rx always save the newest global
parameters,
regardless of whether the command is sent or not. So I think we need
this patch
it complies with the specification requirements.
Thanks!
>
> Thanks
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-22 5:02 ` Heng Qi
@ 2023-09-22 7:32 ` Jason Wang
2023-09-22 7:58 ` Heng Qi
2023-09-22 9:50 ` Xuan Zhuo
0 siblings, 2 replies; 21+ messages in thread
From: Jason Wang @ 2023-09-22 7:32 UTC (permalink / raw)
To: Heng Qi
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
On Fri, Sep 22, 2023 at 1:02 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>
>
> 在 2023/9/22 下午12:29, Jason Wang 写道:
> > On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >> According to the definition of virtqueue coalescing spec[1]:
> >>
> >> 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.
> >>
> >> Upon disabling and re-enabling a receive virtqueue, the device MUST set
> >> the coalescing parameters of the virtqueue to those configured through the
> >> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
> >> any RX coalescing parameters, to 0.
> >>
> >> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
> >>
> >> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
> >>
> >> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
> > I'm not sure this is a real fix as spec allows it to go zero?
>
> The spec says that if the user has configured interrupt coalescing
> parameters,
> parameters need to be restored after vq_reset, otherwise set to 0.
> vi->intr_coal_tx and vi->intr_coal_rx always save the newest global
> parameters,
> regardless of whether the command is sent or not. So I think we need
> this patch
> it complies with the specification requirements.
How can we make sure the old coalescing parameters still make sense
for the new ring size?
Thanks
>
> Thanks!
>
> >
> > Thanks
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-22 7:32 ` Jason Wang
@ 2023-09-22 7:58 ` Heng Qi
2023-09-25 2:29 ` Jason Wang
2023-09-22 9:50 ` Xuan Zhuo
1 sibling, 1 reply; 21+ messages in thread
From: Heng Qi @ 2023-09-22 7:58 UTC (permalink / raw)
To: Jason Wang
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
在 2023/9/22 下午3:32, Jason Wang 写道:
> On Fri, Sep 22, 2023 at 1:02 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>>
>>
>> 在 2023/9/22 下午12:29, Jason Wang 写道:
>>> On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>>>> According to the definition of virtqueue coalescing spec[1]:
>>>>
>>>> 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.
>>>>
>>>> Upon disabling and re-enabling a receive virtqueue, the device MUST set
>>>> the coalescing parameters of the virtqueue to those configured through the
>>>> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
>>>> any RX coalescing parameters, to 0.
>>>>
>>>> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
>>>>
>>>> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
>>>>
>>>> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
>>> I'm not sure this is a real fix as spec allows it to go zero?
>> The spec says that if the user has configured interrupt coalescing
>> parameters,
>> parameters need to be restored after vq_reset, otherwise set to 0.
>> vi->intr_coal_tx and vi->intr_coal_rx always save the newest global
>> parameters,
>> regardless of whether the command is sent or not. So I think we need
>> this patch
>> it complies with the specification requirements.
> How can we make sure the old coalescing parameters still make sense
> for the new ring size?
I'm not sure, ringsize has a wider range of changes. Maybe we should
only keep coalescing
parameters in cases where only vq_reset occurs (no ring size change
involved)?
Thanks!
>
> Thanks
>
>> Thanks!
>>
>>> Thanks
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-22 7:58 ` Heng Qi
@ 2023-09-25 2:29 ` Jason Wang
2023-09-25 2:47 ` Heng Qi
0 siblings, 1 reply; 21+ messages in thread
From: Jason Wang @ 2023-09-25 2:29 UTC (permalink / raw)
To: Heng Qi
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
On Fri, Sep 22, 2023 at 3:58 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>
>
> 在 2023/9/22 下午3:32, Jason Wang 写道:
> > On Fri, Sep 22, 2023 at 1:02 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >>
> >>
> >> 在 2023/9/22 下午12:29, Jason Wang 写道:
> >>> On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >>>> According to the definition of virtqueue coalescing spec[1]:
> >>>>
> >>>> 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.
> >>>>
> >>>> Upon disabling and re-enabling a receive virtqueue, the device MUST set
> >>>> the coalescing parameters of the virtqueue to those configured through the
> >>>> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
> >>>> any RX coalescing parameters, to 0.
> >>>>
> >>>> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
> >>>>
> >>>> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
> >>>>
> >>>> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
> >>> I'm not sure this is a real fix as spec allows it to go zero?
> >> The spec says that if the user has configured interrupt coalescing
> >> parameters,
> >> parameters need to be restored after vq_reset, otherwise set to 0.
> >> vi->intr_coal_tx and vi->intr_coal_rx always save the newest global
> >> parameters,
> >> regardless of whether the command is sent or not. So I think we need
> >> this patch
> >> it complies with the specification requirements.
> > How can we make sure the old coalescing parameters still make sense
> > for the new ring size?
>
> I'm not sure, ringsize has a wider range of changes. Maybe we should
> only keep coalescing
> parameters in cases where only vq_reset occurs (no ring size change
> involved)?
Probably but do we actually have a user other than resize now?
Thanks
>
> Thanks!
>
> >
> > Thanks
> >
> >> Thanks!
> >>
> >>> Thanks
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-25 2:29 ` Jason Wang
@ 2023-09-25 2:47 ` Heng Qi
0 siblings, 0 replies; 21+ messages in thread
From: Heng Qi @ 2023-09-25 2:47 UTC (permalink / raw)
To: Jason Wang
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo, Gavin Li
在 2023/9/25 上午10:29, Jason Wang 写道:
> On Fri, Sep 22, 2023 at 3:58 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>>
>>
>> 在 2023/9/22 下午3:32, Jason Wang 写道:
>>> On Fri, Sep 22, 2023 at 1:02 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>>>>
>>>> 在 2023/9/22 下午12:29, Jason Wang 写道:
>>>>> On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>>>>>> According to the definition of virtqueue coalescing spec[1]:
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>> Upon disabling and re-enabling a receive virtqueue, the device MUST set
>>>>>> the coalescing parameters of the virtqueue to those configured through the
>>>>>> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
>>>>>> any RX coalescing parameters, to 0.
>>>>>>
>>>>>> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
>>>>>>
>>>>>> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
>>>>>>
>>>>>> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
>>>>> I'm not sure this is a real fix as spec allows it to go zero?
>>>> The spec says that if the user has configured interrupt coalescing
>>>> parameters,
>>>> parameters need to be restored after vq_reset, otherwise set to 0.
>>>> vi->intr_coal_tx and vi->intr_coal_rx always save the newest global
>>>> parameters,
>>>> regardless of whether the command is sent or not. So I think we need
>>>> this patch
>>>> it complies with the specification requirements.
>>> How can we make sure the old coalescing parameters still make sense
>>> for the new ring size?
>> I'm not sure, ringsize has a wider range of changes. Maybe we should
>> only keep coalescing
>> parameters in cases where only vq_reset occurs (no ring size change
>> involved)?
> Probably but do we actually have a user other than resize now?
Not yet. What I mean is if there is one in the future, for example in
xdp's processing etc.,
we can reserve parameters for it.
Thanks
>
> Thanks
>
>> Thanks!
>>
>>> Thanks
>>>
>>>> Thanks!
>>>>
>>>>> Thanks
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-22 7:32 ` Jason Wang
2023-09-22 7:58 ` Heng Qi
@ 2023-09-22 9:50 ` Xuan Zhuo
2023-09-25 2:29 ` Jason Wang
1 sibling, 1 reply; 21+ messages in thread
From: Xuan Zhuo @ 2023-09-22 9:50 UTC (permalink / raw)
To: Jason Wang
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Gavin Li,
Heng Qi
On Fri, 22 Sep 2023 15:32:39 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Fri, Sep 22, 2023 at 1:02 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >
> >
> >
> > 在 2023/9/22 下午12:29, Jason Wang 写道:
> > > On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> > >> According to the definition of virtqueue coalescing spec[1]:
> > >>
> > >> 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.
> > >>
> > >> Upon disabling and re-enabling a receive virtqueue, the device MUST set
> > >> the coalescing parameters of the virtqueue to those configured through the
> > >> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
> > >> any RX coalescing parameters, to 0.
> > >>
> > >> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
> > >>
> > >> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
> > >>
> > >> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
> > > I'm not sure this is a real fix as spec allows it to go zero?
> >
> > The spec says that if the user has configured interrupt coalescing
> > parameters,
> > parameters need to be restored after vq_reset, otherwise set to 0.
> > vi->intr_coal_tx and vi->intr_coal_rx always save the newest global
> > parameters,
> > regardless of whether the command is sent or not. So I think we need
> > this patch
> > it complies with the specification requirements.
>
> How can we make sure the old coalescing parameters still make sense
> for the new ring size?
For the user, I don't think we should drop the config for the coalescing.
Maybe the config does not make sense for the new ring size, but when the user
just change the ring size, the config for the coalesing is missing, I think
that is not good.
Thanks.
>
> Thanks
>
> >
> > Thanks!
> >
> > >
> > > Thanks
> >
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-22 9:50 ` Xuan Zhuo
@ 2023-09-25 2:29 ` Jason Wang
2023-09-25 2:45 ` Heng Qi
0 siblings, 1 reply; 21+ messages in thread
From: Jason Wang @ 2023-09-25 2:29 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Gavin Li,
Heng Qi
On Fri, Sep 22, 2023 at 5:56 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Fri, 22 Sep 2023 15:32:39 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Fri, Sep 22, 2023 at 1:02 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> > >
> > >
> > >
> > > 在 2023/9/22 下午12:29, Jason Wang 写道:
> > > > On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> > > >> According to the definition of virtqueue coalescing spec[1]:
> > > >>
> > > >> 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.
> > > >>
> > > >> Upon disabling and re-enabling a receive virtqueue, the device MUST set
> > > >> the coalescing parameters of the virtqueue to those configured through the
> > > >> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
> > > >> any RX coalescing parameters, to 0.
> > > >>
> > > >> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
> > > >>
> > > >> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
> > > >>
> > > >> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
> > > > I'm not sure this is a real fix as spec allows it to go zero?
> > >
> > > The spec says that if the user has configured interrupt coalescing
> > > parameters,
> > > parameters need to be restored after vq_reset, otherwise set to 0.
> > > vi->intr_coal_tx and vi->intr_coal_rx always save the newest global
> > > parameters,
> > > regardless of whether the command is sent or not. So I think we need
> > > this patch
> > > it complies with the specification requirements.
> >
> > How can we make sure the old coalescing parameters still make sense
> > for the new ring size?
>
> For the user, I don't think we should drop the config for the coalescing.
> Maybe the config does not make sense for the new ring size, but when the user
> just change the ring size, the config for the coalesing is missing, I think
> that is not good.
How did other drivers deal with this?
Thanks
>
> Thanks.
>
>
>
>
> >
> > Thanks
> >
> > >
> > > Thanks!
> > >
> > > >
> > > > Thanks
> > >
> >
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize
2023-09-25 2:29 ` Jason Wang
@ 2023-09-25 2:45 ` Heng Qi
0 siblings, 0 replies; 21+ messages in thread
From: Heng Qi @ 2023-09-25 2:45 UTC (permalink / raw)
To: Jason Wang, Xuan Zhuo
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Gavin Li
在 2023/9/25 上午10:29, Jason Wang 写道:
> On Fri, Sep 22, 2023 at 5:56 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>> On Fri, 22 Sep 2023 15:32:39 +0800, Jason Wang <jasowang@redhat.com> wrote:
>>> On Fri, Sep 22, 2023 at 1:02 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>>>>
>>>>
>>>> 在 2023/9/22 下午12:29, Jason Wang 写道:
>>>>> On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>>>>>> According to the definition of virtqueue coalescing spec[1]:
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>> Upon disabling and re-enabling a receive virtqueue, the device MUST set
>>>>>> the coalescing parameters of the virtqueue to those configured through the
>>>>>> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set
>>>>>> any RX coalescing parameters, to 0.
>>>>>>
>>>>>> We need to add this setting for vq resize (ethtool -G) where vq_reset happens.
>>>>>>
>>>>>> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
>>>>>>
>>>>>> Fixes: 394bd87764b6 ("virtio_net: support per queue interrupt coalesce command")
>>>>> I'm not sure this is a real fix as spec allows it to go zero?
>>>> The spec says that if the user has configured interrupt coalescing
>>>> parameters,
>>>> parameters need to be restored after vq_reset, otherwise set to 0.
>>>> vi->intr_coal_tx and vi->intr_coal_rx always save the newest global
>>>> parameters,
>>>> regardless of whether the command is sent or not. So I think we need
>>>> this patch
>>>> it complies with the specification requirements.
>>> How can we make sure the old coalescing parameters still make sense
>>> for the new ring size?
>> For the user, I don't think we should drop the config for the coalescing.
>> Maybe the config does not make sense for the new ring size, but when the user
>> just change the ring size, the config for the coalesing is missing, I think
>> that is not good.
> How did other drivers deal with this?
Test on mlx5, the behavior is the same as what this patch does:
#1 ethtool -g eth0
...
Current hardware settings:
RX: 8192
RX Mini: n/a
RX Jumbo: n/a
Tx: 1024
...
#2 ethtool -c eth0
Coalesce parameters for eth0:
Adaptive RX: off TX: off
...
rx-usecs: 11
rx-frames: 32
rx-usecs-irq: n/a
rx-frames-irq: n/a
...
#3 sudo ethtool -G eth0 rx 4096
#4 ethtool -g eth0
Ring parameters for eth0:
...
Current hardware settings:
RX: 4096
RX Mini: n/a
RX Jumbo: n/a
TX: 1024
...
#4 ethtool -c eth0
Coalesce parameters for eth0:
Adaptive RX: off TX: off
...
rx-usecs: 11
rx-frames: 32
rx-usecs-irq: n/a
rx-frames-irq: n/a
...
And I check the code, except for the parameters related to ring size,
which are changed,
other parameters in structure mlx5e_params are retained.
Thanks
>
> Thanks
>
>> Thanks.
>>
>>
>>
>>
>>> Thanks
>>>
>>>> Thanks!
>>>>
>>>>> Thanks
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net 6/6] virtio-net: a tiny comment update
2023-09-19 7:49 [PATCH net 0/6] virtio-net: Fix and update interrupt moderation Heng Qi
` (4 preceding siblings ...)
2023-09-19 7:49 ` [PATCH net 5/6] virtio-net: fix the vq coalescing setting for vq resize Heng Qi
@ 2023-09-19 7:49 ` Heng Qi
2023-09-22 4:30 ` Jason Wang
5 siblings, 1 reply; 21+ messages in thread
From: Heng Qi @ 2023-09-19 7:49 UTC (permalink / raw)
To: netdev, virtualization
Cc: Michael S . Tsirkin, Jason Wang, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo
Update a comment because virtio-net now supports both
VIRTIO_NET_F_NOTF_COAL and VIRTIO_NET_F_VQ_NOTF_COAL.
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9888218abab4..dd498fa468c0 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3347,7 +3347,7 @@ static int virtnet_send_notf_coal_vq_cmds(struct virtnet_info *vi,
static int virtnet_coal_params_supported(struct ethtool_coalesce *ec)
{
/* usecs coalescing is supported only if VIRTIO_NET_F_NOTF_COAL
- * feature is negotiated.
+ * or VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated.
*/
if (ec->rx_coalesce_usecs || ec->tx_coalesce_usecs)
return -EOPNOTSUPP;
--
2.19.1.6.gb485710b
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net 6/6] virtio-net: a tiny comment update
2023-09-19 7:49 ` [PATCH net 6/6] virtio-net: a tiny comment update Heng Qi
@ 2023-09-22 4:30 ` Jason Wang
0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2023-09-22 4:30 UTC (permalink / raw)
To: Heng Qi
Cc: netdev, virtualization, Michael S . Tsirkin, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Xuan Zhuo
On Tue, Sep 19, 2023 at 3:49 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> Update a comment because virtio-net now supports both
> VIRTIO_NET_F_NOTF_COAL and VIRTIO_NET_F_VQ_NOTF_COAL.
>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
> ---
> drivers/net/virtio_net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 9888218abab4..dd498fa468c0 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3347,7 +3347,7 @@ static int virtnet_send_notf_coal_vq_cmds(struct virtnet_info *vi,
> static int virtnet_coal_params_supported(struct ethtool_coalesce *ec)
> {
> /* usecs coalescing is supported only if VIRTIO_NET_F_NOTF_COAL
> - * feature is negotiated.
> + * or VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated.
> */
> if (ec->rx_coalesce_usecs || ec->tx_coalesce_usecs)
> return -EOPNOTSUPP;
> --
> 2.19.1.6.gb485710b
>
^ permalink raw reply [flat|nested] 21+ messages in thread