From: Patrick Robb <probb@iol.unh.edu>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: jspewock@iol.unh.edu, Honnappa.Nagarahalli@arm.com,
thomas@monjalon.net, wathsala.vithanage@arm.com,
paul.szczepanek@arm.com, dev@dpdk.org
Subject: Re: [PATCH v1 1/1] dts: bind to DPDK driver before running test suites
Date: Wed, 8 Nov 2023 11:44:55 -0500 [thread overview]
Message-ID: <CAJvnSUD_5xZRwHTb2oEgBJJbhZ0xHo35qLn40B8iisS7L7W3UA@mail.gmail.com> (raw)
In-Reply-To: <CAOb5WZZn8HLSzuKgvMc5P=ShoG1RVXNb8MnypysFiEP4_zC3pA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7205 bytes --]
On Wed, Nov 8, 2023 at 6:03 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> On Fri, Oct 27, 2023 at 12:03 AM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Modifies the current process so that we bind to os_driver_for_dpdk from
> > the configuration file before running test suites and bind back to the
> > os_driver afterwards. This allows test suites to assume that the ports
> > are bound to a DPDK supported driver or bind to either driver as needed.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> > dts/framework/remote_session/linux_session.py | 3 ++
> > dts/framework/remote_session/os_session.py | 6 ++++
> > dts/framework/testbed_model/sut_node.py | 34 +++++++++++++++++++
> > dts/tests/TestSuite_os_udp.py | 4 +++
> > dts/tests/TestSuite_smoke_tests.py | 6 ++--
> > 5 files changed, 49 insertions(+), 4 deletions(-)
> >
> > diff --git a/dts/framework/remote_session/linux_session.py
> b/dts/framework/remote_session/linux_session.py
> > index a3f1a6bf3b..7f2453c44c 100644
> > --- a/dts/framework/remote_session/linux_session.py
> > +++ b/dts/framework/remote_session/linux_session.py
> > @@ -199,3 +199,6 @@ def configure_port_ip_address(
> > def configure_ipv4_forwarding(self, enable: bool) -> None:
> > state = 1 if enable else 0
> > self.send_command(f"sysctl -w net.ipv4.ip_forward={state}",
> privileged=True)
> > +
> > + def probe_driver(self, driver_name: str) -> None:
> > + self.send_command(f"modprobe {driver_name}", verify=True)
>
> This may not be something we want to do in DTS, but rather assume it's
> already been configured and maybe add a smoke test that would test
> whether the driver is there.
>
Agreed.
> > diff --git a/dts/framework/remote_session/os_session.py
> b/dts/framework/remote_session/os_session.py
> > index 8a709eac1c..719e815ac8 100644
> > --- a/dts/framework/remote_session/os_session.py
> > +++ b/dts/framework/remote_session/os_session.py
> > @@ -282,3 +282,9 @@ def configure_ipv4_forwarding(self, enable: bool) ->
> None:
> > """
> > Enable IPv4 forwarding in the underlying OS.
> > """
> > +
> > + @abstractmethod
> > + def probe_driver(self, driver_name: str) -> None:
> > + """
> > + Load the module for the driver.
> > + """
> > diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> > index 202aebfd06..5a7dd91cac 100644
> > --- a/dts/framework/testbed_model/sut_node.py
> > +++ b/dts/framework/testbed_model/sut_node.py
> > @@ -89,6 +89,7 @@ class SutNode(Node):
> > _dpdk_version: str | None
> > _node_info: NodeInfo | None
> > _compiler_version: str | None
> > + _path_to_devbind: PurePath | None
>
> I'd add script to the variable so that it's clearer:
> self._path_to_devbind_script
>
> >
> > def __init__(self, node_config: SutNodeConfiguration):
> > super(SutNode, self).__init__(node_config)
> > @@ -105,6 +106,7 @@ def __init__(self, node_config:
> SutNodeConfiguration):
> > self._dpdk_version = None
> > self._node_info = None
> > self._compiler_version = None
> > + self._path_to_devbind = None
> > self._logger.info(f"Created node: {self.name}")
> >
> > @property
> > @@ -155,6 +157,14 @@ def compiler_version(self) -> str:
> > return ""
> > return self._compiler_version
> >
> > + @property
> > + def path_to_devbind(self) -> PurePath:
> > + if self._path_to_devbind is None:
> > + self._path_to_devbind = self.main_session.join_remote_path(
> > + self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
> > + )
> > + return self._path_to_devbind
> > +
> > def get_build_target_info(self) -> BuildTargetInfo:
> > return BuildTargetInfo(
> > dpdk_version=self.dpdk_version,
> compiler_version=self.compiler_version
> > @@ -176,6 +186,14 @@ def _set_up_build_target(
> > self._configure_build_target(build_target_config)
> > self._copy_dpdk_tarball()
> > self._build_dpdk()
> > + self.bind_ports_to_driver()
>
> This only binds the ports on the SUT node, should we also do this on
> the TG node? We may not have access to the devbind script on the TG
> node, but we can just copy it there.
>
>
Jeremy and I discussed this and I'm not sure right now. With the SUT we
have a known default behavior (bind to os_driver_for_dpdk), but for the TG
we don't (really) know yet whether there will be any needs introduced by
adding dperf, trex, ixia, etc. as traffic generators. I prefer to approach
this question when we are adding TGs for perf testing and necessarily
seeing if we need to allow for any configuration we aren't thinking of yet.
> > +
> > + def _tear_down_build_target(self) -> None:
> > + """
> > + This method exists to be optionally overwritten by derived
> classes and
> > + is not decorated so that the derived class doesn't have to use
> the decorator.
> > + """
> > + self.bind_ports_to_driver(for_dpdk=False)
>
> There are three possibilities here:
> 1. We unbind the port
> 2. We bind the port to the OS driver (like you're suggesting)
> 3. We bind the port to the state before self.bind_ports_to_driver()
>
> I don't think it's that big of a deal if we just bind it to the OS
> driver, but 3 is probably the best.
>
I don't have a strong opinion on this other than I like the conf file being
a source of truth, but 3 sounds fine too.
>
> >
> > def _configure_build_target(
> > self, build_target_config: BuildTargetConfiguration
> > @@ -389,3 +407,19 @@ def create_interactive_shell(
> > return super().create_interactive_shell(
> > shell_cls, timeout, privileged, str(eal_parameters)
> > )
> > +
> > + def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> > + """Bind all ports on the SUT to a driver.
> > +
> > + Args:
> > + for_dpdk: Boolean that, when True, binds ports to
> os_driver_for_dpdk
> > + or, when False, binds to os_driver. Defaults to True.
> > + """
> > + for port in self.ports:
> > + driver = port.os_driver_for_dpdk if for_dpdk else
> port.os_driver
> > + self.main_session.probe_driver(driver)
> > + self.main_session.send_command(
> > + f"{self.path_to_devbind} -b {driver} --force
> {port.pci}",
> > + privileged=True,
> > + verify=True,
> > + )
>
> Just a note: I was thinking about creating a dependency on the devbind
> script which is outside DTS and it actually looks like a benefit. Not
> only we don't need to implement anything, we're also testing the tool.
>
>
I think I'm missing something but as we already depend on DPDK, we know we
will have dpdk/usertools/dpdk-devbind.py, right?
[-- Attachment #2: Type: text/html, Size: 9501 bytes --]
prev parent reply other threads:[~2023-11-08 16:45 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-26 21:58 [PATCH v1 0/1] dts: Add the ability to bind ports to drivers jspewock
2023-10-26 21:58 ` [PATCH v1 1/1] dts: bind to DPDK driver before running test suites jspewock
2023-11-08 11:03 ` Juraj Linkeš
2023-11-08 16:38 ` Jeremy Spewock
2023-11-08 16:44 ` Patrick Robb [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJvnSUD_5xZRwHTb2oEgBJJbhZ0xHo35qLn40B8iisS7L7W3UA@mail.gmail.com \
--to=probb@iol.unh.edu \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=dev@dpdk.org \
--cc=jspewock@iol.unh.edu \
--cc=juraj.linkes@pantheon.tech \
--cc=paul.szczepanek@arm.com \
--cc=thomas@monjalon.net \
--cc=wathsala.vithanage@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).