Re: [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config

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

> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index e65ea45058..b31f4d8dbe 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -150,14 +150,21 @@
>             "os": {
>               "$ref": "#/definitions/OS"
>             },
> -          "lcores": {
> -            "type": "string",
> -            "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
> -            "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
> -          },
> -          "memory_channels": {
> -            "type": "integer",
> -            "description": "How many memory channels to use. Optional, defaults to 1."
> +          "dpdk_config": {
> +            "type": "object",
> +            "description": "EAL arguments for DPDK execution",
> +            "properties": {
> +              "lcores": {
> +                "type": "string",
> +                "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
> +                "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
> +              },
> +              "memory_channels": {
> +                "type": "integer",
> +                "description": "How many memory channels to use. Optional, defaults to 1."
> +              }
> +            },
> +            "minimum": 1
>             },
>             "hugepages_2mb": {
>               "$ref": "#/definitions/hugepages_2mb"
> @@ -264,23 +271,15 @@
>               "description": "Optional field that allows you to skip smoke testing",
>               "type": "boolean"
>             },
> +          "vdevs": {
> +            "description": "Optional list of names of vdevs to be used in execution",

Don't forget to check these patches for execution -> test run. Also, 
while we're at it, let's add a dot to the end of this sentence.

> diff --git a/dts/framework/testbed_model/cpu.py b/dts/framework/testbed_model/cpu.py
> index a50cf44c19..cc4ca40ad9 100644
> --- a/dts/framework/testbed_model/cpu.py
> +++ b/dts/framework/testbed_model/cpu.py
> @@ -167,7 +167,6 @@ def __init__(
>   
>           # sorting by core is needed in case hyperthreading is enabled
>           self._lcores_to_filter = sorted(lcore_list, key=lambda x: x.core, reverse=not ascending)
> -        self.filter()

This is a pretty good catch.

>   
>       @abstractmethod
>       def filter(self) -> list[LogicalCore]:
> @@ -210,6 +209,8 @@ def filter(self) -> list[LogicalCore]:
>           Returns:
>               The filtered cores.
>           """
> +        if 0 in self._lcores_to_filter:
> +            self._lcores_to_filter = self._lcores_to_filter[1:]
>           sockets_to_filter = self._filter_sockets(self._lcores_to_filter)
>           filtered_lcores = []
>           for socket_to_filter in sockets_to_filter:
> @@ -328,6 +329,9 @@ def filter(self) -> list[LogicalCore]:
>           Return:
>               The filtered logical CPU cores.
>           """
> +        if 0 not in self._filter_specifier.lcore_list:
> +            self._lcores_to_filter = self._lcores_to_filter[1:]
> +

I see you actually did what I was thinking of. This should be in 2/6.

I'm not a big fan of two different implementation of what should be 
common logic. Can we pass skip_first_core to 
LogicalCoreFilter.__init__() and remove the first core from 
self._lcores_to_filter if True (it would default to False)? skip_first 
core would be determined in SutNode.__init__() and then passed downstream.


> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index a4511157b7..178535b617 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -29,6 +29,7 @@
>   from framework.settings import SETTINGS
>   from framework.utils import MesonArgs
>   
> +from .cpu import LogicalCore, LogicalCoreList
>   from .node import Node
>   from .os_session import OSSession
>   from .virtual_device import VirtualDevice
> @@ -75,6 +76,17 @@ def __init__(self, node_config: SutNodeConfiguration):
>               node_config: The SUT node's test run configuration.
>           """
>           super().__init__(node_config)

Here is where we could determine use_first_core from 
self.config.dpdk_config.lcores and pass that to self.filter_lcores().

> +        self.lcores = self.filter_lcores(LogicalCoreList(self.config.dpdk_config.lcores))
> +        if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
> +            self._logger.info(
> +                """
> +                WARNING: First core being used;
> +                using the first core is considered risky and should only
> +                be done by advanced users.
> +                """
> +            )
> +        else:
> +            self._logger.info("Not using first core")
>           self.virtual_devices = []
>           self.dpdk_prefix_list = []
>           self._build_target_config = None