[PATCH 22.11] test/bonding: fix active backup receive test

[PATCH 22.11] test/bonding: fix active backup receive test

[Stephen Hemminger]

[ upstream commit eb29e625ce41b50898efc8e2618b7eeb128460ed ]

The test had incorrect assumptions about how active backup
should work. When in active backup mode, the secondary (not primary)
ports should be ignored. The test was always broken since initially
written but earlier bug was masking the part of the test which
tested non-primary ports.

Bugzilla ID: 1589
Fixes: 112ce3917674 ("test/bonding: fix loop on members")

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Huisong Li <lihuisong@huawei.com>
Tested-by: Huisong Li <lihuisong@huawei.com>
---
 app/test/test_link_bonding.c | 77 +++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 40 deletions(-)

diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 53f5c13a24..2ea4393476 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -2226,51 +2226,48 @@ test_activebackup_rx_burst(void)
 		virtual_ethdev_add_mbufs_to_rx_queue(test_params->slave_port_ids[i],
 				&gen_pkt_burst[0], burst_size);
 
-		/* Call rx burst on bonded device */
-		TEST_ASSERT_EQUAL(rte_eth_rx_burst(test_params->bonded_port_id, 0,
-				&rx_pkt_burst[0], MAX_PKT_BURST), burst_size,
-				"rte_eth_rx_burst failed");
-
-		if (test_params->slave_port_ids[i] == primary_port) {
-			/* Verify bonded device rx count */
-			rte_eth_stats_get(test_params->bonded_port_id, &port_stats);
-			TEST_ASSERT_EQUAL(port_stats.ipackets, (uint64_t)burst_size,
-					"Bonded Port (%d) ipackets value (%u) not as expected (%d)",
-					test_params->bonded_port_id,
-					(unsigned int)port_stats.ipackets, burst_size);
+		/* Expect burst if this was the active port, zero otherwise */
+		unsigned int rx_expect
+			= (test_params->member_port_ids[i] == primary_port) ? burst_size : 0;
+
+		/* Call rx burst on bonding device */
+		unsigned int rx_count = rte_eth_rx_burst(test_params->bonding_port_id, 0,
+							 &rx_pkt_burst[0], MAX_PKT_BURST);
+		TEST_ASSERT_EQUAL(rx_count, rx_expect,
+				  "rte_eth_rx_burst (%u) not as expected (%u)",
+				  rx_count, rx_expect);
+
+		/* Verify bonding device rx count */
+		rte_eth_stats_get(test_params->bonding_port_id, &port_stats);
+		TEST_ASSERT_EQUAL(port_stats.ipackets, rx_expect,
+				  "Bonding Port (%d) ipackets value (%u) not as expected (%u)",
+					test_params->bonding_port_id,
+				  (unsigned int)port_stats.ipackets, rx_expect);
+
+		for (j = 0; j < test_params->bonding_member_count; j++) {
+			rte_eth_stats_get(test_params->member_port_ids[j], &port_stats);
+			if (i == j) {
+				TEST_ASSERT_EQUAL(port_stats.ipackets, rx_expect,
+					  "Member Port (%d) ipackets (%u) not as expected (%d)",
+					  test_params->member_port_ids[i],
+					  (unsigned int)port_stats.ipackets, rx_expect);
 
-			/* Verify bonded slave devices rx count */
-			for (j = 0; j < test_params->bonded_slave_count; j++) {
-				rte_eth_stats_get(test_params->slave_port_ids[j], &port_stats);
-				if (i == j) {
-					TEST_ASSERT_EQUAL(port_stats.ipackets, (uint64_t)burst_size,
-							"Slave Port (%d) ipackets value (%u) not as "
-							"expected (%d)", test_params->slave_port_ids[i],
-							(unsigned int)port_stats.ipackets, burst_size);
-				} else {
-					TEST_ASSERT_EQUAL(port_stats.ipackets, 0,
-							"Slave Port (%d) ipackets value (%u) not as "
-							"expected (%d)\n", test_params->slave_port_ids[i],
-							(unsigned int)port_stats.ipackets, 0);
-				}
-			}
-		} else {
-			for (j = 0; j < test_params->bonded_slave_count; j++) {
-				rte_eth_stats_get(test_params->slave_port_ids[j], &port_stats);
+				/* reset member device stats */
+				rte_eth_stats_reset(test_params->member_port_ids[j]);
+			} else {
 				TEST_ASSERT_EQUAL(port_stats.ipackets, 0,
-						"Slave Port (%d) ipackets value (%u) not as expected "
-						"(%d)", test_params->slave_port_ids[i],
-						(unsigned int)port_stats.ipackets, 0);
+					  "Member Port (%d) ipackets (%u) not as expected (%d)",
+					  test_params->member_port_ids[i],
+					  (unsigned int)port_stats.ipackets, 0);
 			}
 		}
 
-		/* free mbufs */
-		for (i = 0; i < MAX_PKT_BURST; i++) {
-			if (rx_pkt_burst[i] != NULL) {
-				rte_pktmbuf_free(rx_pkt_burst[i]);
-				rx_pkt_burst[i] = NULL;
-			}
-		}
+		/* extract packets queued to inactive member */
+		if (rx_count == 0)
+			rx_count = rte_eth_rx_burst(test_params->member_port_ids[i], 0,
+						    rx_pkt_burst, MAX_PKT_BURST);
+		if (rx_count > 0)
+			rte_pktmbuf_free_bulk(rx_pkt_burst, rx_count);
 
 		/* reset bonded device stats */
 		rte_eth_stats_reset(test_params->bonded_port_id);
-- 
2.47.2

Re: [PATCH 22.11] test/bonding: fix active backup receive test

[Luca Boccassi]

On Wed, 19 Feb 2025 at 17:34, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> [ upstream commit eb29e625ce41b50898efc8e2618b7eeb128460ed ]
>
> The test had incorrect assumptions about how active backup
> should work. When in active backup mode, the secondary (not primary)
> ports should be ignored. The test was always broken since initially
> written but earlier bug was masking the part of the test which
> tested non-primary ports.
>
> Bugzilla ID: 1589
> Fixes: 112ce3917674 ("test/bonding: fix loop on members")
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Huisong Li <lihuisong@huawei.com>
> Tested-by: Huisong Li <lihuisong@huawei.com>
> ---
>  app/test/test_link_bonding.c | 77 +++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 40 deletions(-)

Thanks, applied with minimal changes to make it build due to struct
fields having different names