netdev - Re: [PATCHv3 net-next 3/3] tools: ynl: add YNL test framework

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: <aRVXEmUowd0rqnLU@fedora>
Date: Thu, 13 Nov 2025 03:57:06 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Donald Hunter <donald.hunter@...il.com>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>, Jan Stancek <jstancek@...hat.com>,
	"Matthieu Baerts (NGI0)" <matttbe@...nel.org>,
	Asbjørn Sloth Tønnesen <ast@...erby.net>,
	Stanislav Fomichev <sdf@...ichev.me>,
	Ido Schimmel <idosch@...dia.com>,
	Guillaume Nault <gnault@...hat.com>,
	Sabrina Dubroca <sd@...asysnail.net>,
	Petr Machata <petrm@...dia.com>
Subject: Re: [PATCHv3 net-next 3/3] tools: ynl: add YNL test framework

On Tue, Nov 11, 2025 at 11:51:38AM +0000, Donald Hunter wrote:
> > diff --git a/tools/net/ynl/tests/Makefile b/tools/net/ynl/tests/Makefile
> > new file mode 100644
> > index 000000000000..4d527f9c3de9
> > --- /dev/null
> > +++ b/tools/net/ynl/tests/Makefile
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Makefile for YNL tests
> > +
> > +TESTS := \
> > +	test_ynl_cli.sh \
> > +	test_ynl_ethtool.sh \
> > +# end of TESTS
> > +
> > +all: $(TESTS)
> > +
> > +run_tests:
> > +	@echo "Running YNL tests..."
> > +	@failed=0; \
> > +	echo "Running test_ynl_cli.sh..."; \
> > +	./test_ynl_cli.sh || failed=$$(($$failed + 1)); \
> > +	echo "Running test_ynl_ethtool.sh..."; \
> > +	./test_ynl_ethtool.sh || failed=$$(($$failed + 1)); \
> 
> This could iterate through $(TESTS) instead of being hard coded.
> 
> > +	if [ $$failed -eq 0 ]; then \
> > +		echo "All tests passed!"; \
> > +	else \
> > +		echo "$$failed test(s) failed!"; \
> 
> AFAICS this will never be reported since the scripts only ever exit 0.
> The message is also a bit misleading since it would be the count of
> scripts that failed, not individual tests.
> 
> It would be great if the scripts exited with the number of test failures
> so the make file could report a total.

Yes, indeed, I will fix it. Thanks!

> > +# Test netdev family operations (dev-get, queue-get)
> > +cli_netdev_ops() {
> > +	local dev_output
> > +	local ifindex
> > +
> > +	ifindex=$(ip netns exec "$testns" cat /sys/class/net/"$NSIM_DEV_NAME"/ifindex 2>/dev/null)
> > +	if [[ -z "$ifindex" ]]; then
> > +		echo "FAIL: YNL CLI netdev operations (failed to get ifindex)"
> 
> This is a bit misleading, there's no ynl command here so I don't think
> it should be a FAIL. Can we just report SKIP when it is an infra issue?

The test reported SKIP for infra issue in setup() function.
Here the netdevsim device has already setup. The ifindex should be there.
Maybe I should use ip cmd to get the ifindex number and skip this checking.

> > +
> > +# Test rt-* family operations (route, addr, link, neigh, rule)
> > +cli_rt_ops() {
> > +	local ifindex
> > +
> > +	if ! $ynl --list-families 2>/dev/null | grep -q "rt-"; then
> > +		echo "SKIP: YNL CLI rt-* operations (no rt-* families available)"
> > +		return
> > +	fi
> > +
> > +	ifindex=$(ip netns exec "$testns" cat /sys/class/net/"$NSIM_DEV_NAME"/ifindex 2>/dev/null)
> > +	if [[ -z "$ifindex" ]]; then
> > +		echo "FAIL: YNL CLI rt-* operations (failed to get ifindex)"
> 
> Also FAIL -> SKIP ?

Same here, the dev has created, I will remove this checking.

Thanks
Hangbin