netdev - Re: [PATCH 1/4] selftests: can: Import tst-filter from can-tests

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: <CAMZ6RqKfdNRBKoH16=7JDC2QKB+XO68mahg2X7zKDcUAM+8bzw@mail.gmail.com>
Date: Thu, 24 Apr 2025 16:42:14 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Felix Maurer <fmaurer@...hat.com>
Cc: socketcan@...tkopp.net, mkl@...gutronix.de, shuah@...nel.org, 
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	horms@...nel.org, linux-can@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, dcaratti@...hat.com, fstornio@...hat.com
Subject: Re: [PATCH 1/4] selftests: can: Import tst-filter from can-tests

On Tue. 22 Apr. 2025 at 21:08, Felix Maurer <fmaurer@...hat.com> wrote:
> diff --git a/tools/testing/selftests/net/can/test_raw_filter.sh b/tools/testing/selftests/net/can/test_raw_filter.sh
> new file mode 100755
> index 000000000000..e5f175c8b27b
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/test_raw_filter.sh
> @@ -0,0 +1,37 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +#set -x

Leftover from the debug?
Can you remove this line?

> +ALL_TESTS="
> +       test_raw_filter
> +"
> +
> +net_dir=$(dirname $0)/..
> +source $net_dir/lib.sh
> +
> +VCANIF="vcan0"

Here, you are making the VCANIF variable configuration, but then, in
your test_raw_filter.c I see:

  #define VCANIF "vcan0"

This means that in order to modify the interface, one would have to
both modify the .sh script and the .c source. Wouldn't it be possible
to centralize this? For example by reading the environment variable in
the C file?

Or maybe there is a smarter way to pass values in the kernel selftests
framework which I am not aware of?

> +setup()
> +{
> +       ip link add name $VCANIF type vcan || exit $ksft_skip
> +       ip link set dev $VCANIF up
> +       pwd
> +}
> +
> +cleanup()
> +{
> +       ip link delete $VCANIF
> +}

I guess that this setup() and this cleanup() is something that you
will also need in the other can tests. Would it make sense to declare
these in a common.sh file and just do a

  source common.sh

here?

> +test_raw_filter()
> +{
> +       ./test_raw_filter
> +}
> +
> +trap cleanup EXIT
> +setup
> +
> +tests_run
> +
> +exit $EXIT_STATUS

Yours sincerely,
Vincent Mailhol