Re: [PATCH] test: Select first reported IPv6 address for guest/host comparison

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4634 bytes --]

On Mon, Dec 11, 2023 at 10:00:58AM +0100, Stefano Brivio wrote:
> On Mon, 11 Dec 2023 13:05:20 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, Dec 08, 2023 at 06:49:32PM +0100, Stefano Brivio wrote:
> > > If we run passt nested (a guest connected via passt to a guest
> > > connected via passt to the host), the first guest (L1) typically has
> > > two IPv6 addresses: one formed from the prefix assigned via SLAAC,
> > > and another one assigned via DHCPv6 (to match the address on the
> > > host).  
> > 
> > Hmm... it can't be just that there are multible IPv6 addresses here
> > that's the problem.  I usually have multiple IPv6 unicast addresses on
> > my laptop (one for the wifi, one for wired) and I already got that
> > working.
> > 
> > Possibly it's that there are multiple addresses on the same interface?
> 
> Yes, the guest has a single interface and multiple addresses for it,
> I'll change the commit message.
> 
> > But see below..
> > 
> > > When we select addresses for comparison, in this case, we have
> > > multiple global unicast addresses. Selecting the first reported one
> > > on both host and guest is not entirely correct (in theory, the order
> > > might differ), but works reasonably well.  
> > 
> > That approach seems good, in principle
> > 
> > > Use the trick from 5beef085978e ("test: Only select a single
> > > interface or gateway in tests") to ask jq(1) for the first address
> > > returned by the query.  
> > 
> > But that patch was selecting gateways, not local addresses, so I don't
> > think  the same syntax makes sense.
> > 
> > > 
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > > ---
> > >  test/passt/dhcp       | 8 ++++----
> > >  test/passt/ndp        | 4 ++--
> > >  test/pasta/dhcp       | 8 ++++----
> > >  test/pasta/ndp        | 4 ++--
> > >  test/two_guests/basic | 6 +++---
> > >  5 files changed, 15 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/test/passt/dhcp b/test/passt/dhcp
> > > index 7272755..b428064 100644
> > > --- a/test/passt/dhcp
> > > +++ b/test/passt/dhcp
> > > @@ -22,8 +22,8 @@ check	[ -n "__IFNAME__" ]
> > >  
> > >  test	DHCP: address
> > >  guest	/sbin/dhclient -4 __IFNAME__
> > > -gout	ADDR ip -j -4 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[0].local'
> > > -hout	HOST_ADDR ip -j -4 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME__").addr_info[0].local'
> > > +gout	ADDR ip -j -4 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[0].local] | .[0]'
> > > +hout	HOST_ADDR ip -j -4 addr show|jq -rM '[.[] | select(.ifname == "__HOST_IFNAME__").addr_info[0].local] | .[0]'  
> > 
> > I'm confused by this.  The 'addr_info[0]' should already be selecting
> > the first address from each interface, so the outer '.[0]' would only
> > be doing something if multiple interfaces are selected, and I'm not
> > quite sure why that would happen.
> > 
> > In fact, I suspect this one was right all along... 
> 
> Right, this doesn't make sense, I just went too far with the copy and
> paste. Same as for the pasta/dhcp change below. There are actually no
> issues with IPv4. I'll drop those.
> 
> > >  check	[ "__ADDR__" = "__HOST_ADDR__" ]
> > >  
> > >  test	DHCP: route
> > > @@ -49,8 +49,8 @@ check	[ "__SEARCH__" = "__HOST_SEARCH__" ]
> > >  
> > >  test	DHCPv6: address
> > >  guest	/sbin/dhclient -6 __IFNAME__
> > > -gout	ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.prefixlen == 128).local'
> > > -hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local'
> > > +gout	ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.prefixlen == 128).local] | .[0]'
> > > +hout	HOST_ADDR6 ip -j -6 addr show|jq -rM '[.[] | select(.ifname ==
> > >  "__HOST_IFNAME6__").addr_info[] | select(.scope == "global").local]
> > >  | .[0]'  
> > 
> > ... but this one isn't, because it is .addr_info[].  Previously the
> > second 'select' has picked out only one, but not in this situation.
> > So here the change makes sense to me.
> 
> Here we want to use .addr_info[] because we want to filter further on
> prefixlen or scope. But once we've done that, if we have two addresses,
> we have to pick one.

Yes, exactly.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]