DPDK patches and discussions
 help / color / mirror / Atom feed
From: Luca Vizzarro <Luca.Vizzarro@arm.com>
To: Dean Marx <dmarx@iol.unh.edu>,
	probb@iol.unh.edu, yoan.picchi@foss.arm.com,
	Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com
Cc: dev@dpdk.org
Subject: Re: [PATCH v1 2/2] dts: add exception handling to checksum verify method
Date: Tue, 29 Jul 2025 13:40:26 +0100	[thread overview]
Message-ID: <a74fe639-e6b1-42d1-b681-9d2158c5be4a@arm.com> (raw)
In-Reply-To: <20250722172214.202308-2-dmarx@iol.unh.edu>

On 22/07/2025 18:22, Dean Marx wrote:
> --- a/dts/tests/TestSuite_checksum_offload.py
> +++ b/dts/tests/TestSuite_checksum_offload.py
> @@ -89,8 +89,11 @@ def send_packet_and_verify_checksum(
>               if testpmd_packet.l4_dport == id:
>                   is_IP = PacketOffloadFlag.RTE_MBUF_F_RX_IP_CKSUM_GOOD in testpmd_packet.ol_flags
>                   is_L4 = PacketOffloadFlag.RTE_MBUF_F_RX_L4_CKSUM_GOOD in testpmd_packet.ol_flags
> -        self.verify(is_L4 == good_L4, "Layer 4 checksum flag did not match expected checksum flag.")
> -        self.verify(is_IP == good_IP, "IP checksum flag did not match expected checksum flag.")
> +        try:
> +            self.verify(is_L4 == good_L4, "Layer 4 checksum flag did not match expected checksum flag.")
> +            self.verify(is_IP == good_IP, "IP checksum flag did not match expected checksum flag.")
> +        except NameError:
> +            self.verify(False, "Test packet was dropped when it should have been received.")

Doesn't really look like the right approach. As it stands I can't tell 
from the code at first glance why are we checking for NameError. I am 
guessing this is because is_L4 is_IP weren't set. We shouldn't cause 
Python to fail on their inexistence and then recover. We should rather 
verify that they exist.

I'd propose to set is_L4 and is_IP to None at the beginning of the 
method. Then after the for loop, check that they are not None through 
self.verify. If mypy becomes unhappy because it can't compare bool with 
bool | None, then verify the non-nullness with an if and do 
self.verify(False, ...) as you are doing already. This will guarantee to 
mypy that the variables won't be None afterwards.

  reply	other threads:[~2025-07-29 12:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-22 17:22 [PATCH v1 1/2] dts: fix docstring typo in checksum suite Dean Marx
2025-07-22 17:22 ` [PATCH v1 2/2] dts: add exception handling to checksum verify method Dean Marx
2025-07-29 12:40   ` Luca Vizzarro [this message]
2025-07-29 12:31 ` [PATCH v1 1/2] dts: fix docstring typo in checksum suite Luca Vizzarro

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=a74fe639-e6b1-42d1-b681-9d2158c5be4a@arm.com \
    --to=luca.vizzarro@arm.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=dev@dpdk.org \
    --cc=dmarx@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).