public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH 0/2] test: Fixes for prefixlen handling
@ 2024-11-06  1:44 David Gibson
  2024-11-06  1:44 ` [PATCH 1/2] test: Don't require 64-bit prefixes in perf tests David Gibson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: David Gibson @ 2024-11-06  1:44 UTC (permalink / raw)
  To: passt-dev, Stefano Brivio; +Cc: David Gibson

Jon hit some problems when attempting to run the testsuite with an
IPv6 prefix length of 40, rather than 64.  Here are a couple of
patches that should fix that, along with a couple of adjacent
oddities.

David Gibson (2):
  test: Don't require 64-bit prefixes in perf tests
  test: Improve test for NDP assigned prefix

 test/passt/ndp      | 4 ++--
 test/pasta/ndp      | 4 ++--
 test/perf/pasta_tcp | 2 +-
 test/perf/pasta_udp | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.47.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] test: Don't require 64-bit prefixes in perf tests
  2024-11-06  1:44 [PATCH 0/2] test: Fixes for prefixlen handling David Gibson
@ 2024-11-06  1:44 ` David Gibson
  2024-11-19 20:43   ` Stefano Brivio
  2024-11-06  1:44 ` [PATCH 2/2] test: Improve test for NDP assigned prefix David Gibson
  2024-11-07 14:54 ` [PATCH 0/2] test: Fixes for prefixlen handling Stefano Brivio
  2 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2024-11-06  1:44 UTC (permalink / raw)
  To: passt-dev, Stefano Brivio; +Cc: David Gibson

When determining the namespace's IPv6 address in the perf test setup, we
explicitly filter for addresses with a 64-bit prefix length.  There's no
real reason we need that - as long as it's a global address we can use it.
I suspect this was copied without thinking from a similar example in the
NDP tests, where the 64-bit prefix length _is_ meaningful (though it's not
entirely clear if the handling is correct there either).

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

diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp
index d1ccf7d..88284b2 100644
--- a/test/perf/pasta_tcp
+++ b/test/perf/pasta_tcp
@@ -211,7 +211,7 @@ tr	TCP throughput over IPv6: host to ns
 iperf3s	ns 10002
 
 nsout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
-nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local'
+nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global").local'
 bw	-
 bw	-
 bw	-
diff --git a/test/perf/pasta_udp b/test/perf/pasta_udp
index 544bf17..3d07091 100644
--- a/test/perf/pasta_udp
+++ b/test/perf/pasta_udp
@@ -196,7 +196,7 @@ tr	UDP throughput over IPv6: host to ns
 iperf3s	ns 10002
 
 nsout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
-nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local'
+nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global").local'
 iperf3	BW host __ADDR6__ 10002 __TIME__ __OPTS__ -b 8G -l 1472
 bw	__BW__ 0.3 0.5
 iperf3	BW host __ADDR6__ 10002 __TIME__ __OPTS__ -b 12G -l 3972
-- 
@@ -196,7 +196,7 @@ tr	UDP throughput over IPv6: host to ns
 iperf3s	ns 10002
 
 nsout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
-nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local'
+nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global").local'
 iperf3	BW host __ADDR6__ 10002 __TIME__ __OPTS__ -b 8G -l 1472
 bw	__BW__ 0.3 0.5
 iperf3	BW host __ADDR6__ 10002 __TIME__ __OPTS__ -b 12G -l 3972
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] test: Improve test for NDP assigned prefix
  2024-11-06  1:44 [PATCH 0/2] test: Fixes for prefixlen handling David Gibson
  2024-11-06  1:44 ` [PATCH 1/2] test: Don't require 64-bit prefixes in perf tests David Gibson
