diff mbox series

[1/5] net/virtio: implement rte_power_monitor API

Message ID 20210910130548.127017-2-miao.li@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series CPU Enabling: Implement rte_power_monitor API in virtio/vhost PMD |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Li, Miao Sept. 10, 2021, 1:05 p.m. UTC 
  This patch implements rte_power_monitor API in virtio PMD to reduce
power consumption when no packet come in. According to current semantics
of power monitor, this commit adds a callback function to decide whether
aborts the sleep by checking current value against the expected value and
virtio_get_monitor_addr to provide address to monitor. When no packet come
in, the value of address will not be changed and the running core will
sleep. Once packets arrive, the value of address will be changed and the
running core will wakeup.

Signed-off-by: Miao Li <miao.li@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 57 ++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

Comments

Chenbo Xia Sept. 15, 2021, 8:45 a.m. UTC | #1 
Hi Miao,

> -----Original Message-----
> From: Li, Miao <miao.li@intel.com>
> Sent: Friday, September 10, 2021 9:06 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com; Li, Miao
> <miao.li@intel.com>
> Subject: [PATCH 1/5] net/virtio: implement rte_power_monitor API
> 
> This patch implements rte_power_monitor API in virtio PMD to reduce
> power consumption when no packet come in. According to current semantics
> of power monitor, this commit adds a callback function to decide whether
> aborts the sleep by checking current value against the expected value and
> virtio_get_monitor_addr to provide address to monitor. When no packet come
> in, the value of address will not be changed and the running core will
> sleep. Once packets arrive, the value of address will be changed and the
> running core will wakeup.
> 
> Signed-off-by: Miao Li <miao.li@intel.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 57 ++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index e58085a2c9..4ce49936f5 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -73,6 +73,8 @@ static int virtio_mac_addr_set(struct rte_eth_dev *dev,
>  				struct rte_ether_addr *mac_addr);
> 
>  static int virtio_intr_disable(struct rte_eth_dev *dev);
> +static int virtio_get_monitor_addr(void *rx_queue,
> +				struct rte_power_monitor_cond *pmc);
> 
>  static int virtio_dev_queue_stats_mapping_set(
>  	struct rte_eth_dev *eth_dev,
> @@ -975,6 +977,7 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
>  	.mac_addr_add            = virtio_mac_addr_add,
>  	.mac_addr_remove         = virtio_mac_addr_remove,
>  	.mac_addr_set            = virtio_mac_addr_set,
> +	.get_monitor_addr        = virtio_get_monitor_addr,
>  };
> 
>  /*
> @@ -1306,6 +1309,60 @@ virtio_mac_addr_set(struct rte_eth_dev *dev, struct
> rte_ether_addr *mac_addr)
>  	return 0;
>  }
> 
> +#define CLB_VAL_IDX 0
> +#define CLB_MSK_IDX 1
> +static int
> +virtio_packed_monitor_callback(const uint64_t value,
> +		const uint64_t opaque[RTE_POWER_MONITOR_OPAQUE_SZ])
> +{
> +	const uint64_t m = opaque[CLB_MSK_IDX];
> +	const uint64_t v = opaque[CLB_VAL_IDX];
> +
> +	return (value & m) == v ? -1 : 0;
> +}
> +
> +static int
> +virtio_split_monitor_callback(const uint64_t value,
> +		const uint64_t opaque[RTE_POWER_MONITOR_OPAQUE_SZ])
> +{
> +	const uint64_t m = opaque[CLB_MSK_IDX];
> +	const uint64_t v = opaque[CLB_VAL_IDX];
> +
> +	return (value & m) == v ? 0 : -1;
> +}
> +
> +static int
> +virtio_get_monitor_addr(void *rx_queue, struct rte_power_monitor_cond *pmc)
> +{
> +	struct virtnet_rx *rxvq = rx_queue;
> +	struct virtqueue *vq = virtnet_rxq_to_vq(rxvq);
> +	struct virtio_hw *hw = vq->hw;
> +	if (vq == NULL)
> +		return -EINVAL;
> +	if (virtio_with_packed_queue(hw)) {
> +		struct vring_packed_desc *desc;
> +		desc = vq->vq_packed.ring.desc;
> +		pmc->addr = &desc[vq->vq_used_cons_idx].flags;
> +		if (vq->vq_packed.used_wrap_counter)
> +			pmc->opaque[CLB_VAL_IDX] =
> +						VRING_PACKED_DESC_F_AVAIL_USED;
> +		else
> +			pmc->opaque[CLB_VAL_IDX] = 0;
> +		pmc->opaque[CLB_MSK_IDX] = VRING_PACKED_DESC_F_AVAIL_USED;
> +		pmc->fn = virtio_packed_monitor_callback;
> +		pmc->size = sizeof(uint16_t);

I suggest to use sizeof(desc[vq->vq_used_cons_idx].flags) or sizeof(desc->flags)
in case the flag type changes.

> +	} else {
> +		pmc->addr = &vq->vq_split.ring.used->idx;
> +		pmc->opaque[CLB_VAL_IDX] = vq->vq_used_cons_idx
> +					& (vq->vq_nentries - 1);
> +		pmc->opaque[CLB_MSK_IDX] = vq->vq_nentries - 1;
> +		pmc->fn = virtio_split_monitor_callback;
> +		pmc->size = sizeof(uint16_t);

Same here.

Thanks,
Chenbo

> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
>  {
> --
> 2.25.1
Li, Miao Sept. 17, 2021, 6:40 a.m. UTC | #2 
Hi chenbo

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, September 15, 2021 4:45 PM
> To: Li, Miao <miao.li@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com
> Subject: RE: [PATCH 1/5] net/virtio: implement rte_power_monitor API
> 
> Hi Miao,
> 
> > -----Original Message-----
> > From: Li, Miao <miao.li@intel.com>
> > Sent: Friday, September 10, 2021 9:06 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com;
> > Li, Miao <miao.li@intel.com>
> > Subject: [PATCH 1/5] net/virtio: implement rte_power_monitor API
> >
> > This patch implements rte_power_monitor API in virtio PMD to reduce
> > power consumption when no packet come in. According to current
> > semantics of power monitor, this commit adds a callback function to
> > decide whether aborts the sleep by checking current value against the
> > expected value and virtio_get_monitor_addr to provide address to
> > monitor. When no packet come in, the value of address will not be
> > changed and the running core will sleep. Once packets arrive, the
> > value of address will be changed and the running core will wakeup.
> >
> > Signed-off-by: Miao Li <miao.li@intel.com>
> > ---
> >  drivers/net/virtio/virtio_ethdev.c | 57
> > ++++++++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> > b/drivers/net/virtio/virtio_ethdev.c
> > index e58085a2c9..4ce49936f5 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -73,6 +73,8 @@ static int virtio_mac_addr_set(struct rte_eth_dev *dev,
> >  				struct rte_ether_addr *mac_addr);
> >
> >  static int virtio_intr_disable(struct rte_eth_dev *dev);
> > +static int virtio_get_monitor_addr(void *rx_queue,
> > +				struct rte_power_monitor_cond *pmc);
> >
> >  static int virtio_dev_queue_stats_mapping_set(
> >  	struct rte_eth_dev *eth_dev,
> > @@ -975,6 +977,7 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
> >  	.mac_addr_add            = virtio_mac_addr_add,
> >  	.mac_addr_remove         = virtio_mac_addr_remove,
> >  	.mac_addr_set            = virtio_mac_addr_set,
> > +	.get_monitor_addr        = virtio_get_monitor_addr,
> >  };
> >
> >  /*
> > @@ -1306,6 +1309,60 @@ virtio_mac_addr_set(struct rte_eth_dev *dev,
> > struct rte_ether_addr *mac_addr)
> >  	return 0;
> >  }
> >
> > +#define CLB_VAL_IDX 0
> > +#define CLB_MSK_IDX 1
> > +static int
> > +virtio_packed_monitor_callback(const uint64_t value,
> > +		const uint64_t opaque[RTE_POWER_MONITOR_OPAQUE_SZ])
> > +{
> > +	const uint64_t m = opaque[CLB_MSK_IDX];
> > +	const uint64_t v = opaque[CLB_VAL_IDX];
> > +
> > +	return (value & m) == v ? -1 : 0;
> > +}
> > +
> > +static int
> > +virtio_split_monitor_callback(const uint64_t value,
> > +		const uint64_t opaque[RTE_POWER_MONITOR_OPAQUE_SZ])
> > +{
> > +	const uint64_t m = opaque[CLB_MSK_IDX];
> > +	const uint64_t v = opaque[CLB_VAL_IDX];
> > +
> > +	return (value & m) == v ? 0 : -1;
> > +}
> > +
> > +static int
> > +virtio_get_monitor_addr(void *rx_queue, struct rte_power_monitor_cond
> > +*pmc) {
> > +	struct virtnet_rx *rxvq = rx_queue;
> > +	struct virtqueue *vq = virtnet_rxq_to_vq(rxvq);
> > +	struct virtio_hw *hw = vq->hw;
> > +	if (vq == NULL)
> > +		return -EINVAL;
> > +	if (virtio_with_packed_queue(hw)) {
> > +		struct vring_packed_desc *desc;
> > +		desc = vq->vq_packed.ring.desc;
> > +		pmc->addr = &desc[vq->vq_used_cons_idx].flags;
> > +		if (vq->vq_packed.used_wrap_counter)
> > +			pmc->opaque[CLB_VAL_IDX] =
> > +
> 	VRING_PACKED_DESC_F_AVAIL_USED;
> > +		else
> > +			pmc->opaque[CLB_VAL_IDX] = 0;
> > +		pmc->opaque[CLB_MSK_IDX] =
> VRING_PACKED_DESC_F_AVAIL_USED;
> > +		pmc->fn = virtio_packed_monitor_callback;
> > +		pmc->size = sizeof(uint16_t);
> 
> I suggest to use sizeof(desc[vq->vq_used_cons_idx].flags) or sizeof(desc->flags)
> in case the flag type changes.

Thanks for your suggestion. I will fix it in the next version.

> 
> > +	} else {
> > +		pmc->addr = &vq->vq_split.ring.used->idx;
> > +		pmc->opaque[CLB_VAL_IDX] = vq->vq_used_cons_idx
> > +					& (vq->vq_nentries - 1);
> > +		pmc->opaque[CLB_MSK_IDX] = vq->vq_nentries - 1;
> > +		pmc->fn = virtio_split_monitor_callback;
> > +		pmc->size = sizeof(uint16_t);
> 
> Same here.

I will fix it in the next version, too.

Thanks,
Miao

> 
> Thanks,
> Chenbo
> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int
> >  virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int
> > on)  {
> > --
> > 2.25.1
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e58085a2c9..4ce49936f5 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -73,6 +73,8 @@  static int virtio_mac_addr_set(struct rte_eth_dev *dev,
 				struct rte_ether_addr *mac_addr);
 
 static int virtio_intr_disable(struct rte_eth_dev *dev);
+static int virtio_get_monitor_addr(void *rx_queue,
+				struct rte_power_monitor_cond *pmc);
 
 static int virtio_dev_queue_stats_mapping_set(
 	struct rte_eth_dev *eth_dev,
@@ -975,6 +977,7 @@  static const struct eth_dev_ops virtio_eth_dev_ops = {
 	.mac_addr_add            = virtio_mac_addr_add,
 	.mac_addr_remove         = virtio_mac_addr_remove,
 	.mac_addr_set            = virtio_mac_addr_set,
+	.get_monitor_addr        = virtio_get_monitor_addr,
 };
 
 /*
@@ -1306,6 +1309,60 @@  virtio_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
 	return 0;
 }
 
+#define CLB_VAL_IDX 0
+#define CLB_MSK_IDX 1
+static int
+virtio_packed_monitor_callback(const uint64_t value,
+		const uint64_t opaque[RTE_POWER_MONITOR_OPAQUE_SZ])
+{
+	const uint64_t m = opaque[CLB_MSK_IDX];
+	const uint64_t v = opaque[CLB_VAL_IDX];
+
+	return (value & m) == v ? -1 : 0;
+}
+
+static int
+virtio_split_monitor_callback(const uint64_t value,
+		const uint64_t opaque[RTE_POWER_MONITOR_OPAQUE_SZ])
+{
+	const uint64_t m = opaque[CLB_MSK_IDX];
+	const uint64_t v = opaque[CLB_VAL_IDX];
+
+	return (value & m) == v ? 0 : -1;
+}
+
+static int
+virtio_get_monitor_addr(void *rx_queue, struct rte_power_monitor_cond *pmc)
+{
+	struct virtnet_rx *rxvq = rx_queue;
+	struct virtqueue *vq = virtnet_rxq_to_vq(rxvq);
+	struct virtio_hw *hw = vq->hw;
+	if (vq == NULL)
+		return -EINVAL;
+	if (virtio_with_packed_queue(hw)) {
+		struct vring_packed_desc *desc;
+		desc = vq->vq_packed.ring.desc;
+		pmc->addr = &desc[vq->vq_used_cons_idx].flags;
+		if (vq->vq_packed.used_wrap_counter)
+			pmc->opaque[CLB_VAL_IDX] =
+						VRING_PACKED_DESC_F_AVAIL_USED;
+		else
+			pmc->opaque[CLB_VAL_IDX] = 0;
+		pmc->opaque[CLB_MSK_IDX] = VRING_PACKED_DESC_F_AVAIL_USED;
+		pmc->fn = virtio_packed_monitor_callback;
+		pmc->size = sizeof(uint16_t);
+	} else {
+		pmc->addr = &vq->vq_split.ring.used->idx;
+		pmc->opaque[CLB_VAL_IDX] = vq->vq_used_cons_idx
+					& (vq->vq_nentries - 1);
+		pmc->opaque[CLB_MSK_IDX] = vq->vq_nentries - 1;
+		pmc->fn = virtio_split_monitor_callback;
+		pmc->size = sizeof(uint16_t);
+	}
+
+	return 0;
+}
+
 static int
 virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 {