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

[Thomas Monjalon]

01/11/2019 12:11, 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)

Yes exactly, this "if" can be removed in several places.