[PATCH net-next 3/3] selftests: bonding: add test for LACP actor port priority

Hangbin Liu posted 3 patches 2 months, 1 week ago
[PATCH net-next 3/3] selftests: bonding: add test for LACP actor port priority
Posted by Hangbin Liu 2 months, 1 week ago
Add a selftest to verify that per-port actor priority (ad_actor_port_prio)
is correctly applied and affects aggregator selection as expected.

Move cmd_jq from forwarding/lib.sh to net/lib.sh.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 .../selftests/drivers/net/bonding/Makefile    |  3 +-
 .../drivers/net/bonding/bond_lacp_prio.sh     | 73 +++++++++++++++++++
 tools/testing/selftests/net/forwarding/lib.sh | 24 ------
 tools/testing/selftests/net/lib.sh            | 24 ++++++
 4 files changed, 99 insertions(+), 25 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/bonding/bond_lacp_prio.sh

diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
index 2b10854e4b1e..32617a834a6b 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_lacp_prio.sh
 
 TEST_FILES := \
 	lag_lib.sh \
diff --git a/tools/testing/selftests/drivers/net/bonding/bond_lacp_prio.sh b/tools/testing/selftests/drivers/net/bonding/bond_lacp_prio.sh
new file mode 100755
index 000000000000..a3f939d12143
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bonding/bond_lacp_prio.sh
@@ -0,0 +1,73 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Testing if bond lacp per port priority works
+
+lib_dir=$(dirname "$0")
+# shellcheck disable=SC1091
+source "$lib_dir"/../../../net/lib.sh
+
+# create client, switch, backup switch netns
+setup_ns c_ns s_ns b_ns
+defer cleanup_all_ns
+
+# setup links
+# shellcheck disable=SC2154
+ip -n "${c_ns}" link add eth0 type veth peer name eth0 netns "${s_ns}"
+ip -n "${c_ns}" link add eth1 type veth peer name eth1 netns "${s_ns}"
+# shellcheck disable=SC2154
+ip -n "${c_ns}" link add eth2 type veth peer name eth0 netns "${b_ns}"
+ip -n "${c_ns}" link add eth3 type veth peer name eth1 netns "${b_ns}"
+
+ip -n "${c_ns}" link add bond0 type bond mode 802.3ad miimon 100 lacp_rate fast ad_select prio
+ip -n "${s_ns}" link add bond0 type bond mode 802.3ad miimon 100 lacp_rate fast
+ip -n "${b_ns}" link add bond0 type bond mode 802.3ad miimon 100 lacp_rate fast
+
+ip -n "${c_ns}" link set eth0 master bond0
+ip -n "${c_ns}" link set eth1 master bond0
+ip -n "${c_ns}" link set eth2 master bond0
+ip -n "${c_ns}" link set eth3 master bond0
+ip -n "${s_ns}" link set eth0 master bond0
+ip -n "${s_ns}" link set eth1 master bond0
+ip -n "${b_ns}" link set eth0 master bond0
+ip -n "${b_ns}" link set eth1 master bond0
+
+ip -n "${c_ns}" link set bond0 up
+ip -n "${s_ns}" link set bond0 up
+ip -n "${b_ns}" link set bond0 up
+
+# set ad actor port priority, default 255
+ip -n "${c_ns}" link set eth0 type bond_slave ad_actor_port_prio 1000
+prio=$(cmd_jq "ip -n ${c_ns} -d -j link show eth0" ".[].linkinfo.info_slave_data.ad_actor_port_prio")
+[ "$prio" -ne 1000 ] && RET=1
+ip -n "${c_ns}" link set eth2 type bond_slave ad_actor_port_prio 10
+prio=$(cmd_jq "ip -n ${c_ns} -d -j link show eth2" ".[].linkinfo.info_slave_data.ad_actor_port_prio")
+[ "$prio" -ne 10 ] && RET=1
+log_test "bond 802.3ad" "ad_actor_port_prio setting"
+
+# Trigger link state change to reselect the aggregator
+ip -n "${c_ns}" link set eth1 down
+ip -n "${c_ns}" link set eth1 up
+# the active agg should be connect to switch
+bond_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show bond0" ".[].linkinfo.info_data.ad_info.aggregator")
+eth0_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show eth0" ".[].linkinfo.info_slave_data.ad_aggregator_id")
+if [ "${bond_agg_id}" -ne "${eth0_agg_id}" ]; then
+	RET=1
+fi
+
+# Change the actor port prio and re-test
+ip -n "${c_ns}" link set eth0 type bond_slave ad_actor_port_prio 10
+ip -n "${c_ns}" link set eth2 type bond_slave ad_actor_port_prio 1000
+# Trigger link state change to reselect the aggregator
+ip -n "${c_ns}" link set eth1 down
+ip -n "${c_ns}" link set eth1 up
+# now the active agg should be connect to backup switch
+bond_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show bond0" ".[].linkinfo.info_data.ad_info.aggregator")
+eth2_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show eth2" ".[].linkinfo.info_slave_data.ad_aggregator_id")
+# shellcheck disable=SC2034
+if [ "${bond_agg_id}" -ne "${eth2_agg_id}" ]; then
+	RET=1
+fi
+log_test "bond 802.3ad" "ad_actor_port_prio switch"
+
+exit "${EXIT_STATUS}"
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 508f3c700d71..09b63c6f3dbd 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -551,30 +551,6 @@ wait_for_dev()
         fi
 }
 
