|SUCCESS| test: sync soft expiry check in inline IPsec

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163578

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-05

git sha: 20260505104351.2678817-1-ajp at marvell.com
Author: Aarnav JP <ajp at marvell.com>
Subject: [PATCH] test: sync soft expiry check in inline IPsec

This patch addresses a race condition in the inline IPsec soft expiry
test where the main test thread checked for event notification before
the asynchronous err-ring polling thread could deliver the event. The
fix adds a bounded poll loop with proper memory barriers to ensure
cross-core visibility of the notification flag.

> diff --git a/app/test/test_security_inline_proto.c b/app/test/test_security_inline_proto.c
> index b0cce5ebd9..5db8718a34 100644
> --- a/app/test/test_security_inline_proto.c
> +++ b/app/test/test_security_inline_proto.c
> @@ -38,6 +38,7 @@ test_inline_ipsec_sg(void)
>  
>  #else
>  
> +#include <rte_cycles.h>
>  #include <rte_eventdev.h>
>  #include <rte_event_eth_rx_adapter.h>
>  #include <rte_event_eth_tx_adapter.h>

[ ... ]

> @@ -1306,7 +1307,6 @@ test_ipsec_inline_sa_exp_event_callback(uint16_t port_id,
>  		printf("Event descriptor not set\n");
>  		return -1;
>  	}
> -	vector->notify_event = true;
>  	if (event_desc->metadata != (uint64_t)vector->sa_data) {
>  		printf("Mismatch in event specific metadata\n");
>  		return -1;
> @@ -1329,6 +1329,10 @@ test_ipsec_inline_sa_exp_event_callback(uint16_t port_id,
>  		return -1;
>  	}
>  
> +	/* Ensure event subtype is visible before signaling notify_event. */
> +	rte_io_wmb();
> +	vector->notify_event = true;
> +
>  	return 0;
>  }
>  

[ ... ]

> @@ -1483,6 +1487,16 @@ test_ipsec_inline_proto_process(struct ipsec_test_data *td,
>  				break;
>  		} while (j++ < 5 || nb_rx == 0);
>  
> +	/* Wait for soft expiry event from the err-ring poll thread. */
> +	if ((flags->sa_expiry_pkts_soft || flags->sa_expiry_bytes_soft) &&
> +	    td->ipsec_xform.direction == RTE_SECURITY_IPSEC_SA_DIR_EGRESS) {
> +		j = 0;
> +		while (!vector.notify_event && j++ < 100) {
> +			rte_delay_us(1000);
> +			rte_io_rmb();
> +		}
> +	}
> +

Can this loop terminate with !vector.notify_event when the polling
thread is slow? If the iteration count is exhausted before the callback
runs, does the code below properly handle this scenario? The loop
doesn't set ret to indicate a timeout condition.

Also, does this polling loop respect the post-loop invariant (j <= 100)
when checking vector.notify_event after the loop?

>  	if (!flags->sa_expiry_pkts_hard &&
>  			!flags->sa_expiry_bytes_hard &&
>  			(nb_rx != nb_sent)) {
> @@ -1546,10 +1560,13 @@ test_ipsec_inline_proto_process(struct ipsec_test_data *td,
>  		destroy_default_flow(port_id);
>  	if (flags->sa_expiry_pkts_soft || flags->sa_expiry_bytes_soft ||
>  		flags->sa_expiry_pkts_hard || flags->sa_expiry_bytes_hard) {
> -		if (vector.notify_event && (vector.event == event))
> -			ret = TEST_SUCCESS;
> -		else
> +		if (vector.notify_event) {
> +			rte_io_rmb();
> +			ret = (vector.event == event) ?
> +				TEST_SUCCESS : TEST_FAILED;
> +		} else {
>  			ret = TEST_FAILED;
> +		}

Does this pattern correctly fail the test if the wait loop above times
out (j >= 100 with !vector.notify_event)? The else branch appears to
handle this, but could the logic be clearer about timeout vs. event
mismatch failures?

Is the rte_io_rmb() placement correct here? It appears after
vector.notify_event was already evaluated as true. Should it be
immediately after the while loop, before reading vector.event?

>  
>  		rte_eth_dev_callback_unregister(port_id, RTE_ETH_EVENT_IPSEC,
>  			test_ipsec_inline_sa_exp_event_callback, &vector);