On Thu, Jun 13, 2024 at 4:21 PM Nicholas Pratte <npra...@iol.unh.edu> wrote: > > Removed use_first_core from the conf.yaml in favor of determining this > within the framework. use_first_core continue to serve a purpose in that > it is only enabled when core 0 is explicitly provided in the > configuration. Any other configuration, including "" or "any," will > omit core 0. > > Documentation reworks are included to reflect the changes made. > > Bugzilla ID: 1360 > Signed-off-by: Nicholas Pratte <npra...@iol.unh.edu> > > --- > doc/guides/tools/dts.rst | 9 +++------ > dts/conf.yaml | 3 +-- > dts/framework/config/__init__.py | 11 +++++++---- > dts/framework/config/conf_yaml_schema.json | 6 +----- > dts/framework/testbed_model/node.py | 9 +++++++++ > 5 files changed, 21 insertions(+), 17 deletions(-) > > diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst > index da85295db9..fbb5c6f17b 100644 > --- a/doc/guides/tools/dts.rst > +++ b/doc/guides/tools/dts.rst > @@ -546,15 +546,12 @@ involved in the testing. These can be defined with the > following mappings: > > +-----------------------+---------------------------------------------------------------------------------------+ > | ``os`` | The operating system of this node. See `OS`_ > for supported values. | > > +-----------------------+---------------------------------------------------------------------------------------+ > - | ``lcores`` | | (*optional*, defaults to 1) *string* – > Comma-separated list of logical | > - | | | cores to use. An empty string means use all > lcores. | > + | ``lcores`` | | (*optional*, defaults to 1 if not used) > *string* – Comma-separated list of logical |
I think just leaving this as "defaults to 1" is fine here. It's more explicit to say "if it isn't used", but just saying it defaults I think implies that enough. > + | | | cores to use. An empty string means use all > lcores except core 0. core 0 is used | > + | | | only when explicitly specified > | > | | > | > | | **Example**: ``1,2,3,4,5,18-22`` > | > > +-----------------------+---------------------------------------------------------------------------------------+ > - | ``use_first_core`` | (*optional*, defaults to ``false``) *boolean* > | > - | | > | > - | | Indicates whether DPDK should use only the > first physical core or not. | > - > +-----------------------+---------------------------------------------------------------------------------------+ > | ``memory_channels`` | (*optional*, defaults to 1) *integer* > | > | | > | > | | The number of the memory channels to use. > | <snip> > diff --git a/dts/framework/config/__init__.py > b/dts/framework/config/__init__.py > index 5a127a1207..6bc290a56a 100644 > --- a/dts/framework/config/__init__.py > +++ b/dts/framework/config/__init__.py > @@ -245,6 +245,9 @@ def from_dict( > hugepage_config_dict["force_first_numa"] = False > hugepage_config = HugepageConfiguration(**hugepage_config_dict) > > + lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in > d["lcores"] else "" > + use_first_core = "0" in lcores > + > # The calls here contain duplicated code which is here because Mypy > doesn't > # properly support dictionary unpacking with TypedDicts > if "traffic_generator" in d: > @@ -255,8 +258,8 @@ def from_dict( > password=d.get("password"), > arch=Architecture(d["arch"]), > os=OS(d["os"]), > - lcores=d.get("lcores", "1"), > - use_first_core=d.get("use_first_core", False), > + lcores=lcores, > + use_first_core=use_first_core, I wonder if we could completely remove the "use_first_core" attribute from the config classes. It seems like it's only used when you're getting remote CPUs to skip core 0 based on the condition. With these new changes it seems like we just assume that if 0 is in the list, we will use it, otherwise we simply won't, so I don't think we need this condition to detect whether we should skip or not anymore, do we? > hugepages=hugepage_config, > ports=[PortConfig.from_dict(d["name"], port) for port in > d["ports"]], > > traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]), > @@ -269,8 +272,8 @@ def from_dict( > password=d.get("password"), > arch=Architecture(d["arch"]), > os=OS(d["os"]), > - lcores=d.get("lcores", "1"), > - use_first_core=d.get("use_first_core", False), > + lcores=lcores, > + use_first_core=use_first_core, > hugepages=hugepage_config, > ports=[PortConfig.from_dict(d["name"], port) for port in > d["ports"]], > memory_channels=d.get("memory_channels", 1), <snip> > diff --git a/dts/framework/testbed_model/node.py > b/dts/framework/testbed_model/node.py > index 74061f6262..470cd18e30 100644 > --- a/dts/framework/testbed_model/node.py > +++ b/dts/framework/testbed_model/node.py > @@ -93,6 +93,15 @@ def __init__(self, node_config: NodeConfiguration): > self.lcores, LogicalCoreList(self.config.lcores) > ).filter() > > + if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores: This condition fits if you completely remove the `use_first_core` boolean from the NodeConfiguration, but if we decide not to I think it could be shortened to just: if config.use_first_core: > + self._logger.info( > + """ > + WARNING: First core being used; > + using the first core is considered risky and should only > + be done by advanced users. > + """ > + ) > + > self._other_sessions = [] > self.virtual_devices = [] > self._init_ports() > -- > 2.44.0 >