Re: [dpdk-dev] [PATCH v2 06/15] app/testpmd: add common fwd wrapper function

["Xueming(Steven) Li" <xuemingl@nvidia.com>]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, August 17, 2021 5:37 PM
> To: Xueming(Steven) Li <xuemingl@nvidia.com>
> Cc: Jack Min <jackmin@nvidia.com>; dpdk-dev <dev@dpdk.org>; Xiaoyun Li <xiaoyun.li@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 06/15] app/testpmd: add common fwd wrapper function
> 
> On Wed, Aug 11, 2021 at 7:35 PM Xueming Li <xuemingl@nvidia.com> wrote:
> >
> > From: Xiaoyu Min <jackmin@nvidia.com>
> >
> > Added an inline common wrapper function for all fwd engines which do
> > the following in common:
> >
> > 1. get_start_cycles
> > 2. rte_eth_rx_burst(...,nb_pkt_per_burst)
> > 3. if rxq_share do forward_shared_rxq(), otherwise do fwd directly 4.
> > get_end_cycle
> >
> > Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> > ---
> >  app/test-pmd/testpmd.h | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > 13141dfed9..b685ac48d6 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -1022,6 +1022,30 @@ void add_tx_dynf_callback(portid_t portid);
> > void remove_tx_dynf_callback(portid_t portid);  int
> > update_jumbo_frame_offload(portid_t portid);
> >
> > +static inline void
> > +do_burst_fwd(struct fwd_stream *fs, packet_fwd_cb fwd) {
> > +       struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> > +       uint16_t nb_rx;
> > +       uint64_t start_tsc = 0;
> > +
> > +       get_start_cycles(&start_tsc);
> > +
> > +       /*
> > +        * Receive a burst of packets and forward them.
> > +        */
> > +       nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue,
> > +                       pkts_burst, nb_pkt_per_burst);
> > +       inc_rx_burst_stats(fs, nb_rx);
> > +       if (unlikely(nb_rx == 0))
> > +               return;
> > +       if (unlikely(rxq_share > 0))
> 
> See below. It reads a global memory.
> 
> > +               forward_shared_rxq(fs, nb_rx, pkts_burst, fwd);
> > +       else
> > +               (*fwd)(fs, nb_rx, pkts_burst);
> 
> New function pointer in fastpath.
> 
> IMO, We should not create performance regression for the existing forward engine.
> Can we have a new forward engine just for shared memory testing?

Yes, fully aware of the performance concern, the global could be defined around record_core_cycles to minimize the impacts.
Based on test data, the impacts almost invisible in legacy mode.

From test perspective, better to have all forward engine to verify shared rxq, test team want to run the
regression with less impacts. Hope to have a solution to utilize all forwarding engines seamlessly.

> 
> > +       get_end_cycles(fs, start_tsc); }
> > +
> >  /*
> >   * Work-around of a compilation error with ICC on invocations of the
> >   * rte_be_to_cpu_16() function.
> > --
> > 2.25.1
> >