On Tue, 8 Oct 2024 17:55:23 +0000
Vladimir Medvedkin <vladimir.medved...@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 <step...@networkplumber.org>
