On Fri, Apr 26, 2024 at 8:06 PM Jeremy Spewock <jspew...@iol.unh.edu> wrote:
>
> 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.
>
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.
