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

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

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.

>
> /Bruce