[PATCH] test: Wait for network before starting passt in two_guests setup

[PATCH] test: Wait for network before starting passt in two_guests setup

[Stefano Brivio]

As pasta now configures that target network namespace with
--config-net, we need to wait for addresses and routes to be actually
present. Just sending netlink messages doesn't mean this is done
synchronously.

A more elegant alternative, which probably makes sense regardless of
this test setup, would be to query, from pasta, addresses and routes
we added, and wait until they're there, before proceeding.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 test/lib/setup | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/lib/setup b/test/lib/setup
index ab6d8d5..07d5056 100755
--- a/test/lib/setup
+++ b/test/lib/setup
@@ -208,6 +208,8 @@ setup_two_guests() {
 
 	__ifname="$(context_run qemu_1 "ip -j link show | jq -rM '.[] | select(.link_type == \"ether\").ifname'")"
 
+	sleep 1
+
 	__opts=
 	[ ${PCAP} -eq 1 ] && __opts="${__opts} -p ${LOGDIR}/passt_1.pcap"
 	[ ${DEBUG} -eq 1 ] && __opts="${__opts} -d"
-- 
@@ -208,6 +208,8 @@ setup_two_guests() {
 
 	__ifname="$(context_run qemu_1 "ip -j link show | jq -rM '.[] | select(.link_type == \"ether\").ifname'")"
 
+	sleep 1
+
 	__opts=
 	[ ${PCAP} -eq 1 ] && __opts="${__opts} -p ${LOGDIR}/passt_1.pcap"
 	[ ${DEBUG} -eq 1 ] && __opts="${__opts} -d"
-- 
2.35.1

Re: [PATCH] test: Wait for network before starting passt in two_guests setup

[David Gibson]

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

On Fri, Nov 04, 2022 at 02:53:36AM +0100, Stefano Brivio wrote:
> As pasta now configures that target network namespace with
> --config-net, we need to wait for addresses and routes to be actually
> present. Just sending netlink messages doesn't mean this is done
> synchronously.
> 
> A more elegant alternative, which probably makes sense regardless of
> this test setup, would be to query, from pasta, addresses and routes
> we added, and wait until they're there, before proceeding.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Ugly, but I see the point.

As you say in future I hope we can actually detect the completion of
interface configuration which would be useful both here and other
places.

One other thing to consider - in the case where pasta is spawing a new
process, rather than attaching to an existing one, it's possibly
counterintuitive that the process starts before pasta has completed
setup.  We possibly should get the spawned thread to wait until the
main pasta thread is entering its main loop before exec()ing the
shell.  Obviously requires some sync mechanism, which is a bit clunky.

> ---
>  test/lib/setup | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/test/lib/setup b/test/lib/setup
> index ab6d8d5..07d5056 100755
> --- a/test/lib/setup
> +++ b/test/lib/setup
> @@ -208,6 +208,8 @@ setup_two_guests() {
>  
>  	__ifname="$(context_run qemu_1 "ip -j link show | jq -rM '.[] | select(.link_type == \"ether\").ifname'")"
>  
> +	sleep 1
> +
>  	__opts=
>  	[ ${PCAP} -eq 1 ] && __opts="${__opts} -p ${LOGDIR}/passt_1.pcap"
>  	[ ${DEBUG} -eq 1 ] && __opts="${__opts} -d"

-- 
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 --]

Re: [PATCH] test: Wait for network before starting passt in two_guests setup

[David Gibson]

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

On Mon, Nov 07, 2022 at 02:21:30PM +1100, David Gibson wrote:
> On Fri, Nov 04, 2022 at 02:53:36AM +0100, Stefano Brivio wrote:
> > As pasta now configures that target network namespace with
> > --config-net, we need to wait for addresses and routes to be actually
> > present. Just sending netlink messages doesn't mean this is done
> > synchronously.
> > 
> > A more elegant alternative, which probably makes sense regardless of
> > this test setup, would be to query, from pasta, addresses and routes
> > we added, and wait until they're there, before proceeding.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> 
> Ugly, but I see the point.

Oops, forgot to add

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> 
> As you say in future I hope we can actually detect the completion of
> interface configuration which would be useful both here and other
> places.
> 
> One other thing to consider - in the case where pasta is spawing a new
> process, rather than attaching to an existing one, it's possibly
> counterintuitive that the process starts before pasta has completed
> setup.  We possibly should get the spawned thread to wait until the
> main pasta thread is entering its main loop before exec()ing the
> shell.  Obviously requires some sync mechanism, which is a bit clunky.
> 
> > ---
> >  test/lib/setup | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/test/lib/setup b/test/lib/setup
> > index ab6d8d5..07d5056 100755
> > --- a/test/lib/setup
> > +++ b/test/lib/setup
> > @@ -208,6 +208,8 @@ setup_two_guests() {
> >  
> >  	__ifname="$(context_run qemu_1 "ip -j link show | jq -rM '.[] | select(.link_type == \"ether\").ifname'")"
> >  
> > +	sleep 1
> > +
> >  	__opts=
> >  	[ ${PCAP} -eq 1 ] && __opts="${__opts} -p ${LOGDIR}/passt_1.pcap"
> >  	[ ${DEBUG} -eq 1 ] && __opts="${__opts} -d"
> 



-- 
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 --]

Re: [PATCH] test: Wait for network before starting passt in two_guests setup

[Stefano Brivio]

On Mon, 7 Nov 2022 14:21:30 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Nov 04, 2022 at 02:53:36AM +0100, Stefano Brivio wrote:
> > As pasta now configures that target network namespace with
> > --config-net, we need to wait for addresses and routes to be actually
> > present. Just sending netlink messages doesn't mean this is done
> > synchronously.
> > 
> > A more elegant alternative, which probably makes sense regardless of
> > this test setup, would be to query, from pasta, addresses and routes
> > we added, and wait until they're there, before proceeding.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> Ugly, but I see the point.
> 
> As you say in future I hope we can actually detect the completion of
> interface configuration which would be useful both here and other
> places.

Yes... the outcome of "pasta --config-net ip address show" (compared to
"pasta --config-net -- sh -c 'sleep 0.05; ip address show'") is a bit
bothering.

> One other thing to consider - in the case where pasta is spawing a new
> process, rather than attaching to an existing one, it's possibly
> counterintuitive that the process starts before pasta has completed
> setup.  We possibly should get the spawned thread to wait until the
> main pasta thread is entering its main loop before exec()ing the
> shell.  Obviously requires some sync mechanism, which is a bit clunky.

Right, that might be a problem if somebody spawns a script that relies
on the tap interface to be working right away (even though the delay is
probably small enough for all practical cases).

I'll file a ticket covering both topics.

-- 
Stefano

Re: [PATCH] test: Wait for network before starting passt in two_guests setup

[Stefano Brivio]

On Mon, 7 Nov 2022 10:51:06 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:

> On Mon, 7 Nov 2022 14:21:30 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, Nov 04, 2022 at 02:53:36AM +0100, Stefano Brivio wrote:  
> > > As pasta now configures that target network namespace with
> > > --config-net, we need to wait for addresses and routes to be actually
> > > present. Just sending netlink messages doesn't mean this is done
> > > synchronously.
> > > 
> > > A more elegant alternative, which probably makes sense regardless of
> > > this test setup, would be to query, from pasta, addresses and routes
> > > we added, and wait until they're there, before proceeding.
> > > 
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>    
> > 
> > Ugly, but I see the point.
> > 
> > As you say in future I hope we can actually detect the completion of
> > interface configuration which would be useful both here and other
> > places.  
> 
> Yes... the outcome of "pasta --config-net ip address show" (compared to
> "pasta --config-net -- sh -c 'sleep 0.05; ip address show'") is a bit
> bothering.
> 
> > One other thing to consider - in the case where pasta is spawing a new
> > process, rather than attaching to an existing one, it's possibly
> > counterintuitive that the process starts before pasta has completed
> > setup.  We possibly should get the spawned thread to wait until the
> > main pasta thread is entering its main loop before exec()ing the
> > shell.  Obviously requires some sync mechanism, which is a bit clunky.  
> 
> Right, that might be a problem if somebody spawns a script that relies
> on the tap interface to be working right away (even though the delay is
> probably small enough for all practical cases).
> 
> I'll file a ticket covering both topics.

... https://bugs.passt.top/show_bug.cgi?id=37.

-- 
Stefano