netdev - Re: [patch net-next v3 3/6] selftests: forwarding: add ability to assemble NETIFS array by driver name

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] [day] [month] [year] [list]

Message-ID: <ZiEOn1GyZhyDfr25@nanopsycho>
Date: Thu, 18 Apr 2024 14:14:23 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Petr Machata <petrm@...dia.com>
Cc: netdev@...r.kernel.org, kuba@...nel.org, pabeni@...hat.com,
	davem@...emloft.net, edumazet@...gle.com, parav@...dia.com,
	mst@...hat.com, jasowang@...hat.com, xuanzhuo@...ux.alibaba.com,
	shuah@...nel.org, liuhangbin@...il.com, vladimir.oltean@....com,
	bpoirier@...dia.com, idosch@...dia.com,
	virtualization@...ts.linux.dev
Subject: Re: [patch net-next v3 3/6] selftests: forwarding: add ability to
 assemble NETIFS array by driver name

Thu, Apr 18, 2024 at 10:11:12AM CEST, petrm@...dia.com wrote:
>
>Jiri Pirko <jiri@...nulli.us> writes:
>
>> From: Jiri Pirko <jiri@...dia.com>
>>
>> Allow driver tests to work without specifying the netdevice names.
>> Introduce a possibility to search for available netdevices according to
>> set driver name. Allow test to specify the name by setting
>> NETIF_FIND_DRIVER variable.
>>
>> Note that user overrides this either by passing netdevice names on the
>> command line or by declaring NETIFS array in custom forwarding.config
>> configuration file.
>>
>> Signed-off-by: Jiri Pirko <jiri@...dia.com>
>> ---
>> v1->v2:
>> - removed unnecessary "-p" and "-e" options
>> - removed unnecessary "! -z" from the check
>> - moved NETIF_FIND_DRIVER declaration from the config options
>> ---
>>  tools/testing/selftests/net/forwarding/lib.sh | 39 +++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
>> index 2e7695b94b6b..b3fd0f052d71 100644
>> --- a/tools/testing/selftests/net/forwarding/lib.sh
>> +++ b/tools/testing/selftests/net/forwarding/lib.sh
>> @@ -94,6 +94,45 @@ if [[ ! -v NUM_NETIFS ]]; then
>>  	exit $ksft_skip
>>  fi
>>  
>> +##############################################################################
>> +# Find netifs by test-specified driver name
>> +
>> +driver_name_get()
>> +{
>> +	local dev=$1; shift
>> +	local driver_path="/sys/class/net/$dev/device/driver"
>> +
>> +	if [ ! -L $driver_path ]; then
>> +		echo ""
>> +	else
>> +		basename `realpath $driver_path`
>> +	fi
>
>This is just:
>
>	if [[ -L $driver_path ]]; then
>		basename `realpath $driver_path`
>	fi

Ok.


>
>> +}
>> +
>> +find_netif()
>
>Maybe name it find_driver_netif? find_netif sounds super generic.

netif_find_driver() to be aligned with the variable name.


>
>Also consider having it take an argument instead of accessing
>environment NETIF_FIND_DRIVER directly.

Considered, don't see any benefit. Touching outside variable NETIFS
anyway.


>
>> +{
>> +	local ifnames=`ip -j link show | jq -r ".[].ifname"`
>> +	local count=0
>> +
>> +	for ifname in $ifnames
>> +	do
>> +		local driver_name=`driver_name_get $ifname`
>> +		if [[ ! -z $driver_name && $driver_name == $NETIF_FIND_DRIVER ]]; then
>> +			count=$((count + 1))
>> +			NETIFS[p$count]="$ifname"
>> +		fi
>> +	done
>> +}
>> +
>> +# Whether to find netdevice according to the specified driver.
>> +: "${NETIF_FIND_DRIVER:=}"
>
>This would be better placed up there in the Topology description
>section. Together with NETIFS and NETIF_NO_CABLE, as it concerns
>specification of which interfaces to use.


You replied yourself.



>
>> +
>> +if [[ $NETIF_FIND_DRIVER ]]; then
>> +	unset NETIFS
>> +	declare -A NETIFS
>> +	find_netif
>> +fi
>> +
>>  net_forwarding_dir=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
>>  
>>  if [[ -f $net_forwarding_dir/forwarding.config ]]; then
>

Thanks!