DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Jeremy Spewock <jspewock@iol.unh.edu>
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
Subject: Re: [PATCH v3 11/12] dts: add Rx offload capabilities
Date: Wed, 18 Sep 2024 16:18:25 +0200	[thread overview]
Message-ID: <dc109457-d7b0-4ecb-a80b-12749eecf10c@pantheon.tech> (raw)
In-Reply-To: <CAAA20URBK5h1=Ej02L=xZ9_8KrHOxftPP1fbOsyXyPS50XBJdA@mail.gmail.com>



On 26. 8. 2024 19:24, Jeremy Spewock wrote:
> On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš
> <juraj.linkes@pantheon.tech> wrote:
> <snip>
>> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/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 = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)"))
>>
>>
>> +class RxOffloadCapability(Flag):
>> +    """Rx offload capabilities of a device."""
>> +
>> +    #:
>> +    RX_OFFLOAD_VLAN_STRIP = auto()
>> +    #: Device supports L3 checksum offload.
>> +    RX_OFFLOAD_IPV4_CKSUM = auto()
>> +    #: Device supports L4 checksum offload.
>> +    RX_OFFLOAD_UDP_CKSUM = auto()
>> +    #: Device supports L4 checksum offload.
>> +    RX_OFFLOAD_TCP_CKSUM = auto()
>> +    #: Device supports Large Receive Offload.
>> +    RX_OFFLOAD_TCP_LRO = auto()
>> +    #: Device supports QinQ (queue in queue) offload.
>> +    RX_OFFLOAD_QINQ_STRIP = auto()
>> +    #: Device supports inner packet L3 checksum.
>> +    RX_OFFLOAD_OUTER_IPV4_CKSUM = auto()
>> +    #: Device supports MACsec.
>> +    RX_OFFLOAD_MACSEC_STRIP = auto()
>> +    #: Device supports filtering of a VLAN Tag identifier.
>> +    RX_OFFLOAD_VLAN_FILTER = 1 << 9
>> +    #: Device supports VLAN offload.
>> +    RX_OFFLOAD_VLAN_EXTEND = auto()
>> +    #: Device supports receiving segmented mbufs.
>> +    RX_OFFLOAD_SCATTER = 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 = 1 << 9 
is the first one). I used the actual values to mirror the flags in DPDK 
code.

