From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E885345913; Tue, 10 Sep 2024 15:34:13 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B76AA42DCD; Tue, 10 Sep 2024 15:34:13 +0200 (CEST) Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) by mails.dpdk.org (Postfix) with ESMTP id 8C21B402AB for ; Tue, 10 Sep 2024 15:34:12 +0200 (CEST) Received: by mail-lj1-f175.google.com with SMTP id 38308e7fff4ca-2f75428b9f8so907181fa.3 for ; Tue, 10 Sep 2024 06:34:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1725975252; x=1726580052; darn=dpdk.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=KOKcMEfF2W4LiErKIdXhnycNmMBz6dsIUYRsnGBXKHU=; b=iOjQ7Awkv3cAl184n6MehcAEVmPhjTuAwH9iw//VAZTH9gmiT5goWoTnIjVA5wzGOc OYE9MoPe5/ou3vl2rus9UP6U6+sYYiFiKA1LCGAaEBWOsR2G1XouMpYYCLc/gvaWdJsC RX1YtuLBGjlER5A3YZrPLdPzH0GYd3MzSk2wNHVP+2V3H45/R4BQ33HNegyon+pCGQC3 Su6Po9EBB4GzZBtY7NWv268P/oHDl58Fb3sVnz6gAQahv/bjRaQvrLZXhy8cX9DKWoeG I3Ba/qOywWvOTRiuLer3aXvUCBzG8ghSV1s36S3/XBVwRjkG36WUqQ10jGz6bipmkRNk hc+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725975252; x=1726580052; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=KOKcMEfF2W4LiErKIdXhnycNmMBz6dsIUYRsnGBXKHU=; b=SGIgKP/PWxSz3gxK1K+k6f6fdanU+84xIKIgIGL+FFS5qIKug/2j6lkf5LzO/6QOEa lL6GmPMOusN3+urx64RN1nU9Alp1K9W0KFszLr5gH17q6niv/Dq83qdWa7FvBBRuTj3M uybeGTbr0BTXKKwuZUhus8Sn+98Md6MgBiTGsybHoOmnZgBwUV3byH6Or5ycdrTDJYQ2 dxa9Kj6qHromGn6RKgusG3uH9tmhL+cfrBuAP0KRHokUujOCa3Y29nN9c+xSbCQOO168 n8t4nOjGKgp6/YrDSa/D881E3X3CzY094/8gX5tfHelYT6/J1iFjMBiVrCoqWxQUVLWM K7mg== X-Gm-Message-State: AOJu0YyTqNyIClcYajlhWpwDNhHo0fOk3IAtueueF/qM0EozcXdjsuPs gXOoZ6yIpmSCw5+2waWgia3hSxSCvxJyq1MDxB9wHSK2W9UuV9ZT+4lEVJgvMtY= X-Google-Smtp-Source: AGHT+IFNtC7p17Jjx3uAowoDWctNGum7VAt0hOZKi7hW41bfNun7glmdpgqqZhVuj0Ouenl75/iJlw== X-Received: by 2002:a05:6512:3d20:b0:52c:deb9:904b with SMTP id 2adb3069b0e04-536587f8770mr10966493e87.38.1725975251468; Tue, 10 Sep 2024 06:34:11 -0700 (PDT) Received: from [192.168.0.113] ([84.245.121.62]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a8d25c72e49sm483212566b.127.2024.09.10.06.34.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 10 Sep 2024 06:34:11 -0700 (PDT) Message-ID: <9b8d2e3c-0008-4ba3-b93e-0a606822757c@pantheon.tech> Date: Tue, 10 Sep 2024 15:34:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 2/6] dts: Use First Core Logic Change To: Nicholas Pratte , probb@iol.unh.edu, dmarx@iol.unh.edu, jspewock@iol.unh.edu, luca.vizzarro@arm.com, yoan.picchi@foss.arm.com, Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com Cc: dev@dpdk.org References: <20240613201831.9748-3-npratte@iol.unh.edu> <20240705171341.23894-6-npratte@iol.unh.edu> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: <20240705171341.23894-6-npratte@iol.unh.edu> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 5. 7. 2024 19:13, Nicholas Pratte 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 > --- > 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 +++++++++ > 4 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/dts/conf.yaml b/dts/conf.yaml > index 56cc08ced2..53192e0761 100644 > --- a/dts/conf.yaml > +++ b/dts/conf.yaml > @@ -29,8 +29,7 @@ nodes: > user: dtsuser > arch: x86_64 > os: linux > - lcores: "" # use all the available logical cores > - use_first_core: false # tells DPDK to use any physical core > + lcores: "" # use all available logical cores (Skips first core) The message in parentheses could be confusing, let's make it explicit that an empty string skips first core. And the comments in this file are all lowercase so let's do that in parentheses as well. > memory_channels: 4 # tells DPDK to use 4 memory channels > hugepages_2mb: # optional; if removed, will use system hugepage configuration > number_of: 256 > diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py > index 456a8a83ab..4c05373ef3 100644 > --- a/dts/framework/config/__init__.py > +++ b/dts/framework/config/__init__.py > @@ -246,6 +246,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 > + I'm wondering whether we want to do this in config. The logic seems to be better placed elsewhere. My thinking is this: 1. Removing use_first_core from Node._get_remote_cpus() (and the downstream OSSession methods) makes sense since the method really looks like it should be getting all remote CPUs. A benefit here is the first core filtering doesn't happen in OSSession where it currently is - it just doesn't make sense there. 2. Putting use_first_core to LogicalCoreList also doesn't make much sense, it's supposed to be a list and not supposed to do any filtering. 3. The two above are used to compose the final list of usable cores on a node. We could either filter the first core from 1. after getting the cores, we could filter it from 2. after getting the core list or filter it after using 1. and 2. with LogicalCoreListFilter(). Or in other words: remote_lcores = self.main_session.get_remote_cpus() lcore_list_config = LogicalCoreList(self.config.lcores) self.lcores = LogicalCoreListFilter(remote_lcores, lcore_list_config).filter() We can either remove the first core from remote_lcores, lcore_list_config or from self.lcores. The result is the same if we remove it from any of these, so maybe we just work with self.lcores as that is also what you've used in the patch to issue the warning. The logic of whether to remove the core or not would be in Node (if self.config.lcores is either "" or "any"). > diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json > index 3f7bc2acae..01a6afdc72 100644 > --- a/dts/framework/config/conf_yaml_schema.json > +++ b/dts/framework/config/conf_yaml_schema.json > @@ -163,13 +163,9 @@ > }, > "lcores": { > "type": "string", > - "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$", > + "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any", This should be added to the description. > "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." > }, > diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py > index 12a40170ac..9b3f01f1e9 100644 > --- a/dts/framework/testbed_model/node.py > +++ b/dts/framework/testbed_model/node.py > @@ -86,6 +86,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: > + self._logger.info( If this is a warning, then we should call self._logger.warning() > + """ > + WARNING: First core being used; > + using the first core is considered risky and should only > + be done by advanced users. > + """ > + ) > + > self._other_sessions = [] > self._init_ports() >