Just a few minor points, otherwise this looks good. > diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst > index 47b218b2c6..71473dbb3d 100644 > --- a/doc/guides/tools/dts.rst > +++ b/doc/guides/tools/dts.rst > @@ -131,7 +131,11 @@ There are two areas that need to be set up on a System > Under Test: > > You may specify the optional hugepage configuration in the DTS config > file. > If you do, DTS will take care of configuring hugepages, > - overwriting your current SUT hugepage configuration. > + overwriting your current SUT hugepage configuration. Configuration of > hugepages via DTS > + allows only for allocation of 2MB hugepages, as doing so prevents > accidental/over > + allocation of hugepages and hugepages sizes not recommended during > runtime due to
This wording is a bit confusing. What would make sense to me is "allocation of hugepages with hugepage sizes not recommended". Or am I missing something? > + contiguous memory space requirements. Thus, if you require hugepage > + sizes not equal to 2MB, then this configuration must be done outside of > the DTS framework. > > * System under test configuration > > diff --git a/dts/conf.yaml b/dts/conf.yaml > index 8068345dd5..56c3ae6f4c 100644 > --- a/dts/conf.yaml > +++ b/dts/conf.yaml > @@ -35,7 +35,7 @@ nodes: > lcores: "" # use all the available logical cores > use_first_core: false # tells DPDK to use any physical core > memory_channels: 4 # tells DPDK to use 4 memory channels > - hugepages: # optional; if removed, will use system hugepage > configuration > + hugepages_2mb: # optional; if removed, will use system hugepage > configuration > amount: 256 I noticed this mistake I made a while back, but this looks like a good opportunity to point it out. Amount is used with uncountable nouns and hugepages is countable. We should correct this mistake (rename to number in all places, I think that's used somewhere in Linux) and this patch could be a good place to do it. Or maybe a separate one or even a separate patchset. What do you think, would you please fix this while you're making hugepage changes? > force_first_numa: false > ports: > @@ -71,7 +71,7 @@ nodes: > os_driver: rdma > peer_node: "SUT 1" > peer_pci: "0000:00:08.1" > - hugepages: # optional; if removed, will use system hugepage > configuration > + hugepages_2mb: # optional; if removed, will use system hugepage > configuration > amount: 256 > force_first_numa: false > traffic_generator: <snip> > diff --git a/dts/framework/testbed_model/node.py > b/dts/framework/testbed_model/node.py > index 74061f6262..056a031ca0 100644 > --- a/dts/framework/testbed_model/node.py > +++ b/dts/framework/testbed_model/node.py > @@ -97,6 +97,10 @@ def __init__(self, node_config: NodeConfiguration): > self.virtual_devices = [] > self._init_ports() > > + @property > + def _hugepage_default_size(self) -> int: > + return 2048 > + I'm thinking about the placement of this and I'm kinda torn between Node and OSSession. In Node, it makes sense as it basically means "no matter what sort of node we're connected to, we're going to use this hugepage size". In OSSession, it would make more sense if we need a different hugepage size based on the OS (and possibly arch, which could be passed to it). For now, leaving it in Node is probably better. We can always move it if need be. But I don't really get why it's a property. Using properties doesn't really make sense if we're not putting any logic into it. This could just as easily be self._hugepage_default_size = 2048 in __init__(). > def _init_ports(self) -> None: > self.ports = [Port(self.name, port_config) for port_config in > self.config.ports] > self.main_session.update_ports(self.ports) > @@ -266,7 +270,9 @@ def _setup_hugepages(self) -> None: > """ > if self.config.hugepages: > self.main_session.setup_hugepages( > - self.config.hugepages.amount, > self.config.hugepages.force_first_numa > + self.config.hugepages.amount, > + self._hugepage_default_size, > + self.config.hugepages.force_first_numa, > ) > > def configure_port_state(self, port: Port, enable: bool = True) -> None: > diff --git a/dts/framework/testbed_model/os_session.py > b/dts/framework/testbed_model/os_session.py > index d5bf7e0401..5d58400cbe 100644 > --- a/dts/framework/testbed_model/os_session.py > +++ b/dts/framework/testbed_model/os_session.py > @@ -345,7 +345,9 @@ def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str: > """ > > @abstractmethod > - def setup_hugepages(self, hugepage_count: int, force_first_numa: bool) > -> None: > + def setup_hugepages( > + self, hugepage_count: int, hugepage_size: int, force_first_numa: bool > + ) -> None: > """Configure hugepages on the node. > > Get the node's Hugepage Size, configure the specified count of > hugepages > @@ -353,6 +355,7 @@ def setup_hugepages(self, hugepage_count: int, > force_first_numa: bool) -> None: > > Args: > hugepage_count: Configure this many hugepages. > + hugepage_size: Configure hugepages of this size (currently not > used in the config) The dot at the end of the sentence is missing. The reminder in the parentheses is not needed, the OSSession class is decoupled from the rest of the classes (and is thus not aware of the config). > force_first_numa: If :data:`True`, configure hugepages just on > the first numa node. > """ > > -- > 2.44.0 >
