2023-07-24 03:59:29

by Gavin Li

[permalink] [raw]
Subject: [PATCH net-next V3 0/4] virtio_net: add per queue interrupt coalescing support

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

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
---
changelog:
v1->v2
- Addressed the comment from Xuan Zhuo
- Allocate memory from heap instead of using stack memory for control vq
messages
v2->v3
- Addressed the comment from Heng Qi
- Use control_buf for control vq messages
---
virtio_net: enable per queue interrupt coalesce feature

drivers/net/virtio_net.c | 172 ++++++++++++++++++++++++++------
include/uapi/linux/virtio_net.h | 14 +++
2 files changed, 157 insertions(+), 29 deletions(-)

--
2.39.1



2023-07-24 04:09:24

by Gavin Li

[permalink] [raw]
Subject: [PATCH net-next V3 3/4] virtio_net: support per queue interrupt coalesce command

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 <[email protected]>
Reviewed-by: Dragos Tatulea <[email protected]>
Reviewed-by: Jiri Pirko <[email protected]>
Acked-by: Michael S. Tsirkin <[email protected]>
---
drivers/net/virtio_net.c | 120 ++++++++++++++++++++++++++++----
include/uapi/linux/virtio_net.h | 14 ++++
2 files changed, 122 insertions(+), 12 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 802ed21453f5..0c3ee1e26ece 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;

@@ -212,6 +216,7 @@ struct control_buf {
struct virtio_net_ctrl_rss rss;
struct virtio_net_ctrl_coal_tx coal_tx;
struct virtio_net_ctrl_coal_rx coal_rx;
+ struct virtio_net_ctrl_coal_vq coal_vq;
};

struct virtnet_info {
@@ -3078,6 +3083,55 @@ 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 scatterlist sgs;
+
+ vi->ctrl->coal_vq.vqn = cpu_to_le16(vqn);
+ vi->ctrl->coal_vq.coal.max_usecs = cpu_to_le32(max_usecs);
+ vi->ctrl->coal_vq.coal.max_packets = cpu_to_le32(max_packets);
+ sg_init_one(&sgs, &vi->ctrl->coal_vq, sizeof(vi->ctrl->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,39 @@ 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;
+ /* All queues that belong to [queue_number, queue_count] will be
+ * updated for the sake of simplicity, which might not be necessary
+ */
+ 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 +3188,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 +3200,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 +3238,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 +3385,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


2023-07-24 04:40:06

by Gavin Li

[permalink] [raw]
Subject: [PATCH net-next V3 4/4] virtio_net: enable per queue interrupt coalesce feature

Enable per queue interrupt coalesce feature bit in driver and validate its
dependency with control queue.

Signed-off-by: Gavin Li <[email protected]>
Reviewed-by: Dragos Tatulea <[email protected]>
Reviewed-by: Jiri Pirko <[email protected]>
Acked-by: Michael S. Tsirkin <[email protected]>
---
drivers/net/virtio_net.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0c3ee1e26ece..a03289da9f51 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -4063,6 +4063,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
"VIRTIO_NET_F_CTRL_VQ") ||
VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_NOTF_COAL,
+ "VIRTIO_NET_F_CTRL_VQ") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_VQ_NOTF_COAL,
"VIRTIO_NET_F_CTRL_VQ"))) {
return false;
}
@@ -4487,6 +4489,7 @@ static struct virtio_device_id id_table[] = {
VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT, VIRTIO_NET_F_NOTF_COAL, \
+ VIRTIO_NET_F_VQ_NOTF_COAL, \
VIRTIO_NET_F_GUEST_HDRLEN

static unsigned int features[] = {
--
2.39.1


2023-07-24 04:53:57

by Gavin Li

[permalink] [raw]
Subject: [PATCH net-next V3 2/4] virtio_net: extract get/set interrupt coalesce to a function

Extract get/set interrupt coalesce to a function to be reused by global
and per queue config.

Signed-off-by: Gavin Li <[email protected]>
Reviewed-by: Dragos Tatulea <[email protected]>
Reviewed-by: Jiri Pirko <[email protected]>
Acked-by: Jason Wang <[email protected]>
Acked-by: Michael S. Tsirkin <[email protected]>
---
drivers/net/virtio_net.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)

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


2023-07-24 05:55:04

by Heng Qi

[permalink] [raw]
Subject: Re: [PATCH net-next V3 0/4] virtio_net: add per queue interrupt coalescing support

On Mon, Jul 24, 2023 at 06:40:44AM +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.
>

For series:

Reviewed-by: Heng Qi <[email protected]>

After this set is merged, I will issue the netdim patchset on top of this.

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
>
> 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
> ---
> changelog:
> v1->v2
> - Addressed the comment from Xuan Zhuo
> - Allocate memory from heap instead of using stack memory for control vq
> messages
> v2->v3
> - Addressed the comment from Heng Qi
> - Use control_buf for control vq messages
> ---
> virtio_net: enable per queue interrupt coalesce feature
>
> drivers/net/virtio_net.c | 172 ++++++++++++++++++++++++++------
> include/uapi/linux/virtio_net.h | 14 +++
> 2 files changed, 157 insertions(+), 29 deletions(-)
>
> --
> 2.39.1
>

2023-07-24 08:25:53

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH net-next V3 3/4] virtio_net: support per queue interrupt coalesce command