@ 2024-11-06  1:44 ` David Gibson
  2024-11-08  9:33   ` Stefano Brivio
  2024-11-07 14:54 ` [PATCH 0/2] test: Fixes for prefixlen handling Stefano Brivio
  2 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2024-11-06  1:44 UTC (permalink / raw)
  To: passt-dev, Stefano Brivio; +Cc: David Gibson

In the NDP tests we search explicitly for a guest address with prefix
length 64.  AFAICT this is an attempt to specifically find the SLAAC
assigned address, rather than something assigned by other means.  We can do
that more explicitly by checking for .protocol == "kernel_ra". however.

The SLAAC prefixes we assigned *will* always be 64-bit, that's hard-coded
into our NDP implementation.  RFC4862 doesn't really allow anything else
since the interface identifiers for an Ethernet-like link are 64-bits.

Let's actually verify that, rather than just assuming it, by extracting the
prefix length assigned in the guest and checking it as well.

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

diff --git a/test/passt/ndp b/test/passt/ndp
index f54b8ce..56b385b 100644
--- a/test/passt/ndp
+++ b/test/passt/ndp
@@ -23,8 +23,8 @@ hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").d
 check	[ -n "__IFNAME__" ]
 
 test	SLAAC: prefix
-gout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local] | .[0]'
-gout	PREFIX6 sipcalc __ADDR6__/64 | grep prefix | cut -d' ' -f4
+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	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
 check	[ "__PREFIX6__" = "__HOST_PREFIX6__" ]
diff --git a/test/pasta/ndp b/test/pasta/ndp
index c59627f..2442ab5 100644
--- a/test/pasta/ndp
+++ b/test/pasta/ndp
@@ -22,8 +22,8 @@ ns	ip link set dev __IFNAME__ up
 ns	while ip -j -6 addr show tentative | jq -e '.[].addr_info'; 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 .prefixlen == 64).local] | .[0]'
-nsout	PREFIX6 sipcalc __ADDR6__/64 | grep prefix | cut -d' ' -f4
+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	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
 check	[ "__PREFIX6__" = "__HOST_PREFIX6__" ]
-- 
@@ -22,8 +22,8 @@ ns	ip link set dev __IFNAME__ up
 ns	while ip -j -6 addr show tentative | jq -e '.[].addr_info'; 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 .prefixlen == 64).local] | .[0]'
-nsout	PREFIX6 sipcalc __ADDR6__/64 | grep prefix | cut -d' ' -f4
+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	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
 check	[ "__PREFIX6__" = "__HOST_PREFIX6__" ]
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] test: Fixes for prefixlen handling
  2024-11-06  1:44 [PATCH 0/2] test: Fixes for prefixlen handling David Gibson
  2024-11-06  1:44 ` [PATCH 1/2] test: Don't require 64-bit prefixes in perf tests David Gibson
  2024-11-06  1:44 ` [PATCH 2/2] test: Improve test for NDP assigned prefix David Gibson
@ 2024-11-07 14:54 ` Stefano Brivio
  2 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2024-11-07 14:54 UTC (permalink / raw)
  To: David Gibson; +Cc: passt-dev

On Wed,  6 Nov 2024 12:44:13 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Jon hit some problems when attempting to run the testsuite with an
> IPv6 prefix length of 40, rather than 64.  Here are a couple of
> patches that should fix that, along with a couple of adjacent
> oddities.

Applied.

-- 
Stefano


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] test: Improve test for NDP assigned prefix
  2024-11-06  1:44 ` [PATCH 2/2] test: Improve test for NDP assigned prefix David Gibson
@ 2024-11-08  9:33   ` Stefano Brivio
  0 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2024-11-08  9:33 UTC (permalink / raw)
  To: David Gibson; +Cc: passt-dev

On Wed,  6 Nov 2024 12:44:15 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> In the NDP tests we search explicitly for a guest address with prefix
> length 64.  AFAICT this is an attempt to specifically find the SLAAC
> assigned address, rather than something assigned by other means.  We can do
> that more explicitly by checking for .protocol == "kernel_ra". however.
> 
> The SLAAC prefixes we assigned *will* always be 64-bit, that's hard-coded
> into our NDP implementation.  RFC4862 doesn't really allow anything else
> since the interface identifiers for an Ethernet-like link are 64-bits.
> 
> Let's actually verify that, rather than just assuming it, by extracting the
> prefix length assigned in the guest and checking it as well.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  test/passt/ndp | 4 ++--
>  test/pasta/ndp | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/test/passt/ndp b/test/passt/ndp
> index f54b8ce..56b385b 100644
> --- a/test/passt/ndp
> +++ b/test/passt/ndp
> @@ -23,8 +23,8 @@ hout	HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").d
>  check	[ -n "__IFNAME__" ]
>  
>  test	SLAAC: prefix
> -gout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local] | .[0]'
> -gout	PREFIX6 sipcalc __ADDR6__/64 | grep prefix | cut -d' ' -f4
> +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]'

