Linux-kselftest Archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Petr Machata <petrm@nvidia.com>
Cc: <davem@davemloft.net>, <netdev@vger.kernel.org>,
	<edumazet@google.com>, <pabeni@redhat.com>,
	<vladimir.oltean@nxp.com>, <shuah@kernel.org>,
	<liuhangbin@gmail.com>, <bpoirier@nvidia.com>,
	<linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH net-next] selftests: net: local_termination: annotate the expected failures
Date: Tue, 14 May 2024 17:43:21 -0700	[thread overview]
Message-ID: <20240514174321.376039a5@kernel.org> (raw)
In-Reply-To: <875xvhu97r.fsf@nvidia.com>

On Mon, 13 May 2024 15:25:38 +0200 Petr Machata wrote:
> For veth specifically there is xfail_on_veth:
> 
> xfail_on_veth $rcv_if_name \
> 	check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
> 		  "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
> 		  false
> 
> Which is IMHO clearer than passing an extra boolean.

The extra boolean is because we want to only fail particular subcases.
I think that's legit. If the other cases regress we want to know.
Otherwise running the test on SW bridge is only useful for catching
crashes (so less useful).

So we probably need to reset FAIL_TO_XFAIL in this case.
Any preference on how to go about that? I'm tempted to:

diff --git a/tools/testing/selftests/net/forwarding/lib.sh
b/tools/testing/selftests/net/forwarding/lib.sh index
112c85c35092..79aa3c8bc288 100644 ---
a/tools/testing/selftests/net/forwarding/lib.sh +++
b/tools/testing/selftests/net/forwarding/lib.sh @@ -473,6 +473,13 @@
ret_set_ksft_status() # Whether FAILs should be interpreted as XFAILs.
Internal. FAIL_TO_XFAIL=
 
+# Clear internal failure tracking for the next test case
+begin_test()
+{
+    RET=0
+    FAIL_TO_XFAIL=
+}
+
 check_err()
 {
 	local err=$1
diff --git
a/tools/testing/selftests/net/forwarding/local_termination.sh
b/tools/testing/selftests/net/forwarding/local_termination.sh index
65694cdf2dbb..0781ceba1348 100755 ---
a/tools/testing/selftests/net/forwarding/local_termination.sh +++
b/tools/testing/selftests/net/forwarding/local_termination.sh @@ -76,7
+76,7 @@ check_rcv() local xfail_sw=$5 
 	[ $should_receive = true ] && should_fail=0 || should_fail=1
-	RET=0
+	begin_test
 
 	tcpdump_show $if_name | grep -q "$pattern"
 

> Not sure what to do about the bridge bit though. In principle the
> various xfail_on_'s can be chained, so e.g. this should work:
> 
> xfail_on_bridge $rcv_if_name \
> xfail_on_veth $rcv_if_name \
> 	check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
> 		  "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
> 		  false
> 
> I find this preferable to adding these ad-hoc tweaks to each test
> individually. Maybe it would make sense to have:
> 
> xfail_on_kind $rcv_if_name veth bridge \
> 	check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
> 		  "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
> 		  false
> 
> And then either replace the existing xfail_on_veth's (there are just a
> handful) or convert xfail_on_veth to a wrapper around xfail_on_kind.

I think the bridge thing we can workaround by just checking
if ${NETIFS[p1]} is veth, rather than $rcv_if_name.
Since the two behave the same.

  reply	other threads:[~2024-05-15  0:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-09 23:55 [PATCH net-next] selftests: net: local_termination: annotate the expected failures Jakub Kicinski
2024-05-10  3:15 ` Hangbin Liu
2024-05-10  3:47   ` Jakub Kicinski
2024-05-10 10:58 ` Vladimir Oltean
2024-05-13 13:25 ` Petr Machata
2024-05-15  0:43   ` Jakub Kicinski [this message]
2024-05-15  9:02     ` Petr Machata
2024-05-15 23:21       ` Jakub Kicinski
2024-05-16  8:42         ` Petr Machata

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=20240514174321.376039a5@kernel.org \
    --to=kuba@kernel.org \
    --cc=bpoirier@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petrm@nvidia.com \
    --cc=shuah@kernel.org \
    --cc=vladimir.oltean@nxp.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 read-only IMAP folder(s) and NNTP newsgroup(s).