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

["lihuisong (C)" <lihuisong@huawei.com>]

LGTM
Acked-by: Huisong Li <lihuisong@huawei.com>
Tested-by: Huisong Li <lihuisong@huawei.com>
在 2024/12/14 1:17, 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: stable@dpdk.org
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v2 - fix checkpatch warning from long line
>
>   app/test/test_link_bonding.c | 69 ++++++++++++++++++------------------
>   1 file changed, 34 insertions(+), 35 deletions(-)
>
> diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
> index b752a5ecbf..19b064771a 100644
> --- a/app/test/test_link_bonding.c
> +++ b/app/test/test_link_bonding.c
> @@ -2246,49 +2246,48 @@ 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);