Re: [PATCH 4/6] dts: use testpmd params for scatter test suite

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

On Fri, Apr 26, 2024 at 8:06 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> On Wed, Apr 10, 2024 at 9:19 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >
> > On Wed, Apr 10, 2024 at 12:53 PM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
> > >
> > > On 09/04/2024 20:12, Juraj Linkeš wrote:
> > > >> @@ -104,16 +108,15 @@ def pmd_scatter(self, mbsize: int) -> None:
> > > >>           """
> > > >>           testpmd = self.sut_node.create_interactive_shell(
> > > >>               TestPmdShell,
> > > >> -            app_parameters=StrParams(
> > > >> -                "--mbcache=200 "
> > > >> -                f"--mbuf-size={mbsize} "
> > > >> -                "--max-pkt-len=9000 "
> > > >> -                "--port-topology=paired "
> > > >> -                "--tx-offloads=0x00008000"
> > > >> +            app_parameters=TestPmdParameters(
> > > >> +                forward_mode=TestPmdForwardingModes.mac,
> > > >> +                mbcache=200,
> > > >> +                mbuf_size=[mbsize],
> > > >> +                max_pkt_len=9000,
> > > >> +                tx_offloads=0x00008000,
> > > >>               ),
> > > >>               privileged=True,
> > > >>           )
> > > >> -        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> > > >
> > > > Jeremy, does this change the test? Instead of configuring the fw mode
> > > > after starting testpmd, we're starting testpmd with fw mode
> > > > configured.
>
> To my knowledge, as Luca mentions below, this does not functionally
> change anything about the test, scatter should just need the MAC
> forwarding mode to be set at some point before forwarding starts, it
> doesn't technically matter when. One thing to note that this does
> change, however, is that we lose the verification step that the method
> within testpmd provides. I'm not sure off the top of my head if
> testpmd just completely fails to start if the forwarding mode flag is
> set and it fails to change modes or if it still starts and then just
> goes back to default (io) which would make the test operate in an
> invalid state without anyway of knowing.
>
> As another note however, I've never seen a mode change fail and I
> don't know what could even make it fail, so this would be a rare thing
> anyway, but still just something to consider.
>

Ok, thanks. This is fine then. If we see a problem with this when
testpmd starts we can just raise a bug against testpmd (as it should
either start with mac forwarding or error).

>
> > >
> > > I am not Jeremy (please Jeremy still reply), but we discussed this on
> > > Slack. Reading through the testpmd source code, setting arguments like
> > > forward-mode in the command line, is the exact equivalent of calling
> > > `set forward mode` right after start-up. So it is equivalent in theory.
> > >
> > > > If not, we should remove the testpmd.set_forward_mode method, as it's
> > > > not used anymore.
> > >
> > > Could there be test cases that change the forward mode multiple times in
> > > the same shell, though? As this could still be needed to cover this.
> >
> > Yes, but we don't have such a test now. It's good practice to remove
> > unused code. We can still bring it back anytime, it'll be in git
> > history.