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 D6A4D45955; Tue, 10 Sep 2024 12:11:40 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9F27542D28; Tue, 10 Sep 2024 12:11:40 +0200 (CEST) Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com [209.85.167.46]) by mails.dpdk.org (Postfix) with ESMTP id 3C2DA402AB for ; Tue, 10 Sep 2024 12:11:39 +0200 (CEST) Received: by mail-lf1-f46.google.com with SMTP id 2adb3069b0e04-5365cf5de24so3828201e87.1 for ; Tue, 10 Sep 2024 03:11:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1725963098; x=1726567898; 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=oHmW1QZvcElLyULUMLV/wQI075xpSc3N9g0P1WfU/Z4=; b=Maxyou2w7HtO0Dg45InNJ5GCYJJytFms3OmM/hDV2f/WaXngYoAhQpb1BCZd0l8Y2G DRkJjAkWo4SzLVv439u5swgJ+SppUy7OB7+3CyO+6a8eVcwQaTE9Sk/FJdCSqkDzZCpf x8dswV+srwpyUwh8RM8wWIC0BTv8tgFtATKkXI3kn8cD3rUtMSb5fY636LXGmPv2mo4e 2MvsoZrSS3d+vXrvF+3aG+GEw0koAHGzU8Jr5Sj+8nvqLdrVnYwryCWaduVwJkO79dEP +hSkb9lJUNIvwmYJb34uFuRvJ/h78FwwLlcQDXIGCZ04ujnIQbDSKR10lNCkDMtBXvgp ceJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725963098; x=1726567898; 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=oHmW1QZvcElLyULUMLV/wQI075xpSc3N9g0P1WfU/Z4=; b=VSmKmLS8Y/OqBp3iZTBnXu2L1h4AgGAvcBSYr/7YAF0+cX+4mQCuGrYoEhZZK5u8yb zEmHS4+2WFkjm1iOb42+aE+hOF6gp2lz7eEilr7oc555USEE/FlrbR7Kkl/24mRD2iM6 s8i6p8AnMD2/mO+AwVfS4tf/3J64A2D539ZYbxt0U5LD5Co+wy7uxf7H7jGiqbwrb1Wg dF1jX7wOYNxCoKymTPqMOhxSwjm7kEPOxZxBLx7sTeG8deuyC5vTqpQ1/ev7IGIUpDCc zWhPaxWh4NhSz8PG0asWtXwFWE879s6AhrCxa9hWdP7f5ugUYqtJgAewIUMkXK2PNLhk XMrg== X-Gm-Message-State: AOJu0Yy1DTFz4hyrTt6uyLYUy/yspSw4PhXK8oFU7tHxOYGqmCXqmjHu 0Xa3RoEXR3RIwXS4YbPOr85Te9jcv/a4qaFNwVGtPnt8PvGcsdNFBJzKbZX9mMs= X-Google-Smtp-Source: AGHT+IEFEihQyw4+oYT20V6VjCX+lM/5RxGgpAzV4af3nW73KCKq5f0HRMABE3CtnZrE+yq9j1CuOw== X-Received: by 2002:a05:6512:15a1:b0:532:fdba:e7bc with SMTP id 2adb3069b0e04-53658810296mr6830475e87.57.1725963098287; Tue, 10 Sep 2024 03:11:38 -0700 (PDT) Received: from [10.12.0.236] (81.89.53.154.host.vnet.sk. [81.89.53.154]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a8d25830f45sm455855366b.3.2024.09.10.03.11.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 10 Sep 2024 03:11:37 -0700 (PDT) Message-ID: Date: Tue, 10 Sep 2024 12:11:36 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 1/3] dts: rework port attributes in config module To: Nicholas Pratte , yoan.picchi@foss.arm.com, luca.vizzarro@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, Honnappa.Nagarahalli@arm.com, dmarx@iol.unh.edu, jspewock@iol.unh.edu, alex.chapman@arm.com Cc: dev@dpdk.org References: <20240821184305.28028-2-npratte@iol.unh.edu> <20240821184305.28028-3-npratte@iol.unh.edu> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: <20240821184305.28028-3-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 As a general note, it's possible we should wait with these changes for the Pydantic changes which could simplify a lot of what we want to do with the config (not just this, but also the split and removing excess attributes). On 21. 8. 2024 20:43, Nicholas Pratte wrote: > The current design requires that a peer pci port is identified so that > test suites can create the correct port links. While this can work, it > also creates a lot of room for user error. Instead, devices should be > given a unique identifier which is referenced in defined test runs. > > Both defined testbeds for the SUT and TG must have an equal number of > specified ports. In each given array or ports, SUT port 0 is connected > to TG port 0, SUT port 1 is connected to TG port 1, etc. > > Bugzilla ID: 1478 > > Signed-off-by: Nicholas Pratte > --- > dts/conf.yaml | 32 ++++++------- > dts/framework/config/__init__.py | 12 +++-- > dts/framework/config/conf_yaml_schema.json | 52 +++++++++++++--------- > dts/framework/config/types.py | 19 +++++--- > 4 files changed, 69 insertions(+), 46 deletions(-) > > diff --git a/dts/conf.yaml b/dts/conf.yaml > index 7d95016e68..16214ee267 100644 > --- a/dts/conf.yaml > +++ b/dts/conf.yaml > @@ -20,10 +20,17 @@ test_runs: > # The machine running the DPDK test executable > system_under_test_node: > node_name: "SUT 1" > + test_bed: test_bed is the whole thing we're testing. Let's find a better name. We're already using unique identifiers for SUTs and TGs and referencing those with node_name, so we could reference port identifiers with port_names. > diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py > index df60a5030e..534821ed22 100644 > --- a/dts/framework/config/__init__.py > +++ b/dts/framework/config/__init__.py > @@ -151,11 +151,10 @@ class PortConfig: > """ > > node: str > + name: str > pci: str > os_driver_for_dpdk: str > os_driver: str > - peer_node: str > - peer_pci: str > > @classmethod > def from_dict(cls, node: str, d: PortConfigDict) -> Self: > @@ -487,12 +486,19 @@ def from_dict( > system_under_test_node, SutNodeConfiguration > ), f"Invalid SUT configuration {system_under_test_node}" > > - tg_name = d["traffic_generator_node"] > + tg_name = d["traffic_generator_node"]["node_name"] > assert tg_name in node_map, f"Unknown TG {tg_name} in test run {d}" > traffic_generator_node = node_map[tg_name] > assert isinstance( > traffic_generator_node, TGNodeConfiguration > ), f"Invalid TG configuration {traffic_generator_node}" > + assert len(traffic_generator_node.ports) == len( > + system_under_test_node.ports > + ), "Insufficient ports defined on nodes." This checks the number of ports specified in the nodes section, but these don't necessarily have to correspond. Consider this scenario: TG has four ports, two connected to SUT1, two connected to SUT2 With the above, we could have two different test run configurations, one with TG and SUT1 and the other with TG and SUT2. We should check that the number of SUT/TG ports specified in test_run is the same. The message should also be more precise - something like "The number of ports on SUT/TG nodes specified in test_run is different.". > + for port_name in d["system_under_test_node"]["test_bed"]: > + assert port_name in {port.name: port for port in system_under_test_node.ports} This is missing an error message. And we're also creating the dictionary for each port, the dictionary could be created before the cycle. Or we could just look at sets of ports of nodes and those specified in test_run: sut_test_run_ports = {port_name for port_name in d["system_under_test_node"]["test_bed"]} tg_test_run_ports = {port_name for port_name in d["traffic_generator_node"]["test_bed"]} assert not sut_test_run_ports - {port.name for port in system_under_test_node.ports}, "err msg" assert not tg_test_run_ports - {port.name for port in traffic_generator_node.ports}, "err msg" We should also enforce that the port name are unique across the node config. > + for port_name in d["traffic_generator_node"]["test_bed"]: > + assert port_name in {port.name: port for port in traffic_generator_node.ports} > > vdevs = ( > d["system_under_test_node"]["vdevs"] if "vdevs" in d["system_under_test_node"] else [] > diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json > index f02a310bb5..91667b01cc 100644 > --- a/dts/framework/config/conf_yaml_schema.json > +++ b/dts/framework/config/conf_yaml_schema.json > @@ -6,6 +6,10 @@ > "type": "string", > "description": "A unique identifier for a node" > }, > + "port_name": { > + "type": "string", > + "description": "A unique identifier for a node's NIC port." > + }, > "NIC": { > "type": "string", > "enum": [ > @@ -190,6 +194,24 @@ > "pmd_buffer_scatter" > ] > }, > + "test_run_node": { > + "type": "object", > + "properties": { > + "node_name": { > + "$ref": "#/definitions/node_name" > + }, > + "test_bed": { > + "type": "array", > + "items": { > + "$ref": "#/definitions/port_name" > + } > + } > + }, > + "required": [ > + "node_name", > + "test_bed" > + ] Ports are not actually strictly required, we do have some simple tests that don't require any ports (hello world and some of the smoke tests).