> > <snip> > > >> +#defineRETA_SZ_MIN2U > >> +#defineRETA_SZ_MAX16U > > > > Should these RETA_SZ defines be in public header? > > So user can know what are allowed values? > > > > I don't think this is necessary, because the user chooses it not > arbitrary, but depending on the NIC.
Sure thing, but it would be goo for the user to know what are the SW Limitations on it without digging through .c. > > >> +#define RETA_SZ_IN_RANGE(reta_sz)((reta_sz >= RETA_SZ_MIN) && \ > > <snip> > > >> +uint32_t i; > > > > Empty line is missing. > > > > Thanks > > >> +if ((name == NULL) || (key_len == 0) || !RETA_SZ_IN_RANGE(reta_sz)) { > >> +rte_errno = EINVAL; > >> +return NULL; > >> +} > > <snip> > > >> +static inline void > >> +set_bit(uint8_t *ptr, uint32_t bit, uint32_t pos) > >> +{ > >> +uint32_t byte_idx = pos >> 3; > > > > Just as a nit to be consistent with the line below: > > pos / CHAR_BIT; > > > > Fixed > > >> +uint32_t bit_idx = (CHAR_BIT - 1) - (pos & (CHAR_BIT - 1)); > >> +uint8_t tmp; > > <snip> > > >> +ent = rte_zmalloc(NULL, sizeof(struct rte_thash_subtuple_helper) + > >> +sizeof(uint32_t) * (1 << ctx->reta_sz_log), 0); > > > > Helper can be used by data-path code (via rte_thash_get_compliment()) right? > > Then might be better to align it at cache-line. > > > > Agree, I'll fix it > > >> +if (ent == NULL) > >> +return -ENOMEM; > > <snip> > > >> uint32_t > >> -rte_thash_get_compliment(struct rte_thash_subtuple_helper *h __rte_unused, > >> -uint32_t hash __rte_unused, uint32_t desired_hash __rte_unused) > >> +rte_thash_get_compliment(struct rte_thash_subtuple_helper *h, > >> +uint32_t hash, uint32_t desired_hash) > >> { > >> -return 0; > >> +return h->compl_table[(hash ^ desired_hash) & h->lsb_msk]; > >> } > > > > Would it make sense to add another-one for multi values: > > rte_thash_get_compliment(uint32_t hash, const uint32_t desired_hashes[], > > uint32_t adj_hash[], uint32_t num); > > So user can get adjustment values for multiple queues at once? > > > > At the moment I can't find scenarios why do we need to have a bulk > version for this function My thought was about case when number of configured HW queues is less than reta_size. Let say reta_size==4, but user configured only 3 queues and reta={0,1,2,0}. In that case for queue 0, both 0 and 3 values would suit.