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

[Thomas Monjalon <thomas@monjalon.net>]

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.