| Message ID | 20200626220943.22526-3-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: > From: Phil Yang <phil.yang@arm.com> > > Add burst stats for noisy vnf mode. > > Fixes: 3c156061b938 ("app/testpmd: add noisy neighbour forwarding mode") > Cc: stable@dpdk.org > Cc: jfreimann@redhat.com > > Signed-off-by: Phil Yang <phil.yang@arm.com> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > Reviewed-by: Jens Freimann <jfreimann@redhat.com> > --- > app/test-pmd/noisy_vnf.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c > index 58c4ee925..24f286da6 100644 > --- a/app/test-pmd/noisy_vnf.c > +++ b/app/test-pmd/noisy_vnf.c > @@ -154,6 +154,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) > > 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 > if (unlikely(nb_rx == 0)) > goto flush; > fs->rx_packets += nb_rx; > @@ -164,6 +167,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) > pkts_burst, nb_rx); > if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) > nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs); > +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS > + fs->tx_burst_stats.pkt_burst_spread[nb_tx]++; > +#endif > fs->tx_packets += nb_tx; > fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx); > return; No objection to add the burst stats to more forwarding engines, but this config does not exist for the meson and make is going away. We need to figure out how to support this option with meson before spreading it out.
<snip> > Subject: Re: [PATCH v2 3/5] app/testpmd: enable burst stats for noisy vnf > mode > > On 6/26/2020 11:09 PM, Honnappa Nagarahalli wrote: > > From: Phil Yang <phil.yang@arm.com> > > > > Add burst stats for noisy vnf mode. > > > > Fixes: 3c156061b938 ("app/testpmd: add noisy neighbour forwarding > > mode") > > Cc: stable@dpdk.org > > Cc: jfreimann@redhat.com > > > > Signed-off-by: Phil Yang <phil.yang@arm.com> > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > > Reviewed-by: Jens Freimann <jfreimann@redhat.com> > > --- > > app/test-pmd/noisy_vnf.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c index > > 58c4ee925..24f286da6 100644 > > --- a/app/test-pmd/noisy_vnf.c > > +++ b/app/test-pmd/noisy_vnf.c > > @@ -154,6 +154,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) > > > > 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 > > if (unlikely(nb_rx == 0)) > > goto flush; > > fs->rx_packets += nb_rx; > > @@ -164,6 +167,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) > > pkts_burst, nb_rx); > > if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) > > nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs); > > +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS > > + fs->tx_burst_stats.pkt_burst_spread[nb_tx]++; > > +#endif > > fs->tx_packets += nb_tx; > > fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx); > > return; > > No objection to add the burst stats to more forwarding engines, but this > config does not exist for the meson and make is going away. > > We need to figure out how to support this option with meson before > spreading it out. Dharmik is working on making this a run time flag that can be enabled or disabled from command line (as was suggested in other emails). That would remove the requirement on meson. This compile time flag existed before and this patch fixes a bug. IMO, we should separate the issue of how to enable this flag using meson from this bug fix.
On 7/6/2020 5:59 PM, Honnappa Nagarahalli wrote: > <snip> > >> Subject: Re: [PATCH v2 3/5] app/testpmd: enable burst stats for noisy vnf >> mode >> >> On 6/26/2020 11:09 PM, Honnappa Nagarahalli wrote: >>> From: Phil Yang <phil.yang@arm.com> >>> >>> Add burst stats for noisy vnf mode. >>> >>> Fixes: 3c156061b938 ("app/testpmd: add noisy neighbour forwarding >>> mode") >>> Cc: stable@dpdk.org >>> Cc: jfreimann@redhat.com >>> >>> Signed-off-by: Phil Yang <phil.yang@arm.com> >>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> >>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> >>> Reviewed-by: Jens Freimann <jfreimann@redhat.com> >>> --- >>> app/test-pmd/noisy_vnf.c | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c index >>> 58c4ee925..24f286da6 100644 >>> --- a/app/test-pmd/noisy_vnf.c >>> +++ b/app/test-pmd/noisy_vnf.c >>> @@ -154,6 +154,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) >>> >>> 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 >>> if (unlikely(nb_rx == 0)) >>> goto flush; >>> fs->rx_packets += nb_rx; >>> @@ -164,6 +167,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) >>> pkts_burst, nb_rx); >>> if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) >>> nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs); >>> +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS >>> + fs->tx_burst_stats.pkt_burst_spread[nb_tx]++; >>> +#endif >>> fs->tx_packets += nb_tx; >>> fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx); >>> return; >> >> No objection to add the burst stats to more forwarding engines, but this >> config does not exist for the meson and make is going away. >> >> We need to figure out how to support this option with meson before >> spreading it out. > Dharmik is working on making this a run time flag that can be enabled or disabled from command line (as was suggested in other emails). That would remove the requirement on meson. Cool, thanks. Based on that work, shouldn't we need to update these lines again, instead why not do the update after Dharmik's patch (or in that patch) based on what the new way is? > > This compile time flag existed before and this patch fixes a bug. IMO, we should separate the issue of how to enable this flag using meson from this bug fix. > I know this flag exists before, but what is the bug this patch fixes? I thought this patch enables burst stat for "noisy vnf" forwarding engine.
<snip> > > > >> Subject: Re: [PATCH v2 3/5] app/testpmd: enable burst stats for noisy > >> vnf mode > >> > >> On 6/26/2020 11:09 PM, Honnappa Nagarahalli wrote: > >>> From: Phil Yang <phil.yang@arm.com> > >>> > >>> Add burst stats for noisy vnf mode. > >>> > >>> Fixes: 3c156061b938 ("app/testpmd: add noisy neighbour forwarding > >>> mode") > >>> Cc: stable@dpdk.org > >>> Cc: jfreimann@redhat.com > >>> > >>> Signed-off-by: Phil Yang <phil.yang@arm.com> > >>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > >>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > >>> Reviewed-by: Jens Freimann <jfreimann@redhat.com> > >>> --- > >>> app/test-pmd/noisy_vnf.c | 12 ++++++++++++ > >>> 1 file changed, 12 insertions(+) > >>> > >>> diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c > >>> index > >>> 58c4ee925..24f286da6 100644 > >>> --- a/app/test-pmd/noisy_vnf.c > >>> +++ b/app/test-pmd/noisy_vnf.c > >>> @@ -154,6 +154,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) > >>> > >>> 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 > >>> if (unlikely(nb_rx == 0)) > >>> goto flush; > >>> fs->rx_packets += nb_rx; > >>> @@ -164,6 +167,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) > >>> pkts_burst, nb_rx); > >>> if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) > >>> nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs); > >>> +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS > >>> + fs->tx_burst_stats.pkt_burst_spread[nb_tx]++; > >>> +#endif > >>> fs->tx_packets += nb_tx; > >>> fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx); > >>> return; > >> > >> No objection to add the burst stats to more forwarding engines, but > >> this config does not exist for the meson and make is going away. > >> > >> We need to figure out how to support this option with meson before > >> spreading it out. > > Dharmik is working on making this a run time flag that can be enabled or > disabled from command line (as was suggested in other emails). That would > remove the requirement on meson. > > Cool, thanks. > Based on that work, shouldn't we need to update these lines again, instead > why not do the update after Dharmik's patch (or in that patch) based on what > the new way is? Yes, agree, these lines need to be updated. I am fine to take this route as well. > > > > > This compile time flag existed before and this patch fixes a bug. IMO, we > should separate the issue of how to enable this flag using meson from this > bug fix. > > > > I know this flag exists before, but what is the bug this patch fixes? I thought > this patch enables burst stat for "noisy vnf" forwarding engine. If one enables 'RTE_TEST_PMD_RECORD_BURST_STATS' expecting the burst stats for 'noisy vnf' engine, it is not reported today.
diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c index 58c4ee925..24f286da6 100644 --- a/app/test-pmd/noisy_vnf.c +++ b/app/test-pmd/noisy_vnf.c @@ -154,6 +154,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) 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 if (unlikely(nb_rx == 0)) goto flush; fs->rx_packets += nb_rx; @@ -164,6 +167,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) pkts_burst, nb_rx); if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs); +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS + fs->tx_burst_stats.pkt_burst_spread[nb_tx]++; +#endif fs->tx_packets += nb_tx; fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx); return; @@ -187,6 +193,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) nb_deqd); if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs); +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS + fs->tx_burst_stats.pkt_burst_spread[nb_tx]++; +#endif fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, nb_tx); } } @@ -211,6 +220,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) tmp_pkts, nb_deqd); if (unlikely(sent < nb_deqd) && fs->retry_enabled) nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs); +#ifdef RTE_TEST_PMD_RECORD_BURST_STATS + fs->tx_burst_stats.pkt_burst_spread[nb_tx]++; +#endif fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, sent); ncf->prev_time = rte_get_timer_cycles(); }