This makes tests occasionally fail for me. The pasta/ndp tests is not
affected. __PREFIX6__ is empty because there are no addresses with
protocol "kernel_ra" when we check. If I do:

@@ -23,6 +23,7 @@ hout  HOST_IFNAME6 ip -j -6 route show|jq -rM '[.[] | select(.dst == "default").d
 check  [ -n "__IFNAME__" ]
 
 test   SLAAC: prefix
+sleep  2
 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   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]'

things seem to work, but I didn't test many times, and looking at
47f0bd503210 ("net: Add new protocol attribute to IP addresses"), I
don't see how that would help.

-- 
Stefano


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] test: Don't require 64-bit prefixes in perf tests
  2024-11-06  1:44 ` [PATCH 1/2] test: Don't require 64-bit prefixes in perf tests David Gibson
@ 2024-11-19 20:43   ` Stefano Brivio
  0 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2024-11-19 20:43 UTC (permalink / raw)
  To: David Gibson; +Cc: passt-dev

On Wed,  6 Nov 2024 12:44:14 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> When determining the namespace's IPv6 address in the perf test setup, we
> explicitly filter for addresses with a 64-bit prefix length.  There's no
> real reason we need that - as long as it's a global address we can use it.
> I suspect this was copied without thinking from a similar example in the
> NDP tests, where the 64-bit prefix length _is_ meaningful (though it's not
> entirely clear if the handling is correct there either).

So, I'm not sure why we had those filters, I also suspect a copy and
past, but:

> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  test/perf/pasta_tcp | 2 +-
>  test/perf/pasta_udp | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp
> index d1ccf7d..88284b2 100644
> --- a/test/perf/pasta_tcp
> +++ b/test/perf/pasta_tcp
> @@ -211,7 +211,7 @@ tr	TCP throughput over IPv6: host to ns
>  iperf3s	ns 10002
>  
>  nsout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
> -nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local'
> +nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global").local'
>  bw	-
>  bw	-
>  bw	-
> diff --git a/test/perf/pasta_udp b/test/perf/pasta_udp
> index 544bf17..3d07091 100644
> --- a/test/perf/pasta_udp
> +++ b/test/perf/pasta_udp
> @@ -196,7 +196,7 @@ tr	UDP throughput over IPv6: host to ns
>  iperf3s	ns 10002
>  
>  nsout	IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname'
> -nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local'
> +nsout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global").local'

...these cause failures on a "typical" pasta-in-passt-in-passt setup,
because in the guest we typically have two addresses, one assigned via
SLAAC, and one assigned via DHCPv6, say:

    inet6 2a01:4f8:222:904::2/128 scope global noprefixroute 
       valid_lft forever preferred_lft forever
    inet6 2a01:4f8:222:904:5054:ff:fe7c:cea1/64 scope global mngtmpaddr noprefixroute 
       valid_lft forever preferred_lft forever

and, whoops:

ns$ ip -j -6 addr show|jq -rM '.[] | select(.ifname == "ens3").addr_info[] | select(.scope == "global").local'
2a01:4f8:222:904:5054:ff:fe7c:cea1
2a01:4f8:222:904::2

I guess we should come up with a jq expression selecting the longest
prefix. Any address will do, but it's probably better to keep this
consistent. On the subject, see also:

  https://github.com/siderolabs/talos/issues/9725#issuecomment-2478509622

...or should we just pick the first one as we do elsewhere?

-- 
Stefano


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-11-19 20:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-06  1:44 [PATCH 0/2] test: Fixes for prefixlen handling David Gibson
2024-11-06  1:44 ` [PATCH 1/2] test: Don't require 64-bit prefixes in perf tests David Gibson
2024-11-19 20:43   ` Stefano Brivio
2024-11-06  1:44 ` [PATCH 2/2] test: Improve test for NDP assigned prefix David Gibson
2024-11-08  9:33   ` Stefano Brivio
2024-11-07 14:54 ` [PATCH 0/2] test: Fixes for prefixlen handling Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

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 IMAP folder(s).