[dpdk-dev] [PATCH v7 1/5] net/virtio: implement rte_power_monitor API

[Ferruh Yigit]

On 10/22/2021 9:51 AM, Li, Miao wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Li, Miao
>> Sent: Friday, October 22, 2021 4:28 PM
>> To: Yigit, Ferruh <ferruh.yigit at intel.com>; dev at dpdk.org
>> Cc: Xia, Chenbo <Chenbo.Xia at intel.com>; maxime.coquelin at redhat.com
>> Subject: RE: [dpdk-dev] [PATCH v7 1/5] net/virtio: implement
>> rte_power_monitor API
>>
>> Hi,
>>
>>> -----Original Message-----
>>> From: Yigit, Ferruh <ferruh.yigit at intel.com>
>>> Sent: Friday, October 22, 2021 12:59 AM
>>> To: Li, Miao <miao.li at intel.com>; dev at dpdk.org
>>> Cc: Xia, Chenbo <chenbo.xia at intel.com>; maxime.coquelin at redhat.com
>>> Subject: Re: [dpdk-dev] [PATCH v7 1/5] net/virtio: implement
>>> rte_power_monitor API
>>>
>>> On 10/18/2021 3:16 PM, Miao Li wrote:
>>>> 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.
>>>>
>>>
>>> A minor comment but instead of patch title mentioning what is implemented,
>>> better to describe what feature is added, like:
>>> net/virtio: support power monitor
>>
>> I will modify it in the next version.
>>
>>>
>>>> Signed-off-by: Miao Li <miao.li at intel.com>
>>>> Reviewed-by: Chenbo Xia <chenbo.xia at intel.com>
>>>> ---
>>>>    doc/guides/rel_notes/release_21_11.rst |  4 ++
>>>>    drivers/net/virtio/virtio_ethdev.c     | 56 ++++++++++++++++++++++++++
>>>>    2 files changed, 60 insertions(+)
>>>>
>>>> diff --git a/doc/guides/rel_notes/release_21_11.rst
>>> b/doc/guides/rel_notes/release_21_11.rst
>>>> index d5435a64aa..c298844898 100644
>>>> --- a/doc/guides/rel_notes/release_21_11.rst
>>>> +++ b/doc/guides/rel_notes/release_21_11.rst
>>>> @@ -80,6 +80,10 @@ New Features
>>>>      Added macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now
>>> IPv4 and
>>>>      TCP/UDP/SCTP header checksum field can be used as input set for RSS.
>>>>
>>>> +* **Updated virtio PMD.**
>>>> +
>>>> +  Implement rte_power_monitor API in virtio PMD.
>>>> +
>>>
>>> The release note updates are grouped as described in the documents section
>>> comment, and in a group it is sorted alphabetically, can you please re-arrange
>>> the update accordingly. And similar comment, instead of documenting what
>> API
>>> implemented, you can mention what support is added.
>>
>> I will re-arrange the update and modify the description in the next version.
> 
> The updates are requested to be ordered alphabetically by vendor name. But
> Vhost/virtio do not have a vendor name. So should I order virtio/vhost by 'v'?
> 

I think sort it by 'v' is good, unless Maxime & Chenbo has better idea.