[v3,11/12] dts: add Rx offload capabilities

Message ID 20240821145315.97974-12-juraj.linkes@pantheon.tech (mailing list archive)
State Superseded
Delegated to: Juraj Linkeš
Headers
Series dts: add test skipping based on capabilities |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Juraj Linkeš Aug. 21, 2024, 2:53 p.m. UTC
The scatter Rx offload capability is needed for the pmd_buffer_scatter
test suite. The command that retrieves the capability is:
show port <port_id> rx_offload capabilities

The command also retrieves a lot of other capabilities (RX_OFFLOAD_*)
which are all added into a Flag. The Flag members correspond to NIC
capability names so a convenience function that looks for the supported
Flags in a testpmd output is also added.

The NIC capability names (mentioned above) are copy-pasted from the
Flag. Dynamic addition of Enum members runs into problems with typing
(mypy doesn't know about the members) and documentation generation
(Sphinx doesn't know about the members).

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 dts/framework/remote_session/testpmd_shell.py | 213 ++++++++++++++++++
 dts/tests/TestSuite_pmd_buffer_scatter.py     |   1 +
 2 files changed, 214 insertions(+)
  

Comments

Jeremy Spewock Aug. 26, 2024, 5:24 p.m. UTC | #1
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.

> +    #: 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.

> +        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.

>  class TestPmdBufferScatter(TestSuite):
>      """DPDK PMD packet scattering test suite.
>
> --
> 2.34.1
>
  
Jeremy Spewock Aug. 28, 2024, 5:44 p.m. UTC | #2
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()

One other thought that I had about this; was there a specific reason
that you decided to prefix all of these with `RX_OFFLOAD_`? I am
working on a test suite right now that uses both RX and TX offloads
and thought that it would be a great use of capabilities, so I am
working on adding a TxOffloadCapability flag as well and, since the
output is essentially the same, it made a lot of sense to make it a
sibling class of this one with similar parsing functionality. In what
I was writing, I found it much easier to remove this prefix so that
the parsing method can be the same for both RX and TX, and I didn't
have to restate some options that are shared between both (like
IPv4_CKSUM, UDP_CKSUM, etc.). Is there a reason you can think of why
removing this prefix is a bad idea? Hopefully I will have a patch out
soon that shows this extension that I've made so that you can see
in-code what I was thinking.

> +    #: 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
> +    #: 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>
>
  
Jeremy Spewock Aug. 29, 2024, 3:40 p.m. UTC | #3
On Wed, Aug 28, 2024 at 1:44 PM Jeremy Spewock <jspewock@iol.unh.edu> 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()
>
> One other thought that I had about this; was there a specific reason
> that you decided to prefix all of these with `RX_OFFLOAD_`? I am
> working on a test suite right now that uses both RX and TX offloads
> and thought that it would be a great use of capabilities, so I am
> working on adding a TxOffloadCapability flag as well and, since the
> output is essentially the same, it made a lot of sense to make it a
> sibling class of this one with similar parsing functionality. In what
> I was writing, I found it much easier to remove this prefix so that
> the parsing method can be the same for both RX and TX, and I didn't
> have to restate some options that are shared between both (like
> IPv4_CKSUM, UDP_CKSUM, etc.). Is there a reason you can think of why
> removing this prefix is a bad idea? Hopefully I will have a patch out
> soon that shows this extension that I've made so that you can see
> in-code what I was thinking.

I see now that you actually already answered this question, I was just
looking too much at that piece of code, and clearly not looking
further down at the helper-method mapping or the commit message that
you left :).

"The Flag members correspond to NIC
capability names so a convenience function that looks for the supported
Flags in a testpmd output is also added."

Having it prefixed with RX_OFFLOAD_ in NicCapability makes a lot of
sense since it is more explicit. Since there is a good reason to have
it like this, then the redundancy makes sense I think. There are some
ways to potentially avoid this like creating a StrFlag class that
overrides the __str__ method, or something like an additional type
that would contain a toString method, but it feels very situational
and specific to this one use-case so it probably isn't going to be
super valuable. Another thing I could think of to do would be allowing
the user to pass in a function or something to the helper-method that
mapped Flag names to their respective NicCapability name, or just
doing it in the method that gets the offloads instead of using a
helper at all, but this also just makes it more complicated and maybe
it isn't worth it.

I apologize for asking you about something that you already explained,
but maybe something we can get out of this is that, since these names
have to be consistent, it might be worth putting that in the
doc-strings of the flag for when people try to make further expansions
or changes in the future. Or it could also be generally clear that
flags used for capabilities should follow this idea, let me know what
you think.

>
> > +    #: 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
> > +    #: 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>
> >
  
Dean Marx Sept. 3, 2024, 7:49 p.m. UTC | #4
On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> The scatter Rx offload capability is needed for the pmd_buffer_scatter
> test suite. The command that retrieves the capability is:
> show port <port_id> rx_offload capabilities
>
> The command also retrieves a lot of other capabilities (RX_OFFLOAD_*)
> which are all added into a Flag. The Flag members correspond to NIC
> capability names so a convenience function that looks for the supported
> Flags in a testpmd output is also added.
>
> The NIC capability names (mentioned above) are copy-pasted from the
> Flag. Dynamic addition of Enum members runs into problems with typing
> (mypy doesn't know about the members) and documentation generation
> (Sphinx doesn't know about the members).
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>

<snip>

> +    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
>

This was an interesting section, I'm not super familiar with bitwise
shifting in python flags so I figured I'd ask while it's in mind if there's
any specific reason for shifting these two flags? Not a critique of the
code, just genuinely curious.

Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
  
Juraj Linkeš Sept. 18, 2024, 1:59 p.m. UTC | #5
On 3. 9. 2024 21:49, Dean Marx wrote:
> On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš 
> <juraj.linkes@pantheon.tech> wrote:
> 
>     The scatter Rx offload capability is needed for the pmd_buffer_scatter
>     test suite. The command that retrieves the capability is:
>     show port <port_id> rx_offload capabilities
> 
>     The command also retrieves a lot of other capabilities (RX_OFFLOAD_*)
>     which are all added into a Flag. The Flag members correspond to NIC
>     capability names so a convenience function that looks for the supported
>     Flags in a testpmd output is also added.
> 
>     The NIC capability names (mentioned above) are copy-pasted from the
>     Flag. Dynamic addition of Enum members runs into problems with typing
>     (mypy doesn't know about the members) and documentation generation
>     (Sphinx doesn't know about the members).
> 
>     Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> 
> 
> <snip>
> 
>     +    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
> 
> 
> This was an interesting section, I'm not super familiar with bitwise 
> shifting in python flags so I figured I'd ask while it's in mind if 
> there's any specific reason for shifting these two flags? Not a critique 
> of the code, just genuinely curious.
> 

It's there just to mirror the flags in DPDK code.

> Reviewed-by: Dean Marx <dmarx@iol.unh.edu <mailto:dmarx@iol.unh.edu>>
  
Juraj Linkeš Sept. 18, 2024, 2:18 p.m. UTC | #6
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
>>
  
Juraj Linkeš Sept. 18, 2024, 2:27 p.m. UTC | #7
On 29. 8. 2024 17:40, Jeremy Spewock wrote:
> On Wed, Aug 28, 2024 at 1:44 PM Jeremy Spewock <jspewock@iol.unh.edu> 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()
>>
>> One other thought that I had about this; was there a specific reason
>> that you decided to prefix all of these with `RX_OFFLOAD_`? I am
>> working on a test suite right now that uses both RX and TX offloads
>> and thought that it would be a great use of capabilities, so I am
>> working on adding a TxOffloadCapability flag as well and, since the
>> output is essentially the same, it made a lot of sense to make it a
>> sibling class of this one with similar parsing functionality. In what
>> I was writing, I found it much easier to remove this prefix so that
>> the parsing method can be the same for both RX and TX, and I didn't
>> have to restate some options that are shared between both (like
>> IPv4_CKSUM, UDP_CKSUM, etc.). Is there a reason you can think of why
>> removing this prefix is a bad idea? Hopefully I will have a patch out
>> soon that shows this extension that I've made so that you can see
>> in-code what I was thinking.
> 
> I see now that you actually already answered this question, I was just
> looking too much at that piece of code, and clearly not looking
> further down at the helper-method mapping or the commit message that
> you left :).
> 
> "The Flag members correspond to NIC
> capability names so a convenience function that looks for the supported
> Flags in a testpmd output is also added."
> 
> Having it prefixed with RX_OFFLOAD_ in NicCapability makes a lot of
> sense since it is more explicit. Since there is a good reason to have
> it like this, then the redundancy makes sense I think. There are some
> ways to potentially avoid this like creating a StrFlag class that
> overrides the __str__ method, or something like an additional type
> that would contain a toString method, but it feels very situational
> and specific to this one use-case so it probably isn't going to be
> super valuable. Another thing I could think of to do would be allowing
> the user to pass in a function or something to the helper-method that
> mapped Flag names to their respective NicCapability name, or just
> doing it in the method that gets the offloads instead of using a
> helper at all, but this also just makes it more complicated and maybe
> it isn't worth it.
> 

I also had it without the prefix, but then I also realized it's needed 
in NicCapability so this is where I ended. I'm not sure complicating 
things to remove the prefix is worth it, especially when these names are 
basically only used internally. The prefix could actually confer some 
benefit if the name appears in a log somewhere (although overriding 
__str__ could be the way; maybe I'll think about that).

> I apologize for asking you about something that you already explained,
> but maybe something we can get out of this is that, since these names
> have to be consistent, it might be worth putting that in the
> doc-strings of the flag for when people try to make further expansions
> or changes in the future. Or it could also be generally clear that
> flags used for capabilities should follow this idea, let me know what
> you think.
> 

Adding things to docstring is usually a good thing. What should I 
document? I guess the correspondence between the flag and NicCapability, 
anything else?

>>
>>> +    #: 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
>>> +    #: 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>
>>>
  
Jeremy Spewock Sept. 18, 2024, 4:53 p.m. UTC | #8
On Wed, Sep 18, 2024 at 10:18 AM Juraj Linkeš
<juraj.linkes@pantheon.tech> wrote:
>
>
>
> 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.

Gotcha, that makes sense.

>
> >> +    #: 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.

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 = 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.

Sure, sounds good to me.


>
> >>   class TestPmdBufferScatter(TestSuite):
> >>       """DPDK PMD packet scattering test suite.
> >>
> >> --
> >> 2.34.1
> >>
>
  
Jeremy Spewock Sept. 18, 2024, 4:57 p.m. UTC | #9
On Wed, Sep 18, 2024 at 10:27 AM Juraj Linkeš
<juraj.linkes@pantheon.tech> wrote:
>
>
>
> On 29. 8. 2024 17:40, Jeremy Spewock wrote:
> > On Wed, Aug 28, 2024 at 1:44 PM Jeremy Spewock <jspewock@iol.unh.edu> 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()
> >>
> >> One other thought that I had about this; was there a specific reason
> >> that you decided to prefix all of these with `RX_OFFLOAD_`? I am
> >> working on a test suite right now that uses both RX and TX offloads
> >> and thought that it would be a great use of capabilities, so I am
> >> working on adding a TxOffloadCapability flag as well and, since the
> >> output is essentially the same, it made a lot of sense to make it a
> >> sibling class of this one with similar parsing functionality. In what
> >> I was writing, I found it much easier to remove this prefix so that
> >> the parsing method can be the same for both RX and TX, and I didn't
> >> have to restate some options that are shared between both (like
> >> IPv4_CKSUM, UDP_CKSUM, etc.). Is there a reason you can think of why
> >> removing this prefix is a bad idea? Hopefully I will have a patch out
> >> soon that shows this extension that I've made so that you can see
> >> in-code what I was thinking.
> >
> > I see now that you actually already answered this question, I was just
> > looking too much at that piece of code, and clearly not looking
> > further down at the helper-method mapping or the commit message that
> > you left :).
> >
> > "The Flag members correspond to NIC
> > capability names so a convenience function that looks for the supported
> > Flags in a testpmd output is also added."
> >
> > Having it prefixed with RX_OFFLOAD_ in NicCapability makes a lot of
> > sense since it is more explicit. Since there is a good reason to have
> > it like this, then the redundancy makes sense I think. There are some
> > ways to potentially avoid this like creating a StrFlag class that
> > overrides the __str__ method, or something like an additional type
> > that would contain a toString method, but it feels very situational
> > and specific to this one use-case so it probably isn't going to be
> > super valuable. Another thing I could think of to do would be allowing
> > the user to pass in a function or something to the helper-method that
> > mapped Flag names to their respective NicCapability name, or just
> > doing it in the method that gets the offloads instead of using a
> > helper at all, but this also just makes it more complicated and maybe
> > it isn't worth it.
> >
>
> I also had it without the prefix, but then I also realized it's needed
> in NicCapability so this is where I ended. I'm not sure complicating
> things to remove the prefix is worth it, especially when these names are
> basically only used internally. The prefix could actually confer some
> benefit if the name appears in a log somewhere (although overriding
> __str__ could be the way; maybe I'll think about that).

It could be done with modifying str, but I found that an approach that
was easier was just adding an optional prefix to the
_update_capabilities_from_flag() method since you will know whether
the capability is Rx or Tx at the point of calling this method. I feel
like either or could work, I'm not sure exactly which is better. The
change that adds the prefix is in the Rx/Tx offload suite in the first
commit [1] if you wanted to look at it. This commit and the one after
it are isolated to be only changes to the capabilities series.

[1] https://patchwork.dpdk.org/project/dpdk/patch/20240903194642.24458-2-jspewock@iol.unh.edu/

>
> > I apologize for asking you about something that you already explained,
> > but maybe something we can get out of this is that, since these names
> > have to be consistent, it might be worth putting that in the
> > doc-strings of the flag for when people try to make further expansions
> > or changes in the future. Or it could also be generally clear that
> > flags used for capabilities should follow this idea, let me know what
> > you think.
> >
>
> Adding things to docstring is usually a good thing. What should I
> document? I guess the correspondence between the flag and NicCapability,
> anything else?

The only thing I was thinking was that the flag values have to match
the values in NicCapability. I think explaining it this way is enough
just to make it clear that it is done that way for a purpose and
cannot be different (unless of course the no-prefix way is favorable).

>
> >>
> >>> +    #: 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
> >>> +    #: 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>
> >>>
>
  

Patch

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
+    #: 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
+    )
+
+    @classmethod
+    def from_string(cls, line: str) -> Self:
+        """Make an instance from a string containing the flag names separated with a space.
+
+        Args:
+            line: The line to parse.
+
+        Returns:
+            A new instance containing all found flags.
+        """
+        flag = cls(0)
+        for flag_name in line.split():
+            flag |= cls[f"RX_OFFLOAD_{flag_name}"]
+        return flag
+
+    @classmethod
+    def make_parser(cls, per_port: bool) -> ParserFn:
+        """Make a parser function.
+
+        Args:
+            per_port: If :data:`True`, will return capabilities per port. If :data:`False`,
+                will return capabilities per queue.
+
+        Returns:
+            ParserFn: A dictionary for the `dataclasses.field` metadata argument containing a
+                parser function that makes an instance of this flag from text.
+        """
+        granularity = "Port" if per_port else "Queue"
+        return TextParser.wrap(
+            TextParser.find(rf"Per {granularity}\s+:(.*)$", re.MULTILINE),
+            cls.from_string,
+        )
+
+
+@dataclass
+class RxOffloadCapabilities(TextParser):
+    """The result of testpmd's ``show port <port_id> rx_offload capabilities`` command."""
+
+    #:
+    port_id: int = field(
+        metadata=TextParser.find_int(r"Rx Offloading Capabilities of port (\d+) :")
+    )
+    #: Per-queue Rx offload capabilities.
+    per_queue: RxOffloadCapability = field(metadata=RxOffloadCapability.make_parser(False))
+    #: Capabilities other than per-queue Rx offload capabilities.
+    per_port: RxOffloadCapability = field(metadata=RxOffloadCapability.make_parser(True))
+
+
 T = TypeVarTuple("T")  # type: ignore[misc]
 
 
@@ -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"
+        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,
+        )
+
+    def _update_capabilities_from_flag(
+        self,
+        supported_capabilities: MutableSet["NicCapability"],
+        unsupported_capabilities: MutableSet["NicCapability"],
+        flag_class: type[Flag],
+        supported_flags: Flag,
+    ) -> None:
+        """Divide all flags from `flag_class` into supported and unsupported."""
+        for flag in flag_class:
+            if flag in supported_flags:
+                supported_capabilities.add(NicCapability[str(flag.name)])
+            else:
+                unsupported_capabilities.add(NicCapability[str(flag.name)])
+
     def get_capabilities_rxq_info(
         self,
         supported_capabilities: MutableSet["NicCapability"],
@@ -1119,6 +1252,86 @@  class NicCapability(NoAliasEnum):
     SCATTERED_RX_ENABLED: TestPmdShellCapabilityMethod = partial(
         TestPmdShell.get_capabilities_rxq_info
     )
+    #:
+    RX_OFFLOAD_VLAN_STRIP: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports L3 checksum offload.
+    RX_OFFLOAD_IPV4_CKSUM: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports L4 checksum offload.
+    RX_OFFLOAD_UDP_CKSUM: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports L4 checksum offload.
+    RX_OFFLOAD_TCP_CKSUM: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports Large Receive Offload.
+    RX_OFFLOAD_TCP_LRO: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports QinQ (queue in queue) offload.
+    RX_OFFLOAD_QINQ_STRIP: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports inner packet L3 checksum.
+    RX_OFFLOAD_OUTER_IPV4_CKSUM: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports MACsec.
+    RX_OFFLOAD_MACSEC_STRIP: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports filtering of a VLAN Tag identifier.
+    RX_OFFLOAD_VLAN_FILTER: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports VLAN offload.
+    RX_OFFLOAD_VLAN_EXTEND: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports receiving segmented mbufs.
+    RX_OFFLOAD_SCATTER: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports Timestamp.
+    RX_OFFLOAD_TIMESTAMP: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports crypto processing while packet is received in NIC.
+    RX_OFFLOAD_SECURITY: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports CRC stripping.
+    RX_OFFLOAD_KEEP_CRC: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports L4 checksum offload.
+    RX_OFFLOAD_SCTP_CKSUM: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports inner packet L4 checksum.
+    RX_OFFLOAD_OUTER_UDP_CKSUM: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports RSS hashing.
+    RX_OFFLOAD_RSS_HASH: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports scatter Rx packets to segmented mbufs.
+    RX_OFFLOAD_BUFFER_SPLIT: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports all checksum capabilities.
+    RX_OFFLOAD_CHECKSUM: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
+    #: Device supports all VLAN capabilities.
+    RX_OFFLOAD_VLAN: TestPmdShellCapabilityMethod = partial(
+        TestPmdShell.get_capabilities_rx_offload
+    )
 
     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)
 class TestPmdBufferScatter(TestSuite):
     """DPDK PMD packet scattering test suite.