Re: [dpdk-dev] [PATCH 4/5] mlx5: add support for flow director

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

Hi Bruce,

On Wed, Feb 17, 2016 at 05:13:44PM +0000, Bruce Richardson wrote:
> On Fri, Jan 29, 2016 at 11:32:01AM +0100, Adrien Mazarguil wrote:
> > From: Yaacov Hazan <yaacovh@mellanox.com>
> > 
> > Add support for flow director filters (RTE_FDIR_MODE_PERFECT and
> > RTE_FDIR_MODE_PERFECT_MAC_VLAN modes).
> > 
> > This feature requires MLNX_OFED 3.2.
> > 
> > Signed-off-by: Yaacov Hazan <yaacovh@mellanox.com>
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > ---
> Hi Adrien, Yaacov,
> 
> this patch raises a lot of warnings (17) with checkpatch. Can you perhaps look
> to see if this number can be reduced.

We actually did it before submitting that patch, there is indeed a bunch of
remaining warnings that have been left on purpose. Not sure if we have the
same configuration for checkpatch, but they should fall into the following
categories:

- "WARNING: return of an errno should typically be negative" - All return
  values are documented in the code. Since this PMD uses syscalls in its
  control path, it uses positive errno values internally for
  consistency. Public callback functions however return negative error
  values.

- "WARNING: line over 80 characters" - Well, although I'm a big fan of the
  80 characters limit, breaking those would have made the code harder to
  read.

- "WARNING: Missing a blank line after declarations" - It's actually a
  declaration through a macro, there is no missing blank line.

- "WARNING: networking block comments don't use an empty /* line" - Not sure
  if we really care? I don't particularly mind.

- "CHECK: Comparison to NULL could be written "!" - I do not mind either,
  writing the full check seems clearer to me.

- "CHECK: Unnecessary parentheses around fdir_info->mask" - Looks like a
  valid, although minor error.

Please tell me which of these still need to be fixed.

-- 
Adrien Mazarguil
6WIND