[dpdk-dev] [PATCH v15 4/7] drivers/net: update Rx RSS hash offload capabilities

[Andrew Rybchenko]

On 10/31/19 7:51 PM, Pavan Nikhilesh Bhagavatula wrote:
> 
>> 29/10/2019 16:37, pbhagavatula at marvell.com:
>>> From: Pavan Nikhilesh <pbhagavatula at marvell.com>
>>>
>>> Add DEV_RX_OFFLOAD_RSS_HASH flag for all PMDs that support RSS hash delivery.
>>>
>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula at marvell.com>
>>> Reviewed-by: Andrew Rybchenko <arybchenko at solarflare.com>
>>> Reviewed-by: Hemant Agrawal <hemant.agrawal at nxp.com>
>>> Acked-by: Jerin Jacob <jerinj at marvell.com>
>>> Acked-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
>>> ---
>>> +	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)