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

[Sabrina Dubroca <sd@queasysnail.net>]

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.


> 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?

> +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)))

> +	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?

> +
> +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

-- 
Sabrina