> -----Original Message-----
> From: Li, Xiaoyun <xiaoyun...@intel.com>
> Sent: Saturday, September 18, 2021 4:31 PM
> To: Zhang, AlvinX <alvinx.zh...@intel.com>; Ananyev, Konstantin
> <konstantin.anan...@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v3 1/2] app/testpmd: update forward engine beginning
>
> Hi
>
> > -----Original Message-----
> > From: Zhang, AlvinX <alvinx.zh...@intel.com>
> > Sent: Saturday, September 18, 2021 11:07
> > To: Li, Xiaoyun <xiaoyun...@intel.com>; Ananyev, Konstantin
> > <konstantin.anan...@intel.com>
> > Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zh...@intel.com>
> > Subject: [PATCH v3 1/2] app/testpmd: update forward engine beginning
> >
> > For each forward engine, there may be some special conditions must be
> > meet
>
> met
Hi Xiaoyun,
Thanks for your attentively reviewing. I will update them in V4.
>
> > before the forwarding run.
>
> is running / runs
>
> >
> > Adding checks for these conditions in configuring is not suitable,
> > because one condition may rely on multiple configurations, and the
> > conditions required by each forward engine is not general.
> >
> > The best solution is each forward engine has a callback to check
> > whether these conditions are meet, and then testpmd can call the
> > callback to determine
>
> met
>
> > whether the forwarding can be started.
> >
> > There was a void callback 'port_fwd_begin' in forward engine, it did
> > some initialization for forwarding, this patch updates it's return
> > type, then we can add
>
> its return value then we can
>
> > some checks in it to confirm whether the forwarding can be started. In
> > addition, this patch puts the calling part of the callback up to
> > before some forwarding related status being set.
>
> this patch calls this callback before the forwarding stats is reset and then
> launches the forwarding engine.
>
> Also the cc stable issue I state in the 2nd patch.
> But overall the patch looks good to me.
>
> BRs
> Xiaoyun
>
> >
> > Signed-off-by: Alvin Zhang <alvinx.zh...@intel.com>
> > ---
> > app/test-pmd/flowgen.c | 3 ++-
> > app/test-pmd/ieee1588fwd.c | 3 ++-
> > app/test-pmd/noisy_vnf.c | 4 +++-
> > app/test-pmd/testpmd.c | 38
> ++++++++++++++++++++++++++------------
> > app/test-pmd/testpmd.h | 2 +-
> > app/test-pmd/txonly.c | 3 ++-
> > 6 files changed, 36 insertions(+), 17 deletions(-)
> >
> > diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c index
> > 0d3664a..83234fc 100644
> > --- a/app/test-pmd/flowgen.c
> > +++ b/app/test-pmd/flowgen.c
> > @@ -201,10 +201,11 @@
> > get_end_cycles(fs, start_tsc);
> > }
> >
> > -static void
> > +static int
> > flowgen_begin(portid_t pi)
> > {
> > printf(" number of flows for port %u: %d\n", pi, nb_flows_flowgen);
> > + return 0;
> > }
> >
> > struct fwd_engine flow_gen_engine = { diff --git
> > a/app/test-pmd/ieee1588fwd.c b/app/test-pmd/ieee1588fwd.c index
> > 034f238..81624a7 100644
> > --- a/app/test-pmd/ieee1588fwd.c
> > +++ b/app/test-pmd/ieee1588fwd.c
> > @@ -198,10 +198,11 @@ struct ptpv2_msg {
> > port_ieee1588_tx_timestamp_check(fs->rx_port);
> > }
> >
> > -static void
> > +static int
> > port_ieee1588_fwd_begin(portid_t pi)
> > {
> > rte_eth_timesync_enable(pi);
> > + return 0;
> > }
> >
> > static void
> > diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c index
> > 382a4c2..e4434be 100644
> > --- a/app/test-pmd/noisy_vnf.c
> > +++ b/app/test-pmd/noisy_vnf.c
> > @@ -231,7 +231,7 @@ struct noisy_config {
> > rte_free(noisy_cfg[pi]);
> > }
> >
> > -static void
> > +static int
> > noisy_fwd_begin(portid_t pi)
> > {
> > struct noisy_config *n;
> > @@ -273,6 +273,8 @@ struct noisy_config {
> > rte_exit(EXIT_FAILURE,
> > "--noisy-lkup-memory-size must be > 0\n");
> > }
> > +
> > + return 0;
> > }
> >
> > struct fwd_engine noisy_vnf_engine = { diff --git
> > a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 97ae52e..0345b2e 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2172,16 +2172,10 @@ struct extmem_param { static void
> > launch_packet_forwarding(lcore_function_t *pkt_fwd_on_lcore) {
> > - port_fwd_begin_t port_fwd_begin;
> > unsigned int i;
> > unsigned int lc_id;
> > int diag;
> >
> > - port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
> > - if (port_fwd_begin != NULL) {
> > - for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++)
> > - (*port_fwd_begin)(fwd_ports_ids[i]);
> > - }
> > for (i = 0; i < cur_fwd_config.nb_fwd_lcores; i++) {
> > lc_id = fwd_lcores_cpuids[i];
> > if ((interactive == 0) || (lc_id != rte_lcore_id())) { @@
> > -2227,10
> > +2221,35 @@ struct extmem_param {
> > fprintf(stderr, "Packet forwarding already started\n");
> > return;
> > }
> > - test_done = 0;
> >
> > fwd_config_setup();
> >
> > + port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
> > + if (port_fwd_begin != NULL) {
> > + for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
> > + if (port_fwd_begin(fwd_ports_ids[i])) {
> > + fprintf(stderr,
> > + "Packet forwarding not ready\n");
> > + return;
> > + }
> > + }
> > + }
> > +
> > + if (with_tx_first) {
> > + port_fwd_begin = tx_only_engine.port_fwd_begin;
> > + if (port_fwd_begin != NULL) {
> > + for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
> > + if (port_fwd_begin(fwd_ports_ids[i])) {
> > + fprintf(stderr,
> > + "Packet forwarding not
> > ready\n");
> > + return;
> > + }
> > + }
> > + }
> > + }
> > +
> > + test_done = 0;
> > +
> > if(!no_flush_rx)
> > flush_fwd_rx_queues();
> >
> > @@ -2239,11 +2258,6 @@ struct extmem_param {
> >
> > fwd_stats_reset();
> > if (with_tx_first) {
> > - port_fwd_begin = tx_only_engine.port_fwd_begin;
> > - if (port_fwd_begin != NULL) {
> > - for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++)
> > - (*port_fwd_begin)(fwd_ports_ids[i]);
> > - }
> > while (with_tx_first--) {
> > launch_packet_forwarding(
> > run_one_txonly_burst_on_core);
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > 5863b2f..e9d9db0 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -268,7 +268,7 @@ struct fwd_lcore {
> > * Forwards packets unchanged on the same port.
> > * Check that sent IEEE1588 PTP packets are timestamped by the
> hardware.
> > */
> > -typedef void (*port_fwd_begin_t)(portid_t pi);
> > +typedef int (*port_fwd_begin_t)(portid_t pi);
> > typedef void (*port_fwd_end_t)(portid_t pi); typedef void
> > (*packet_fwd_t)(struct fwd_stream *fs);
> >
> > diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c index
> > aed820f..386a4ff 100644
> > --- a/app/test-pmd/txonly.c
> > +++ b/app/test-pmd/txonly.c
> > @@ -435,7 +435,7 @@
> > get_end_cycles(fs, start_tsc);
> > }
> >
> > -static void
> > +static int
> > tx_only_begin(portid_t pi)
> > {
> > uint16_t pkt_data_len;
> > @@ -467,6 +467,7 @@
> > timestamp_init_req++;
> > /* Make sure all settings are visible on forwarding cores.*/
> > rte_wmb();
> > + return 0;
> > }
> >
> > struct fwd_engine tx_only_engine = {
> > --
> > 1.8.3.1
BRs
Alvin
