Netdev 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: Wed, 15 May 2024 16:21:32 -0700	[thread overview]
Message-ID: <20240515162132.476a6b43@kernel.org> (raw)
In-Reply-To: <87y18bju1n.fsf@nvidia.com>

On Wed, 15 May 2024 11:02:28 +0200 Petr Machata wrote:
> >> 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.  
> 
> I don't follow. The test has two legs, one creates a VRF and attaches
> p2, the other creates a bridge and attaches p2. Whether p1 and p2 are
> veth or HW seems orthogonal to whether $rcv_if_name is a bridge or a
> veth.

Right, my superficial understanding was that the main distinction is
whether p2/h2 can do the filtering (or possibly some offload happens).
So if p1,p2 are veths we know to XFAIL, doesn't matter if we're in 
the vrf or bridge configuration, cause these construct will not filter
either.

If I'm not making sense - I'm probably confused, I can code up what you
suggested, it will work, just more LoC :)

--->8------

From 7a645eb425530f647e88590ba7ba01681e73812e Mon Sep 17 00:00:00 2001
From: Jakub Kicinski <kuba@kernel.org>
Date: Thu, 9 May 2024 16:28:41 -0700
Subject: selftests: net: local_termination: annotate the expected failures

Vladimir said when adding this test:

  The bridge driver fares particularly badly [...] mainly because
  it does not implement IFF_UNICAST_FLT.

See commit 90b9566aa5cd ("selftests: forwarding: add a test for
local_termination.sh").

We don't want to hide the known gaps, but having a test which
always fails prevents us from catching regressions. Report
the cases we know may fail as XFAIL.

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: liuhangbin@gmail.com
CC: shuah@kernel.org
CC: linux-kselftest@vger.kernel.org

v2:
 - remove duplicated log_test_xfail
v1: https://lore.kernel.org/all/20240509235553.5740-1-kuba@kernel.org/
---
 tools/testing/selftests/net/forwarding/lib.sh       |  7 +++++++
 .../selftests/net/forwarding/local_termination.sh   | 13 ++++++++-----
 2 files changed, 15 insertions(+), 5 deletions(-)

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 c5b0cbc85b3e..a241acc02498 100755
--- a/tools/testing/selftests/net/forwarding/local_termination.sh
+++ b/tools/testing/selftests/net/forwarding/local_termination.sh
@@ -73,9 +73,12 @@ check_rcv()
 	local pattern=$3
 	local should_receive=$4
 	local should_fail=
+	local xfail_sw=$5
 
 	[ $should_receive = true ] && should_fail=0 || should_fail=1
-	RET=0
+	begin_test
+	# check if main interface is veth
+	[ "$xfail_sw" == true ] && xfail_on_veth $h1
 
 	tcpdump_show $if_name | grep -q "$pattern"
 
@@ -157,7 +160,7 @@ run_test()
 
 	check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
 		"$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
-		false
+		false true
 
 	check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, promisc" \
 		"$smac > $UNKNOWN_UC_ADDR2, ethertype IPv4 (0x0800)" \
@@ -165,7 +168,7 @@ run_test()
 
 	check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, allmulti" \
 		"$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \
-		false
+		false true
 
 	check_rcv $rcv_if_name "Multicast IPv4 to joined group" \
 		"$smac > $JOINED_MACV4_MC_ADDR, ethertype IPv4 (0x0800)" \
@@ -173,7 +176,7 @@ run_test()
 
 	check_rcv $rcv_if_name "Multicast IPv4 to unknown group" \
 		"$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 (0x0800)" \
-		false
+		false true
 
 	check_rcv $rcv_if_name "Multicast IPv4 to unknown group, promisc" \
 		"$smac > $UNKNOWN_MACV4_MC_ADDR2, ethertype IPv4 (0x0800)" \
@@ -189,7 +192,7 @@ run_test()
 
 	check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \
 		"$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 (0x86dd)" \
-		false
+		false true
 
 	check_rcv $rcv_if_name "Multicast IPv6 to unknown group, promisc" \
 		"$smac > $UNKNOWN_MACV6_MC_ADDR2, ethertype IPv6 (0x86dd)" \
-- 
2.45.0


  reply	other threads:[~2024-05-15 23:21 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
2024-05-15  9:02     ` Petr Machata
2024-05-15 23:21       ` Jakub Kicinski [this message]
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=20240515162132.476a6b43@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).