+ if (adapter->last_rss_indices != rss_i ||
+ adapter->last_reta_entries != reta_entries) {
+ for (i = 0; i < reta_entries; i++)
+ adapter->rss_indir_tbl[i] = i % rss_i;
Are you sure rss_i never ever can be a 0?
This is the only thing I'm worrying about.
Oops, you're exactly right. Good catch!
I see the original code assigns 0 to rss_indir_tbl[i] when rss_i is
0,
like:
adapter->rss_indir_tbl[i] = 0;
Ahh, that's not true, my brain was not working... Sorry for messing
up.
Anyway, in a situation where rss_i == 0, we should handle it somehow
to avoid zero-divisor.
To handle this with keeping the behavior when rss_i == 0, I'm
considering Option 1:
adapter->rss_indir_tbl[i] = rss_i ? i % rss_i : 0;
Option 2:
if (rss_i)
for (i = 0; i < reta_entries; i++)
adapter->rss_indir_tbl[i] = i % rss_i;
else
memset(adapter->rss_indir_tbl, 0, reta_entries);
Since this is not in the data path, the overhead of checking rss_i in
each iteration might be acceptable. Therefore I'd like to adopt the
option 1 for simplicity.
Do you have any preference or other suggestions?
I lean toward option 2, as the explicit if (rss_i) guard makes the logic
clearer and easier to follow.
Handling the simplified case first with:
if (unlikely(!rss_i))
memset(adapter->rss_indir_tbl, 0, reta_entries);
else
for (i = 0; i < reta_entries; i++)
adapter->rss_indir_tbl[i] = i % rss_i;
Improves readability and separates the edge case from the main logic.
While it's possible to use a ternary expression like adapter->rss_indir_tbl[i]
= rss_i ? i % rss_i : 0;,
I find the conditional block more maintainable, especially if this logic
evolves later.
Regarding unlikely(), unless there's profiling data showing a performance
benefit,
I'd avoid it here - this isn't in the fast path, and clarity should take
precedence.
With the best regards Alex
I would make it even simpler (than if/else paths):
if (!rss_i)
rss_i = 1;
(which looks better than "should be obvious" oneliner, rss_i += !rss_i;)
