Hi Stephen,
Thanks for the review, I'll address your comments in v3
On 08/10/2024 19:28, Stephen Hemminger wrote:
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>
--
Regards,
Vladimir
