Re: [PATCH] test/bonding: fix active backup rx test

[lihuisong (C)]

Hi Stephen,

This patch looks good to me.
But this test case still runs failure when I run "link_bonding_autotest" 
twice. like:
-->
...
EAL: Test assert configure_ethdev line 181 failed: rte_eth_dev_configure 
for port 7 failed
EAL: Test assert initialize_bonding_device_with_members line 1138 
failed: Failed to configure bonding port (7) in mode 3 with (4) members.
EAL: Test assert 
test_broadcast_verify_member_link_status_change_behaviour line 4060 
failed: Failed to initialise bonding device
 + TestCase [62] : 
test_broadcast_verify_member_link_status_change_behaviour failed
ETHDEV: Invalid port_id=7
EAL: Test assert configure_ethdev line 181 failed: rte_eth_dev_configure 
for port 7 failed
EAL: Test assert test_reconfigure_bonding_device line 4162 failed: 
failed to reconfigure bonding device
 + TestCase [63] : test_reconfigure_bonding_device failed
ETHDEV: Invalid port_id=7
 + TestCase [64] : test_close_bonding_device succeeded
ETHDEV: Invalid port_id=7
EAL: Test assert remove_members_and_stop_bonding_device line 659 failed: 
Failed to stop bonding port 7
 + ------------------------------------------------------- +
 + Test Suite Summary : Link Bonding Unit Test Suite
 + ------------------------------------------------------- +
 + Tests Total :       65
 + Tests Skipped :      0
 + Tests Executed :    65
 + Tests Unsupported:   0
 + Tests Passed :       5
 + Tests Failed :      60
 + ------------------------------------------------------- +
Test Failed


I guess the bonding use case needs to close all bonding devices.


/Huisong

在 2024/12/13 9:51, Stephen Hemminger 写道:
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")
Cc: sta...@dpdk.org

Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
---
  app/test/test_link_bonding.c | 68 +++++++++++++++++-------------------
  1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index b752a5ecbf..512e79365f 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -2246,49 +2246,47 @@ test_activebackup_rx_burst(void)
                
virtual_ethdev_add_mbufs_to_rx_queue(test_params->member_port_ids[i],
                                &gen_pkt_burst[0], 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 */
-               
TEST_ASSERT_EQUAL(rte_eth_rx_burst(test_params->bonding_port_id, 0,
-                               &rx_pkt_burst[0], MAX_PKT_BURST), burst_size,
-                               "rte_eth_rx_burst failed");
+               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);
  
-		if (test_params->member_port_ids[i] == primary_port) {
-                       /* Verify bonding device rx count */
-                       rte_eth_stats_get(test_params->bonding_port_id, 
&port_stats);
-                       TEST_ASSERT_EQUAL(port_stats.ipackets, 
(uint64_t)burst_size,
-                                       "Bonding Port (%d) ipackets value (%u) not 
as expected (%d)",
+               /* 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, 
burst_size);
+                                 (unsigned int)port_stats.ipackets, rx_expect);
  
-			/* Verify bonding member devices rx count */
-                       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, 
(uint64_t)burst_size,
-                                                       "Member Port (%d) ipackets 
value (%u) not as "
-                                                       "expected (%d)",
-                                                       
test_params->member_port_ids[i],
-                                                       (unsigned 
int)port_stats.ipackets,
-                                                       burst_size);
-                               } else {
-                                       TEST_ASSERT_EQUAL(port_stats.ipackets, 
0,
-                                                       "Member Port (%d) ipackets 
value (%u) not as "
-                                                       "expected (%d)\n",
-                                                       
test_params->member_port_ids[i],
-                                                       (unsigned 
int)port_stats.ipackets, 0);
-                               }
-                       }
-               } else {
-                       for (j = 0; j < test_params->bonding_member_count; j++) 
{
-                               rte_eth_stats_get(test_params->member_port_ids[j], 
&port_stats);
+               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);
+
+                               /* reset member device stats */
+                               
rte_eth_stats_reset(test_params->member_port_ids[j]);
+                       } else {
                                TEST_ASSERT_EQUAL(port_stats.ipackets, 0,
-                                               "Member Port (%d) ipackets value 
(%u) not as expected "
-                                               "(%d)", 
test_params->member_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 */
-               rte_pktmbuf_free_bulk(rx_pkt_burst, burst_size);
+               /* 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 bonding device stats */
                rte_eth_stats_reset(test_params->bonding_port_id);