Re: [PATCH net-next V1 0/4] virtio_net: add per queue interrupt coalescing support

Re: [PATCH net-next V1 0/4] virtio_net: add per queue interrupt coalescing support

[Xuan Zhuo]

On Mon, 10 Jul 2023 12:20:01 +0300, Gavin Li <gavinl@nvidia.com> wrote:

As far as I know, Heng Qi does that. I'm not sure, it's the same piece.

cc @Heng Qi

Thanks.


> Currently, coalescing parameters are grouped for all transmit and receive
> virtqueues. This patch series add support to set or get the parameters for
> a specified virtqueue.
>
> When the traffic between virtqueues is unbalanced, for example, one virtqueue
> is busy and another virtqueue is idle, then it will be very useful to
> control coalescing parameters at the virtqueue granularity.
>
> Example command:
> $ ethtool -Q eth5 queue_mask 0x1 --coalesce tx-packets 10
> Would set max_packets=10 to VQ 1.
> $ ethtool -Q eth5 queue_mask 0x1 --coalesce rx-packets 10
> Would set max_packets=10 to VQ 0.
> $ ethtool -Q eth5 queue_mask 0x1 --show-coalesce
>  Queue: 0
>  Adaptive RX: off  TX: off
>  stats-block-usecs: 0
>  sample-interval: 0
>  pkt-rate-low: 0
>  pkt-rate-high: 0
>
>  rx-usecs: 222
>  rx-frames: 0
>  rx-usecs-irq: 0
>  rx-frames-irq: 256
>
>  tx-usecs: 222
>  tx-frames: 0
>  tx-usecs-irq: 0
>  tx-frames-irq: 256
>
>  rx-usecs-low: 0
>  rx-frame-low: 0
>  tx-usecs-low: 0
>  tx-frame-low: 0
>
>  rx-usecs-high: 0
>  rx-frame-high: 0
>  tx-usecs-high: 0
>  tx-frame-high: 0
>
> In this patch series:
> Patch-1: Extract interrupt coalescing settings to a structure.
> Patch-2: Extract get/set interrupt coalesce to a function.
> Patch-3: Support per queue interrupt coalesce command.
> Patch-4: Enable per queue interrupt coalesce feature.
>
> Gavin Li (4):
>   virtio_net: extract interrupt coalescing settings to a structure
>   virtio_net: extract get/set interrupt coalesce to a function
>   virtio_net: support per queue interrupt coalesce command
>   virtio_net: enable per queue interrupt coalesce feature
>
>  drivers/net/virtio_net.c        | 169 ++++++++++++++++++++++++++------
>  include/uapi/linux/virtio_net.h |  14 +++
>  2 files changed, 154 insertions(+), 29 deletions(-)
>
> --
> 2.39.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next V1 0/4] virtio_net: add per queue interrupt coalescing support

[Xuan Zhuo]

On Tue, 11 Jul 2023 10:34:33 +0800, Gavin Li <gavinl@nvidia.com> wrote:
>
> On 7/10/2023 5:25 PM, Xuan Zhuo wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Mon, 10 Jul 2023 12:20:01 +0300, Gavin Li <gavinl@nvidia.com> wrote:
> >
> > As far as I know, Heng Qi does that. I'm not sure, it's the same piece.
> >
> > cc @Heng Qi
> >
> > Thanks.
> Do you mean the one below?

YES


>
> https://lists.oasis-open.org/archives/virtio-dev/202303/msg00415.html
>
> The code is to implement what it described and I don't have better
> words. So, I copied the text from Heng Qi's.

Maybe he wrote code for it. I think you should ask him about his plans first.

Thanks.


>
> >
> >
> >> Currently, coalescing parameters are grouped for all transmit and receive
> >> virtqueues. This patch series add support to set or get the parameters for
> >> a specified virtqueue.
> >>
> >> When the traffic between virtqueues is unbalanced, for example, one virtqueue
> >> is busy and another virtqueue is idle, then it will be very useful to
> >> control coalescing parameters at the virtqueue granularity.
> >>
> >> Example command:
> >> $ ethtool -Q eth5 queue_mask 0x1 --coalesce tx-packets 10
> >> Would set max_packets=10 to VQ 1.
> >> $ ethtool -Q eth5 queue_mask 0x1 --coalesce rx-packets 10
> >> Would set max_packets=10 to VQ 0.
> >> $ ethtool -Q eth5 queue_mask 0x1 --show-coalesce
> >>   Queue: 0
> >>   Adaptive RX: off  TX: off
> >>   stats-block-usecs: 0
> >>   sample-interval: 0
> >>   pkt-rate-low: 0
> >>   pkt-rate-high: 0
> >>
> >>   rx-usecs: 222
> >>   rx-frames: 0
> >>   rx-usecs-irq: 0
> >>   rx-frames-irq: 256
> >>
> >>   tx-usecs: 222
> >>   tx-frames: 0
> >>   tx-usecs-irq: 0
> >>   tx-frames-irq: 256
> >>
> >>   rx-usecs-low: 0
> >>   rx-frame-low: 0
> >>   tx-usecs-low: 0
> >>   tx-frame-low: 0
> >>
> >>   rx-usecs-high: 0
> >>   rx-frame-high: 0
> >>   tx-usecs-high: 0
> >>   tx-frame-high: 0
> >>
> >> In this patch series:
> >> Patch-1: Extract interrupt coalescing settings to a structure.
> >> Patch-2: Extract get/set interrupt coalesce to a function.
> >> Patch-3: Support per queue interrupt coalesce command.
> >> Patch-4: Enable per queue interrupt coalesce feature.
> >>
> >> Gavin Li (4):
> >>    virtio_net: extract interrupt coalescing settings to a structure
> >>    virtio_net: extract get/set interrupt coalesce to a function
> >>    virtio_net: support per queue interrupt coalesce command
> >>    virtio_net: enable per queue interrupt coalesce feature
> >>
> >>   drivers/net/virtio_net.c        | 169 ++++++++++++++++++++++++++------
> >>   include/uapi/linux/virtio_net.h |  14 +++
> >>   2 files changed, 154 insertions(+), 29 deletions(-)
> >>
> >> --
> >> 2.39.1
> >>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next V1 1/4] virtio_net: extract interrupt coalescing settings to a structure

