* [PATCH net-next v2 0/2] virtio_net: support getting initial value of irq coalesce @ 2024-04-26 6:54 Heng Qi 2024-04-26 6:54 ` [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device Heng Qi 2024-04-26 6:54 ` [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe Heng Qi 0 siblings, 2 replies; 11+ messages in thread From: Heng Qi @ 2024-04-26 6:54 UTC (permalink / raw) To: netdev, virtualization Cc: Michael S . Tsirkin, Jason Wang, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device 2024-04-26 6:54 [PATCH net-next v2 0/2] virtio_net: support getting initial value of irq coalesce Heng Qi @ 2024-04-26 6:54 ` Heng Qi 2024-05-07 6:24 ` Jason Wang 2024-04-26 6:54 ` [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe Heng Qi 1 sibling, 1 reply; 11+ messages in thread From: Heng Qi @ 2024-04-26 6:54 UTC (permalink / raw) To: netdev, virtualization Cc: Michael S . Tsirkin, Jason Wang, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device 2024-04-26 6:54 ` [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device Heng Qi @ 2024-05-07 6:24 ` Jason Wang 2024-05-07 6:53 ` Heng Qi 0 siblings, 1 reply; 11+ messages in thread From: Jason Wang @ 2024-05-07 6:24 UTC (permalink / raw) To: Heng Qi Cc: netdev, virtualization, Michael S . Tsirkin, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device 2024-05-07 6:24 ` Jason Wang @ 2024-05-07 6:53 ` Heng Qi 2024-05-08 2:20 ` Jason Wang 0 siblings, 1 reply; 11+ messages in thread From: Heng Qi @ 2024-05-07 6:53 UTC (permalink / raw) To: Jason Wang Cc: netdev, virtualization, Michael S . Tsirkin, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device 2024-05-07 6:53 ` Heng Qi @ 2024-05-08 2:20 ` Jason Wang 2024-05-08 2:44 ` Heng Qi 0 siblings, 1 reply; 11+ messages in thread From: Jason Wang @ 2024-05-08 2:20 UTC (permalink / raw) To: Heng Qi Cc: netdev, virtualization, Michael S . Tsirkin, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device 2024-05-08 2:20 ` Jason Wang @ 2024-05-08 2:44 ` Heng Qi 0 siblings, 0 replies; 11+ messages in thread From: Heng Qi @ 2024-05-08 2:44 UTC (permalink / raw) To: Jason Wang Cc: netdev, virtualization, Michael S . Tsirkin, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe 2024-04-26 6:54 [PATCH net-next v2 0/2] virtio_net: support getting initial value of irq coalesce Heng Qi 2024-04-26 6:54 ` [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device Heng Qi @ 2024-04-26 6:54 ` Heng Qi 2024-05-07 6:24 ` Jason Wang 1 sibling, 1 reply; 11+ messages in thread From: Heng Qi @ 2024-04-26 6:54 UTC (permalink / raw) To: netdev, virtualization Cc: Michael S . Tsirkin, Jason Wang, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe 2024-04-26 6:54 ` [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe Heng Qi @ 2024-05-07 6:24 ` Jason Wang 2024-05-07 6:56 ` Heng Qi 0 siblings, 1 reply; 11+ messages in thread From: Jason Wang @ 2024-05-07 6:24 UTC (permalink / raw) To: Heng Qi Cc: netdev, virtualization, Michael S . Tsirkin, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe 2024-05-07 6:24 ` Jason Wang @ 2024-05-07 6:56 ` Heng Qi 2024-05-08 2:22 ` Jason Wang 0 siblings, 1 reply; 11+ messages in thread From: Heng Qi @ 2024-05-07 6:56 UTC (permalink / raw) To: Jason Wang Cc: netdev, virtualization, Michael S . Tsirkin, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe 2024-05-07 6:56 ` Heng Qi @ 2024-05-08 2:22 ` Jason Wang 2024-05-08 2:34 ` Heng Qi 0 siblings, 1 reply; 11+ messages in thread From: Jason Wang @ 2024-05-08 2:22 UTC (permalink / raw) To: Heng Qi Cc: netdev, virtualization, Michael S . Tsirkin, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe 2024-05-08 2:22 ` Jason Wang @ 2024-05-08 2:34 ` Heng Qi 0 siblings, 0 replies; 11+ messages in thread From: Heng Qi @ 2024-05-08 2:34 UTC (permalink / raw) To: Jason Wang Cc: netdev, virtualization, Michael S . Tsirkin, Xuan Zhuo, Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni 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 > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-05-08 2:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-26 6:54 [PATCH net-next v2 0/2] virtio_net: support getting initial value of irq coalesce Heng Qi 2024-04-26 6:54 ` [PATCH net-next v2 1/2] virtio_net: introduce ability to get reply info from device Heng Qi 2024-05-07 6:24 ` Jason Wang 2024-05-07 6:53 ` Heng Qi 2024-05-08 2:20 ` Jason Wang 2024-05-08 2:44 ` Heng Qi 2024-04-26 6:54 ` [PATCH net-next v2 2/2] virtio_net: get init coalesce value when probe Heng Qi 2024-05-07 6:24 ` Jason Wang 2024-05-07 6:56 ` Heng Qi 2024-05-08 2:22 ` Jason Wang 2024-05-08 2:34 ` Heng Qi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).