-cmd_jq()
-{
-	local cmd=$1
-	local jq_exp=$2
-	local jq_opts=$3
-	local ret
-	local output
-
-	output="$($cmd)"
-	# it the command fails, return error right away
-	ret=$?
-	if [[ $ret -ne 0 ]]; then
-		return $ret
-	fi
-	output=$(echo $output | jq -r $jq_opts "$jq_exp")
-	ret=$?
-	if [[ $ret -ne 0 ]]; then
-		return $ret
-	fi
-	echo $output
-	# return success only in case of non-empty output
-	[ ! -z "$output" ]
-}
-
 pre_cleanup()
 {
 	if [ "${PAUSE_ON_CLEANUP}" = "yes" ]; then
diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index 006fdadcc4b9..4c278829e04c 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -616,3 +616,27 @@ wait_local_port_listen()
 		sleep 0.1
 	done
 }
+
+cmd_jq()
+{
+	local cmd=$1
+	local jq_exp=$2
+	local jq_opts=$3
+	local ret
+	local output
+
+	output="$($cmd)"
+	# it the command fails, return error right away
+	ret=$?
+	if [[ $ret -ne 0 ]]; then
+		return $ret
+	fi
+	output=$(echo $output | jq -r $jq_opts "$jq_exp")
+	ret=$?
+	if [[ $ret -ne 0 ]]; then
+		return $ret
+	fi
+	echo $output
+	# return success only in case of non-empty output
+	[ ! -z "$output" ]
+}
-- 
2.46.0
Re: [PATCH net-next 3/3] selftests: bonding: add test for LACP actor port priority
Posted by Hangbin Liu 2 months, 1 week ago
Hi Jakub,

Should I drop this selftest as it needs the new iproute2 feature that has
not applied yet?

Thanks
Hangbin
On Thu, Jul 24, 2025 at 08:16:32AM +0000, Hangbin Liu wrote:
> Add a selftest to verify that per-port actor priority (ad_actor_port_prio)
> is correctly applied and affects aggregator selection as expected.
> 
> Move cmd_jq from forwarding/lib.sh to net/lib.sh.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  .../selftests/drivers/net/bonding/Makefile    |  3 +-
>  .../drivers/net/bonding/bond_lacp_prio.sh     | 73 +++++++++++++++++++
>  tools/testing/selftests/net/forwarding/lib.sh | 24 ------
>  tools/testing/selftests/net/lib.sh            | 24 ++++++
>  4 files changed, 99 insertions(+), 25 deletions(-)
>  create mode 100755 tools/testing/selftests/drivers/net/bonding/bond_lacp_prio.sh
> 
> diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
> index 2b10854e4b1e..32617a834a6b 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_lacp_prio.sh
>  
>  TEST_FILES := \
>  	lag_lib.sh \
> diff --git a/tools/testing/selftests/drivers/net/bonding/bond_lacp_prio.sh b/tools/testing/selftests/drivers/net/bonding/bond_lacp_prio.sh
> new file mode 100755
> index 000000000000..a3f939d12143
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/bonding/bond_lacp_prio.sh
> @@ -0,0 +1,73 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Testing if bond lacp per port priority works
> +
> +lib_dir=$(dirname "$0")
> +# shellcheck disable=SC1091
> +source "$lib_dir"/../../../net/lib.sh
> +
> +# create client, switch, backup switch netns
> +setup_ns c_ns s_ns b_ns
> +defer cleanup_all_ns
> +
> +# setup links
> +# shellcheck disable=SC2154
> +ip -n "${c_ns}" link add eth0 type veth peer name eth0 netns "${s_ns}"
> +ip -n "${c_ns}" link add eth1 type veth peer name eth1 netns "${s_ns}"
> +# shellcheck disable=SC2154
> +ip -n "${c_ns}" link add eth2 type veth peer name eth0 netns "${b_ns}"
> +ip -n "${c_ns}" link add eth3 type veth peer name eth1 netns "${b_ns}"
> +
> +ip -n "${c_ns}" link add bond0 type bond mode 802.3ad miimon 100 lacp_rate fast ad_select prio
> +ip -n "${s_ns}" link add bond0 type bond mode 802.3ad miimon 100 lacp_rate fast
> +ip -n "${b_ns}" link add bond0 type bond mode 802.3ad miimon 100 lacp_rate fast
> +
> +ip -n "${c_ns}" link set eth0 master bond0
> +ip -n "${c_ns}" link set eth1 master bond0
> +ip -n "${c_ns}" link set eth2 master bond0
> +ip -n "${c_ns}" link set eth3 master bond0
> +ip -n "${s_ns}" link set eth0 master bond0
> +ip -n "${s_ns}" link set eth1 master bond0
> +ip -n "${b_ns}" link set eth0 master bond0
> +ip -n "${b_ns}" link set eth1 master bond0
> +
> +ip -n "${c_ns}" link set bond0 up
> +ip -n "${s_ns}" link set bond0 up
> +ip -n "${b_ns}" link set bond0 up
> +
> +# set ad actor port priority, default 255
> +ip -n "${c_ns}" link set eth0 type bond_slave ad_actor_port_prio 1000
> +prio=$(cmd_jq "ip -n ${c_ns} -d -j link show eth0" ".[].linkinfo.info_slave_data.ad_actor_port_prio")
> +[ "$prio" -ne 1000 ] && RET=1
> +ip -n "${c_ns}" link set eth2 type bond_slave ad_actor_port_prio 10
> +prio=$(cmd_jq "ip -n ${c_ns} -d -j link show eth2" ".[].linkinfo.info_slave_data.ad_actor_port_prio")
> +[ "$prio" -ne 10 ] && RET=1
> +log_test "bond 802.3ad" "ad_actor_port_prio setting"
> +
> +# Trigger link state change to reselect the aggregator
> +ip -n "${c_ns}" link set eth1 down
> +ip -n "${c_ns}" link set eth1 up
> +# the active agg should be connect to switch
> +bond_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show bond0" ".[].linkinfo.info_data.ad_info.aggregator")
> +eth0_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show eth0" ".[].linkinfo.info_slave_data.ad_aggregator_id")
> +if [ "${bond_agg_id}" -ne "${eth0_agg_id}" ]; then
> +	RET=1
> +fi
> +
> +# Change the actor port prio and re-test
> +ip -n "${c_ns}" link set eth0 type bond_slave ad_actor_port_prio 10
> +ip -n "${c_ns}" link set eth2 type bond_slave ad_actor_port_prio 1000
> +# Trigger link state change to reselect the aggregator
> +ip -n "${c_ns}" link set eth1 down
> +ip -n "${c_ns}" link set eth1 up
> +# now the active agg should be connect to backup switch
> +bond_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show bond0" ".[].linkinfo.info_data.ad_info.aggregator")
> +eth2_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show eth2" ".[].linkinfo.info_slave_data.ad_aggregator_id")
> +# shellcheck disable=SC2034
> +if [ "${bond_agg_id}" -ne "${eth2_agg_id}" ]; then
> +	RET=1
> +fi
> +log_test "bond 802.3ad" "ad_actor_port_prio switch"
> +
> +exit "${EXIT_STATUS}"
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index 508f3c700d71..09b63c6f3dbd 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -551,30 +551,6 @@ wait_for_dev()
>          fi
>  }
>  
> -cmd_jq()
> -{
> -	local cmd=$1
> -	local jq_exp=$2
> -	local jq_opts=$3
> -	local ret
> -	local output
> -
> -	output="$($cmd)"
> -	# it the command fails, return error right away
> -	ret=$?
> -	if [[ $ret -ne 0 ]]; then
> -		return $ret
> -	fi
> -	output=$(echo $output | jq -r $jq_opts "$jq_exp")
> -	ret=$?
> -	if [[ $ret -ne 0 ]]; then
> -		return $ret
> -	fi
> -	echo $output
> -	# return success only in case of non-empty output
> -	[ ! -z "$output" ]
> -}
> -
>  pre_cleanup()
>  {
>  	if [ "${PAUSE_ON_CLEANUP}" = "yes" ]; then
> diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> index 006fdadcc4b9..4c278829e04c 100644
> --- a/tools/testing/selftests/net/lib.sh
> +++ b/tools/testing/selftests/net/lib.sh
> @@ -616,3 +616,27 @@ wait_local_port_listen()
>  		sleep 0.1
>  	done
>  }
> +
> +cmd_jq()
> +{
> +	local cmd=$1
> +	local jq_exp=$2
> +	local jq_opts=$3
> +	local ret
> +	local output
> +
> +	output="$($cmd)"
> +	# it the command fails, return error right away
> +	ret=$?
> +	if [[ $ret -ne 0 ]]; then
> +		return $ret
> +	fi
> +	output=$(echo $output | jq -r $jq_opts "$jq_exp")
> +	ret=$?
> +	if [[ $ret -ne 0 ]]; then
> +		return $ret
> +	fi
> +	echo $output
> +	# return success only in case of non-empty output
> +	[ ! -z "$output" ]
> +}
> -- 
> 2.46.0
>
Re: [PATCH net-next 3/3] selftests: bonding: add test for LACP actor port priority
Posted by Jakub Kicinski 2 months, 1 week ago
On Thu, 24 Jul 2025 14:31:43 +0000 Hangbin Liu wrote:
> Should I drop this selftest as it needs the new iproute2 feature that has
> not applied yet?

