From: Jeremy Spewock <jspewock@iol.unh.edu>
To: Dean Marx <dmarx@iol.unh.edu>
Cc: Honnappa.Nagarahalli@arm.com, juraj.linkes@pantheon.tech,
probb@iol.unh.edu, paul.szczepanek@arm.com,
yoan.picchi@foss.arm.com, bruce.richardson@intel.com,
luca.vizzarro@arm.com, dev@dpdk.org
Subject: Re: [PATCH v5 2/3] dts: refactored VLAN test suite
Date: Fri, 21 Jun 2024 16:53:47 -0400 [thread overview]
Message-ID: <CAAA20UTmej0HgM0TfoRbJw7Nji5f=d3L6cNUJUG3OTAneC6X+A@mail.gmail.com> (raw)
In-Reply-To: <20240618162939.23339-2-dmarx@iol.unh.edu>
The subject for this commit should likely explain that this patch adds
the VLAN test suite rather than refacoring it. This sort of falls in
line with what I mentioned on the previous patch about the commit
messages being about what the entire patch adds rather than the
difference from the last version.
Additionally, I think you will find that a lot of my comments on the
previous version of this commit [1] still apply, but I'll try to point
out anything I didn't mention there in this email.
[1] http://inbox.dpdk.org/dev/CAAA20UTSSNB-t-ty+qpWJaz_hRJ1JX9HtM_kP_utnbynpPB0zw@mail.gmail.com/
On Tue, Jun 18, 2024 at 12:30 PM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Tweaked logic on sending and verifying packets for
> more concise code, added verbose and promisc
> function calls from pmd shell module.
I mentioned this in more detail in the previous patch, but these
descriptions should be more about what the entire patch adds than a
change log from the previous.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
> dts/tests/TestSuite_vlan.py | 167 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 167 insertions(+)
> create mode 100644 dts/tests/TestSuite_vlan.py
>
<snip>
> +
> +class TestVlan(TestSuite):
> + """DPDK VLAN test suite.
> +
> + Ensures VLAN packet reception on the Poll Mode Driver when certain conditions are met.
> + If one or more of these conditions are not met, the packet reception should be unsuccessful.
The conditions should probably be stated here again briefly, and this
doc-string can go more in depth about how the testing is done than the
module which might help explain the test. You can also touch more here
on what specific cases you are testing.
> + """
> +
<snip>
> + def send_vlan_packet_and_verify(
> + self, should_receive: bool = True, strip: bool = False, vlan_id: int = -1
> + ) -> None:
> + """Generate a vlan packet, send and verify on the dut.
It probably makes more sense to call this a SUT rather than DUT since
SUT is what we call it elsewhere in the code.
> +
> + Args:
> + should_receive: indicate whether the packet should be successfully received
> + vlan_id: expected vlan ID
> + strip: indicates whether stripping is on or off,
> + and when the vlan tag is checked for a match
> + """
> + packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load='xxxxx')
It looks like you are using this payload to filter the received
packets but that might not be immediately obvious to other developers.
A comment about what this is for might help make it more clear.
Additionally, since this same filter is used later to check if you
captured the packet you are looking for, it is probably better to pull
this out into a seperate variable so it is only hard-coded in one
place.
> + received_packets = self.send_packet_and_capture(packet)
> + test_packet = None
> + for packet in received_packets:
> + if packet.haslayer(Raw) and packet[Raw].load == b'xxxxx':
> + test_packet = packet
> + break
I said in the previous version you could use filter() to shrink the
list, if you just want to find one that matches like this you could
instead use the built in function called any() in python to get a
boolean on if the packet exists or not. It does essentially the same
thing you are doing but is a little more concise.
I did something similar with the any function in this patch:
https://patchwork.dpdk.org/project/dpdk/patch/20240613181510.30135-5-jspewock@iol.unh.edu/
> + if should_receive:
> + self.verify(
> + test_packet is not None, "Packet was dropped when it should have been received"
> + )
> + if strip:
> + self.verify(Dot1Q not in test_packet, "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",
> + )
> +
> + def send_packet_and_verify_insertion(self, expected_id: int = -1) -> None:
> + """Generate a packet with no vlan tag, send and verify on the dut.
> +
> + Args:
> + expected_id: the vlan id that is being inserted through tx_offload configuration
> + should_receive: indicate whether the packet should be successfully received
> + """
> + packet = Ether() / Raw(load='xxxxx')
We could do the same thing here with pulling out the filtering into
its own variable.
> + received_packets = self.send_packet_and_capture(packet)
> + test_packet = None
> + for packet in received_packets:
<snip>
> 2.44.0
>
next prev parent reply other threads:[~2024-06-21 20:54 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-11 16:15 [PATCH v2 0/2] " 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 [this message]
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
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
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='CAAA20UTmej0HgM0TfoRbJw7Nji5f=d3L6cNUJUG3OTAneC6X+A@mail.gmail.com' \
--to=jspewock@iol.unh.edu \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=dmarx@iol.unh.edu \
--cc=juraj.linkes@pantheon.tech \
--cc=luca.vizzarro@arm.com \
--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).