Hi Konstantin,
On 12/04/2021 12:47, Ananyev, Konstantin wrote:
<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.
I see, I'll move it to .h
+#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.
Ah, yes, I remember out discussion about this. I'll think about this and
add next releases.
I should be kind of
int
rte_thash_get_compliment_reta(struct rte_thash_subtuple_helper *helper,
uint32_t hash, const uint16_t queue_id, struct rte_eth_rss_reta_entry64
*reta, int reta_sz);
where reta is completed by rte_eth_dev_rss_reta_query()
--
Regards,
Vladimir
