Re: [PATCH v4 00/10] dts: add hello world testcase

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Feb 23, 2023 at 09:27:05AM +0100, Juraj Linkeš wrote:
> On Wed, Feb 22, 2023 at 5:43 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Mon, Feb 20, 2023 at 11:13:45AM +0100, Juraj Linkeš wrote:
> > > Thanks for the comments, Bruce.
> > >
> > > On Fri, Feb 17, 2023 at 6:26 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > On Mon, Feb 13, 2023 at 04:28:36PM +0100, Juraj Linkeš wrote:
> > > > > Add code needed to run the HelloWorld testcase which just runs the hello
> > > > > world dpdk application.
> > > > >
> > > > > The patchset currently heavily refactors this original DTS code needed
> > > > > to run the testcase:
> > > > > * The whole architecture has been redone into more sensible class
> > > > >   hierarchy
> > > > > * DPDK build on the System under Test
> > > > > * DPDK eal args construction, app running and shutting down
> > > > > * Optional SUT hugepage memory configuration
> > > > > * Test runner
> > > > > * Test results
> > > > > * TestSuite class
> > > > > * Test runner parts interfacing with TestSuite
> > > > > * The HelloWorld testsuite itself
> > > > >
> > <snip>
> > >
> > > > * When running as root, things progressed further but I hit an error when
> > > >   DTS was trying to get the CPU config. No idea what is happening here,
> > > >   because running the same commands manually over ssh seemed to work fine.
> > > >   Below is the error. Any hints as to what is the problem appreciated.
> > > >
> > >
> > > I remember running into the same issue as well. I think it's related
> > > to the bracketed paste feature of some terminal emulators:
> > > https://askubuntu.com/questions/662222/why-bracketed-paste-mode-is-enabled-sporadically-in-my-terminal-screen
> > > Please try disabling it and see whether that helps.
> > > I haven't gone to great lengths to harden this part of SSH
> > > implementation as we'll be moving to Fabric (from pexpect) after this
> > > patch (which uses a mature Python SSH implementation instead of
> > > expect).
> > >
> >
> > Adding things to my environment, e.g. bashrc didn't seem to work for me,
> > but the following change fixed this particular error. Might be worth
> > including in the code to avoid others hitting an issue?
> 
> I didn't really want to modify the code that's about to be replaced,
> but this is a small and bening change, so I don't mind.
> 
> >
> > index d0863d8791..936d5f4642 100644
> > --- a/dts/framework/remote_session/remote/ssh_session.py
> > +++ b/dts/framework/remote_session/remote/ssh_session.py
> > @@ -68,6 +68,7 @@ def _connect(self) -> None:
> >
> >              self.send_expect("stty -echo", "#")
> >              self.send_expect("stty columns 1000", "#")
> > +            self.send_expect("bind 'set enable-bracketed-paste off'", "#")
> >          except Exception as e:
> >              self._logger.error(RED(str(e)))
> >              if getattr(self, "port", None):
> >
> > Unfortunately, things still aren't running correctly for me. The code gets
> > copied over and builds, and then the first hello-world test case runs ok.
> > However, things don't work after that - something seems wrong with the
> > lcore detection or filtering logic on my system.
> >
> >   File "/home/bruce/dpdk.org/dts/framework/testbed_model/hw/cpu.py", line 206, in _filter_cores
> >     raise ValueError(
> > ValueError: The amount of logical cores per core to use (1) exceeds the actual amount present. Is hyperthreading enabled?
> >
> > To the suggestion on hyperthreading, I then checked, and yes, I have HT
> > enabled on the system. Any suggestions what is wrong?
> 
> Interesting. The first test case runs hello world on all cores
> specified in conf.yaml (or all system cores if lcores is empty).
> The second one tries to run it on just one core and, interestingly,
> that fails. It's definitely related to hyperthreading, which I've
> tested a bit (or I thought so), but apparently missed something.
> 
> Looking at the code, there's something wrong when checking the number
> of lcores per core (with hyperthreading, more than 1 core per core
> could be present) requested by filter (in this case, the test case
> supplies the filter) and the lcores on the system.
> 
> I'll try to fix it and send v5 right away. If the fix doesn't work, we
> could look at what "lscpu -p=CPU,CORE,SOCKET,NODE | grep -v #" returns
> on your system. It's also captured in dts/output/suite.log. The lcore
> config in conf.yaml could also be relevant, but I assume you didn't
> change that. We could also check the test case output. It's also in
> dts/output/suite.log
> 
> >
> > BTW: suggest the following changes to the error message:
> > * s/amount/number/ - as cores are countable.
> 
> Thanks. I've used it inappropriately in a number of places.
> 
> > * "Is hyperthreading enabled?" -> "This test requires SMT/hyperthreading be
> > enabled". By asking if it's enabled, you don't make it clear whether it
> > should be enabled or not. Since I had it enabled, the question implied to
> > me that it should be disabled. It's only on reading the code I see the comment
> > that it is meant to be enabled.
> 
> I see where the confusion is. The question is just a mere suggestion
> as to where the problem could be, but the logic in code is faulty,
> leading to this unclear error message. I'll fix the logic and probably
> modify the message so it makes more sense.
> 

Thanks, if you do a new version I'm happy enough to retest today.

/Bruce