> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Hideyuki Yamashita > Sent: Tuesday, December 22, 2020 3:50 AM > > Hello, > > Thanks for your comments. > Please see my comments inline tagged with [HY]. > > > > diff --git a/lib/librte_ethdev/rte_ethdev.h > b/lib/librte_ethdev/rte_ethdev.h > > > index f5f8919..bef9bc6 100644 > > > --- a/lib/librte_ethdev/rte_ethdev.h > > > +++ b/lib/librte_ethdev/rte_ethdev.h > > > @@ -160,6 +160,7 @@ extern "C" { > > > > > > #include "rte_ethdev_trace_fp.h" > > > #include "rte_dev_info.h" > > > +#include <rte_apistats.h> > > > > > > extern int rte_eth_dev_logtype; > > > > > > @@ -4849,6 +4850,9 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t > queue_id, > > > nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id], > > > rx_pkts, nb_pkts); > > > > > > + int lcore_id = rte_lcore_id(); > > > + rte_apicounts->rx_burst_counts[lcore_id]++;
[...] > > As a generic one: such sort of statistics can be easily collected by the > app itself. > > Either by just incrementing counters before rx/tx_burst function call > directly or > > via rx/tx callbacks. > > So I don't see any point to push it inside ethdev layer. > > Konstantin > [HY] > You are correct. > Application can do it. > But if applications want to do it, then every applicaiton needs > to do it. > The reason why I propose my patchset is to provide such > common function (count invocation of rx_burst/tx_burst) > so that application only needs to "retrieve the information". > > I think it is benefitical than all application do similar thing. > Your feedback is highly appreciated. For performance reasons, I am strongly opposed to adding anything into the ethdev rx/tx functions, unless it is absolutely critical for a large user base. I get your argument about avoiding additional application code by doing it directly in the ethdev rx/tx functions - this is the benefit that this library adds to DPDK. So as a compromise, I suggest surrounding the added code in these functions by #ifdef/#endif, so there is no performance degradation if the library is not used. Personally, I would prefer a much more advanced library for measuring CPU load and RX/TX usage. However, I can also imagine that simple DPDK applications would benefit from this library, because is easy to understand and requires nearly no effort to use. > > Thanks! > BR, > Hideyuki Yamashita > NTT TechnoCross > > > > #ifdef RTE_ETHDEV_RXTX_CALLBACKS > > > struct rte_eth_rxtx_callback *cb; > > > > > > @@ -5124,6 +5128,9 @@ rte_eth_tx_burst(uint16_t port_id, uint16_t > queue_id, > > > } > > > #endif > > > > > > + int lcore_id = rte_lcore_id(); > > > + rte_apicounts->tx_burst_counts[lcore_id]++; > > > #ifdef RTE_ETHDEV_RXTX_CALLBACKS > > > struct rte_eth_rxtx_callback *cb; > > >
