Re: [dpdk-dev] [PATCH v2] app/testpmd: fix missing set for current forward engine

[Phil Yang <Phil.Yang@arm.com>]

<snip>

> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Lijun Ou
> > Sent: Wednesday, July 1, 2020 3:43 PM
> > To: wenzhuo.lu@intel.com; beilei.xing@intel.com;
> > bernard.iremonger@intel.com; ferruh.yigit@intel.com
> > Cc: dev@dpdk.org; linuxarm@huawei.com
> > Subject: [dpdk-dev] [PATCH v2] app/testpmd: fix missing set for current
> > forward engine
> >
> > When set the forwarding mode of the pkt, it needs to update
> > the fwd_eng that it is the member of cur_fwd_config. Otherwise,
> > an error will occur when executing the set command and show
> > confg fwd.
> > the flows as follows:
> >    1. ./tespmd -w xxx -l xx -n 4 -- i
> >    2. set fwd mac
> >       testpmd> set fwd mac
> >       Set mac packet forwarding mode
> >
> >    3. show config fwd
> >       testpmd> show config fwd
> >       io packet forwarding - ports=1 - cores=1 - streams=8 -...
> >       Logical Core 1 (socket 0) forwards packets on 8 streams:
> 
> I think we need to stop the running engine then start the new engine to
> make the configuration work.
> The Testpmd can't switch forward engine on runtime. When it is forwarding
> packets, the standing operating procedures should be:
> 1. testpmd> set fwd mac
> 2. testpmd> stop
> 3. testpmd> start
> 
> We can add a patch to document this standing operating procedure.
> Or we can refactor the set_pkt_forwarding_mode API to support this
> feature. I prefer this approach.
> 
> >
> > When run the 1 ~ 3 step, it should show the packet forwarding is
> > mac.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: wenzhuo.lu@intel.com
> > Cc: beilei.xing@intel.com
> > Cc: bernard.iremonger@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > ---
> > v2:
> > -- fix the title
> > -- add fixes and Cc
> > ---
> >  app/test-pmd/config.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index a7112c9..ee0e904 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -3158,6 +3158,7 @@ set_pkt_forwarding_mode(const char
> > *fwd_mode_name)
> >  			       fwd_mode_name,
> >  			       retry_enabled == 0 ? "" : " with retry");
> >  			cur_fwd_eng = fwd_eng;
> > +			cur_fwd_config.fwd_eng = fwd_eng;
> 
> When you stop and restart the forwarding engine, it overwrites by the old
> engine.


Please check the code below. 

-                      cur_fwd_eng = fwd_eng;
+                      if (cur_fwd_config.fwd_eng != cur_fwd_eng) {
+                              cur_fwd_eng = fwd_eng;
+                              fwd_config_setup();
+                      }

If people like this fix, I can add a patch for it.


> >  			return;
> >  		}
> >  		i++;
> > --
> > 2.7.4