On Mon, Jul 24, 2023 at 06:40:47AM +0300, Gavin Li 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 <[email protected]>
> Reviewed-by: Dragos Tatulea <[email protected]>
> Reviewed-by: Jiri Pirko <[email protected]>
> Acked-by: Michael S. Tsirkin <[email protected]>
> ---
> drivers/net/virtio_net.c | 120 ++++++++++++++++++++++++++++----
> include/uapi/linux/virtio_net.h | 14 ++++
> 2 files changed, 122 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 802ed21453f5..0c3ee1e26ece 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;
>
> @@ -212,6 +216,7 @@ struct control_buf {
> struct virtio_net_ctrl_rss rss;
> struct virtio_net_ctrl_coal_tx coal_tx;
> struct virtio_net_ctrl_coal_rx coal_rx;
> + struct virtio_net_ctrl_coal_vq coal_vq;
> };
>
> struct virtnet_info {
> @@ -3078,6 +3083,55 @@ 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 scatterlist sgs;
> +
> + vi->ctrl->coal_vq.vqn = cpu_to_le16(vqn);
> + vi->ctrl->coal_vq.coal.max_usecs = cpu_to_le32(max_usecs);
> + vi->ctrl->coal_vq.coal.max_packets = cpu_to_le32(max_packets);
> + sg_init_one(&sgs, &vi->ctrl->coal_vq, sizeof(vi->ctrl->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,39 @@ 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;

Actually can't we refactor this? This whole function is littered
with if/else branches. just code it separately - the only
common part is:

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
update_napi = true;
}

so just move this to a helper and have two functions - global and
per queue.



> 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;
> + /* All queues that belong to [queue_number, queue_count] will be
> + * updated for the sake of simplicity, which might not be necessary
> + */
> + 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 +3188,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 +3200,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 +3238,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 +3385,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


2023-07-24 08:47:18

by Gavin Li

[permalink] [raw]
Subject: Re: [PATCH net-next V3 3/4] virtio_net: support per queue interrupt coalesce command



On 7/24/2023 3:32 PM, Michael S. Tsirkin wrote:
> On Mon, Jul 24, 2023 at 06:40:47AM +0300, Gavin Li 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 <[email protected]>
>> Reviewed-by: Dragos Tatulea <[email protected]>
>> Reviewed-by: Jiri Pirko <[email protected]>
>> Acked-by: Michael S. Tsirkin <[email protected]>
>> ---
>> drivers/net/virtio_net.c | 120 ++++++++++++++++++++++++++++----
>> include/uapi/linux/virtio_net.h | 14 ++++
>> 2 files changed, 122 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 802ed21453f5..0c3ee1e26ece 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;
>>
>> @@ -212,6 +216,7 @@ struct control_buf {
>> struct virtio_net_ctrl_rss rss;
>> struct virtio_net_ctrl_coal_tx coal_tx;
>> struct virtio_net_ctrl_coal_rx coal_rx;
>> + struct virtio_net_ctrl_coal_vq coal_vq;
>> };
>>
>> struct virtnet_info {
>> @@ -3078,6 +3083,55 @@ 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 scatterlist sgs;
>> +
>> + vi->ctrl->coal_vq.vqn = cpu_to_le16(vqn);
>> + vi->ctrl->coal_vq.coal.max_usecs = cpu_to_le32(max_usecs);
>> + vi->ctrl->coal_vq.coal.max_packets = cpu_to_le32(max_packets);
>> + sg_init_one(&sgs, &vi->ctrl->coal_vq, sizeof(vi->ctrl->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,39 @@ 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;
>
> Actually can't we refactor this? This whole function is littered
> with if/else branches. just code it separately - the only
> common part is:
>
> 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
> update_napi = true;
> }
>
> so just move this to a helper and have two functions - global and
> per queue.
ACK
>
>
>
>> 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;
>> + /* All queues that belong to [queue_number, queue_count] will be
>> + * updated for the sake of simplicity, which might not be necessary
>> + */
>> + 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 +3188,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 +3200,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 +3238,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 +3385,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
>