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: initial queue start/stop suite implementation
Date: Wed, 10 Jul 2024 11:36:59 -0400 [thread overview]
Message-ID: <CAAA20USesOSXKg58GaqjZAJazZJwwWsJy+Wc=+Y4UoCea+3kkA@mail.gmail.com> (raw)
In-Reply-To: <20240703180820.18723-2-dmarx@iol.unh.edu>
Hey Dean, everything looks good here to me, but I had one thought
about how we do the verification below.
On Wed, Jul 3, 2024 at 2:08 PM Dean Marx <dmarx@iol.unh.edu> wrote:
<snip>
> +
> + def test_rx_queue_start_stop(self) -> None:
> + """Verify packets are not received by port 0 when Rx queue is disabled.
> +
> + Test:
> + Create an interactive testpmd session, stop Rx queue on port 0, verify
> + packets are not received.
> + """
> + testpmd = TestPmdShell(node=self.sut_node)
> + testpmd.set_forward_mode(SimpleForwardingModes.mac)
> + testpmd.stop_port_queue(0, 0, True)
> +
> + testpmd.start()
> + self.send_packet_and_verify(should_receive=False)
> + stats = testpmd.show_port_stats(port_id=0)
> + self.verify(
> + stats.rx_packets == 0,
> + "Packets were received on Rx queue when it should've been disabled",
> + )
Thinking more about this verification step, I think it makes a lot of
sense why we can verify here using port stats even though we usually
can't. One thing I was thinking about with this however is I think
there is a (probably small) chance that before you get the chance to
stop the port after opening testpmd, a packet is sent to it. When I
tested this it seemed like this verification step worked with some
NICs (like one I ran with on the bnxt_en driver), but specifically
because when you stop the Rx queue it clears all of the Rx stats on
the port. I'm not sure if this is something PMD specific or not, but
it might be worth putting a small comment in here that explains that
disabling the queue will reset the stats and that we then don't expect
the stat to grow from that point on at all.
If handling this number is something PMD specific, another alternative
route we could take would be taking the statistics that are returned
from the "stop" command. I think this would work because the
statistics from the stop command are only aggregated while packet
forwarding is running, and we know that we stopped the queue before
starting forwarding so that number should truly always be 0.
> + testpmd.close()
> +
> + def test_tx_queue_start_stop(self) -> None:
> + """Verify packets are not forwarded by port 1 when Tx queue is disabled.
> +
> + Test:
> + Create an interactive testpmd session, stop Tx queue on port 1, verify
> + packets are not forwarded.
> + """
> + testpmd = TestPmdShell(node=self.sut_node)
> + testpmd.set_forward_mode(SimpleForwardingModes.mac)
> + testpmd.stop_port_queue(1, 0, False)
> + testpmd.start()
> + self.send_packet_and_verify(should_receive=False)
> + stats = testpmd.show_port_stats(port_id=1)
> + self.verify(
> + stats.tx_packets == 0,
> + "Packets were forwarded on Tx queue when it should've been disabled",
> + )
> + testpmd.close()
> --
> 2.44.0
>
next prev parent reply other threads:[~2024-07-10 15:37 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-17 19:46 [PATCH v2 0/3] dts: queue start/stop suite Dean Marx
2024-06-17 19:46 ` [PATCH v2 1/3] dts: initial queue start/stop suite implementation Dean Marx
2024-06-21 21:27 ` Jeremy Spewock
2024-06-17 19:46 ` [PATCH v2 2/3] dts: added promisc/verbose func to testpmd shell Dean Marx
2024-06-21 21:28 ` Jeremy Spewock
2024-06-17 19:46 ` [PATCH v2 3/3] dts: queue suite conf schema Dean Marx
2024-06-21 21:28 ` Jeremy Spewock
2024-06-21 21:27 ` [PATCH v2 0/3] dts: queue start/stop suite Jeremy Spewock
2024-06-26 13:51 ` [PATCH v3 1/3] dts: initial queue start/stop suite implementation Dean Marx
2024-06-26 13:51 ` [PATCH v3 2/3] dts: add functions to testpmd shell Dean Marx
2024-06-26 19:51 ` Jeremy Spewock
2024-06-26 13:51 ` [PATCH v3 3/3] dts: queue suite conf schema Dean Marx
2024-06-26 19:51 ` Jeremy Spewock
2024-06-26 19:50 ` [PATCH v3 1/3] dts: initial queue start/stop suite implementation Jeremy Spewock
2024-06-28 16:19 ` [PATCH v4 1/3] dts: add functions to testpmd shell Dean Marx
2024-06-28 16:19 ` [PATCH v4 2/3] dts: initial queue start/stop suite implementation Dean Marx
2024-07-01 20:17 ` Jeremy Spewock
2024-06-28 16:19 ` [PATCH v4 3/3] dts: queue suite conf schema Dean Marx
2024-07-01 20:17 ` [PATCH v4 1/3] dts: add functions to testpmd shell Jeremy Spewock
2024-07-03 18:08 ` [PATCH v5 " Dean Marx
2024-07-03 18:08 ` [PATCH v5 2/3] dts: initial queue start/stop suite implementation Dean Marx
2024-07-10 15:36 ` Jeremy Spewock [this message]
2024-07-03 18:08 ` [PATCH v5 3/3] dts: queue suite conf schema Dean Marx
2024-07-10 15:37 ` Jeremy Spewock
2024-07-10 15:36 ` [PATCH v5 1/3] dts: add functions to testpmd shell Jeremy Spewock
2024-07-10 16:01 ` [PATCH v6 " Dean Marx
2024-07-10 16:01 ` [PATCH v6 2/3] dts: initial queue start/stop suite implementation Dean Marx
2024-07-11 13:58 ` Jeremy Spewock
2024-07-10 16:01 ` [PATCH v6 3/3] dts: queue suite conf schema Dean Marx
2024-07-11 13:59 ` [PATCH v6 1/3] dts: add functions to testpmd shell Jeremy Spewock
2024-07-17 20:23 ` [PATCH v7 " Dean Marx
2024-07-17 20:23 ` [PATCH v7 2/3] dts: initial queue start/stop suite implementation Dean Marx
2024-07-19 13:34 ` Jeremy Spewock
2024-07-23 17:04 ` Adam Hassick
2024-07-17 20:23 ` [PATCH v7 3/3] dts: queue suite conf schema Dean Marx
2024-07-19 13:34 ` Jeremy Spewock
2024-07-19 13:34 ` [PATCH v7 1/3] dts: add functions to testpmd shell Jeremy Spewock
2024-07-24 18:32 ` [PATCH v8 0/3] dts: refactored queue start/stop suite Dean Marx
2024-07-24 18:32 ` [PATCH v8 1/3] dts: add functions to testpmd shell Dean Marx
2024-07-26 19:19 ` Jeremy Spewock
2024-07-24 18:32 ` [PATCH v8 2/3] dts: initial queue start/stop suite implementation Dean Marx
2024-07-26 19:19 ` Jeremy Spewock
2024-07-24 18:32 ` [PATCH v8 3/3] dts: queue suite conf schema Dean Marx
2024-07-26 19:19 ` Jeremy Spewock
2024-08-07 19:36 ` [PATCH v9 0/2] dts: refactored queue start/stop suite Dean Marx
2024-08-07 19:36 ` [PATCH v9 1/2] dts: add functions to testpmd shell Dean Marx
2024-08-09 15:51 ` Jeremy Spewock
2024-08-07 19:36 ` [PATCH v9 2/2] dts: initial queue start/stop suite implementation Dean Marx
2024-08-09 15:52 ` Jeremy Spewock
2024-10-08 21:48 ` [PATCH v10 0/1] dts: port over queue start/stop suite Dean Marx
2024-10-08 21:48 ` [PATCH v10] " Dean Marx
2024-11-18 19:59 ` [PATCH v11] " 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='CAAA20USesOSXKg58GaqjZAJazZJwwWsJy+Wc=+Y4UoCea+3kkA@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).