All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: Hangbin Liu <liuhangbin@gmail.com>,
	John Fastabend <john.fastabend@gmail.com>
Cc: netdev@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	Mathieu Xhonneux <m.xhonneux@gmail.com>,
	Lorenz Bauer <lmb@cloudflare.com>,
	William Tu <u9012063@gmail.com>,
	Toshiaki Makita <toshiaki.makita1@gmail.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>
Subject: Re: [PATCH bpf 1/7] selftests/bpf/test_xdp_redirect_multi: use temp netns for testing
Date: Thu, 27 Jan 2022 18:53:59 -0800	[thread overview]
Message-ID: <61f35ac73faa4_738dc20880@john.notmuch> (raw)
In-Reply-To: <YfI+b3JQw5w355Zd@Laptop-X1>

Hangbin Liu wrote:
> On Wed, Jan 26, 2022 at 09:34:52PM -0800, John Fastabend wrote:
> > Hangbin Liu wrote:
> > > Use temp netns instead of hard code name for testing in case the netns
> > > already exists.
> > > 
> > > Remove the hard code interface index when creating the veth interfaces.
> > > Because when the system loads some virtual interface modules, e.g. tunnels.
> > > the ifindex of 2 will be used and the cmd will fail.
> > > 
> > > As the netns has not created if checking environment failed. Trap the
> > > clean up function after checking env.
> > > 
> > > Fixes: 8955c1a32987 ("selftests/bpf/xdp_redirect_multi: Limit the tests in netns")
> > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > > ---
> > >  .../selftests/bpf/test_xdp_redirect_multi.sh  | 60 ++++++++++---------
> > >  1 file changed, 31 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh
> > > index 05f872740999..cc57cb87e65f 100755
> > > --- a/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh
> > > +++ b/tools/testing/selftests/bpf/test_xdp_redirect_multi.sh
> > > @@ -32,6 +32,11 @@ DRV_MODE="xdpgeneric xdpdrv xdpegress"
> > >  PASS=0
> > >  FAIL=0
> > >  LOG_DIR=$(mktemp -d)
> > > +declare -a NS
> > > +NS[0]="ns0-$(mktemp -u XXXXXX)"
> > > +NS[1]="ns1-$(mktemp -u XXXXXX)"
> > > +NS[2]="ns2-$(mktemp -u XXXXXX)"
> > > +NS[3]="ns3-$(mktemp -u XXXXXX)"
> > >  
> > >  test_pass()
> > >  {
> > > @@ -47,11 +52,9 @@ test_fail()
> > >  
> > >  clean_up()
> > >  {
> > > -	for i in $(seq $NUM); do
> > > -		ip link del veth$i 2> /dev/null
> > > -		ip netns del ns$i 2> /dev/null
> > > +	for i in $(seq 0 $NUM); do
> > > +		ip netns del ${NS[$i]} 2> /dev/null
> > 
> > You dropped the `ip link del veth$i` why is this ok?
> 
> All the veth interfaces are created and attached in the netns. So remove
> the netns should also remove related veth interfaces.

Assumed so and just checked. Would have been nice to mention in the
commit so it couldn't be mistaken for a typo. Anyways Ack from me.

  reply	other threads:[~2022-01-28  2:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-25  8:17 [PATCH bpf 0/7] selftests/bpf: use temp netns for testing Hangbin Liu
2022-01-25  8:17 ` [PATCH bpf 1/7] selftests/bpf/test_xdp_redirect_multi: " Hangbin Liu
2022-01-27  5:24   ` William Tu
2022-01-27  5:34   ` John Fastabend
2022-01-27  6:40     ` Hangbin Liu
2022-01-28  2:53       ` John Fastabend [this message]
2022-01-25  8:17 ` [PATCH bpf 2/7] selftests/bpf/test_xdp_veth: " Hangbin Liu
2022-01-25  8:17 ` [PATCH bpf 3/7] selftests/bpf/test_xdp_vlan: " Hangbin Liu
2022-01-25  8:17 ` [PATCH bpf 4/7] selftests/bpf/test_lwt_seg6local: " Hangbin Liu
2022-01-25  8:17 ` [PATCH bpf 5/7] selftests/bpf/test_tcp_check_syncookie: " Hangbin Liu
2022-01-26  9:32   ` Lorenz Bauer
2022-01-25  8:17 ` [PATCH bpf 6/7] selftests/bpf/test_xdp_meta: " Hangbin Liu
2022-01-25  8:17 ` [PATCH bpf 7/7] selftests/bpf/test_xdp_redirect: " Hangbin Liu
2022-01-27  5:26   ` William Tu
2022-01-28  2:54 ` [PATCH bpf 0/7] selftests/bpf: " John Fastabend

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=61f35ac73faa4_738dc20880@john.notmuch \
    --to=john.fastabend@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=lmb@cloudflare.com \
    --cc=m.xhonneux@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=toshiaki.makita1@gmail.com \
    --cc=u9012063@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.