netdev - Re: [PATCH net 5/5] selftests: net: add very basic test for netdev names and namespaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231017075858.66863311@kernel.org>
Date: Tue, 17 Oct 2023 07:58:58 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: <netdev@...r.kernel.org>, <edumazet@...gle.com>, <pabeni@...hat.com>,
 <davem@...emloft.net>
Subject: Re: [PATCH net 5/5] selftests: net: add very basic test for netdev
 names and namespaces

On Tue, 17 Oct 2023 13:25:10 +0200 Przemek Kitszel wrote:
> On 10/16/23 22:16, Jakub Kicinski wrote:
> > +fail() {
> > +    if [ ! -z "$1" ]; then  
> 
> s/! -z/-n/

I find ! -z easier to read.
! -z == "not zero", -n == "not??"

> > +	echo "ERROR: $1"
> > +    else
> > +	echo "ERROR: unexpected return code"  
> 
> I see that in some cases unexpected rc is 0, but it's worth printing.
> 
> At the expense of requiring reader to know about default values syntax,
> whole if could become:
>    echo "ERROR: ${1-unexpected return code} (rc=$_)"

SG!

> I didn't do my homework (of checking expectations of selftests
> "framework"), but perhaps errors/diag messages should go to stderr? >&2

Hm, I've never done that but won't hurt!

> > +    fi
> > +    RET_CODE=1
> > +}
> > +
> > +ip netns add $NS
> > +
> > +#
> > +# Test basic move without a rename
> > +#
> > +ip -netns $NS link add name $DEV type dummy || fail
> > +ip -netns $NS link set dev $DEV netns 1 || \  
> 
> \ after ||, |, && is redundant, not sure if it improves readability or not

Roger.

> I like this patch (and the rest of the series), especially for the fact
> how easy it is to test (compared to internals of HW drivers ;) )

Or notification fixes without something more flexible like YNL :(