| Message ID | 20200626220943.22526-2-honnappa.nagarahalli@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/5] app/testpmd: clock gettime call in throughput calculation | expand |
On 6/26/2020 11:09 PM, Honnappa Nagarahalli wrote: > Enable missing burst stats for rx path for flowgen mode. > > Fixes: e9e23a617eb8 ("app/testpmd: add flowgen forwarding engine") > Cc: stable@dpdk.org > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > Reviewed-by: Phil Yang <phil.yang@arm.com> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > Tested-by: Phil Yang <phil.yang@arm.com> > --- > app/test-pmd/flowgen.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c > index 4bd351e67..011887c61 100644 > --- a/app/test-pmd/flowgen.c > +++ b/app/test-pmd/flowgen.c > @@ -111,6 +111,10 @@ pkt_burst_flow_gen(struct fwd_stream *fs) > /* Receive a burst of packets and discard them. */ > nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst, > nb_pkt_per_burst); > +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS > + fs->rx_burst_stats.pkt_burst_spread[nb_rx]++; > +#endif > + Flow generator is for Tx, what is the point of getting stats for the Rx?
<snip> > Subject: Re: [PATCH v2 2/5] app/testpmd: enable burst stats for flowgen mode > rx path > > On 6/26/2020 11:09 PM, Honnappa Nagarahalli wrote: > > Enable missing burst stats for rx path for flowgen mode. > > > > Fixes: e9e23a617eb8 ("app/testpmd: add flowgen forwarding engine") > > Cc: stable@dpdk.org > > > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > > Reviewed-by: Phil Yang <phil.yang@arm.com> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > > Tested-by: Phil Yang <phil.yang@arm.com> > > --- > > app/test-pmd/flowgen.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c index > > 4bd351e67..011887c61 100644 > > --- a/app/test-pmd/flowgen.c > > +++ b/app/test-pmd/flowgen.c > > @@ -111,6 +111,10 @@ pkt_burst_flow_gen(struct fwd_stream *fs) > > /* Receive a burst of packets and discard them. */ > > nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst, > > nb_pkt_per_burst); > > +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS > > + fs->rx_burst_stats.pkt_burst_spread[nb_rx]++; > > +#endif > > + > > Flow generator is for Tx, what is the point of getting stats for the Rx? Is there any reason for this mode to receive the packets? The reason I ask is, the core cycles include the RX call as well. So, we have cycles/packet that includes the RX call and burst stats that does not show the RX burst stats. Do we need to remove the call to rte_eth_rx_burst API?
On 7/6/2020 6:06 PM, Honnappa Nagarahalli wrote: > <snip> > >> Subject: Re: [PATCH v2 2/5] app/testpmd: enable burst stats for flowgen mode >> rx path >> >> On 6/26/2020 11:09 PM, Honnappa Nagarahalli wrote: >>> Enable missing burst stats for rx path for flowgen mode. >>> >>> Fixes: e9e23a617eb8 ("app/testpmd: add flowgen forwarding engine") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> >>> Reviewed-by: Phil Yang <phil.yang@arm.com> >>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> >>> Tested-by: Phil Yang <phil.yang@arm.com> >>> --- >>> app/test-pmd/flowgen.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c index >>> 4bd351e67..011887c61 100644 >>> --- a/app/test-pmd/flowgen.c >>> +++ b/app/test-pmd/flowgen.c >>> @@ -111,6 +111,10 @@ pkt_burst_flow_gen(struct fwd_stream *fs) >>> /* Receive a burst of packets and discard them. */ >>> nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst, >>> nb_pkt_per_burst); >>> +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS >>> + fs->rx_burst_stats.pkt_burst_spread[nb_rx]++; >>> +#endif >>> + >> >> Flow generator is for Tx, what is the point of getting stats for the Rx? > Is there any reason for this mode to receive the packets? > The reason I ask is, the core cycles include the RX call as well. So, we have cycles/packet that includes the RX call and burst stats that does not show the RX burst stats. > Do we need to remove the call to rte_eth_rx_burst API? > The implementation receives packets and frees them immediately. I assume the Rx is implemented in case port receives packets, like if other end forwards the generated packets back to same port. Yes it consumes cycles for Rx but received packets are ignored, I am not sure if received packets have any meaning in flowgen to get additional burst stats.
<snip> > > > >> Subject: Re: [PATCH v2 2/5] app/testpmd: enable burst stats for > >> flowgen mode rx path > >> > >> On 6/26/2020 11:09 PM, Honnappa Nagarahalli wrote: > >>> Enable missing burst stats for rx path for flowgen mode. > >>> > >>> Fixes: e9e23a617eb8 ("app/testpmd: add flowgen forwarding engine") > >>> Cc: stable@dpdk.org > >>> > >>> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > >>> Reviewed-by: Phil Yang <phil.yang@arm.com> > >>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > >>> Tested-by: Phil Yang <phil.yang@arm.com> > >>> --- > >>> app/test-pmd/flowgen.c | 4 ++++ > >>> 1 file changed, 4 insertions(+) > >>> > >>> diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c index > >>> 4bd351e67..011887c61 100644 > >>> --- a/app/test-pmd/flowgen.c > >>> +++ b/app/test-pmd/flowgen.c > >>> @@ -111,6 +111,10 @@ pkt_burst_flow_gen(struct fwd_stream *fs) > >>> /* Receive a burst of packets and discard them. */ > >>> nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst, > >>> nb_pkt_per_burst); > >>> +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS > >>> + fs->rx_burst_stats.pkt_burst_spread[nb_rx]++; > >>> +#endif > >>> + > >> > >> Flow generator is for Tx, what is the point of getting stats for the Rx? > > Is there any reason for this mode to receive the packets? > > The reason I ask is, the core cycles include the RX call as well. So, we have > cycles/packet that includes the RX call and burst stats that does not show the > RX burst stats. > > Do we need to remove the call to rte_eth_rx_burst API? > > > > The implementation receives packets and frees them immediately. > I assume the Rx is implemented in case port receives packets, like if other end > forwards the generated packets back to same port. I tried to find more info, there is not a lot. The original commit message says 'it is similar to 'txonly' engine, but will generate multiple L4 flows'. Your assumption seems reasonable to me. In that case, I think it makes sense to include 'core cycles' only for TX. Otherwise, there is not enough information to debug any issues. > Yes it consumes cycles for Rx but received packets are ignored, I am not sure if > received packets have any meaning in flowgen to get additional burst stats. Please see above, I think we need to adjust how the 'core cycles' are counted in this case.
diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c index 4bd351e67..011887c61 100644 --- a/app/test-pmd/flowgen.c +++ b/app/test-pmd/flowgen.c @@ -111,6 +111,10 @@ pkt_burst_flow_gen(struct fwd_stream *fs) /* Receive a burst of packets and discard them. */ nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst, nb_pkt_per_burst); +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS + fs->rx_burst_stats.pkt_burst_spread[nb_rx]++; +#endif + fs->rx_packets += nb_rx; for (i = 0; i < nb_rx; i++)
Enable missing burst stats for rx path for flowgen mode. Fixes: e9e23a617eb8 ("app/testpmd: add flowgen forwarding engine") Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> Tested-by: Phil Yang <phil.yang@arm.com> --- app/test-pmd/flowgen.c | 4 ++++ 1 file changed, 4 insertions(+) -- 2.17.1