01/11/2019 12:11, Andrew Rybchenko: > 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)
Yes exactly, this "if" can be removed in several places.
