On 24/04/2025 at 23:02, Felix Maurer wrote:
> On 24.04.25 09:42, Vincent Mailhol wrote:
>> On Tue. 22 Apr. 2025 at 21:08, Felix Maurer <fmau...@redhat.com> wrote:
> [...]
>>> +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?
>
> Good point, I'll try to come up with something to avoid the duplication
> (either from the selftest framework or just for the CAN tests). I'd
> prefer an argument to the program though, as I find this the more usual
> way to pass info if one ever wants to run the test directly.
Passing an argument would be the best. I am not sure how to do this with the
selftests (but I did not investigate either).
>>> +setup()
>>> +{
>>> + ip link add name $VCANIF type vcan || exit $ksft_skip
>>> + ip link set dev $VCANIF up
>>> + pwd
>>> +}
Speaking of which, if you allow the user to modify the interface, then you will
one additional check here to see whether it is a virtual can interface or not
(the ip link commands are not the same for the vcan and the physical can).
Something like:
CANIF="${CANIF:-vcan}"
BITRATE="${BITRATE:-500000}"
setup()
{
if [ $CANIF == vcan* ]; then
ip link add name $CANIF type vcan || exit $ksft_skip
else
ip link set dev $CANIF type can $BITRATE 500000
fi
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?
>
> I usually try to avoid making changes in anticipation of the future. I'm
> not sure if all the tests need a similar environment and would prefer to
> split this when we encounter that they do. Are you okay with that?
Yes, this works. Keep this idea in back of your mind and if there is a need to
reuse those in the future, then it will be a good timing to do the factorize the
code.
Yours sincerely,
Vincent Mailhol
