[PATCH] test: Improve logic for waiting for SLAAC & DAD to complete in NDP tests

[PATCH] test: Improve logic for waiting for SLAAC & DAD to complete in NDP tests

[David Gibson]

Since 9a0e544f05bf the NDP tests attempt to explicitly wait for DAD to
complete, rather than just having a hard coded sleep.  However, the
conditions we use are a bit sloppy and allow for a number of possible cases
where it might not work correctly.  Stefano seems to be hitting one of
these (though I'm not sure which) with some later patches.

 - We wait for *lack* of a tentative address, so if the first check occurs
   before we have even a tentative address it will bypass the delay
 - It's not entirely clear if the permanent address will always appear
   as soon as the tentative address disappears
 - We weren't filtering on interface
 - We were doing the filtering with ip-address options rather than in jq.
   However in at least in some circumstances this seems to result in an
   empty .addr_info field, rather than omitting it entirely, which could
   cause us to get the wrong result

So, instead, explicitly wait for the address we need to be present: an
RA provided address on the external interface.  While we're here we remove
the requirement that it have global scope: the "kernel_ra" check is already
sufficient to make sure this address comes from an NDP RA, not something
else.  If it's not the global scope address we expect, better to check it
and fail, rather than keep waiting.

Fixes: 9a0e544f05bf ("test: Improve test for NDP assigned prefix")

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/passt/ndp | 6 +++---
 test/pasta/ndp | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/test/passt/ndp b/test/passt/ndp
index 56b385b..516cd6b 100644
--- a/test/passt/ndp
+++ b/test/passt/ndp
@@ -17,13 +17,13 @@ htools	ip jq sipcalc grep cut
 test	Interface name
 gout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 guest	ip link set dev __IFNAME__ up
-# Wait for DAD to complete
-guest	while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; done
+# Wait for SLAAC & DAD to complete
+guest	while ! ip -j -6 addr show dev __IFNAME__ | jq -e '.[].addr_info.[] | select(.protocol == "kernel_ra")'; do sleep 0.1; done
 hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").dev] | .[0]'
 check	[ -n "__IFNAME__" ]
 
 test	SLAAC: prefix
-gout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .protocol == "kernel_ra") | .local + "/" + (.prefixlen | tostring)] | .[0]'
+gout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.protocol == "kernel_ra") | .local + "/" + (.prefixlen | tostring)] | .[0]'
 gout	PREFIX6 sipcalc __ADDR6__ | grep prefix | cut -d' ' -f4
 hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global" and .deprecated != true).local] | .[0]'
 hout	HOST_PREFIX6 sipcalc __HOST_ADDR6__/64 | grep prefix | cut -d' ' -f4
diff --git a/test/pasta/ndp b/test/pasta/ndp
index 2442ab5..952c1ea 100644
--- a/test/pasta/ndp
+++ b/test/pasta/ndp
@@ -18,11 +18,11 @@ test	Interface name
 nsout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 check	[ -n "__IFNAME__" ]
 ns	ip link set dev __IFNAME__ up
-# Wait for DAD to complete
-ns	while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; done
+# Wait for SLAAC & DAD to complete
+ns	while ! ip -j -6 addr show dev __IFNAME__ | jq -e '.[].addr_info.[] | select(.protocol == "kernel_ra")'; do sleep 0.1; done
 
 test	SLAAC: prefix
-nsout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .protocol == "kernel_ra") | .local + "/" + (.prefixlen | tostring)] | .[0]'
+nsout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.protocol == "kernel_ra") | .local + "/" + (.prefixlen | tostring)] | .[0]'
 nsout	PREFIX6 sipcalc __ADDR6__ | grep prefix | cut -d' ' -f4
 hout	HOST_ADDR6 ip -j -6 addr show|jq -rM ['.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .deprecated != true).local] | .[0]'
 hout	HOST_PREFIX6 sipcalc __HOST_ADDR6__/64 | grep prefix | cut -d' ' -f4
-- 
@@ -18,11 +18,11 @@ test	Interface name
 nsout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
 check	[ -n "__IFNAME__" ]
 ns	ip link set dev __IFNAME__ up
-# Wait for DAD to complete
-ns	while ip -j -6 addr show tentative | jq -e '.[].addr_info'; do sleep 0.1; done
+# Wait for SLAAC & DAD to complete
+ns	while ! ip -j -6 addr show dev __IFNAME__ | jq -e '.[].addr_info.[] | select(.protocol == "kernel_ra")'; do sleep 0.1; done
 
 test	SLAAC: prefix
-nsout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .protocol == "kernel_ra") | .local + "/" + (.prefixlen | tostring)] | .[0]'
+nsout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.protocol == "kernel_ra") | .local + "/" + (.prefixlen | tostring)] | .[0]'
 nsout	PREFIX6 sipcalc __ADDR6__ | grep prefix | cut -d' ' -f4
 hout	HOST_ADDR6 ip -j -6 addr show|jq -rM ['.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .deprecated != true).local] | .[0]'
 hout	HOST_PREFIX6 sipcalc __HOST_ADDR6__/64 | grep prefix | cut -d' ' -f4
-- 
2.47.0

Re: [PATCH] test: Improve logic for waiting for SLAAC & DAD to complete in NDP tests

[Stefano Brivio]

On Tue, 26 Nov 2024 14:27:27 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Since 9a0e544f05bf the NDP tests attempt to explicitly wait for DAD to
> complete, rather than just having a hard coded sleep.  However, the
> conditions we use are a bit sloppy and allow for a number of possible cases
> where it might not work correctly.  Stefano seems to be hitting one of
> these (though I'm not sure which) with some later patches.
> 
>  - We wait for *lack* of a tentative address, so if the first check occurs
>    before we have even a tentative address it will bypass the delay
>  - It's not entirely clear if the permanent address will always appear
>    as soon as the tentative address disappears
>  - We weren't filtering on interface
>  - We were doing the filtering with ip-address options rather than in jq.
>    However in at least in some circumstances this seems to result in an
>    empty .addr_info field, rather than omitting it entirely, which could
>    cause us to get the wrong result
> 
> So, instead, explicitly wait for the address we need to be present: an
> RA provided address on the external interface.  While we're here we remove
> the requirement that it have global scope: the "kernel_ra" check is already
> sufficient to make sure this address comes from an NDP RA, not something
> else.  If it's not the global scope address we expect, better to check it
> and fail, rather than keep waiting.
> 
> Fixes: 9a0e544f05bf ("test: Improve test for NDP assigned prefix")
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied.

-- 
Stefano