From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH 4/5] mlx5: add support for flow director Date: Thu, 18 Feb 2016 17:10:16 +0100 Message-ID: <20160218161016.GQ27079@6wind.com> References: <1454063522-1948-1-git-send-email-adrien.mazarguil@6wind.com> <1454063522-1948-5-git-send-email-adrien.mazarguil@6wind.com> <20160217171343.GA11736@bricha3-MOBL3> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: Bruce Richardson Return-path: Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by dpdk.org (Postfix) with ESMTP id ACBDCC3A2 for ; Thu, 18 Feb 2016 17:10:32 +0100 (CET) Received: by mail-wm0-f50.google.com with SMTP id g62so32705426wme.0 for ; Thu, 18 Feb 2016 08:10:32 -0800 (PST) Content-Disposition: inline In-Reply-To: <20160217171343.GA11736@bricha3-MOBL3> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > > > > 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 > > Signed-off-by: Adrien Mazarguil > > --- > 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