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 6C328459CE; Wed, 18 Sep 2024 18:53:47 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5C2A04025E; Wed, 18 Sep 2024 18:53:47 +0200 (CEST) Received: from mail-pj1-f47.google.com (mail-pj1-f47.google.com [209.85.216.47]) by mails.dpdk.org (Postfix) with ESMTP id 8AC344003C for ; Wed, 18 Sep 2024 18:53:46 +0200 (CEST) Received: by mail-pj1-f47.google.com with SMTP id 98e67ed59e1d1-2d87176316eso816169a91.0 for ; Wed, 18 Sep 2024 09:53:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1726678426; x=1727283226; 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=Z7/MU2wgCUCltoeuPTvsngUgclNA9H8s+t9fAXyPhTs=; b=jJwkKQL+1dE+H98v5XEeqI7iRzyyZ7/orAFPMQdeWgSTkGN4/vP2yKQLnWR0vF24EY 78/KBmhZQHmRByugXt+vRTJN+fu0eI0/h2SnJVAYeQAoFE0Jo+dOEKBhQv342S1b34Vq D4wA4S8y5w2zod4fN+i9hvsf6g+Iqmp0qqAlQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726678426; x=1727283226; 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=Z7/MU2wgCUCltoeuPTvsngUgclNA9H8s+t9fAXyPhTs=; b=rEbpt/5HuscnZ6vSnZ2ZHithF9kmzkvhQA+32Nd4R64lx7YYx3TBXZHFa0WXniE8af gHBZ26ZlYE4+fW5WDBn6mIwQKzlQJLrHoZN0laSyBn20Ulfivmfh75t7yomo2edpIHYH 1n+s4zKO25eukPdlUkB1bhywsUbiRe3yoYXc/pbabdv/U1sWf/neJ0fKE8+h/Rh05kZc CjHCeKFZvkP8GE42Y9OjD7mOth3W0hpnaGwAkxfpdTxbmueyMz5OOeL6W1FJVrYxkRCm s9Ew6gbVZXWXYgoY32VAc6ou/GqjV4bPbCaGcdOxkemcN0A2PBRcCXYdCX3qT++3LfR1 oN3w== X-Forwarded-Encrypted: i=1; AJvYcCWWXr9J769ZJfzftZvJ5edwyLZ30mPvCzxWC7v5P+baYMjQ73sTg9znBBVtnCeV5O6sTnM=@dpdk.org X-Gm-Message-State: AOJu0YwOb2CLrapF0Igea5zO2XssAUezX5K7UnGKVAbE2lS8+hp4KHPq ULnBsHy1pXKzEibBL8vkJvSNo/ytgh2g6oZCd4xn4zhjEeR2sWzzHEiRxo1RaqMEroi7G7MTFR7 Rqm+sJNrJh7T/jvwx5Y9f2xKPScPkRADCwiIoQA== X-Google-Smtp-Source: AGHT+IF7K2g3+YyCtBS/uHjXFMpSW4QvDzUOO9+R+UoVCE90m4RvYhoZmoBKz+T6HA1cz4b4JXI8ewbMYq+ZEtdJxZ4= X-Received: by 2002:a17:90b:4f4e:b0:2da:55a5:959f with SMTP id 98e67ed59e1d1-2dd6cdd5b27mr418530a91.3.1726678425617; Wed, 18 Sep 2024 09:53:45 -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: From: Jeremy Spewock Date: Wed, 18 Sep 2024 12:53:34 -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, Sep 18, 2024 at 10:18=E2=80=AFAM Juraj Linke=C5=A1 wrote: > > > > On 26. 8. 2024 19:24, Jeremy Spewock wrote: > > 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/frame= work/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. > > > > Since both you and Dean asked, I'll add something to the docstring about > this. > > There are actually two non-auto values (RX_OFFLOAD_VLAN_FILTER =3D 1 << 9 > is the first one). I used the actual values to mirror the flags in DPDK > code. Gotcha, that makes sense. > > >> + #: 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_CK= SUM | 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 suppo= rted and unsupported. > >> + > >> + Args: > >> + supported_capabilities: Supported capabilities will be ad= ded to this set. > >> + unsupported_capabilities: Unsupported capabilities will b= e added to this set. > >> + """ > >> + self._logger.debug("Getting rx offload capabilities.") > >> + command =3D f"show port {self.ports[0].id} rx_offload capabil= ities" > > > > 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= well. > > > > No parts of the framework are adjusted to use multiple NIC in a single > test run (because we assume we're testing only one NIC at a time). If we > add this support, it's going to be a broader change. > > I approached this with the above assumption in mind and in that case, > testing just one port of the NIC seemed just fine. That's a good point that making the adjustment to allow for multiple devices is a bigger change that is definitely out of scope for this series. Makes sense to put it off and go with the current assumptions, I only asked in case it was something simple so it would be one less thing to do in the future :). This is fine as is then I think. > > >> + rx_offload_capabilities_out =3D self.send_command(command) > >> + rx_offload_capabilities =3D RxOffloadCapabilities.parse(rx_of= fload_capabilities_out) > >> + self._update_capabilities_from_flag( > >> + supported_capabilities, > >> + unsupported_capabilities, > >> + RxOffloadCapability, > >> + rx_offload_capabilities.per_port | rx_offload_capabilitie= s.per_queue, > >> + ) > >> + > > > >> > >> def __call__( > >> self, > >> diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/Tes= tSuite_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, requir= es > >> > >> > >> +@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. > > > > I'm not sure I understand what your point is. Let's talk about it in the > call. Sure, sounds good to me. > > >> class TestPmdBufferScatter(TestSuite): > >> """DPDK PMD packet scattering test suite. > >> > >> -- > >> 2.34.1 > >> >