Re: [dpdk-dev] [PATCH v2 5/5] net/mlx5: fix non-dv compilation errors

[Gregory Etelson <getelson@nvidia.com>]

> 14/11/2020 19:31, Gregory Etelson:
> > > 14/11/2020 18:41, Gregory Etelson:
> > > > > 13/11/2020 15:52, Gregory Etelson:
> > > > > > +       ret = mlx5_flow_group_to_table(dev, tunnel, jump_data-
> > > >group,
> > > > > > +                                      &flow_table, grp_info,
> > > > > > + error);
> > > > >
> > > > > The parameter grp_info is a struct passed as value.
> > > > > I believe it should be passed as a pointer.
> > > >
> > > > struct flow_grp_info is a 64 bit-field:
> > > > struct flow_grp_info {
> > > >       uint64_t external:1;
> > > >       uint64_t transfer:1;
> > > >       uint64_t fdb_def_rule:1;
> > > >       /* force standard group translation */
> > > >       uint64_t std_tbl_fix:1;
> > > >       uint64_t skip_scale:1;
> > > > };
> > > > Since mlx5_flow_group_to_table() does not change bits
> > > > configuration, there is no need to pass this type as a pointer.
> > >
> > > I feel passing struct as pointer is a better practice.
> >
> > The parameter in question is 64 bit unsigned long value.
> > Structure coating is a syntactic sugar, because C language does not
> > have bit-field types.
> > In general, if structure size does not exceed 64 bit it can be passed
> > by value.
> > Passing it by reference would create unnecessary indirect access.
> 
> Did you measure a performance difference?
> 
> This current code triggers this compiler note:
> drivers/net/mlx5/mlx5_flow.c:7106:1: note:
> parameter passing for argument of type 'struct flow_grp_info' changed in
> GCC 9.1
> 
> I'm afraid it can be a problem.
> In general we don't have such note on the whole DPDK code base.
> 

I'll handle this issue in DV compilation patch.