Jakub Kicinski <kuba@xxxxxxxxxx> writes:
> 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 :)
I'm not sure myself, but from the commit message it looks like the issue
is with $rcv_if_name being the bridge.
But the patch that you inline is R-b'd and T-b'd by Vladimir, so I'm
going to assume it's doing the right thing.
> +# 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
If xfail_on_veth $h1 is all that's needed, then I really don't see a
reason why not just do this:
check_rcv $rcv_if_name "Unicast IPv4 to primary MAC address" \
"$smac > $rcv_dmac, ethertype IPv4 (0x0800)" \
true
check_rcv $rcv_if_name "Unicast IPv4 to macvlan MAC address" \
"$smac > $MACVLAN_ADDR, ethertype IPv4 (0x0800)" \
true
xfail_on_veth $h1 \
check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
"$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
false
This should work now, in much the same way as this patch, but the intent
is IMHO clearer (vs. passing a mystery true), and FAIL_TO_XFAIL is
cleanly scoped and doesn't run the risk of leaking out of the test.
> 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)" \
