[PATCH net-next 1/2] bonding: 3ad: Receive packets in COLLECTING state

[PATCH net-next 1/2] bonding: 3ad: Receive packets in COLLECTING state

[Ido Schimmel]

When coupled control is disabled, a port in COLLECTING state is supposed
to receive packets, but it does not. The bond only delivers packets from
slaves whose 'inactive' flag is cleared, whereas
bond_set_slave_rx_enabled_flags() only clears 'rx_disabled'.

Fix by clearing 'inactive' as well.

Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---

Notes:
    Targeting at net-next since this never worked.
    
    Known issues that this patch does not address:
    
    1. Changing 'all_slaves_active' from 1 to 0 while a port is in
       COLLECTING state sets its 'inactive' flag and the port stops
       receiving until the next mux state transition. This is not new,
       COLLECTING is a transient state and the option is off by default, so I
       do not think it is worth handling.
    
    2. The 'inactive' flag shares a bitfield with other slave flags. The
       802.3ad state machine writes them under the mode lock, while the
       'all_slaves_active' handler writes 'inactive' under RTNL only. This is
       not new.

 include/net/bonding.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/bonding.h b/include/net/bonding.h
index 598d56b1bc97..cb89aff437f4 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -594,6 +594,7 @@ static inline void bond_set_slave_active_flags(struct slave *slave,
 static inline void bond_set_slave_rx_enabled_flags(struct slave *slave,
 					       bool notify)
 {
+	slave->inactive = 0;
 	slave->rx_disabled = 0;
 }
 
-- 
2.56.0

[PATCH net-next 2/2] selftests: bonding: Add a test for 802.3ad with coupled control disabled

[Ido Schimmel]

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
+
+	h1_create
+	h2_create
+}
+
+collecting()
+{
+	local pkts
+
+	RET=0
+
+	# Clear the Synchronization (0x08) and Collecting (0x10) bits in the
+	# partner's state to move $h1 to ATTACHED state and then only clear
+	# the Collecting bit to move it to COLLECTING state.
+	partner_state_clear 0x18
+	defer tc filter del dev "$h1" ingress pref 1 handle 101 flower
+	lacp_state_wait "$h1" 0x0f
+	check_err $? "Port did not reach ATTACHED state"
+
+	partner_state_clear 0x10
+	lacp_state_wait "$h1" 0x1f
+	check_err $? "Port did not reach COLLECTING state"
+
+	# $h1 cannot reply to ARP requests in COLLECTING state.
+	ip neigh add 192.0.2.1 lladdr "$(mac_get bond1)" dev bond2 \
+		nud permanent
+	defer ip neigh del 192.0.2.1 dev bond2
+
+	ping_do bond2 192.0.2.1
+	pkts=$(tc_rule_handle_stats_get "dev bond1 ingress" 101)
+	((pkts >= PING_COUNT))
+	check_err $? "Packets not received in COLLECTING state"
+
+	log_test "Receive in COLLECTING state"
+}
+
+distributing()
+{
+	RET=0
+
+	lacp_state_wait "$h1" 0x3f
+	check_err $? "Port did not reach DISTRIBUTING state"
+
+	ping_do bond2 192.0.2.1
+	check_err $? "Ping failed"
+
+	log_test "Ping in DISTRIBUTING state"
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit "$EXIT_STATUS"
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
 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
 CONFIG_NLMON=y
 CONFIG_VETH=y
 CONFIG_VLAN_8021Q=m
-- 
2.56.0