linux-kernel - Re: [PATCHv2 net 3/3] selftests: bonding: add test for passive LACP mode

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] [day] [month] [year] [list]

Message-ID: <aJ2yckfzToXfCiYa@fedora>
Date: Thu, 14 Aug 2025 09:54:58 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, Jay Vosburgh <jv@...sburgh.net>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Simon Horman <horms@...nel.org>, Shuah Khan <shuah@...nel.org>,
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2 net 3/3] selftests: bonding: add test for passive LACP
 mode

On Tue, Aug 12, 2025 at 11:23:17AM +0200, Paolo Abeni wrote:
> On 8/5/25 11:46 AM, Hangbin Liu wrote:
> @@ -0,0 +1,95 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Test if a bond interface works with lacp_active=off.
> > +
> > +# shellcheck disable=SC2034
> > +REQUIRE_MZ=no
> > +NUM_NETIFS=0
> > +lib_dir=$(dirname "$0")
> > +# shellcheck disable=SC1091
> > +source "$lib_dir"/../../../net/forwarding/lib.sh
> > +
> > +check_port_state()
> > +{
> > +	local netns=$1
> > +	local port=$2
> > +	local state=$3
> > +
> > +	ip -n "${netns}" -d -j link show "$port" | \
> > +		jq -e ".[].linkinfo.info_slave_data.ad_actor_oper_port_state_str | index(\"${state}\") != null" > /dev/null
> > +}
> > +
> > +trap cleanup EXIT
> > +setup_ns c_ns s_ns
> > +defer cleanup_all_ns
> > +
> > +# 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}"
> > +ip -n "${s_ns}" link set eth0 up
> > +ip -n "${s_ns}" link set eth1 up
> > +ip -n "${c_ns}" link add bond0 type bond mode 802.3ad lacp_active off 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 bond0 up
> > +
> > +# 1. The passive side shouldn't send LACPDU.
> > +RET=0
> > +client_mac=$(cmd_jq "ip -j -n ${c_ns} link show bond0" ".[].address")
> > +# Wait for the first LACPDU due to state change.
> > +sleep 5
> > +timeout 62 ip netns exec "${c_ns}" tcpdump --immediate-mode -c 1 -i eth0 \
> > +	-nn -l -vvv ether proto 0x8809 2> /dev/null > /tmp/client_init.out
> > +grep -q "System $client_mac" /tmp/client_init.out && RET=1
> > +log_test "802.3ad" "init port pkt lacp_active off"
> > +
> > +# 2. The passive side should not have the 'active' flag.
> > +RET=0
> > +check_port_state "${c_ns}" "eth0" "active" && RET=1
> > +log_test "802.3ad" "port state lacp_active off"
> > +
> > +# Set up the switch side with active mode.
> > +ip -n "${s_ns}" link set eth0 down
> > +ip -n "${s_ns}" link set eth1 down
> > +ip -n "${s_ns}" link add bond0 type bond mode 802.3ad lacp_active on lacp_rate fast
> > +ip -n "${s_ns}" link set eth0 master bond0
> > +ip -n "${s_ns}" link set eth1 master bond0
> > +ip -n "${s_ns}" link set bond0 up
> > +# Make sure the negotiation finished
> > +sleep 5
> 
> Minor nit: I guess the above sleep time depends on some kernel constant,
> but it's not obvious to me if the minimum time is mandated by the RFC,
> nor how long is such interval.

We need to make sure the lacp negotiation finished. There is no definition
in IEEE standard. 

> 
> A comment explaining the rationale behind such sleep will help, and
> possibly a loop with smaller minimal wait and periodic checks for the
> expected condition up to a significantly higher timeout could make the
> test both faster on average and more robust.

I will use tc rules to catch pkts and see if we can reduce some time.

Thanks
Hangbin