Re: [PATCHv3 net 3/3] selftests: bonding: add ipsec offload test

Petr Machata
2025-02-27T14:24:08Z

Hangbin Liu <liuhangbin@gmail.com> writes:

> This introduces a test for IPSec offload over bonding, utilizing netdevsim
> for the testing process, as veth interfaces do not support IPSec offload.
> The test will ensure that the IPSec offload functionality remains operational
> even after a failover event occurs in the bonding configuration.
>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> .../selftests/drivers/net/bonding/Makefile | 3 +-
> .../drivers/net/bonding/bond_ipsec_offload.sh | 155 ++++++++++++++++++
> .../selftests/drivers/net/bonding/config | 4 +
> 3 files changed, 161 insertions(+), 1 deletion(-)
> create mode 100755 tools/testing/selftests/drivers/net/bonding/bond_ipsec_offload.sh
>
> diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
> index 2b10854e4b1e..d5a7de16d33a 100644
> --- a/tools/testing/selftests/drivers/net/bonding/Makefile
> +++ b/tools/testing/selftests/drivers/net/bonding/Makefile
> @@ -10,7 +10,8 @@ TEST_PROGS := \
> mode-2-recovery-updelay.sh \
> bond_options.sh \
> bond-eth-type-change.sh \
> - bond_macvlan_ipvlan.sh
> + bond_macvlan_ipvlan.sh \
> + bond_ipsec_offload.sh
>
> TEST_FILES := \
> lag_lib.sh \
> diff --git a/tools/testing/selftests/drivers/net/bonding/bond_ipsec_offload.sh b/tools/testing/selftests/drivers/net/bonding/bond_ipsec_offload.sh
> new file mode 100755
> index 000000000000..169866b47a67
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/bonding/bond_ipsec_offload.sh
> @@ -0,0 +1,155 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# IPsec over bonding offload test:
> +#
> +# +----------------+
> +# | bond0 |
> +# | | |
> +# | eth0 eth1 |
> +# +---+-------+----+
> +#
> +# We use netdevsim instead of physical interfaces
> +#-------------------------------------------------------------------
> +# Example commands
> +# ip x s add proto esp src 192.0.2.1 dst 192.0.2.2 \
> +# spi 0x07 mode transport reqid 0x07 replay-window 32 \
> +# aead 'rfc4106(gcm(aes))' 1234567890123456dcba 128 \
> +# sel src 192.0.2.1/24 dst 192.0.2.2/24
> +# offload dev bond0 dir out
> +# ip x p add dir out src 192.0.2.1/24 dst 192.0.2.2/24 \
> +# tmpl proto esp src 192.0.2.1 dst 192.0.2.2 \
> +# spi 0x07 mode transport reqid 0x07
> +#
> +#-------------------------------------------------------------------
> +
> +lib_dir=$(dirname "$0")
> +source "$lib_dir"/../../../net/lib.sh
> +algo="aead rfc4106(gcm(aes)) 0x3132333435363738393031323334353664636261 128"
> +srcip=192.0.2.1
> +dstip=192.0.2.2
> +ipsec0=/sys/kernel/debug/netdevsim/netdevsim0/ports/0/ipsec
> +ipsec1=/sys/kernel/debug/netdevsim/netdevsim0/ports/1/ipsec
> +ret=0
> +
> +cleanup()
> +{
> + modprobe -r netdevsim
> + cleanup_ns $ns
> +}
> +
> +active_slave_changed()
> +{
> + local old_active_slave=$1
> + local new_active_slave=$(ip -n ${ns} -d -j link show bond0 | \
> + jq -r ".[].linkinfo.info_data.active_slave")
> + [ "$new_active_slave" != "$old_active_slave" -a "$new_active_slave" != "null" ]
> +}
> +
> +test_offload()
> +{
> + # use ping to exercise the Tx path
> + ip netns exec $ns ping -I bond0 -c 3 -W 1 -i 0 $dstip >/dev/null
> +
> + active_slave=$(ip -n ${ns} -d -j link show bond0 | \
> + jq -r ".[].linkinfo.info_data.active_slave")
> +
> + if [ $active_slave = $nic0 ]; then
> + sysfs=$ipsec0
> + elif [ $active_slave = $nic1 ]; then
> + sysfs=$ipsec1
> + else
> + echo "FAIL: bond_ipsec_offload invalid active_slave $active_slave"
> + ret=1
> + fi
> +
> + # The tx/rx order in sysfs may changed after failover
> + if grep -q "SA count=2 tx=3" $sysfs && grep -q "tx ipaddr=$dstip" $sysfs; then
> + echo "PASS: bond_ipsec_offload has correct tx count with link ${active_slave}"
> + else
> + echo "FAIL: bond_ipsec_offload incorrect tx count with link ${active_slave}"
> + ret=1
> + fi

lib.sh got all sorts of logging and checking helpers that were
previously in forwarding/, I think it makes sense to use them. Would the
following make sense to you?

test_offload()
{
# use ping to exercise the Tx path
ip netns exec $ns ping -I bond0 -c 3 -W 1 -i 0 $dstip >/dev/null

active_slave=$(ip -n ${ns} -d -j link show bond0 | \
jq -r ".[].linkinfo.info_data.active_slave")

RET=0

if [ $active_slave = $nic0 ]; then
sysfs=$ipsec0
elif [ $active_slave = $nic1 ]; then
sysfs=$ipsec1
else
check_err 1 "bond_ipsec_offload invalid active_slave $active_slave"
fi

# The tx/rx order in sysfs may changed after failover
grep -q "SA count=2 tx=3" $sysfs && grep -q "tx ipaddr=$dstip" $sysfs
check_err $? "incorrect tx count with link ${active_slave}"

log_test bond_ipsec_offload
}

... etc. below.

> +}
> +
> +if ! mount | grep -q debugfs; then
> + mount -t debugfs none /sys/kernel/debug/ &> /dev/null

