>
> +
> + used_nic_info: list[dict[str, object]] = (
> + self.test_run.ctx.sut_node.main_session.get_nic_info()
> + )
> + with open(f"{SETTINGS.output_dir}/dut_info.json", "w") as file:
> + json.dump(used_nic_info, file, indent=3)
> + file.close()
>
file.close() is redundant and can be removed.
More importantly, the info is gathered after the dut nics have been bound
to the dpdk driver. This is an issue if a driver is bound to vfio-pci since
it has limited visibility from the commands used to gather info. I tested
this with broadcom cards which showed various info fields that were empty
or None. This is resolved by moving this snippet before the DUT ports are
bound. Keep in mind to test your patches against multiple NICs,
preferably with different drivers.
> + self.logger.info(f"DUT NIC info written to:
> {SETTINGS.output_dir}/dut_info.json")
> +
> return TestRunExecution(test_run, self.result)
>
> def on_error(self, ex: BaseException) -> State | None:
> diff --git a/dts/framework/testbed_model/linux_session.py
> b/dts/framework/testbed_model/linux_session.py
> index 30c89ad70d..67eba33ce8 100644
> --- a/dts/framework/testbed_model/linux_session.py
> +++ b/dts/framework/testbed_model/linux_session.py
> @@ -197,6 +197,70 @@ def unbind_ports(self, ports: list[Port]):
> if self._lshw_net_info:
> del self._lshw_net_info
>
> + def get_nic_info(self) -> list[dict[str, object]]:
>
object may be too generic, are there issues tightening the type hinting on
this signature?
> + """Overrides :meth`~.os_session.OSSession.get_nic_info`.
> +
> + Raises:
> + ConfigurationError: If the NIC info could not be found.
> + """
> + port_data = {
> + port.get("businfo"): port for port in self._lshw_net_info if
> port.get("businfo")
> + }
> +
> + all_nic_info = []
>
all_nic_info should be type hinted
> + for port in self._config.ports:
> + pci_addr = port.pci
> +
> + command_result = self.send_command(
> + f"sudo lshw -c network -businfo | grep '{pci_addr}' | cut
> -d'@' -f1"
> + )
>
> + bus_type = (
> + command_result.stdout
> + if command_result.return_code == 0 and
> command_result.stdout
> + else None
> + )
> + if bus_type is None:
> + raise ConfigurationError(f"Unable to get bus type for
> port {pci_addr}.")
>
There is a redundant check on setting the variable and then verifying.
Since you are throwing an exception if there is no valid value, you could
do a check once and throw an error and then set the variable after with no
further checks required. This makes it more readable in my opinion but let
me know if you disagree, for example...
if command.code != 0 or command.stdout == '':
raise ConfigurationError()
bus_type = command.stdout
+ bus_info = f"{bus_type}@{pci_addr}"
> +
> + nic_port: LshwOutput | None = port_data.get(bus_info)
> + if nic_port is None:
> + raise ConfigurationError(f"Port {pci_addr} could not be
> found on the node.")
> +
> + config: LshwConfigurationOutput | None =
> nic_port.get("configuration")
> + if config is None:
> + raise ConfigurationError(
> + f"Configuration info for port {pci_addr} could not be
> found on the node."
> + )
> +
> + nic_name = nic_port.get("logicalname")
> + nic_speed = None
> + if nic_name is not None:
> + command_result = self.send_command(
> + f"ethtool {nic_name} | grep 'Speed:' | awk '{{print
> $2}}'"
> + )
> + nic_speed = (
> + command_result.stdout
> + if command_result.return_code == 0 and
> command_result.stdout
> + else None
> + )
> + if nic_speed is None:
> + self._logger.error(f"Unable to get speed for NIC:
> {pci_addr}")
>
this code can be condensed as well, redundant check but logic is good.
If the intention is to report unknown in this case, it should be mentioned
in this log message.
Also I'm not sure if this has a logger level error if everything runs fine
in this case.
Maybe info or warning level is sufficient.
> +
+ dut_json = {
> + "make": nic_port.get("vendor"),
> + "model": nic_port.get("product"),
> + "hardware version": nic_port.get("version") or "Unknown",
> + "firmware version": config.get("firmware"),
> + "deviceBusType": bus_type,
> + "deviceId": nic_port.get("serial"),
> + "pmd": config.get("driver"),
> + "speed": nic_speed or "Unknown",
> + }
> + all_nic_info.append(dut_json)
> +
> + return all_nic_info
> +
> def bind_ports_to_driver(self, ports: list[Port], driver_name: str)
> -> None:
> """Overrides :meth:`~.os_session.OSSession.bind_ports_to_driver
>
>
