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 12E5145A2C; Wed, 25 Sep 2024 14:05:11 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9295B4025D; Wed, 25 Sep 2024 14:05:10 +0200 (CEST) Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by mails.dpdk.org (Postfix) with ESMTP id 2632E400EF for ; Wed, 25 Sep 2024 14:05:09 +0200 (CEST) Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-a8d4979b843so837547266b.3 for ; Wed, 25 Sep 2024 05:05:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1727265909; x=1727870709; 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=IFoUe3Ul1+JF/sj82LsKnCC3zppjN/6RHjJbdph4qKs=; b=HWoLs/U/wHRaXOzWrk8fhxCpMKujYper2WFbZZXw8oghPwVY8uLWZiEpFvMC+odpyh ymQnucM11JkJE9QWQ4yr0ZreRd9AcwMtwMhrYMKTWjSHOIdlcfXhVjy7vnX2z7K+XRN0 SCnJEIKeLSbu2J4QOycEuGzPs999E0+lekCaL0FQFhdS6ZnkbVtSm9uWBdNx/b7xsPyH jGxHE11L8Oka3aHPQ/KEHg3K4rf6ETbgThWhL5h+D4VNOol0qU26DvKxPBeOsB03bw24 aDot0okdKCndwiy9+v+9Y50u0sLmIANVwvTjFC645QGJjDkUTo+YFjze5mslHAPFVPtM F2aQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727265909; x=1727870709; 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=IFoUe3Ul1+JF/sj82LsKnCC3zppjN/6RHjJbdph4qKs=; b=dKkbFerK0xwJmeQyvgXz/fQpsrsmUjN7HRBx+G809b9ap1CkceqnAdWxH7bwS73a0d qJXFpAomuaQO8WVrV4K21mEHcymXa6i+BfM27/+jLVYlJ7tJSLMwWXL0kjDGJY+8f0cJ THmiYKtAWU5mRZXcHWULDmWXR0t1Lcdt8cEszkcliglPxQEzR/OiUwcQi/SEg277SfF/ CtOAJxQP3rOsVJKdkJOlix/unzTzidd/LkEMlRw42Bq+IAtJmaNbzchYQ8DPqU54F1Rd taihRPmVSpMEi2qjTnIBnpaggpHOUIavNM6vVxWz8XHEENHjvgcGJnzJwiCMIBGhjRFp sx+g== X-Gm-Message-State: AOJu0Yx2Z42t/X8XMTL1sgy+EwgfVqRYFP1aAJllbVDQepcuSxkxzC+p eieTVetQOgJSHTfVHUTAOeNCw3bQAjYhaVa3Q9k/+iDOO5pFzEuTfcua66T7818= X-Google-Smtp-Source: AGHT+IHaHVg9lA0BIJbv8GRofC7TStayMPLK1rHnTJHkFpDKFnk1IEr3uDoLEW8oXLthD8Mtm0LHvw== X-Received: by 2002:a17:907:9487:b0:a86:8e3d:86e2 with SMTP id a640c23a62f3a-a93a0330c54mr239414666b.11.1727265908509; Wed, 25 Sep 2024 05:05:08 -0700 (PDT) Received: from [192.168.200.22] ([84.245.121.62]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9392f33df1sm202441766b.43.2024.09.25.05.05.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 25 Sep 2024 05:05:08 -0700 (PDT) Message-ID: <5b229d0b-768b-4cdf-b18e-c37300d93ab3@pantheon.tech> Date: Wed, 25 Sep 2024 14:05:06 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 4/5] dts: add OS abstractions for creating virtual functions To: jspewock@iol.unh.edu, npratte@iol.unh.edu, yoan.picchi@foss.arm.com, thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu, wathsala.vithanage@arm.com, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, alex.chapman@arm.com Cc: dev@dpdk.org References: <20240821191557.18744-1-jspewock@iol.unh.edu> <20240923184235.22582-1-jspewock@iol.unh.edu> <20240923184235.22582-5-jspewock@iol.unh.edu> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: <20240923184235.22582-5-jspewock@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 > diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py > @@ -210,3 +214,37 @@ def configure_ipv4_forwarding(self, enable: bool) -> None: > """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forwarding`.""" > state = 1 if enable else 0 > self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True) > + > + def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool: > + """Overrides :meth:`~.os_session.OSSession.set_num_virtual_functions`.""" > + sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", "\\:") > + curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout) > + if num > 0 and curr_num_vfs >= num: > + self._logger.info( > + f"{curr_num_vfs} VFs already configured on port {pf_port.identifier.pci} on node " > + f"{pf_port.identifier.node}." > + ) > + return False > + elif num > 0 and curr_num_vfs > 0: These two conditions could be written as: if num > 0: if curr_num_vfs >= num: return False elif curr_num_vfs > 0: raise InternalError() Maybe it's not worth the extra indent, but it's easier to understand, so I lean towards doing it this way. > + def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]: > + """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`.""" > + sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", "\\:") > + curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}/sriov_numvfs").stdout) > + if curr_num_vfs > 0: > + pci_addrs = self.send_command( > + 'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" ' > + + f"{sys_bus_path}/virtfn*/uevent", We could use a TextParser here. Not sure if it's a good fit though. > diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py > @@ -395,3 +395,43 @@ def configure_ipv4_forwarding(self, enable: bool) -> None: > + @abstractmethod > + def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool: > + """Update the number of virtual functions (VFs) on a port. > + > + It should be noted that, due to the nature of VFs, if there are already VFs that exist on > + the physical function (PF) prior to calling this function, additional ones cannot be added. > + The only way to add more VFs is to remove the existing and then set the desired amount. For > + this reason, this method will handle creation in the following order: > + > + 1. Use existing VFs on the PF if the number of existing VFs is greater than or equal to > + `num` > + 2. Throw an exception noting that VFs cannot be created if the PF has some VFs already set > + on it, but the total VFs that it has are less then `num`. > + 3. Create `num` VFs on the PF if there are none on it already > + > + Args: > + num: The number of VFs to set on the port. > + pf_port: The port to add the VFs to. > + > + Raises: > + InternalError: If `pf_port` has less than `num` VFs configured on it > + already. > + > + Returns: > + :data:`True` if this method successfully created VFs, :data:`False` if existing VFs > + were used instead. > + """ The whole docstring talks only about the creation of VFs, but we can also remove VFs with this.