No need, I'll add the iproute2 patch in the CI.
Re: [PATCH net-next 3/3] selftests: bonding: add test for LACP actor port priority
Posted by Hangbin Liu 2 months, 1 week ago
On Thu, Jul 24, 2025 at 07:35:01AM -0700, Jakub Kicinski wrote:
> On Thu, 24 Jul 2025 14:31:43 +0000 Hangbin Liu wrote:
> > Should I drop this selftest as it needs the new iproute2 feature that has
> > not applied yet?
> 
> No need, I'll add the iproute2 patch in the CI.

Cool! Do you run CI for all posted patches?

I just found that I forgot to rebase the code to latest net-next, which will
have a conflict in include/net/bond_options.h... I see your email that the
net-next is closed. So I think we can just leave this patchset for reviewing.
I can post the next version after net-next re-open.

Thanks
Hangbin
Re: [PATCH net-next 3/3] selftests: bonding: add test for LACP actor port priority
Posted by Jakub Kicinski 2 months, 1 week ago
On Fri, 25 Jul 2025 02:20:01 +0000 Hangbin Liu wrote:
> On Thu, Jul 24, 2025 at 07:35:01AM -0700, Jakub Kicinski wrote:
> > On Thu, 24 Jul 2025 14:31:43 +0000 Hangbin Liu wrote:  
> > > Should I drop this selftest as it needs the new iproute2 feature that has
> > > not applied yet?  
> > 
> > No need, I'll add the iproute2 patch in the CI.  
> 
> Cool! Do you run CI for all posted patches?

Yes, the quick checks like shellcheck we run on individual patches,
but selftests get batched. I mean we collect patches from n hours
(currently 3) and test them all together.

> I just found that I forgot to rebase the code to latest net-next, which will
> have a conflict in include/net/bond_options.h... I see your email that the
> net-next is closed. So I think we can just leave this patchset for reviewing.
> I can post the next version after net-next re-open.

Right..