On Wed, Apr 10, 2024 at 9:19 AM Juraj Linkeš <juraj.lin...@pantheon.tech> wrote:
>
> On Wed, Apr 10, 2024 at 12:53 PM Luca Vizzarro <luca.vizza...@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.
> >
> > 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.
