[dpdk-dev] [RFC] net/mlx5: support count flow action

[Nélio Laranjeiro]

Hi Ori,

On Thu, Aug 24, 2017 at 02:04:32PM +0000, Ori Kam wrote:
> Hi Nelio,
> 
> Please see my comments in line.
> 
> Ori
> 
> > -----Original Message-----
> > From: Nélio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
> > Sent: Thursday, August 24, 2017 9:54 AM
> > To: Ori Kam <orika at mellanox.com>
> > Cc: adrien.mazaruil at 6wind.com; dev at dpdk.org
> > Subject: Re: [RFC] net/mlx5: support count flow action
> > 
> > Hi Ori,
> > 
> > Please keep the coding style of the file, and pass checkpatch before
> > submitting a patch on the mailing list.  It helps the review by having a correct
> > patch respecting the coding style of the file.
> > I won't spot out here all the coding style issues, if you need some help, feel
> > free to ask.
> > 
> Sorry won't happen again.

No problem, first contribution is always complicate.

> > On Mon, Aug 21, 2017 at 03:35:41PM +0300, Ori Kam wrote:
> > > Support count flow action.
> > 
> > Why copy/pasting the title in the commit message?
> > 
> I was under the impression that main function of the RFC should also be in the message body.

No, it is not necessary, the commit message should bring useful information by
still being short and precise.

>[...]
> > > ---
> > >  drivers/net/mlx5/mlx5.h      |   4 ++
> > >  drivers/net/mlx5/mlx5_flow.c | 163
> > > ++++++++++++++++++++++++++++++++++++++++++-
> > 
> > There are missing changes in the Makefile to have the
> > HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_COUNT and the include of the
> > mlx5_autoconf.h in mlx5_flow.c.
> > 
> I haven't added them since this feature is not supported yet, and 
> I don't want anybody trying to activate them.
> When the feature will be supported on the verbs then I will update
> those files. 

Ok, so a new version should be sent soon :)

>[...]
> > 
> Will be update according to your suggestion.
 
Thanks,

-- 
Nélio Laranjeiro
6WIND