On 12/22/2020 2:22 AM, Hideyuki Yamashita wrote:

Hello,

Thanks for your comments.
Please see my comments inline tagged with [HY].

quoted

04/12/2020 08:51, Hideyuki Yamashita:

quoted

In general, DPDK application consumes CPU usage because it polls
incoming packets using rx_burst API in infinite loop.
This makes difficult to estimate how much CPU usage is really
used to send/receive packets by the DPDK application.

For example, even if no incoming packets arriving, CPU usage
looks nearly 100% when observed by top command.

It is beneficial if developers can observe real CPU usage of the
DPDK application.
Such information can be exported to monitoring application like
prometheus/graphana and shows CPU usage graphically.

To achieve above, this patch set provides apistats functionality.
apistats provides the followiing two counters for each lcore.
- rx_burst_counts[RTE_MAX_LCORE]
- tx_burst_counts[RTE_MAX_LCORE]
Those accumulates rx_burst/tx_burst counts since the application starts.

Please could you compare with what rte_jobstats offers?
http://code.dpdk.org/dpdk/latest/source/lib/librte_jobstats/rte_jobstats.h

I feel all of this shares the same goals as librte_power work.

[HY]
Thanks for your commetns.
You are correct. As you well know, l3fwd-power measures "how cpu cores
are busy".
And in that sense, the goal of my proposal is the same with yours .
Moreover l3fwd-power is more precise than my proposal.

Point of my proposal is
- more easy to use
- less code impact on application code

I think that if application developer wants to need to measure "how cpu
cores are busy" he/she will needs to implement
- logic similar with l3fwd-power
or
- use jobstats API

But it is rather heavy for existing applications.
By using my proposal, it is "much easier" to implement.
(But it is "rough" measurement. I think it is trade-off)
  
How do you think about the idea?

quoted

[...]

quoted

- basic API counting functionality(apistats) into librte_ethdev

Could it be it be accessible via rte_telemetry?
http://code.dpdk.org/dpdk/latest/source/lib/librte_telemetry/rte_telemetry.h

[HY]
OK.
Currently, no reason not using telemetry.

I think telemetry is useful for applications which does NOT call DPDK
API(C lang API) directly.
My patchset provide only C API to retrieve apistats.
But if assuming not all applications call C API, then I think it is
reasonable to add telemetry in addition to C API for exposing stats.

Do you think "exposure via C API" is not needed?

Hi Hideyuki,

With current implementation the change will cause a performance degradation for 
all users, even if they use the stats or not.

Even if the statics update wrapped with #ifdef, as suggested, the functionality 
is simple and it can be easily implemented by application using Rx/Tx callbacks, 
again as suggested, without introducing this new complexity.

And for more complex usage, the jobstats already tries to provide a generic 
library for it, or application specific needs can be implemented in application 
level as done is l3fwd-power.


Overall I agree the problem this patch is trying to solve is a valid one, but 
not sure about the current implementation, and I am marking current version as 
'rejected', please feel free to send a new version with a new approach.

Thanks,
ferruh