All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hangbin Liu <liuhangbin@gmail.com>
To: Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org, Jay Vosburgh <jv@jvosburgh.net>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	Simon Horman <horms@kernel.org>, Shuah Khan <shuah@kernel.org>,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv2 net 3/3] selftests: bonding: add test for passive LACP mode
Date: Thu, 14 Aug 2025 09:54:58 +0000	[thread overview]
Message-ID: <aJ2yckfzToXfCiYa@fedora> (raw)
In-Reply-To: <4a8266bf-5e33-4a38-a569-2a1e13633696@redhat.com>

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

      reply	other threads:[~2025-08-14  9:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-05  9:46 [PATCHv2 net 0/3] bonding: fix negotiation flapping in 802.3ad passive mode Hangbin Liu
2025-08-05  9:46 ` [PATCHv2 net 1/3] bonding: update LACP activity flag after setting lacp_active Hangbin Liu
2025-08-05  9:46 ` [PATCHv2 net 2/3] bonding: send LACPDUs periodically in passive mode after receiving partner's LACPDU Hangbin Liu
2025-08-12  9:14   ` Paolo Abeni
2025-08-05  9:46 ` [PATCHv2 net 3/3] selftests: bonding: add test for passive LACP mode Hangbin Liu
2025-08-12  9:23   ` Paolo Abeni
2025-08-14  9:54     ` Hangbin Liu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJ2yckfzToXfCiYa@fedora \
    --to=liuhangbin@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jv@jvosburgh.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.