diff mbox series

[v2,2/5] app/testpmd: enable burst stats for flowgen mode rx path

Message ID 20200626220943.22526-2-honnappa.nagarahalli@arm.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [v2,1/5] app/testpmd: clock gettime call in throughput calculation |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Honnappa Nagarahalli June 26, 2020, 10:09 p.m. UTC 
  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(+)

Comments

Ferruh Yigit July 6, 2020, 4:02 p.m. UTC | #1 
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?
Honnappa Nagarahalli July 6, 2020, 5:06 p.m. UTC | #2 
<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?
Ferruh Yigit July 6, 2020, 5:17 p.m. UTC | #3 
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.
Honnappa Nagarahalli July 6, 2020, 6:46 p.m. UTC | #4 
<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 mbox series

Patch

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++)