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 1CB36457BD; Mon, 26 Aug 2024 19:24:40 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C547400EF; Mon, 26 Aug 2024 19:24:40 +0200 (CEST) Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) by mails.dpdk.org (Postfix) with ESMTP id 832AA4003C for ; Mon, 26 Aug 2024 19:24:38 +0200 (CEST) Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-2d3c0d587e4so3145963a91.2 for ; Mon, 26 Aug 2024 10:24:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1724693077; x=1725297877; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=MB9T5ROciZVo+yGi6pIEVqdDl2bnOe2uyMk8h4fnhFM=; b=MMiQg4tYrTM4slS9ZovvlbUkShRcN789W5gzzR07ZE7/H+hAue+Y4l6iKMTQVQ2ehb pYVFJ+s3WfZIs4E8wH1ia0j7Q5OBcvSG2d+yxm8IVIh7KCFY5OX2H/H4ziZQirnQbZub fy4i9ncoRMq4EcZmyHAny4AAmnggTn5ObzfH0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724693077; x=1725297877; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=MB9T5ROciZVo+yGi6pIEVqdDl2bnOe2uyMk8h4fnhFM=; b=h1EbbE/WTioM5ve/91bKnZ+9gcFkUwXVy798tfqAEb7GxPYeAG6N+qeN28FvMLz6ZX CqSD9sL6+OjSZQfnpXcnnVOAqqAugBFHjg1+jKPH2hh9lZDc0iGZXeSWP2fNbRthirS2 9gvqWWRkBIdkSG/iY+wsL03GcL2jKFNXUFlLPUTWPkZra9DCdqAIAT2I0ckDUaIcSBwr DBRAtEt1XBzuYMzSIvAXzwZBq7DkvpS+VvW8+fUnlIVwCmNCQLqx6ZW7XpvRqNRiqquA 3BC4Q9XoI6ZPFhRyqFJLqp+R7UTMxpQD4OzqLMAFTqRaqdPmAtWoaEoZtZ5fzX2MBZbs rQUw== X-Forwarded-Encrypted: i=1; AJvYcCVIVIOOSiL3CbNqUYcC5seaBmCVQPMGadgR2YjjFzM6/KQo8jNeNmbecgUw/1b0NlCuN/U=@dpdk.org X-Gm-Message-State: AOJu0Yz0EjmDz9Ez8SjoktZqDqYmF9Bq5mxxpnfe1Hfm1UljGmdsYozz yQoPkpLb3YJsZQbD/bGl38lZc7C7HQn/ANrpPQOFl8EI/X8hzrpC6E/NUGzSXzjaL/zlV1mY0wa OrST6tDkMWOmB/XgMEymS8Lu6H1qGXa+YThwTkA== X-Google-Smtp-Source: AGHT+IFmNkSApABDjAnzfA+2DpCsppJJzQoDuocZaImGPActhHgu87Fn8MqLPZwIRPgkPNguV4fdAMjhg/8J1fJKS3w= X-Received: by 2002:a17:90a:1509:b0:2c2:5f25:5490 with SMTP id 98e67ed59e1d1-2d8259d5830mr333839a91.34.1724693077496; Mon, 26 Aug 2024 10:24:37 -0700 (PDT) MIME-Version: 1.0 References: <20240301155416.96960-1-juraj.linkes@pantheon.tech> <20240821145315.97974-1-juraj.linkes@pantheon.tech> <20240821145315.97974-12-juraj.linkes@pantheon.tech> In-Reply-To: <20240821145315.97974-12-juraj.linkes@pantheon.tech> From: Jeremy Spewock Date: Mon, 26 Aug 2024 13:24:26 -0400 Message-ID: Subject: Re: [PATCH v3 11/12] dts: add Rx offload capabilities To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, npratte@iol.unh.edu, dmarx@iol.unh.edu, alex.chapman@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Wed, Aug 21, 2024 at 10:53=E2=80=AFAM Juraj Linke=C5=A1 wrote: > diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framewor= k/remote_session/testpmd_shell.py > index 48c31124d1..f83569669e 100644 > --- a/dts/framework/remote_session/testpmd_shell.py > +++ b/dts/framework/remote_session/testpmd_shell.py > @@ -659,6 +659,103 @@ class TestPmdPortStats(TextParser): > tx_bps: int =3D field(metadata=3DTextParser.find_int(r"Tx-bps:\s+(\d= +)")) > > > +class RxOffloadCapability(Flag): > + """Rx offload capabilities of a device.""" > + > + #: > + RX_OFFLOAD_VLAN_STRIP =3D auto() > + #: Device supports L3 checksum offload. > + RX_OFFLOAD_IPV4_CKSUM =3D auto() > + #: Device supports L4 checksum offload. > + RX_OFFLOAD_UDP_CKSUM =3D auto() > + #: Device supports L4 checksum offload. > + RX_OFFLOAD_TCP_CKSUM =3D auto() > + #: Device supports Large Receive Offload. > + RX_OFFLOAD_TCP_LRO =3D auto() > + #: Device supports QinQ (queue in queue) offload. > + RX_OFFLOAD_QINQ_STRIP =3D auto() > + #: Device supports inner packet L3 checksum. > + RX_OFFLOAD_OUTER_IPV4_CKSUM =3D auto() > + #: Device supports MACsec. > + RX_OFFLOAD_MACSEC_STRIP =3D auto() > + #: Device supports filtering of a VLAN Tag identifier. > + RX_OFFLOAD_VLAN_FILTER =3D 1 << 9 > + #: Device supports VLAN offload. > + RX_OFFLOAD_VLAN_EXTEND =3D auto() > + #: Device supports receiving segmented mbufs. > + RX_OFFLOAD_SCATTER =3D 1 << 13 I know you mentioned in the commit message that the auto() can cause problems with mypy/sphinx, is that why this one is a specific value instead? Regardless, I think we should probably make it consistent so that either all of them are bit-shifts or none of them are unless there is a specific reason that the scatter offload is different. > + #: Device supports Timestamp. > + RX_OFFLOAD_TIMESTAMP =3D auto() > + #: Device supports crypto processing while packet is received in NIC= . > + RX_OFFLOAD_SECURITY =3D auto() > + #: Device supports CRC stripping. > + RX_OFFLOAD_KEEP_CRC =3D auto() > + #: Device supports L4 checksum offload. > + RX_OFFLOAD_SCTP_CKSUM =3D auto() > + #: Device supports inner packet L4 checksum. > + RX_OFFLOAD_OUTER_UDP_CKSUM =3D auto() > + #: Device supports RSS hashing. > + RX_OFFLOAD_RSS_HASH =3D auto() > + #: Device supports > + RX_OFFLOAD_BUFFER_SPLIT =3D auto() > + #: Device supports all checksum capabilities. > + RX_OFFLOAD_CHECKSUM =3D RX_OFFLOAD_IPV4_CKSUM | RX_OFFLOAD_UDP_CKSUM= | RX_OFFLOAD_TCP_CKSUM > + #: Device supports all VLAN capabilities. > + RX_OFFLOAD_VLAN =3D ( > + RX_OFFLOAD_VLAN_STRIP > + | RX_OFFLOAD_VLAN_FILTER > + | RX_OFFLOAD_VLAN_EXTEND > + | RX_OFFLOAD_QINQ_STRIP > + ) > > @@ -1048,6 +1145,42 @@ def _close(self) -> None: > =3D=3D=3D=3D=3D=3D Capability retrieval methods =3D=3D=3D=3D=3D=3D > """ > > + def get_capabilities_rx_offload( > + self, > + supported_capabilities: MutableSet["NicCapability"], > + unsupported_capabilities: MutableSet["NicCapability"], > + ) -> None: > + """Get all rx offload capabilities and divide them into supporte= d and unsupported. > + > + Args: > + supported_capabilities: Supported capabilities will be added= to this set. > + unsupported_capabilities: Unsupported capabilities will be a= dded to this set. > + """ > + self._logger.debug("Getting rx offload capabilities.") > + command =3D f"show port {self.ports[0].id} rx_offload capabiliti= es" Is it desirable to only get the capabilities of the first port? In the current framework I suppose it doesn't matter all that much since you can only use the first few ports in the list of ports anyway, but will there ever be a case where a test run has 2 different devices included in the list of ports? Of course it's possible that it will happen, but is it practical? Because, if so, then we would want this to aggregate what all the devices are capable of and have capabilities basically say "at least one of the ports in the list of ports is capable of these things." This consideration also applies to the rxq info capability gathering as wel= l. > + rx_offload_capabilities_out =3D self.send_command(command) > + rx_offload_capabilities =3D RxOffloadCapabilities.parse(rx_offlo= ad_capabilities_out) > + self._update_capabilities_from_flag( > + supported_capabilities, > + unsupported_capabilities, > + RxOffloadCapability, > + rx_offload_capabilities.per_port | rx_offload_capabilities.p= er_queue, > + ) > + > > def __call__( > self, > diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSu= ite_pmd_buffer_scatter.py > index 89ece2ef56..64c48b0793 100644 > --- a/dts/tests/TestSuite_pmd_buffer_scatter.py > +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py > @@ -28,6 +28,7 @@ > from framework.testbed_model.capability import NicCapability, requires > > > +@requires(NicCapability.RX_OFFLOAD_SCATTER) I know that we talked about this and how, in the environments we looked at, it was true that the offload was supported in all cases where the "native" or non-offloaded was supported, but thinking about this more, I wonder if it is worth generalizing this assumption to all NICs or if we can just decorate the second test case that I wrote which uses the offloaded support. As long as the capabilities exposed by testpmd are accurate, even if this assumption was true, the capability for the non-offloaded one would show False when this offload wasn't usable and it would skip the test case anyway, so I don't think we lose anything by not including this test-suite-level requirement and making it more narrow to the test cases that require it. Let me know your thoughts on that though and I would be interested to hear if anyone else has any. > class TestPmdBufferScatter(TestSuite): > """DPDK PMD packet scattering test suite. > > -- > 2.34.1 >