netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "Aaron Conole" <aconole@redhat.com>,
	netdev@vger.kernel.org, dev@openvswitch.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Pravin B Shelar" <pshelar@ovn.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Adrián Moreno" <amorenoz@redhat.com>,
	"Simon Horman" <horms@kernel.org>
Subject: Re: [PATCH v2 net-next 0/7] selftests: net: Switch pmtu.sh to use the internal ovs script.
Date: Tue, 25 Jun 2024 11:52:17 +0200	[thread overview]
Message-ID: <20240625115217.07c820c9@elisabeth> (raw)
In-Reply-To: <20240624153023.6fabd9f1@kernel.org>

On Mon, 24 Jun 2024 15:30:23 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 24 Jun 2024 12:53:45 -0400 Aaron Conole wrote:
> > Additionally, the "Cannot find device ..." text comes from an iproute2
> > utility output.  The only place we actually interact with that is via
> > the call at pmtu.sh:973:
> > 
> > 	run_cmd ip link set ovs_br0 up
> > 
> > Maybe it is possible that the link isn't up (could some port memory
> > allocation or message be delaying it?) yet in the virtual environment.  
> 
> Depends on how the creation is implemented, normally device creation
> over netlink is synchronous.

It also looks like pyroute2 would keep everything synchronous (unless
you call NetlinkSocket.bind(async_cache=True))... weird.

> Just to be sure have you tried to repro with vng:
> 
> https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style
> 
> ? It could be the base OS difference, too, but that's harder to confirm.
> 
> > To confirm, is it possible to run in the constrained environment, but
> > put a 5s sleep or something?  I will add the following either as a
> > separate patch (ie 7/8), or I can fold it into 6/7 (and drop Stefano's
> > ACK waiting for another review):
> > 
> > 
> > wait_for_if() {
> >    if ip link show "$2" >/dev/null 2>&1; then return 0; fi
> > 
> >    for d in `seq 1 30`; do
> >       sleep 1
> >       if ip link show "$2" >/dev/null 2>&1; then return 0; fi
> >    done
> >    return 1
> > }
> > 
> > ....
> >  	setup_ovs_br_internal || setup_ovs_br_vswitchd || return $ksft_skip
> > +	wait_for_if "ovs_br0"
> >  	run_cmd ip link set ovs_br0 up
> > ....
> > 
> > Does it make sense or does it seem like I am way off base?  
> 
> sleep 1 is a bit high (sleep does accept fractional numbers!)

This script was originally (and mostly is) all nice and POSIX (where
sleep doesn't take fractional numbers), so, if you don't mind, I'd
rather prefer "sleep 0.1 || sleep 1". :)

-- 
Stefano


  reply	other threads:[~2024-06-25  9:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-20 12:55 [PATCH v2 net-next 0/7] selftests: net: Switch pmtu.sh to use the internal ovs script Aaron Conole
2024-06-20 12:55 ` [PATCH v2 net-next 1/7] selftests: openvswitch: Support explicit tunnel port creation Aaron Conole
2024-06-20 12:55 ` [PATCH v2 net-next 2/7] selftests: openvswitch: Refactor actions parsing Aaron Conole
2024-06-20 12:55 ` [PATCH v2 net-next 3/7] selftests: openvswitch: Add set() and set_masked() support Aaron Conole
2024-06-20 12:55 ` [PATCH v2 net-next 4/7] selftests: openvswitch: Add support for tunnel() key Aaron Conole
2024-06-20 12:55 ` [PATCH v2 net-next 5/7] selftests: openvswitch: Support implicit ipv6 arguments Aaron Conole
2024-06-20 12:56 ` [PATCH v2 net-next 6/7] selftests: net: Use the provided dpctl rather than the vswitchd for tests Aaron Conole
2024-06-20 12:56 ` [PATCH v2 net-next 7/7] selftests: net: add config for openvswitch Aaron Conole
2024-06-22  1:01 ` [PATCH v2 net-next 0/7] selftests: net: Switch pmtu.sh to use the internal ovs script Jakub Kicinski
2024-06-23 19:26   ` Aaron Conole
2024-06-24 16:53     ` Aaron Conole
2024-06-24 22:30       ` Jakub Kicinski
2024-06-25  9:52         ` Stefano Brivio [this message]
2024-06-25 13:18         ` Aaron Conole
2024-06-25  8:27       ` Paolo Abeni
2024-06-25 13:20         ` Aaron Conole
2024-06-25 14:06           ` Jakub Kicinski
2024-06-25 14:14             ` Aaron Conole
2024-06-25 14:41               ` Jakub Kicinski
2024-06-25 15:17                 ` Aaron Conole
2024-06-25 16:30                   ` Jakub Kicinski
2024-06-25 15:41             ` Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240625115217.07c820c9@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=aconole@redhat.com \
    --cc=amorenoz@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dev@openvswitch.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pshelar@ovn.org \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).