On 10/31/19 7:51 PM, Pavan Nikhilesh Bhagavatula wrote:
>
>> 29/10/2019 16:37, [email protected]:
>>> From: Pavan Nikhilesh <[email protected]>
>>>
>>> Add DEV_RX_OFFLOAD_RSS_HASH flag for all PMDs that support RSS hash
>>> delivery.
>>>
>>> Signed-off-by: Pavan Nikhilesh <[email protected]>
>>> Reviewed-by: Andrew Rybchenko <[email protected]>
>>> Reviewed-by: Hemant Agrawal <[email protected]>
>>> Acked-by: Jerin Jacob <[email protected]>
>>> Acked-by: Ajit Khaparde <[email protected]>
>>> ---
>>> + if (!(dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_RSS_HASH))
>>> + dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
>>
>> Excuse me, I miss why you need a check before setting the bit.
>
> Currently, none of the PMDs support disabling RSS_HASH (except octeontx2)
> since it involves
> adding an if check in Rx routine that might lead to perf impact.
> So, we are implicitly enabling the offload for all the PMDs if an application
> decides to disable
> RSS_HASH. In future if PMD maintainer decides to add this feature she/he can
> remove the check.
As I understand Thomas says that it is just sufficient to do:
dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
without any if before.
Yes, it is true since right now it looks a bit strange.
I guess it is the result of code evolution. Initially
it was logging inside if, but logging is moved to ethdev.
(Of course, it is true for such trivial checks only)