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