[v2,3/6] dts: Self-Discovering Architecture Change
Checks
| Context |
Check |
Description |
| ci/Intel-compilation |
warning
|
apply issues
|
Commit Message
The 'arch' attribute in the conf.yaml is unnecessary, as this can be
readily discovered within the constructor of any given node. Since OS is
determined within user configuration, finding system arch can be done
both reliably and easily within the framework.
For Linux/Posix systems, the 'uname' command is used to determine system
architecture. I believe that this is posix-standard and utilizes a
standardized output.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 2 --
dts/framework/config/__init__.py | 4 ----
dts/framework/config/conf_yaml_schema.json | 12 ------------
dts/framework/config/types.py | 2 --
dts/framework/testbed_model/node.py | 4 +++-
dts/framework/testbed_model/os_session.py | 8 ++++++++
dts/framework/testbed_model/posix_session.py | 6 ++++++
7 files changed, 17 insertions(+), 21 deletions(-)
Comments
On 5. 7. 2024 19:13, Nicholas Pratte wrote:
> The 'arch' attribute in the conf.yaml is unnecessary, as this can be
> readily discovered within the constructor of any given node. Since OS is
> determined within user configuration, finding system arch can be done
> both reliably and easily within the framework.
>
> For Linux/Posix systems, the 'uname' command is used to determine system
> architecture. I believe that this is posix-standard and utilizes a
> standardized output.
From what I can tell, uname is it POSIX compliant. Let's reword this to
remove the uncertainty.
> diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
> index 79f56b289b..02277eee1f 100644
> --- a/dts/framework/testbed_model/os_session.py
> +++ b/dts/framework/testbed_model/os_session.py
> @@ -342,6 +342,14 @@ def get_node_info(self) -> NodeInfo:
> Node information.
> """
>
> + @abstractmethod
> + def get_arch_info(self) -> str:
I'd rename this to just get_arch(), as get_arch_info implies we're
getting more than just a string representation of the archirecture.
> + """Discover CPU architecture of the remote host.
The CPU architecture
Here again I am on the same level as Juraj. Repeating my previous
comments on commit subject and Bugzilla ID.
Moreover, the subject should be of imperative form according to the
contributing guidelines. In other words, the first word is always an
imperative verb. Something like this could work:
dts: enable self-discovering architecture
About the commit body indicating uncertainty ("I believe..."), there
shouldn't be space for uncertainty on the tree, but just backed-up
facts. If you are uncertain about something, make sure to find out if
the statement stands true and provide the reasoning around it.
You can find out if the call is a standard by looking up the manual[1].
In this case it doesn't mention anything except of being related to the
syscall[2], which is indeed backed up by the POSIX.1 standard. But you
can attempt to compare it to any other POSIX OS and draw some
conclusions. For example FreeBSD's manual[3], explicitly states that
their command is conform to the POSIX.2 standard. And you'll notice that
FreeBSD's has more options than Linux's. The conclusion I can gather
here is that Linux's version is not entirely conform and implements only
a subset. For our usage, this is good enough as it still falls under the
POSIX umbrella. Therefore, you can change your paragraph into something
like:
The POSIX-compliant options of the command `uname` are used
to determine the system architecture.
Hope this helps!
Best,
Luca
[1] https://man7.org/linux/man-pages/man1/uname.1.html
[2] https://man7.org/linux/man-pages/man2/uname.2.html
[3] https://man.freebsd.org/cgi/man.cgi?uname
@@ -27,7 +27,6 @@ nodes:
- name: "SUT 1"
hostname: sut1.change.me.localhost
user: dtsuser
- arch: x86_64
os: linux
lcores: "" # use all available logical cores (Skips first core)
memory_channels: 4 # tells DPDK to use 4 memory channels
@@ -52,7 +51,6 @@ nodes:
- name: "TG 1"
hostname: tg1.change.me.localhost
user: dtsuser
- arch: x86_64
os: linux
ports:
# sets up the physical link between "TG 1"@000:00:08.0 and "SUT 1"@0000:00:08.0
@@ -208,7 +208,6 @@ class NodeConfiguration:
the :class:`~framework.testbed_model.node.Node`.
password: The password of the user. The use of passwords is heavily discouraged.
Please use keys instead.
- arch: The architecture of the :class:`~framework.testbed_model.node.Node`.
os: The operating system of the :class:`~framework.testbed_model.node.Node`.
lcores: A comma delimited list of logical cores to use when running DPDK.
use_first_core: If :data:`True`, the first logical core won't be used.
@@ -220,7 +219,6 @@ class NodeConfiguration:
hostname: str
user: str
password: str | None
- arch: Architecture
os: OS
lcores: str
use_first_core: bool
@@ -257,7 +255,6 @@ def from_dict(
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
- arch=Architecture(d["arch"]),
os=OS(d["os"]),
lcores=lcores,
use_first_core=use_first_core,
@@ -271,7 +268,6 @@ def from_dict(
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
- arch=Architecture(d["arch"]),
os=OS(d["os"]),
lcores=lcores,
use_first_core=use_first_core,
@@ -6,14 +6,6 @@
"type": "string",
"description": "A unique identifier for a node"
},
- "ARCH": {
- "type": "string",
- "enum": [
- "x86_64",
- "arm64",
- "ppc64le"
- ]
- },
"OS": {
"type": "string",
"enum": [
@@ -155,9 +147,6 @@
"type": "string",
"description": "The password to use on this node. Use only as a last resort. SSH keys are STRONGLY preferred."
},
- "arch": {
- "$ref": "#/definitions/ARCH"
- },
"os": {
"$ref": "#/definitions/OS"
},
@@ -233,7 +222,6 @@
"name",
"hostname",
"user",
- "arch",
"os"
]
},
@@ -56,8 +56,6 @@ class NodeConfigDict(TypedDict):
#:
password: str
#:
- arch: str
- #:
os: str
#:
lcores: str
@@ -17,7 +17,7 @@
from ipaddress import IPv4Interface, IPv6Interface
from typing import Any, Callable, Union
-from framework.config import OS, NodeConfiguration, TestRunConfiguration
+from framework.config import OS, Architecture, NodeConfiguration, TestRunConfiguration
from framework.exception import ConfigurationError
from framework.logger import DTSLogger, get_dts_logger
from framework.settings import SETTINGS
@@ -55,6 +55,7 @@ class Node(ABC):
main_session: OSSession
config: NodeConfiguration
name: str
+ arch: Architecture
lcores: list[LogicalCore]
ports: list[Port]
_logger: DTSLogger
@@ -77,6 +78,7 @@ def __init__(self, node_config: NodeConfiguration):
self.name = node_config.name
self._logger = get_dts_logger(self.name)
self.main_session = create_session(self.config, self.name, self._logger)
+ self.arch = Architecture(self.main_session.get_arch_info())
self._logger.info(f"Connected to node: {self.name}")
@@ -342,6 +342,14 @@ def get_node_info(self) -> NodeInfo:
Node information.
"""
+ @abstractmethod
+ def get_arch_info(self) -> str:
+ """Discover CPU architecture of the remote host.
+
+ Returns:
+ Remote host CPU architecture.
+ """
+
@abstractmethod
def update_ports(self, ports: list[Port]) -> None:
"""Get additional information about ports from the operating system and update them.
@@ -295,3 +295,9 @@ def get_node_info(self) -> NodeInfo:
).stdout.split("\n")
kernel_version = self.send_command("uname -r", SETTINGS.timeout).stdout
return NodeInfo(os_release_info[0].strip(), os_release_info[1].strip(), kernel_version)
+
+ def get_arch_info(self) -> str:
+ """Overrides :meth'~.os_session.OSSession.get_arch_info'."""
+ # return str(self.send_command('arch')).stdout
+
+ return str(self.send_command("uname -m").stdout.removesuffix("\n"))