Clean this up at exit?

defer umount /sys/kernel/debug/

(But then the cleanup trap needs to be registered sooner, and cleanup()
needs to invoke defer_scopes_cleanup.)

> +fi
> +
> +# setup netdevsim since dummy/veth dev doesn't have offload support
> +if [ ! -w /sys/bus/netdevsim/new_device ] ; then
> + modprobe -q netdevsim
> + if [ $? -ne 0 ]; then
> + echo "SKIP: can't load netdevsim for ipsec offload"
> + exit $ksft_skip
> + fi

And here you can just schedule a cleanup, as above.

defer modprobe -r netdevsim

> +fi
> +
> +trap cleanup EXIT
> +
> +setup_ns ns

defer cleanup_ns $ns

And at that point you can drop cleanup altogether, and just have:

trap defer_scopes_cleanup EXIT

> +ip -n $ns link add bond0 type bond mode active-backup miimon 100
> +ip -n $ns addr add $srcip/24 dev bond0
> +ip -n $ns link set bond0 up
> +
> +ifaces=$(ip netns exec $ns bash -c '
> + sysfsnet=/sys/bus/netdevsim/devices/netdevsim0/net/
> + echo "0 2" > /sys/bus/netdevsim/new_device
> + while [ ! -d $sysfsnet ] ; do :; done
> + udevadm settle
> + ls $sysfsnet
> +')
> +nic0=$(echo $ifaces | cut -f1 -d ' ')
> +nic1=$(echo $ifaces | cut -f2 -d ' ')
> +ip -n $ns link set $nic0 master bond0
> +ip -n $ns link set $nic1 master bond0
> +
> +# create offloaded SAs, both in and out
> +ip -n $ns x p add dir out src $srcip/24 dst $dstip/24 \
> + tmpl proto esp src $srcip dst $dstip spi 9 \
> + mode transport reqid 42
> +
> +ip -n $ns x p add dir in src $dstip/24 dst $srcip/24 \
> + tmpl proto esp src $dstip dst $srcip spi 9 \
> + mode transport reqid 42
> +
> +ip -n $ns x s add proto esp src $srcip dst $dstip spi 9 \
> + mode transport reqid 42 $algo sel src $srcip/24 dst $dstip/24 \
> + offload dev bond0 dir out
> +
> +ip -n $ns x s add proto esp src $dstip dst $srcip spi 9 \
> + mode transport reqid 42 $algo sel src $dstip/24 dst $srcip/24 \
> + offload dev bond0 dir in
> +
> +# does offload show up in ip output
> +lines=`ip -n $ns x s list | grep -c "crypto offload parameters: dev bond0 dir"`
> +if [ $lines -ne 2 ] ; then
> + echo "FAIL: bond_ipsec_offload SA offload missing from list output"
> + ret=1
> +fi
> +
> +# we didn't create a peer, make sure we can Tx by adding a permanent neighbour
> +# this need to be added after enslave
> +ip -n $ns neigh add $dstip dev bond0 lladdr 00:11:22:33:44:55
> +
> +# start Offload testing
> +test_offload
> +
> +# do failover
> +ip -n $ns link set $active_slave down
> +slowwait 5 active_slave_changed $active_slave
> +test_offload

Hm, active_slave being overriden in the function is a bit sneaky. But
shifting the assignment out of the function is not great, because then
it would just needs to be done twice. Ho hum. This might just be the
least annoying way to write it after all.

> +
> +# make sure offload get removed from driver
> +ip -n $ns x s flush
> +ip -n $ns x p flush
> +line0=$(grep -c "SA count=0" $ipsec0)
> +line1=$(grep -c "SA count=0" $ipsec1)
> +if [ $line0 -ne 1 -o $line1 -ne 1 ] ; then
> + echo "FAIL: bond_ipsec_offload SA not removed from driver"
> + ret=1
> +else
> + echo "PASS: bond_ipsec_offload SA removed from driver"
> +fi
> +
> +exit $ret

With log_test this would be. It merges results from individual tests to
get the right exit status.

exit $EXIT_STATUS

> diff --git a/tools/testing/selftests/drivers/net/bonding/config b/tools/testing/selftests/drivers/net/bonding/config
> index dad4e5fda4db..054fb772846f 100644
> --- a/tools/testing/selftests/drivers/net/bonding/config
> +++ b/tools/testing/selftests/drivers/net/bonding/config
> @@ -9,3 +9,7 @@ CONFIG_NET_CLS_FLOWER=y
> CONFIG_NET_SCH_INGRESS=y
> CONFIG_NLMON=y
> CONFIG_VETH=y
> +CONFIG_INET_ESP=y
> +CONFIG_INET_ESP_OFFLOAD=y
> +CONFIG_XFRM_USER=m
> +CONFIG_NETDEVSIM=m