[Jason Wang]

[-- Attachment #1.1: Type: text/plain, Size: 4127 bytes --]

On Mon, Jul 10, 2023 at 5:21 PM Gavin Li <gavinl@nvidia.com> wrote:

> Extract interrupt coalescing settings to a structure so that it could be
> reused in other data structures.
>
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  drivers/net/virtio_net.c | 35 +++++++++++++++++++----------------
>  1 file changed, 19 insertions(+), 16 deletions(-)
>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0db14f6b87d3..dd5fec073a27 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -126,6 +126,11 @@ static const struct virtnet_stat_desc
> virtnet_rq_stats_desc[] = {
>  #define VIRTNET_SQ_STATS_LEN   ARRAY_SIZE(virtnet_sq_stats_desc)
>  #define VIRTNET_RQ_STATS_LEN   ARRAY_SIZE(virtnet_rq_stats_desc)
>
> +struct virtnet_interrupt_coalesce {
> +       u32 max_packets;
> +       u32 max_usecs;
> +};
> +
>  /* Internal representation of a send virtqueue */
>  struct send_queue {
>         /* Virtqueue associated with this send _queue */
> @@ -281,10 +286,8 @@ struct virtnet_info {
>         u32 speed;
>
>         /* Interrupt coalescing settings */
> -       u32 tx_usecs;
> -       u32 rx_usecs;
> -       u32 tx_max_packets;
> -       u32 rx_max_packets;
> +       struct virtnet_interrupt_coalesce intr_coal_tx;
> +       struct virtnet_interrupt_coalesce intr_coal_rx;
>
>         unsigned long guest_offloads;
>         unsigned long guest_offloads_capable;
> @@ -3056,8 +3059,8 @@ static int virtnet_send_notf_coal_cmds(struct
> virtnet_info *vi,
>                 return -EINVAL;
>
>         /* Save parameters */
> -       vi->tx_usecs = ec->tx_coalesce_usecs;
> -       vi->tx_max_packets = ec->tx_max_coalesced_frames;
> +       vi->intr_coal_tx.max_usecs = ec->tx_coalesce_usecs;
> +       vi->intr_coal_tx.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);
> @@ -3069,8 +3072,8 @@ static int virtnet_send_notf_coal_cmds(struct
> virtnet_info *vi,
>                 return -EINVAL;
>
>         /* Save parameters */
> -       vi->rx_usecs = ec->rx_coalesce_usecs;
> -       vi->rx_max_packets = ec->rx_max_coalesced_frames;
> +       vi->intr_coal_rx.max_usecs = ec->rx_coalesce_usecs;
> +       vi->intr_coal_rx.max_packets = ec->rx_max_coalesced_frames;
>
>         return 0;
>  }
> @@ -3132,10 +3135,10 @@ static int virtnet_get_coalesce(struct net_device
> *dev,
>         struct virtnet_info *vi = netdev_priv(dev);
>
>         if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
> -               ec->rx_coalesce_usecs = vi->rx_usecs;
> -               ec->tx_coalesce_usecs = vi->tx_usecs;
> -               ec->tx_max_coalesced_frames = vi->tx_max_packets;
> -               ec->rx_max_coalesced_frames = vi->rx_max_packets;
> +               ec->rx_coalesce_usecs = vi->intr_coal_rx.max_usecs;
> +               ec->tx_coalesce_usecs = vi->intr_coal_tx.max_usecs;
> +               ec->tx_max_coalesced_frames = vi->intr_coal_tx.max_packets;
> +               ec->rx_max_coalesced_frames = vi->intr_coal_rx.max_packets;
>         } else {
>                 ec->rx_max_coalesced_frames = 1;
>
> @@ -4119,10 +4122,10 @@ static int virtnet_probe(struct virtio_device
> *vdev)
>         }
>
>         if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
> -               vi->rx_usecs = 0;
> -               vi->tx_usecs = 0;
> -               vi->tx_max_packets = 0;
> -               vi->rx_max_packets = 0;
> +               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))
> --
> 2.39.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 5467 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next V1 2/4] virtio_net: extract get/set interrupt coalesce to a function

[Jason Wang]

[-- Attachment #1.1: Type: text/plain, Size: 2637 bytes --]

On Mon, Jul 10, 2023 at 5:21 PM Gavin Li <gavinl@nvidia.com> wrote:

> Extract get/set interrupt coalesce to a function to be reused by global
> and per queue config.
>
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  drivers/net/virtio_net.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index dd5fec073a27..802ed21453f5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3093,10 +3093,8 @@ static int virtnet_coal_params_supported(struct
> ethtool_coalesce *ec)
>         return 0;
>  }
>
> -static int virtnet_set_coalesce(struct net_device *dev,
> -                               struct ethtool_coalesce *ec,
> -                               struct kernel_ethtool_coalesce
> *kernel_coal,
> -                               struct netlink_ext_ack *extack)
> +static int virtnet_set_coalesce_one(struct net_device *dev,
> +                                   struct ethtool_coalesce *ec)
>  {
>         struct virtnet_info *vi = netdev_priv(dev);
>         int ret, i, napi_weight;
> @@ -3127,10 +3125,16 @@ static int virtnet_set_coalesce(struct net_device
> *dev,
>         return ret;
>  }
>
> -static int virtnet_get_coalesce(struct net_device *dev,
> +static int virtnet_set_coalesce(struct net_device *dev,
>                                 struct ethtool_coalesce *ec,
>                                 struct kernel_ethtool_coalesce
> *kernel_coal,
>                                 struct netlink_ext_ack *extack)
> +{
> +       return virtnet_set_coalesce_one(dev, ec);
> +}
> +
> +static int virtnet_get_coalesce_one(struct net_device *dev,
> +                                   struct ethtool_coalesce *ec)
>  {
>         struct virtnet_info *vi = netdev_priv(dev);
>
> @@ -3149,6 +3153,14 @@ static int virtnet_get_coalesce(struct net_device
> *dev,
>         return 0;
>  }
>
> +static int virtnet_get_coalesce(struct net_device *dev,
> +                               struct ethtool_coalesce *ec,
> +                               struct kernel_ethtool_coalesce
> *kernel_coal,
> +                               struct netlink_ext_ack *extack)
> +{
> +       return virtnet_get_coalesce_one(dev, ec);
> +}
> +
>  static void virtnet_init_settings(struct net_device *dev)
>  {
>         struct virtnet_info *vi = netdev_priv(dev);
> --
> 2.39.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 3691 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next V1 3/4] virtio_net: support per queue interrupt coalesce command

[Jason Wang]

[-- Attachment #1.1: Type: text/plain, Size: 10958 bytes --]

On Mon, Jul 10, 2023 at 5:21 PM Gavin Li <gavinl@nvidia.com> wrote:

> Add interrupt_coalesce config in send_queue and receive_queue to cache user
> config.
>
> Send per virtqueue interrupt moderation config to underline device in order
> to have more efficient interrupt moderation and cpu utilization of guest
> VM.
>
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  drivers/net/virtio_net.c        | 117 ++++++++++++++++++++++++++++----
>  include/uapi/linux/virtio_net.h |  14 ++++
>  2 files changed, 119 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 802ed21453f5..333a38e1941f 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -144,6 +144,8 @@ struct send_queue {
>
>         struct virtnet_sq_stats stats;
>
> +       struct virtnet_interrupt_coalesce intr_coal;
> +
>         struct napi_struct napi;
>
>         /* Record whether sq is in reset state. */
> @@ -161,6 +163,8 @@ struct receive_queue {
>
>         struct virtnet_rq_stats stats;
>
> +       struct virtnet_interrupt_coalesce intr_coal;
> +
>         /* Chain pages by the private ptr. */
>         struct page *pages;
>
> @@ -3078,6 +3082,56 @@ static int virtnet_send_notf_coal_cmds(struct
> virtnet_info *vi,
>         return 0;
>  }
>
> +static int virtnet_send_ctrl_coal_vq_cmd(struct virtnet_info *vi,
> +                                        u16 vqn, u32 max_usecs, u32
> max_packets)
> +{
> +       struct virtio_net_ctrl_coal_vq coal_vq = {};
> +       struct scatterlist sgs;
> +
> +       coal_vq.vqn = cpu_to_le16(vqn);
> +       coal_vq.coal.max_usecs = cpu_to_le32(max_usecs);
> +       coal_vq.coal.max_packets = cpu_to_le32(max_packets);
> +       sg_init_one(&sgs, &coal_vq, sizeof(coal_vq));
> +
> +       if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
> +                                 VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET,
> +                                 &sgs))
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
> +static int virtnet_send_notf_coal_vq_cmds(struct virtnet_info *vi,
> +                                         struct ethtool_coalesce *ec,
> +                                         u16 queue)
> +{
> +       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;
> +       }
> +
> +       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;
> +       }
> +
> +       return 0;
> +}
> +
>  static int virtnet_coal_params_supported(struct ethtool_coalesce *ec)
>  {
>         /* usecs coalescing is supported only if VIRTIO_NET_F_NOTF_COAL
> @@ -3094,23 +3148,36 @@ static int virtnet_coal_params_supported(struct
> ethtool_coalesce *ec)
>  }
>
>  static int virtnet_set_coalesce_one(struct net_device *dev,
> -                                   struct ethtool_coalesce *ec)
> +                                   struct ethtool_coalesce *ec,
> +                                   bool per_queue,
> +                                   u32 queue)
>  {
>         struct virtnet_info *vi = netdev_priv(dev);
> -       int ret, i, napi_weight;
> +       int queue_count = per_queue ? 1 : vi->max_queue_pairs;
> +       int queue_number = per_queue ? queue : 0;
>         bool update_napi = false;
> +       int ret, i, napi_weight;
> +
> +       if (queue >= vi->max_queue_pairs)
> +               return -EINVAL;
>
>         /* Can't change NAPI weight if the link is up */
>         napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
> -       if (napi_weight ^ vi->sq[0].napi.weight) {
> -               if (dev->flags & IFF_UP)
> -                       return -EBUSY;
> -               else
> +       for (i = queue_number; i < queue_count; i++) {
> +               if (napi_weight ^ vi->sq[i].napi.weight) {
> +                       if (dev->flags & IFF_UP)
> +                               return -EBUSY;
> +
>                         update_napi = true;
> +                       queue_number = i;
>


This seems to assume all queues that belong to [queue_number, queue_count)
need an updating of the NAPI which is not necessarily the case.  Worth a
comment to mention that the updating of the NAPI might not be necessary.

Thanks


> +                       break;
> +               }
>         }
>
> -       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL))
> +       if (!per_queue && virtio_has_feature(vi->vdev,
> VIRTIO_NET_F_NOTF_COAL))
>                 ret = virtnet_send_notf_coal_cmds(vi, ec);
> +       else if (per_queue && virtio_has_feature(vi->vdev,
> VIRTIO_NET_F_VQ_NOTF_COAL))
> +               ret = virtnet_send_notf_coal_vq_cmds(vi, ec, queue);
>         else
>                 ret = virtnet_coal_params_supported(ec);
>
> @@ -3118,7 +3185,7 @@ static int virtnet_set_coalesce_one(struct
> net_device *dev,
>                 return ret;
>
>         if (update_napi) {
> -               for (i = 0; i < vi->max_queue_pairs; i++)
> +               for (i = queue_number; i < queue_count; i++)
>                         vi->sq[i].napi.weight = napi_weight;
>         }
>
> @@ -3130,19 +3197,29 @@ static int virtnet_set_coalesce(struct net_device
> *dev,
>                                 struct kernel_ethtool_coalesce
> *kernel_coal,
>                                 struct netlink_ext_ack *extack)
>  {
> -       return virtnet_set_coalesce_one(dev, ec);
> +       return virtnet_set_coalesce_one(dev, ec, false, 0);
>  }
>
>  static int virtnet_get_coalesce_one(struct net_device *dev,
> -                                   struct ethtool_coalesce *ec)
> +                                   struct ethtool_coalesce *ec,
> +                                   bool per_queue,
> +                                   u32 queue)
>  {
>         struct virtnet_info *vi = netdev_priv(dev);
>
> -       if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
> +       if (queue >= vi->max_queue_pairs)
> +               return -EINVAL;
> +
> +       if (!per_queue && virtio_has_feature(vi->vdev,
> VIRTIO_NET_F_NOTF_COAL)) {
>                 ec->rx_coalesce_usecs = vi->intr_coal_rx.max_usecs;
>                 ec->tx_coalesce_usecs = vi->intr_coal_tx.max_usecs;
>                 ec->tx_max_coalesced_frames = vi->intr_coal_tx.max_packets;
>                 ec->rx_max_coalesced_frames = vi->intr_coal_rx.max_packets;
> +       } else if (per_queue && virtio_has_feature(vi->vdev,
> VIRTIO_NET_F_VQ_NOTF_COAL)) {
> +               ec->rx_coalesce_usecs = vi->rq[queue].intr_coal.max_usecs;
> +               ec->tx_coalesce_usecs = vi->sq[queue].intr_coal.max_usecs;
> +               ec->tx_max_coalesced_frames =
> vi->sq[queue].intr_coal.max_packets;
> +               ec->rx_max_coalesced_frames =
> vi->rq[queue].intr_coal.max_packets;
>         } else {
>                 ec->rx_max_coalesced_frames = 1;
>
> @@ -3158,7 +3235,21 @@ static int virtnet_get_coalesce(struct net_device
> *dev,
>                                 struct kernel_ethtool_coalesce
> *kernel_coal,
>                                 struct netlink_ext_ack *extack)
>  {
> -       return virtnet_get_coalesce_one(dev, ec);
> +       return virtnet_get_coalesce_one(dev, ec, false, 0);
> +}
> +
> +static int virtnet_set_per_queue_coalesce(struct net_device *dev,
> +                                         u32 queue,
> +                                         struct ethtool_coalesce *ec)
> +{
> +       return virtnet_set_coalesce_one(dev, ec, true, queue);
> +}
> +
> +static int virtnet_get_per_queue_coalesce(struct net_device *dev,
> +                                         u32 queue,
> +                                         struct ethtool_coalesce *ec)
> +{
> +       return virtnet_get_coalesce_one(dev, ec, true, queue);
>  }
>
>  static void virtnet_init_settings(struct net_device *dev)
> @@ -3291,6 +3382,8 @@ static const struct ethtool_ops virtnet_ethtool_ops
> = {
>         .set_link_ksettings = virtnet_set_link_ksettings,
>         .set_coalesce = virtnet_set_coalesce,
>         .get_coalesce = virtnet_get_coalesce,
> +       .set_per_queue_coalesce = virtnet_set_per_queue_coalesce,
> +       .get_per_queue_coalesce = virtnet_get_per_queue_coalesce,
>         .get_rxfh_key_size = virtnet_get_rxfh_key_size,
>         .get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
>         .get_rxfh = virtnet_get_rxfh,
> diff --git a/include/uapi/linux/virtio_net.h
> b/include/uapi/linux/virtio_net.h
> index 12c1c9699935..cc65ef0f3c3e 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -56,6 +56,7 @@
>  #define VIRTIO_NET_F_MQ        22      /* Device supports Receive Flow
>                                          * Steering */
>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
> +#define VIRTIO_NET_F_VQ_NOTF_COAL 52   /* Device supports virtqueue
> notification coalescing */
>  #define VIRTIO_NET_F_NOTF_COAL 53      /* Device supports notifications
> coalescing */
>  #define VIRTIO_NET_F_GUEST_USO4        54      /* Guest can handle USOv4
> in. */
>  #define VIRTIO_NET_F_GUEST_USO6        55      /* Guest can handle USOv6
> in. */
> @@ -391,5 +392,18 @@ struct virtio_net_ctrl_coal_rx {
>  };
>
>  #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET               1
> +#define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET               2
> +#define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET               3
> +
> +struct virtio_net_ctrl_coal {
> +       __le32 max_packets;
> +       __le32 max_usecs;
> +};
> +
> +struct  virtio_net_ctrl_coal_vq {
> +       __le16 vqn;
> +       __le16 reserved;
> +       struct virtio_net_ctrl_coal coal;
> +};
>
>  #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> --
> 2.39.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 13637 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next V1 3/4] virtio_net: support per queue interrupt coalesce command

[Xuan Zhuo]

On Mon, 10 Jul 2023 12:20:04 +0300, Gavin Li <gavinl@nvidia.com> wrote:
> Add interrupt_coalesce config in send_queue and receive_queue to cache user
> config.
>
> Send per virtqueue interrupt moderation config to underline device in order
> to have more efficient interrupt moderation and cpu utilization of guest
> VM.
>
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  drivers/net/virtio_net.c        | 117 ++++++++++++++++++++++++++++----
>  include/uapi/linux/virtio_net.h |  14 ++++
>  2 files changed, 119 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 802ed21453f5..333a38e1941f 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -144,6 +144,8 @@ struct send_queue {
>
>  	struct virtnet_sq_stats stats;
>
> +	struct virtnet_interrupt_coalesce intr_coal;
> +
>  	struct napi_struct napi;
>
>  	/* Record whether sq is in reset state. */
> @@ -161,6 +163,8 @@ struct receive_queue {
>
>  	struct virtnet_rq_stats stats;
>
> +	struct virtnet_interrupt_coalesce intr_coal;
> +
>  	/* Chain pages by the private ptr. */
>  	struct page *pages;
>
> @@ -3078,6 +3082,56 @@ static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,
>  	return 0;
>  }
>
> +static int virtnet_send_ctrl_coal_vq_cmd(struct virtnet_info *vi,
> +					 u16 vqn, u32 max_usecs, u32 max_packets)
> +{
> +	struct virtio_net_ctrl_coal_vq coal_vq = {};

We should alloc this on the heap.

Thanks.


> +	struct scatterlist sgs;
> +
> +	coal_vq.vqn = cpu_to_le16(vqn);
> +	coal_vq.coal.max_usecs = cpu_to_le32(max_usecs);
> +	coal_vq.coal.max_packets = cpu_to_le32(max_packets);
> +	sg_init_one(&sgs, &coal_vq, sizeof(coal_vq));
> +
> +	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
> +				  VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET,
> +				  &sgs))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int virtnet_send_notf_coal_vq_cmds(struct virtnet_info *vi,
> +					  struct ethtool_coalesce *ec,
> +					  u16 queue)
> +{
> +	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;
> +	}
> +
> +	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;
> +	}
> +
> +	return 0;
> +}
> +
>  static int virtnet_coal_params_supported(struct ethtool_coalesce *ec)
>  {
>  	/* usecs coalescing is supported only if VIRTIO_NET_F_NOTF_COAL
> @@ -3094,23 +3148,36 @@ static int virtnet_coal_params_supported(struct ethtool_coalesce *ec)
>  }
>
>  static int virtnet_set_coalesce_one(struct net_device *dev,
> -				    struct ethtool_coalesce *ec)
> +				    struct ethtool_coalesce *ec,
> +				    bool per_queue,
> +				    u32 queue)
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
> -	int ret, i, napi_weight;
> +	int queue_count = per_queue ? 1 : vi->max_queue_pairs;
> +	int queue_number = per_queue ? queue : 0;
>  	bool update_napi = false;
> +	int ret, i, napi_weight;
> +
> +	if (queue >= vi->max_queue_pairs)
> +		return -EINVAL;
>
>  	/* Can't change NAPI weight if the link is up */
>  	napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
> -	if (napi_weight ^ vi->sq[0].napi.weight) {
> -		if (dev->flags & IFF_UP)
> -			return -EBUSY;
> -		else
> +	for (i = queue_number; i < queue_count; i++) {
> +		if (napi_weight ^ vi->sq[i].napi.weight) {
> +			if (dev->flags & IFF_UP)
> +				return -EBUSY;
> +
>  			update_napi = true;
> +			queue_number = i;
> +			break;
> +		}
>  	}
>
> -	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL))
> +	if (!per_queue && virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL))
>  		ret = virtnet_send_notf_coal_cmds(vi, ec);
> +	else if (per_queue && virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
> +		ret = virtnet_send_notf_coal_vq_cmds(vi, ec, queue);
>  	else
>  		ret = virtnet_coal_params_supported(ec);
>
> @@ -3118,7 +3185,7 @@ static int virtnet_set_coalesce_one(struct net_device *dev,
>  		return ret;
>
>  	if (update_napi) {
> -		for (i = 0; i < vi->max_queue_pairs; i++)
> +		for (i = queue_number; i < queue_count; i++)
>  			vi->sq[i].napi.weight = napi_weight;
>  	}
>
> @@ -3130,19 +3197,29 @@ static int virtnet_set_coalesce(struct net_device *dev,
>  				struct kernel_ethtool_coalesce *kernel_coal,
>  				struct netlink_ext_ack *extack)
>  {
> -	return virtnet_set_coalesce_one(dev, ec);
> +	return virtnet_set_coalesce_one(dev, ec, false, 0);
>  }
>
>  static int virtnet_get_coalesce_one(struct net_device *dev,
> -				    struct ethtool_coalesce *ec)
> +				    struct ethtool_coalesce *ec,
> +				    bool per_queue,
> +				    u32 queue)
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
>
> -	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
> +	if (queue >= vi->max_queue_pairs)
> +		return -EINVAL;
> +
> +	if (!per_queue && virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
>  		ec->rx_coalesce_usecs = vi->intr_coal_rx.max_usecs;
>  		ec->tx_coalesce_usecs = vi->intr_coal_tx.max_usecs;
>  		ec->tx_max_coalesced_frames = vi->intr_coal_tx.max_packets;
>  		ec->rx_max_coalesced_frames = vi->intr_coal_rx.max_packets;
> +	} else if (per_queue && virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
> +		ec->rx_coalesce_usecs = vi->rq[queue].intr_coal.max_usecs;
> +		ec->tx_coalesce_usecs = vi->sq[queue].intr_coal.max_usecs;
> +		ec->tx_max_coalesced_frames = vi->sq[queue].intr_coal.max_packets;
> +		ec->rx_max_coalesced_frames = vi->rq[queue].intr_coal.max_packets;
>  	} else {
>  		ec->rx_max_coalesced_frames = 1;
>
> @@ -3158,7 +3235,21 @@ static int virtnet_get_coalesce(struct net_device *dev,
>  				struct kernel_ethtool_coalesce *kernel_coal,
>  				struct netlink_ext_ack *extack)
>  {
> -	return virtnet_get_coalesce_one(dev, ec);
> +	return virtnet_get_coalesce_one(dev, ec, false, 0);
> +}
> +
> +static int virtnet_set_per_queue_coalesce(struct net_device *dev,
> +					  u32 queue,
> +					  struct ethtool_coalesce *ec)
> +{
> +	return virtnet_set_coalesce_one(dev, ec, true, queue);
> +}
> +
> +static int virtnet_get_per_queue_coalesce(struct net_device *dev,
> +					  u32 queue,
> +					  struct ethtool_coalesce *ec)
> +{
> +	return virtnet_get_coalesce_one(dev, ec, true, queue);
>  }
>
>  static void virtnet_init_settings(struct net_device *dev)
> @@ -3291,6 +3382,8 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>  	.set_link_ksettings = virtnet_set_link_ksettings,
>  	.set_coalesce = virtnet_set_coalesce,
>  	.get_coalesce = virtnet_get_coalesce,
> +	.set_per_queue_coalesce = virtnet_set_per_queue_coalesce,
> +	.get_per_queue_coalesce = virtnet_get_per_queue_coalesce,
>  	.get_rxfh_key_size = virtnet_get_rxfh_key_size,
>  	.get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
>  	.get_rxfh = virtnet_get_rxfh,
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index 12c1c9699935..cc65ef0f3c3e 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -56,6 +56,7 @@
>  #define VIRTIO_NET_F_MQ	22	/* Device supports Receive Flow
>  					 * Steering */
>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
> +#define VIRTIO_NET_F_VQ_NOTF_COAL 52	/* Device supports virtqueue notification coalescing */
>  #define VIRTIO_NET_F_NOTF_COAL	53	/* Device supports notifications coalescing */
>  #define VIRTIO_NET_F_GUEST_USO4	54	/* Guest can handle USOv4 in. */
>  #define VIRTIO_NET_F_GUEST_USO6	55	/* Guest can handle USOv6 in. */
> @@ -391,5 +392,18 @@ struct virtio_net_ctrl_coal_rx {
>  };
>
>  #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET		1
> +#define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET		2
> +#define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET		3
> +
> +struct virtio_net_ctrl_coal {
> +	__le32 max_packets;
> +	__le32 max_usecs;
> +};
> +
> +struct  virtio_net_ctrl_coal_vq {
> +	__le16 vqn;
> +	__le16 reserved;
> +	struct virtio_net_ctrl_coal coal;
> +};
>
>  #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> --
> 2.39.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next V1 0/4] virtio_net: add per queue interrupt coalescing support

[Michael S. Tsirkin]

On Mon, Jul 10, 2023 at 12:20:01PM +0300, Gavin Li wrote:
> Currently, coalescing parameters are grouped for all transmit and receive
> virtqueues. This patch series add support to set or get the parameters for
> a specified virtqueue.
> 
> When the traffic between virtqueues is unbalanced, for example, one virtqueue
> is busy and another virtqueue is idle, then it will be very useful to
> control coalescing parameters at the virtqueue granularity.

series:

Acked-by: Michael S. Tsirkin <mst@redhat.com>



> Example command:
> $ ethtool -Q eth5 queue_mask 0x1 --coalesce tx-packets 10
> Would set max_packets=10 to VQ 1.
> $ ethtool -Q eth5 queue_mask 0x1 --coalesce rx-packets 10
> Would set max_packets=10 to VQ 0.
> $ ethtool -Q eth5 queue_mask 0x1 --show-coalesce
>  Queue: 0
>  Adaptive RX: off  TX: off
>  stats-block-usecs: 0
>  sample-interval: 0
>  pkt-rate-low: 0
>  pkt-rate-high: 0
> 
>  rx-usecs: 222
>  rx-frames: 0
>  rx-usecs-irq: 0
>  rx-frames-irq: 256
> 
>  tx-usecs: 222
>  tx-frames: 0
>  tx-usecs-irq: 0
>  tx-frames-irq: 256
> 
>  rx-usecs-low: 0
>  rx-frame-low: 0
>  tx-usecs-low: 0
>  tx-frame-low: 0
> 
>  rx-usecs-high: 0
>  rx-frame-high: 0
>  tx-usecs-high: 0
>  tx-frame-high: 0
> 
> In this patch series:
> Patch-1: Extract interrupt coalescing settings to a structure.
> Patch-2: Extract get/set interrupt coalesce to a function.
> Patch-3: Support per queue interrupt coalesce command.
> Patch-4: Enable per queue interrupt coalesce feature.
> 
> Gavin Li (4):
>   virtio_net: extract interrupt coalescing settings to a structure
>   virtio_net: extract get/set interrupt coalesce to a function
>   virtio_net: support per queue interrupt coalesce command
>   virtio_net: enable per queue interrupt coalesce feature
> 
>  drivers/net/virtio_net.c        | 169 ++++++++++++++++++++++++++------
>  include/uapi/linux/virtio_net.h |  14 +++
>  2 files changed, 154 insertions(+), 29 deletions(-)
> 
> -- 
> 2.39.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next V1 0/4] virtio_net: add per queue interrupt coalescing support

[Xuan Zhuo]

On Thu, 13 Jul 2023 07:40:12 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Jul 10, 2023 at 12:20:01PM +0300, Gavin Li wrote:
> > Currently, coalescing parameters are grouped for all transmit and receive
> > virtqueues. This patch series add support to set or get the parameters for
> > a specified virtqueue.
> >
> > When the traffic between virtqueues is unbalanced, for example, one virtqueue
> > is busy and another virtqueue is idle, then it will be very useful to
> > control coalescing parameters at the virtqueue granularity.
>
> series:
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>


Why?

This series has the bug I reported.

Are you thinking that is ok? Or this is not a bug?

Thanks.



>
>
>
> > Example command:
> > $ ethtool -Q eth5 queue_mask 0x1 --coalesce tx-packets 10
> > Would set max_packets=10 to VQ 1.
> > $ ethtool -Q eth5 queue_mask 0x1 --coalesce rx-packets 10
> > Would set max_packets=10 to VQ 0.
> > $ ethtool -Q eth5 queue_mask 0x1 --show-coalesce
> >  Queue: 0
> >  Adaptive RX: off  TX: off
> >  stats-block-usecs: 0
> >  sample-interval: 0
> >  pkt-rate-low: 0
> >  pkt-rate-high: 0
> >
> >  rx-usecs: 222
> >  rx-frames: 0
> >  rx-usecs-irq: 0
> >  rx-frames-irq: 256
> >
> >  tx-usecs: 222
> >  tx-frames: 0
> >  tx-usecs-irq: 0
> >  tx-frames-irq: 256
> >
> >  rx-usecs-low: 0
> >  rx-frame-low: 0
> >  tx-usecs-low: 0
> >  tx-frame-low: 0
> >
> >  rx-usecs-high: 0
> >  rx-frame-high: 0
> >  tx-usecs-high: 0
> >  tx-frame-high: 0
> >
> > In this patch series:
> > Patch-1: Extract interrupt coalescing settings to a structure.
> > Patch-2: Extract get/set interrupt coalesce to a function.
> > Patch-3: Support per queue interrupt coalesce command.
> > Patch-4: Enable per queue interrupt coalesce feature.
> >
> > Gavin Li (4):
> >   virtio_net: extract interrupt coalescing settings to a structure
> >   virtio_net: extract get/set interrupt coalesce to a function
> >   virtio_net: support per queue interrupt coalesce command
> >   virtio_net: enable per queue interrupt coalesce feature
> >
> >  drivers/net/virtio_net.c        | 169 ++++++++++++++++++++++++++------
> >  include/uapi/linux/virtio_net.h |  14 +++
> >  2 files changed, 154 insertions(+), 29 deletions(-)
> >
> > --
> > 2.39.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next V1 0/4] virtio_net: add per queue interrupt coalescing support

[Michael S. Tsirkin]

On Fri, Jul 14, 2023 at 10:10:51AM +0800, Xuan Zhuo wrote:
> On Thu, 13 Jul 2023 07:40:12 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Mon, Jul 10, 2023 at 12:20:01PM +0300, Gavin Li wrote:
> > > Currently, coalescing parameters are grouped for all transmit and receive
> > > virtqueues. This patch series add support to set or get the parameters for
> > > a specified virtqueue.
> > >
> > > When the traffic between virtqueues is unbalanced, for example, one virtqueue
> > > is busy and another virtqueue is idle, then it will be very useful to
> > > control coalescing parameters at the virtqueue granularity.
> >
> > series:
> >
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> 
> Why?
> 
> This series has the bug I reported.
> 
> Are you thinking that is ok? Or this is not a bug?
> 
> Thanks.
> 
> 


I missed that mail. What's the bug?

> >
> >
> >
> > > Example command:
> > > $ ethtool -Q eth5 queue_mask 0x1 --coalesce tx-packets 10
> > > Would set max_packets=10 to VQ 1.
> > > $ ethtool -Q eth5 queue_mask 0x1 --coalesce rx-packets 10
> > > Would set max_packets=10 to VQ 0.
> > > $ ethtool -Q eth5 queue_mask 0x1 --show-coalesce
> > >  Queue: 0
> > >  Adaptive RX: off  TX: off
> > >  stats-block-usecs: 0
> > >  sample-interval: 0
> > >  pkt-rate-low: 0
> > >  pkt-rate-high: 0
> > >
> > >  rx-usecs: 222
> > >  rx-frames: 0
> > >  rx-usecs-irq: 0
> > >  rx-frames-irq: 256
> > >
> > >  tx-usecs: 222
> > >  tx-frames: 0
> > >  tx-usecs-irq: 0
> > >  tx-frames-irq: 256
> > >
> > >  rx-usecs-low: 0
> > >  rx-frame-low: 0
> > >  tx-usecs-low: 0
> > >  tx-frame-low: 0
> > >
> > >  rx-usecs-high: 0
> > >  rx-frame-high: 0
> > >  tx-usecs-high: 0
> > >  tx-frame-high: 0
> > >
> > > In this patch series:
> > > Patch-1: Extract interrupt coalescing settings to a structure.
> > > Patch-2: Extract get/set interrupt coalesce to a function.
> > > Patch-3: Support per queue interrupt coalesce command.
> > > Patch-4: Enable per queue interrupt coalesce feature.
> > >
> > > Gavin Li (4):
> > >   virtio_net: extract interrupt coalescing settings to a structure
> > >   virtio_net: extract get/set interrupt coalesce to a function
> > >   virtio_net: support per queue interrupt coalesce command
> > >   virtio_net: enable per queue interrupt coalesce feature
> > >
> > >  drivers/net/virtio_net.c        | 169 ++++++++++++++++++++++++++------
> > >  include/uapi/linux/virtio_net.h |  14 +++
> > >  2 files changed, 154 insertions(+), 29 deletions(-)
> > >
> > > --
> > > 2.39.1
> >

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization