DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jeremy Spewock <jspewock@iol.unh.edu>
To: Dean Marx <dmarx@iol.unh.edu>
Cc: probb@iol.unh.edu, npratte@iol.unh.edu, luca.vizzarro@arm.com,
	 yoan.picchi@foss.arm.com, Honnappa.Nagarahalli@arm.com,
	 paul.szczepanek@arm.com, juraj.linkes@pantheon.tech,
	dev@dpdk.org
Subject: Re: [PATCH v13 2/2] dts: VLAN test suite implementation
Date: Fri, 9 Aug 2024 13:23:22 -0400	[thread overview]
Message-ID: <CAAA20UQ48KpvuEPxVoTg_Od-NSi2+FKhCp2Lc4H3XD1e8j074w@mail.gmail.com> (raw)
In-Reply-To: <20240807194341.26102-3-dmarx@iol.unh.edu>

On Wed, Aug 7, 2024 at 3:43 PM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Test suite for verifying VLAN filtering, stripping, and insertion
> functionality on Poll Mode Driver.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
<snip>
> +++ b/dts/tests/TestSuite_vlan.py
> @@ -0,0 +1,168 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2024 University of New Hampshire
> +
> +"""Test the support of VLAN Offload Features by Poll Mode Drivers.
> +
> +The test suite ensures that with the correct configuration, a port
> +will not drop a VLAN tagged packet. In order for this to be successful,
> +packet header stripping and packet receipts must be enabled on the Poll Mode Driver.

This sentence is a little confusing to me. I'm not sure exactly what
you mean by packet receipts being enabled, and it's probably better to
not specify that stripping must be enabled since there are cases
tested where it isn't and we still expect to get the packets. It seems
like the main thing you are testing for in the suite is whether the
VLAN functions perform their expected behavior, so maybe it would be
better to focus more on that in this doc-string than if the packet is
received.

> +The test suite checks that when these conditions are met, the packet is received without issue.
> +The suite also checks to ensure that when these conditions are not met, as in the cases where
> +stripping is disabled, or VLAN packet receipts are disabled, the packet is not received.

There are test cases where we also test that a packet is received even
when the stripping is disabled, so we should likely remove that from
here as well.

> +Additionally, it checks the case where VLAN header insertion is enabled in transmitted packets,
> +which should be successful if the previous cases pass.
> +
> +"""
> +
> +from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
> +from scapy.packet import Raw  # type: ignore[import-untyped]
> +
> +from framework.remote_session.testpmd_shell import SimpleForwardingModes, TestPmdShell
> +from framework.test_suite import TestSuite
> +
> +
<snip>
> +    def send_vlan_packet_and_verify(self, should_receive: bool, strip: bool, vlan_id: int) -> None:
> +        """Generate a vlan packet, send and verify packet with same payload is received on the dut.
> +
> +        Args:
> +            should_receive: Indicate whether the packet should be successfully received.
> +            strip: Indicates whether stripping is on or off, and when the vlan tag is
> +                checked for a match.

Reading the code it makes more sense what you mean in the second part
of this sentence, but it might be better here to be more explicit and
say that, when strip is false, we expect there to be a vlan tag on the
received packet and said tag will be checked against `vlan_id`.

> +            vlan_id: Expected vlan ID.
> +        """
> +        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load="xxxxx")
> +        received_packets = self.send_packet_and_capture(packet)
> +        test_packet = None
> +        for packet in received_packets:
> +            if b"xxxxx" in packet.load:

I believe there could be cases where this "load" attribute might not
exist on some noise packets and that's why oftentimes this check in
other suites will have a `hasattr()` check in it. If that's not the
case and all packets do have a load attribute that would be great, but
it might be better to add the hasattr check just for certainty.

