Netdev List
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@idosch.org>
To: Hangbin Liu <hangbin.liu@linux.dev>
Cc: Ido Schimmel <idosch@nvidia.com>,
	netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org,
	pabeni@redhat.com, edumazet@kernel.org, jv@jvosburgh.net,
	andrew+netdev@lunn.ch, razor@blackwall.org, aahila@google.com
Subject: Re: [PATCH net-next 2/2] selftests: bonding: Add a test for 802.3ad with coupled control disabled
Date: Fri, 13 Nov 2026 09:07:32 +0200	[thread overview]
Message-ID: <20261113070732.GC3517282@shredder> (raw)
In-Reply-To: <avZ07oXIXo7xZyCC@fedora>

On Fri, Nov 13, 2026 at 10:24:46AM +0800, Hangbin Liu wrote:
> Hi Ido,
> On Wed, Nov 11, 2026 at 07:19:47PM +0200, Ido Schimmel wrote:
> > Test that with coupled control disabled, a port in COLLECTING state
> > receives packets.
> > 
> > In this mode, a port only transitions from COLLECTING to DISTRIBUTING
> > state if its partner sets the Collecting bit in its LACPDUs. Therefore,
> > keep the port in COLLECTING state by clearing the Collecting bit in
> > received LACPDUs.
> > 
> > Also test that ping works once the port moves to DISTRIBUTING state.
> > 
> > Use VRFs rather than network namespaces so that the test could be run
> > with a veth pair or a pair of physically connected ports.
> > 
> > Output without "bonding: 3ad: Receive packets in COLLECTING state":
> > 
> >  # ./bond_lacp_independent_control.sh
> >  TEST: Receive in COLLECTING state                                   [FAIL]
> >          Packets not received in COLLECTING state
> >  TEST: Ping in DISTRIBUTING state                                    [ OK ]
> > 
> > Output with the fix:
> > 
> >  # ./bond_lacp_independent_control.sh
> >  TEST: Receive in COLLECTING state                                   [ OK ]
> >  TEST: Ping in DISTRIBUTING state                                    [ OK ]
> > 
> > Reviewed-by: Petr Machata <petrm@nvidia.com>
> > Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> > ---
> >  .../selftests/drivers/net/bonding/Makefile    |   1 +
> >  .../bonding/bond_lacp_independent_control.sh  | 144 ++++++++++++++++++
> >  .../selftests/drivers/net/bonding/config      |   6 +
> >  3 files changed, 151 insertions(+)
> >  create mode 100755 tools/testing/selftests/drivers/net/bonding/bond_lacp_independent_control.sh
> > 
> > diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
> > index e337c099fb40..46d9fede5918 100644
> > --- a/tools/testing/selftests/drivers/net/bonding/Makefile
> > +++ b/tools/testing/selftests/drivers/net/bonding/Makefile
> > @@ -7,6 +7,7 @@ TEST_PROGS := \
> >  	bond-eth-type-change.sh \
> >  	bond-lladdr-target.sh \
> >  	bond_ipsec_offload.sh \
> > +	bond_lacp_independent_control.sh \
> >  	bond_lacp_prio.sh \
> >  	bond_lacp_strict.sh \
> >  	bond_macvlan_ipvlan.sh \
> > diff --git a/tools/testing/selftests/drivers/net/bonding/bond_lacp_independent_control.sh b/tools/testing/selftests/drivers/net/bonding/bond_lacp_independent_control.sh
> > new file mode 100755
> > index 000000000000..c29712e01e49
> > --- /dev/null
> > +++ b/tools/testing/selftests/drivers/net/bonding/bond_lacp_independent_control.sh
> > @@ -0,0 +1,144 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +# +-------------------------+ +-------------------------+
> > +# | H1 (vrf)                | | H2 (vrf)                |
> > +# |  + bond1 (802.3ad)      | |  + bond2 (802.3ad)      |
> > +# |  | coupled_control off  | |  |                      |
> > +# |  | 192.0.2.1/28         | |  | 192.0.2.2/28         |
> > +# |  |                      | |  |                      |
> > +# |  + $h1                  | |  + $h2                  |
> > +# +--|----------------------+ +--|----------------------+
> > +#    |                           |
> > +#    +---------------------------+
> > +
> > +# shellcheck disable=SC2034,SC2329
> > +ALL_TESTS="
> > +	collecting
> > +	distributing
> > +"
> > +NUM_NETIFS=2
> > +REQUIRE_MZ=no
> > +lib_dir=$(dirname "$0")
> > +# shellcheck disable=SC1091
> > +source "$lib_dir"/../../../net/forwarding/lib.sh
> > +
> > +lacp_state_get()
> > +{
> > +	local dev=$1; shift
> > +
> > +	ip -d -j link show dev "$dev" | \
> > +		jq '.[].linkinfo.info_slave_data.ad_actor_oper_port_state'
> > +}
> > +
> > +# The actor state of a port in ATTACHED state is 0x0f: Activity (0x01),
> > +# Timeout (0x02), Aggregation (0x04) and Synchronization (0x08). The
> > +# Collecting (0x10) bit is added in COLLECTING state (0x1f) and the
> > +# Distributing (0x20) bit in DISTRIBUTING state (0x3f).
> > +lacp_state_wait()
> > +{
> > +	local dev=$1; shift
> > +	local state=$1; shift
> > +
> > +	slowwait "$WAIT_TIMEOUT" until_counter_is "== $state" \
> > +		lacp_state_get "$dev" > /dev/null
> > +}
> > +
> > +partner_state_clear()
> > +{
> > +	local bits=$1; shift
> > +
> > +	tc filter replace dev "$h1" ingress pref 1 handle 101 protocol 0x8809 \
> > +		flower skip_hw action pedit munge offset 18 u8 set 0 \
> > +		retain "$bits"
> > +}
> > +
> > +h1_create()
> > +{
> > +	adf_ip_link_add bond1 type bond mode 802.3ad lacp_rate fast \
> > +		coupled_control off
> > +	adf_ip_link_set_master "$h1" bond1
> > +	adf_simple_if_init bond1 192.0.2.1/28
> > +
> > +	tc qdisc add dev "$h1" clsact
> > +	defer tc qdisc del dev "$h1" clsact
> > +
> > +	tc qdisc add dev bond1 clsact
> > +	defer tc qdisc del dev bond1 clsact
> > +
> > +	tc filter add dev bond1 ingress pref 1 handle 101 proto ip \
> > +		flower skip_hw src_ip 192.0.2.2 dst_ip 192.0.2.1 \
> > +		ip_proto icmp type 8 action pass
> > +	defer tc filter del dev bond1 ingress pref 1 handle 101 flower
> > +}
> > +
> > +h2_create()
> > +{
> > +	adf_ip_link_add bond2 type bond mode 802.3ad lacp_rate fast
> > +	adf_ip_link_set_master "$h2" bond2
> > +	adf_simple_if_init bond2 192.0.2.2/28
> > +}
> > +
> > +setup_prepare()
> > +{
> > +	h1=${NETIFS[p1]}
> > +	h2=${NETIFS[p2]}
> > +
> > +	adf_vrf_prepare
> 
> Nit: why we need the vrf prepare? The h1/h2 should be veth interface, no?

No. The VRF usage is intentional. From the commit message:

"Use VRFs rather than network namespaces so that the test could be run
with a veth pair or a pair of physically connected ports."

> 
> > diff --git a/tools/testing/selftests/drivers/net/bonding/config b/tools/testing/selftests/drivers/net/bonding/config
> > index b62c70715293..71c6d69d98a1 100644
> > --- a/tools/testing/selftests/drivers/net/bonding/config
> > +++ b/tools/testing/selftests/drivers/net/bonding/config
> > @@ -1,13 +1,17 @@
> >  CONFIG_BONDING=y
> > +CONFIG_BPF_SYSCALL=y
> >  CONFIG_BRIDGE=y
> > +CONFIG_CGROUP_BPF=y
> >  CONFIG_CONFIGFS_FS=y
> >  CONFIG_DUMMY=y
> >  CONFIG_INET_ESP=y
> >  CONFIG_INET_ESP_OFFLOAD=y
> >  CONFIG_IPV6=y
> > +CONFIG_IPV6_MULTIPLE_TABLES=y
> 
> Do we need this config ?

Yes. CONFIG_NET_VRF depends on it when IPv6 is enabled and it's not
enabled by default. Other config files that enable CONFIG_NET_VRF list
it as well.

> 
> >  CONFIG_IPVLAN=y
> >  CONFIG_MACVLAN=y
> >  CONFIG_NET_ACT_GACT=y
> > +CONFIG_NET_ACT_PEDIT=m
> >  CONFIG_NET_CLS_FLOWER=y
> >  CONFIG_NET_CLS_MATCHALL=m
> >  CONFIG_NETCONSOLE=m
> > @@ -15,7 +19,9 @@ CONFIG_NETCONSOLE_DYNAMIC=y
> >  CONFIG_NETCONSOLE_EXTENDED_LOG=y
> >  CONFIG_NETDEVSIM=m
> >  CONFIG_NET_IPGRE=y
> > +CONFIG_NET_L3_MASTER_DEV=y
> >  CONFIG_NET_SCH_INGRESS=y
> > +CONFIG_NET_VRF=y
> 
> If yes, this config could be removed. And the H1 (vrf) description in the
> chart.

See above. VRF is needed.

  reply	other threads:[~2026-11-13  7:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-11 17:19 [PATCH net-next 1/2] bonding: 3ad: Receive packets in COLLECTING state Ido Schimmel
2026-11-11 17:19 ` [PATCH net-next 2/2] selftests: bonding: Add a test for 802.3ad with coupled control disabled Ido Schimmel
2026-11-12 20:21   ` netdev-bot+sashiko
2026-11-13  7:01     ` Ido Schimmel
2026-11-13  2:16   ` Hangbin Liu
2026-11-13  2:24   ` Hangbin Liu
2026-11-13  7:07     ` Ido Schimmel [this message]
2026-11-12 20:21 ` [PATCH net-next 1/2] bonding: 3ad: Receive packets in COLLECTING state netdev-bot+sashiko
2026-11-13  1:32   ` Hangbin Liu
2026-11-13  6:52   ` Ido Schimmel

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=20261113070732.GC3517282@shredder \
    --to=idosch@idosch.org \
    --cc=aahila@google.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=hangbin.liu@linux.dev \
    --cc=idosch@nvidia.com \
    --cc=jv@jvosburgh.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox