Re: [PATCH v2 1/2] fib: implement RCU rule reclamation

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue,  8 Oct 2024 17:55:23 +0000
Vladimir Medvedkin <vladimir.medvedkin@intel.com> wrote:
> +	if ((tbl8_idx == -ENOSPC) && dp->dq != NULL) {

Better to either drop the parenthesis here, or put it on both conditions.

> +		/* If there are no tbl8 groups try to reclaim one. */
> +		if (rte_rcu_qsbr_dq_reclaim(dp->dq, 1,
> +				NULL, NULL, NULL) == 0)
> +			tbl8_idx = tbl8_get_idx(dp);
> +	}

Could add unlikely() to this expression.

	/* If there are no tbl8 groups try to reclaim one. */
	if (unlikely(tbl8_idx == -ENOSPC && dp->dq &&
		     !rte_rcu_qsbr_dq_reclaim(dp->dq, 1, NULL, NULL, NULL)))
	    tbl8_idx = tbl8_get_idx(dp);


> +static void
> +__rcu_qsbr_free_resource(void *p, void *data, unsigned int n)
> +{
> +	struct dir24_8_tbl *dp = p;
> +	uint64_t tbl8_idx = *(uint64_t *)data;
> +	RTE_SET_USED(n);
> +
> +	tbl8_cleanup_and_free(dp, tbl8_idx);
> +}

My preference (not a requirement) is to use __rte_unused attribute
instead of RTE_SET_USED

> +	if (dp->v == NULL)
> +		tbl8_cleanup_and_free(dp, tbl8_idx);
> +	else if (dp->rcu_mode == RTE_FIB_QSBR_MODE_SYNC) {
> +		rte_rcu_qsbr_synchronize(dp->v,
> +			RTE_QSBR_THRID_INVALID);
> +		tbl8_cleanup_and_free(dp, tbl8_idx);
> +	} else { /* RTE_FIB_QSBR_MODE_DQ */
> +		if (rte_rcu_qsbr_dq_enqueue(dp->dq,
> +				(void *)&tbl8_idx))

Minor nit: cast to void * is not necessary in C (only in C++).
And can fit on one line; max line length now for DPDK is 100 characters.

Overall, looks good.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>