From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; Wed, 18 Sep 2024 18:53:46 +0200 (CEST)
Received: by mail-pj1-f47.google.com with SMTP id
 98e67ed59e1d1-2d87176316eso816169a91.0
 for <dev@dpdk.org>; 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>
 <CAAA20URBK5h1=Ej02L=xZ9_8KrHOxftPP1fbOsyXyPS50XBJdA@mail.gmail.com>
 <dc109457-d7b0-4ecb-a80b-12749eecf10c@pantheon.tech>
In-Reply-To: <dc109457-d7b0-4ecb-a80b-12749eecf10c@pantheon.tech>
From: Jeremy Spewock <jspewock@iol.unh.edu>
Date: Wed, 18 Sep 2024 12:53:34 -0400
Message-ID: <CAAA20UQKQh=Qy=QZh7RUHFcnD_TF2ooTjR2AD+fwiHsw_MJB3Q@mail.gmail.com>
Subject: Re: [PATCH v3 11/12] dts: add Rx offload capabilities
To: =?UTF-8?Q?Juraj_Linke=C5=A1?= <juraj.linkes@pantheon.tech>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Wed, Sep 18, 2024 at 10:18=E2=80=AFAM Juraj Linke=C5=A1
<juraj.linkes@pantheon.tech> 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
> > <juraj.linkes@pantheon.tech> wrote:
> > <snip>
> >> 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
> >> +    )
> > <snip>
> >>
> >> @@ -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,
> >> +        )
> >> +
> > <snip>
> >>
> >>       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
> >>
>