>> +    #: Device supports Timestamp.
>> +    RX_OFFLOAD_TIMESTAMP = auto()
>> +    #: Device supports crypto processing while packet is received in NIC.
>> +    RX_OFFLOAD_SECURITY = auto()
>> +    #: Device supports CRC stripping.
>> +    RX_OFFLOAD_KEEP_CRC = auto()
>> +    #: Device supports L4 checksum offload.
>> +    RX_OFFLOAD_SCTP_CKSUM = auto()
>> +    #: Device supports inner packet L4 checksum.
>> +    RX_OFFLOAD_OUTER_UDP_CKSUM = auto()
>> +    #: Device supports RSS hashing.
>> +    RX_OFFLOAD_RSS_HASH = auto()
>> +    #: Device supports
>> +    RX_OFFLOAD_BUFFER_SPLIT = auto()
>> +    #: Device supports all checksum capabilities.
>> +    RX_OFFLOAD_CHECKSUM = RX_OFFLOAD_IPV4_CKSUM | RX_OFFLOAD_UDP_CKSUM | RX_OFFLOAD_TCP_CKSUM
>> +    #: Device supports all VLAN capabilities.
>> +    RX_OFFLOAD_VLAN = (
>> +        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:
>>       ====== Capability retrieval methods ======
>>       """
>>
>> +    def get_capabilities_rx_offload(
>> +        self,
>> +        supported_capabilities: MutableSet["NicCapability"],
>> +        unsupported_capabilities: MutableSet["NicCapability"],
>> +    ) -> None:
>> +        """Get all rx offload capabilities and divide them into supported and unsupported.
>> +
>> +        Args:
>> +            supported_capabilities: Supported capabilities will be added to this set.
>> +            unsupported_capabilities: Unsupported capabilities will be added to this set.
>> +        """
>> +        self._logger.debug("Getting rx offload capabilities.")
>> +        command = f"show port {self.ports[0].id} rx_offload capabilities"
> 
> 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.

>> +        rx_offload_capabilities_out = self.send_command(command)
>> +        rx_offload_capabilities = RxOffloadCapabilities.parse(rx_offload_capabilities_out)
>> +        self._update_capabilities_from_flag(
>> +            supported_capabilities,
>> +            unsupported_capabilities,
>> +            RxOffloadCapability,
>> +            rx_offload_capabilities.per_port | rx_offload_capabilities.per_queue,
>> +        )
>> +
> <snip>
>>
>>       def __call__(
>>           self,
>> diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_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.
> 

I'm not sure I understand what your point is. Let's talk about it in the 
call.

>>   class TestPmdBufferScatter(TestSuite):
>>       """DPDK PMD packet scattering test suite.
>>
>> --
>> 2.34.1
>>


  reply	other threads:[~2024-09-18 14:18 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-01 15:54 [RFC PATCH v1] dts: skip test cases based on capabilities Juraj Linkeš
2024-04-11  8:48 ` [RFC PATCH v2] " Juraj Linkeš
2024-05-21 15:47   ` Luca Vizzarro
2024-05-22 14:58   ` Luca Vizzarro
2024-06-07 13:13     ` Juraj Linkeš
2024-06-11  9:51       ` Luca Vizzarro
2024-06-12  9:15         ` Juraj Linkeš
2024-06-17 15:07           ` Luca Vizzarro
2024-05-24 20:51   ` Nicholas Pratte
2024-05-31 16:44   ` Luca Vizzarro
2024-06-05 13:55     ` Patrick Robb
2024-06-06 13:36       ` Jeremy Spewock
2024-06-03 14:40   ` Nicholas Pratte
2024-06-07 13:20     ` Juraj Linkeš
2024-08-21 14:53 ` [PATCH v3 00/12] dts: add test skipping " Juraj Linkeš
2024-08-21 14:53   ` [PATCH v3 01/12] dts: fix default device error handling mode Juraj Linkeš
2024-08-26 16:42     ` Jeremy Spewock
2024-08-27 16:15     ` Dean Marx
2024-08-27 20:09     ` Nicholas Pratte
2024-08-21 14:53   ` [PATCH v3 02/12] dts: add the aenum dependency Juraj Linkeš
2024-08-26 16:42     ` Jeremy Spewock
2024-08-27 16:28     ` Dean Marx
2024-08-27 20:21     ` Nicholas Pratte
2024-08-21 14:53   ` [PATCH v3 03/12] dts: add test case decorators Juraj Linkeš
2024-08-26 16:50     ` Jeremy Spewock
2024-09-05  8:07       ` Juraj Linkeš
2024-09-05 15:24         ` Jeremy Spewock
2024-08-28 20:09     ` Dean Marx
2024-08-30 15:50     ` Nicholas Pratte
2024-08-21 14:53   ` [PATCH v3 04/12] dts: add mechanism to skip test cases or suites Juraj Linkeš
2024-08-26 16:52     ` Jeremy Spewock
2024-09-05  9:23       ` Juraj Linkeš
2024-09-05 15:26         ` Jeremy Spewock
2024-08-28 20:37     ` Dean Marx
2024-08-21 14:53   ` [PATCH v3 05/12] dts: add support for simpler topologies Juraj Linkeš
2024-08-26 16:54     ` Jeremy Spewock
2024-09-05  9:42       ` Juraj Linkeš
2024-08-28 20:56     ` Dean Marx
2024-08-21 14:53   ` [PATCH v3 06/12] dst: add basic capability support Juraj Linkeš
2024-08-26 16:56     ` Jeremy Spewock
2024-09-05  9:50       ` Juraj Linkeš
2024-09-05 15:27         ` Jeremy Spewock
2024-09-03 16:03     ` Dean Marx
2024-09-05  9:51       ` Juraj Linkeš
2024-08-21 14:53   ` [PATCH v3 07/12] dts: add testpmd port information caching Juraj Linkeš
2024-08-26 16:56     ` Jeremy Spewock
2024-09-03 16:12     ` Dean Marx
2024-08-21 14:53   ` [PATCH v3 08/12] dts: add NIC capability support Juraj Linkeš
2024-08-26 17:11     ` Jeremy Spewock
2024-09-05 11:56       ` Juraj Linkeš
2024-09-05 15:30         ` Jeremy Spewock
2024-08-27 16:36     ` Jeremy Spewock
2024-09-18 12:58       ` Juraj Linkeš
2024-09-18 16:52         ` Jeremy Spewock
2024-09-03 19:13     ` Dean Marx
2024-08-21 14:53   ` [PATCH v3 09/12] dts: add topology capability Juraj Linkeš
2024-08-26 17:13     ` Jeremy Spewock
2024-09-03 17:50     ` Dean Marx
2024-08-21 14:53   ` [PATCH v3 10/12] doc: add DTS capability doc sources Juraj Linkeš
2024-08-26 17:13     ` Jeremy Spewock
2024-09-03 17:52     ` Dean Marx
2024-08-21 14:53   ` [PATCH v3 11/12] dts: add Rx offload capabilities Juraj Linkeš
2024-08-26 17:24     ` Jeremy Spewock
2024-09-18 14:18       ` Juraj Linkeš [this message]
2024-09-18 16:53         ` Jeremy Spewock
2024-08-28 17:44     ` Jeremy Spewock
2024-08-29 15:40       ` Jeremy Spewock
2024-09-18 14:27         ` Juraj Linkeš
2024-09-18 16:57           ` Jeremy Spewock
2024-09-03 19:49     ` Dean Marx
2024-09-18 13:59       ` Juraj Linkeš
2024-08-21 14:53   ` [PATCH v3 12/12] dts: add NIC capabilities from show port info Juraj Linkeš
2024-08-26 17:24     ` Jeremy Spewock
2024-09-03 18:02     ` Dean Marx
2024-08-26 17:25   ` [PATCH v3 00/12] dts: add test skipping based on capabilities Jeremy Spewock
2024-09-23 15:02 ` [PATCH v4 01/11] dts: add the aenum dependency Juraj Linkeš
2024-09-23 15:02   ` [PATCH v4 02/11] dts: add test case decorators Juraj Linkeš
2024-09-23 19:26     ` Jeremy Spewock
2024-09-24  8:00       ` Juraj Linkeš
2024-09-27 12:36     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 03/11] dts: add mechanism to skip test cases or suites Juraj Linkeš
2024-09-23 19:26     ` Jeremy Spewock
2024-09-27 12:37     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 04/11] dts: add support for simpler topologies Juraj Linkeš
2024-09-27 12:37     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 05/11] dts: add basic capability support Juraj Linkeš
2024-09-27 12:37     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 06/11] dts: add NIC " Juraj Linkeš
2024-09-23 19:26     ` Jeremy Spewock
2024-09-24  8:02       ` Juraj Linkeš
2024-09-27 12:42     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 07/11] dts: add NIC capabilities from show rxq info Juraj Linkeš
2024-09-23 19:26     ` Jeremy Spewock
2024-09-27 13:00     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 08/11] dts: add topology capability Juraj Linkeš
2024-09-23 19:26     ` Jeremy Spewock
2024-09-27 13:04     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 09/11] doc: add DTS capability doc sources Juraj Linkeš
2024-09-27 13:04     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 10/11] dts: add Rx offload capabilities Juraj Linkeš
2024-09-23 19:26     ` Jeremy Spewock
2024-09-27 13:11     ` Luca Vizzarro
2024-09-23 15:02   ` [PATCH v4 11/11] dts: add NIC capabilities from show port info Juraj Linkeš
2024-09-27 13:12     ` Luca Vizzarro
2024-09-27 12:36   ` [PATCH v4 01/11] dts: add the aenum dependency Luca Vizzarro
2024-09-24  8:20 ` [PATCH v4 00/11] dts: add test skipping based on capabilities Juraj Linkeš
2024-09-30 13:43   ` Juraj Linkeš

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dc109457-d7b0-4ecb-a80b-12749eecf10c@pantheon.tech \
    --to=juraj.linkes@pantheon.tech \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Luca.Vizzarro@arm.com \
    --cc=alex.chapman@arm.com \
    --cc=dev@dpdk.org \
    --cc=dmarx@iol.unh.edu \
    --cc=jspewock@iol.unh.edu \
    --cc=npratte@iol.unh.edu \
    --cc=paul.szczepanek@arm.com \
    --cc=probb@iol.unh.edu \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).