netdev - Re: [RFC net-next 08/13] selftests: ovpn: add test for the FW mark feature

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 for Android: free password hash cracker in your pocket

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

Message-ID: <217d870daf5d9880fa37116161dc3e7c49947060.camel@mandelbit.com>
Date: Tue, 02 Dec 2025 17:22:03 +0100
From: Ralf Lici <ralf@...delbit.com>
To: Sabrina Dubroca <sd@...asysnail.net>, Antonio Quartulli
	 <antonio@...nvpn.net>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>, 
	linux-kselftest@...r.kernel.org, Shuah Khan <shuah@...nel.org>
Subject: Re: [RFC net-next 08/13] selftests: ovpn: add test for the FW mark
 feature

On Thu, 2025-11-27 at 12:09 +0100, Sabrina Dubroca wrote:
> 2025-11-21, 01:20:39 +0100, Antonio Quartulli wrote:
> > diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c
> > b/tools/testing/selftests/net/ovpn/ovpn-cli.c
> > index baabb4c9120e..4df596d29b8c 100644
> > --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
> > +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
> > @@ -1693,12 +1705,13 @@ static void usage(const char *cmd)
> >  	fprintf(stderr, "\tvpnaddr: peer VPN IP\n");
> >  
> >  	fprintf(stderr,
> > -		"* new_multi_peer <iface> <lport> <peers_file>: add
> > multiple peers as listed in the file\n");
> > +		"* new_multi_peer <iface> <lport> <peers_file>
> > [mark]: add multiple peers as listed in the file\n");
> >  	fprintf(stderr, "\tiface: ovpn interface name\n");
> >  	fprintf(stderr, "\tlport: local UDP port to bind to\n");
> >  	fprintf(stderr,
> >  		"\tpeers_file: text file containing one peer per
> > line. Line format:\n");
> > -	fprintf(stderr, "\t\t<peer_id> <tx_id> <raddr> <rport>
> > <laddr> <lport> <vpnaddr>\n");
> > +	fprintf(stderr, "\t\t<peer_id> <tx_id> <raddr> <rport>
> > <laddr> <lport> <vpnaddr> [mark]\n");
> 
> This line should be dropped, this patch doesn't have the corresponding
> change to parse mark while we're looping over the lines of peers_file.

Right, the usage() strings can sometimes be a bit hard to visually
parse.

> 
> > diff --git a/tools/testing/selftests/net/ovpn/test-mark.sh
> > b/tools/testing/selftests/net/ovpn/test-mark.sh
> > new file mode 100755
> > index 000000000000..a4bfe938118d
> > --- /dev/null
> > +++ b/tools/testing/selftests/net/ovpn/test-mark.sh
> [...]
> > +for p in $(seq 1 3); do
> > +	ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 60 120
> > +	ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} $((${p}
> > + 9)) 60 120
> > +done
> > +
> > +sleep 1
> > +
> > +for p in $(seq 1 3); do
> > +	ip netns exec peer0 ping -qfc 500 -s 3000 -w 3
> > 5.5.5.$((${p} + 1))
> 
> Any reason this ping (and the final one) uses -s 3000 while the one we
> expect to fail doesn't?

No, I think it's a copy-paste leftover. I'll drop this option since it
doesn’t add much value to the test.

> 
> > +done
> > +
> > +echo "Adding an nftables drop rule based on mark value ${MARK}"
> > +ip netns exec peer0 nft flush ruleset
> > +ip netns exec peer0 nft 'add table inet filter'
> > +ip netns exec peer0 nft 'add chain inet filter output { type filter
> > hook output priority 0; policy accept; }'
> > +ip netns exec peer0 nft add rule inet filter output meta mark ==
> > ${MARK} counter drop
> > +
> > +DROP_COUNTER=$(ip netns exec peer0 nft list chain inet filter
> > output | sed -n 's/.*packets \([0-9]*\).*/\1/p')
> > +sleep 1
> > +
> > +# ping should fail
> > +for p in $(seq 1 3); do
> > +	PING_OUTPUT=$(ip netns exec peer0 ping -qfc 500 -w 1
> > 5.5.5.$((p+1)) 2>&1) && exit 1
> 
> nit: inconsistent syntax for p+1 compared to the other pings ($((p+1))
> vs $((${p} + 1)))

Ok, I'll keep a consistent style for pre-existing files and follow
shellcheck's suggestions for new ones. Later we can then cleanup the
existing shellcheck warning on the older files.

> 
> > +	echo "${PING_OUTPUT}"
> > +	LOST_PACKETS=$(echo "$PING_OUTPUT" | grep 'packets
> > transmitted' | awk '{ print $1 }')
> > +	# increment the drop counter by the amount of lost packets
> > +	DROP_COUNTER=$(($DROP_COUNTER+$LOST_PACKETS))
> > +done
> > +
> > +# check if the final nft counter matches our counter
> > +TOTAL_COUNT=$(ip netns exec peer0 nft list chain inet filter output
> > | sed -n 's/.*packets \([0-9]*\).*/\1/p')
> > +[ ${DROP_COUNTER} -eq ${TOTAL_COUNT} ] || exit 1
> 
> Maybe add something like
>     echo "Expected ${TOTAL_COUNT} drops, got ${DROP_COUNTER}"
> if we're failing at this stage?

ACK.

> > +
> > +echo "Removing the drop rule"
> > +ip netns exec peer0 nft flush ruleset
> > +sleep 1
> > +
> > +for p in $(seq 1 3); do
> > +	ip netns exec peer0 ping -qfc 500 -s 3000 -w 3
> > 5.5.5.$((${p} + 1))
> > +done
> > +
> > +cleanup
> > +
> > +modprobe -r ovpn || true

-- 
Ralf Lici
Mandelbit Srl