> +                test_packet = packet
> +                break
> +        if should_receive:
> +            self.verify(
> +                test_packet is not None, "Packet was dropped when it should have been received"
> +            )
> +            if test_packet is not None:
> +                if strip:
> +                    self.verify(
> +                        not test_packet.haslayer(Dot1Q), "Vlan tag was not stripped successfully"
> +                    )
> +                else:
> +                    self.verify(
> +                        test_packet.vlan == vlan_id,
> +                        "The received tag did not match the expected tag",
> +                    )
> +        else:
> +            self.verify(
> +                test_packet is None,
> +                "Packet was received when it should have been dropped",
> +            )
> +
<snip>
> +    def vlan_setup(self, testpmd: TestPmdShell, port_id: int, filtered_id: int) -> TestPmdShell:
> +        """Setup method for all test cases.
> +
> +        Args:
> +            testpmd: Testpmd shell session to send commands to.
> +            port_id: Number of port to use for setup.
> +            filtered_id: ID to be added to the vlan filter list.
> +
> +        Returns:
> +            TestPmdShell: Testpmd session being configured.
> +        """
> +        testpmd.set_forward_mode(SimpleForwardingModes.mac)
> +        testpmd.set_promisc(port_id, False)
> +        testpmd.vlan_filter_set(port=port_id, on=True)
> +        testpmd.rx_vlan(vlan=filtered_id, port=port_id, add=True)
> +        return testpmd
> +
> +    def test_vlan_receipt_no_stripping(self) -> None:
> +        """Ensure vlan packet is dropped when receipts are enabled and header stripping is disabled.

This test case isn't testing for a packet being dropped, we should
probably adjust the doc-string accordingly.

> +
> +        Test:
> +            Create an interactive testpmd shell and verify a vlan packet.
> +        """
> +        with TestPmdShell(node=self.sut_node) as testpmd:
> +            testpmd = self.vlan_setup(testpmd=testpmd, port_id=0, filtered_id=1)
> +            testpmd.start()
> +            self.send_vlan_packet_and_verify(True, strip=False, vlan_id=1)
> +
<snip>
> 2.44.0
>

  reply	other threads:[~2024-08-09 17:23 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-11 16:15 [PATCH v2 0/2] VLAN test suite Dean Marx
2024-06-11 16:15 ` [PATCH v2 1/2] Initial implementation for " Dean Marx
2024-06-11 16:15 ` [PATCH v2 2/2] conf schema Dean Marx
2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
2024-06-14 15:02   ` [PATCH v3 1/3] Added VLAN commands to testpmd_shell class Dean Marx
2024-06-14 15:59     ` Patrick Robb
2024-06-14 20:29       ` Jeremy Spewock
2024-06-14 21:24         ` Patrick Robb
2024-06-17 14:37     ` Jeremy Spewock
2024-06-14 15:02   ` [PATCH v3 2/3] Initial implementation for VLAN test suite Dean Marx
2024-06-14 16:19     ` Patrick Robb
2024-06-17 14:56     ` Jeremy Spewock
2024-06-14 15:02   ` [PATCH v3 3/3] Config schema Dean Marx
2024-06-17 14:59     ` Jeremy Spewock
2024-06-17 14:35   ` [PATCH v3 0/3] VLAN Test Suite Jeremy Spewock
2024-06-17 17:50   ` Patrick Robb
2024-06-18 15:20   ` [PATCH v4 1/3] dts: refactored VLAN test suite Dean Marx
2024-06-18 15:20     ` [PATCH v4 2/3] dts: updated testpmd shell class Dean Marx
2024-06-18 15:20     ` [PATCH v4 3/3] dts: config schema Dean Marx
2024-06-18 16:29   ` [PATCH v5 1/3] dts: updated testpmd shell class Dean Marx
2024-06-18 16:29     ` [PATCH v5 2/3] dts: refactored VLAN test suite Dean Marx
2024-06-21 20:53       ` Jeremy Spewock
2024-06-18 16:29     ` [PATCH v5 3/3] dts: config schema Dean Marx
2024-06-21 20:53       ` Jeremy Spewock
2024-06-21 20:50     ` [PATCH v5 1/3] dts: updated testpmd shell class Jeremy Spewock
2024-06-24 18:17   ` [PATCH v6 " Dean Marx
2024-06-24 18:17     ` [PATCH v6 2/3] dts: refactored VLAN test suite Dean Marx
2024-06-24 18:17     ` [PATCH v6 3/3] dts: config schema Dean Marx
2024-06-25 15:33   ` [PATCH v7 1/3] dts: VLAN test suite implementation Dean Marx
2024-06-25 15:33     ` [PATCH v7 2/3] dts: add VLAN methods to testpmd shell Dean Marx
2024-06-26 18:22       ` Jeremy Spewock
2024-06-25 15:33     ` [PATCH v7 3/3] dts: config schema Dean Marx
2024-06-26 18:23       ` Jeremy Spewock
2024-06-26 18:21     ` [PATCH v7 1/3] dts: VLAN test suite implementation Jeremy Spewock
2024-06-28 14:00   ` [PATCH v8 1/3] dts: add VLAN methods to testpmd shell Dean Marx
2024-06-28 14:00     ` [PATCH v8 2/3] dts: VLAN test suite implementation Dean Marx
2024-07-01 19:52       ` Jeremy Spewock
2024-06-28 14:00     ` [PATCH v8 3/3] dts: config schema Dean Marx
2024-07-01 19:48     ` [PATCH v8 1/3] dts: add VLAN methods to testpmd shell Jeremy Spewock
2024-07-03 16:47   ` [PATCH v9 1/3] dts: config schema Dean Marx
2024-07-03 16:47     ` [PATCH v9 2/3] dts: VLAN test suite implementation Dean Marx
2024-07-03 16:47     ` [PATCH v9 3/3] dts: add VLAN methods to testpmd shell Dean Marx
2024-07-09 21:22       ` Jeremy Spewock
2024-07-03 16:50   ` [PATCH v10 1/3] " Dean Marx
2024-07-03 16:50     ` [PATCH v10 2/3] dts: VLAN test suite implementation Dean Marx
2024-07-09 21:22       ` Jeremy Spewock
2024-07-03 16:50     ` [PATCH v10 3/3] dts: config schema Dean Marx
2024-07-05 15:55       ` Patrick Robb
2024-07-10 15:38       ` Jeremy Spewock
2024-07-17 20:31   ` [PATCH v11 1/3] dts: add VLAN methods to testpmd shell Dean Marx
2024-07-17 20:31     ` [PATCH v11 2/3] dts: VLAN test suite implementation Dean Marx
2024-07-19 15:35       ` Jeremy Spewock
2024-07-17 20:31     ` [PATCH v11 3/3] dts: config schema Dean Marx
2024-07-19 15:35       ` Jeremy Spewock
2024-07-19 15:35     ` [PATCH v11 1/3] dts: add VLAN methods to testpmd shell Jeremy Spewock
2024-07-24 16:30 ` [PATCH v12 0/3] dts: refactored VLAN test suite Dean Marx
2024-07-24 16:30   ` [PATCH v12 1/3] dts: add VLAN methods to testpmd shell Dean Marx
2024-07-24 16:30   ` [PATCH v12 2/3] dts: VLAN test suite implementation Dean Marx
2024-07-24 16:30   ` [PATCH v12 3/3] dts: config schema Dean Marx
2024-08-07 19:43   ` [PATCH v13 0/2] dts: refactored VLAN test suite Dean Marx
2024-08-07 19:43     ` [PATCH v13 1/2] dts: add VLAN methods to testpmd shell Dean Marx
2024-08-09 17:23       ` Jeremy Spewock
2024-08-07 19:43     ` [PATCH v13 2/2] dts: VLAN test suite implementation Dean Marx
2024-08-09 17:23       ` Jeremy Spewock [this message]
2024-08-23 21:16     ` [PATCH v14 0/2] dts: port over VLAN test suite Dean Marx
2024-08-23 21:16       ` [PATCH v14 1/2] dts: add VLAN methods to testpmd shell Dean Marx
2024-09-04 19:49         ` Jeremy Spewock
2024-08-23 21:16       ` [PATCH v14 2/2] dts: VLAN test suite implementation Dean Marx
2024-09-04 19:49         ` Jeremy Spewock
2024-09-11 17:43     ` [PATCH v14 0/1] dts: port over VLAN test suite Dean Marx
2024-09-11 17:43       ` [PATCH v14 1/1] dts: VLAN test suite implementation Dean Marx
2024-09-18 20:38       ` [PATCH v15 0/1] dts: port over VLAN test suite Dean Marx
2024-09-18 20:38         ` [PATCH v15 1/1] dts: VLAN test suite implementation Dean Marx
2024-09-18 20:49       ` [PATCH v15 0/1] dts: port over VLAN test suite Dean Marx
2024-09-18 20:49         ` [PATCH v15] dts: VLAN test suite implementation Dean Marx
2024-10-08 17:20           ` [PATCH v16 0/1] dts: port over VLAN test suite Dean Marx
2024-10-08 17:20             ` [PATCH v16] dts: VLAN test suite implementation Dean Marx
2024-10-08 19:18               ` [PATCH v17 0/1] port over VLAN test suite Dean Marx
2024-10-08 19:18                 ` [PATCH v17 1/1] dts: VLAN test suite implementation Dean Marx

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=CAAA20UQ48KpvuEPxVoTg_Od-NSi2+FKhCp2Lc4H3XD1e8j074w@mail.gmail.com \
    --to=jspewock@iol.unh.edu \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=dev@dpdk.org \
    --cc=dmarx@iol.unh.edu \
    --cc=juraj.linkes@pantheon.tech \
    --cc=luca.vizzarro@arm.com \
    --cc=npratte@iol.unh.edu \
    --cc=paul.szczepanek@arm.com \
    --cc=probb@iol.unh.edu \
    --cc=yoan.picchi@foss.arm.com \
    /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).