From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lb0-f172.google.com (mail-lb0-f172.google.com [209.85.217.172]) by dpdk.org (Postfix) with ESMTP id 9297FC478 for ; Wed, 22 Jun 2016 11:30:08 +0200 (CEST) Received: by mail-lb0-f172.google.com with SMTP id ak10so18848732lbc.3 for ; Wed, 22 Jun 2016 02:30:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=yz1gCWNAIVo/Uod1oeCvGLv8xiOHJTXhGZWiGP2DScs=; b=m+ZO60RMhtDQIN44/8SmOVq/tNZTATsZTGEIu4QrPbyAVxNGMpVX04XSKSyK9sjG03 QsWTioNgNvrwJZWUbg9ueEuS0AYK+V4CsJgkI72tA6pScVH8RZ5UZrr/pj/dR+2Mpfo5 jXmt2BzRQCpIb844yLqAA0p+BkjArSxRUazVrTSxvvwrVntHTitaYTM84iht2MdTjB0P 3ajcySQ8eYRMo/JdSh6DRiCWtfdaV45tsp+ndMDD55j/Pt96x6E97a3YSeo/zyIb9iNe NiIHz0FXowUw5PYWDo0xANGtQVdA6XhH4TuyYhV7alWSYh4H9V2ZC7r6pA6XTyJVq86r 1+lg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=yz1gCWNAIVo/Uod1oeCvGLv8xiOHJTXhGZWiGP2DScs=; b=a+ceb9hg+IG0UdFfIMOo9wnHs7e2qi6X48Ds62WNK0+PZEMNzPQnO05X2tVzfiOeaM ssHGaUIbZ8WsAI/wrr8D8qwW1PRU0Dg5d4Ql8+3xynKRtOMKc8+TTGQdw1VbjVawkogw oCXsZ4v64J3G9whN5tjhJ9B/iagEQvm0Sotob0xsZw4QNVW6RoaYACnGVOjWLOaNk2di 8ifR+VQF3BEDdiZ3Rj9EdtunvpL6SIojs7C/r2HiS4uZcVZ6eH9TyWwukANAdR+sgbwe jsN2fXNwMQlFfFVNYTfu9fiw7sbLDJiXqw9s1y0jKDkHGM7sg4n4zlARlkOAZFSJBbJv DLgg== X-Gm-Message-State: ALyK8tKJsvSIn/PbPF+yVSeRzmJwjNMQ8znDaxNw/RYz8ahaX5na6liT1PCnZox6++Fhx82b X-Received: by 10.194.108.4 with SMTP id hg4mr17794060wjb.140.1466587808144; Wed, 22 Jun 2016 02:30:08 -0700 (PDT) Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id t188sm7157350wma.8.2016.06.22.02.30.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Jun 2016 02:30:07 -0700 (PDT) Date: Wed, 22 Jun 2016 11:30:05 +0200 From: Adrien Mazarguil To: Bruce Richardson Cc: Ferruh Yigit , Nelio Laranjeiro , dev@dpdk.org Message-ID: <20160622093005.GA7621@6wind.com> Mail-Followup-To: Bruce Richardson , Ferruh Yigit , Nelio Laranjeiro , dev@dpdk.org References: <1465379291-25310-1-git-send-email-nelio.laranjeiro@6wind.com> <1466493818-1877-1-git-send-email-nelio.laranjeiro@6wind.com> <57696E75.8050402@intel.com> <20160622082054.GY7621@6wind.com> <20160622091914.GA9728@bricha3-MOBL3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160622091914.GA9728@bricha3-MOBL3> Subject: Re: [dpdk-dev] [PATCH v3 00/25] Refactor mlx5 to improve performance X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Jun 2016 09:30:08 -0000 On Wed, Jun 22, 2016 at 10:19:14AM +0100, Bruce Richardson wrote: > On Wed, Jun 22, 2016 at 10:20:54AM +0200, Adrien Mazarguil wrote: > > On Tue, Jun 21, 2016 at 05:42:29PM +0100, Ferruh Yigit wrote: > > > On 6/21/2016 8:23 AM, Nelio Laranjeiro wrote: > > > > Enhance mlx5 with a data path that bypasses Verbs. > > > > > > > > The first half of this patchset removes support for functionality completely > > > > rewritten in the second half (scatter/gather, inline send), while the data > > > > path is refactored without Verbs. > > > > > > > > The PMD remains usable during the transition. > > > > > > > > This patchset must be applied after "Miscellaneous fixes for mlx4 and mlx5". > > > > > > > > Changes in v3: > > > > - Rebased patchset on top of next-net/rel_16_07. > > > > > > > > Changes in v2: > > > > - Rebased patchset on top of dpdk/master. > > > > - Fixed CQE size on Power8. > > > > - Fixed mbuf assertion failure in debug mode. > > > > - Fixed missing class_id field in rte_pci_id by using RTE_PCI_DEVICE. > > > > > > > > Adrien Mazarguil (8): > > > > mlx5: replace countdown with threshold for Tx completions > > > > mlx5: add debugging information about Tx queues capabilities > > > > mlx5: check remaining space while processing Tx burst > > > > mlx5: resurrect Tx gather support > > > > mlx5: work around spurious compilation errors > > > > mlx5: remove redundant Rx queue initialization code > > > > mlx5: make Rx queue reinitialization safer > > > > mlx5: resurrect Rx scatter support > > > > > > > > Nelio Laranjeiro (16): > > > > drivers: fix PCI class id support > > > > mlx5: split memory registration function > > > > mlx5: remove Tx gather support > > > > mlx5: remove Rx scatter support > > > > mlx5: remove configuration variable > > > > mlx5: remove inline Tx support > > > > mlx5: split Tx queue structure > > > > mlx5: split Rx queue structure > > > > mlx5: update prerequisites for upcoming enhancements > > > > mlx5: add definitions for data path without Verbs > > > > mlx5: add support for configuration through kvargs > > > > mlx5: add Tx/Rx burst function selection wrapper > > > > mlx5: refactor Rx data path > > > > mlx5: refactor Tx data path > > > > mlx5: handle Rx CQE compression > > > > mlx5: add support for multi-packet send > > > > > > > > Yaacov Hazan (1): > > > > mlx5: add support for inline send > > > > > > > > > > Patchset applies and compiles fine, thanks. > > > > > > But still has some checkpatch warnings, -btw, I am using the checkpatch > > > script from latest master branch of Linux repo. > > > > > > Following is the sample type of warnings (not instances, there are more > > > than one instance per type): > > > > While Nelio is preparing a v4 to address the kvargs issue, the remaining > > warnings can be safely ignored. > > > > A few of them are in relocated but unmodified code as this patchset > > refactors the entire PMD, others are documented. We settled on positive > > errno values internally because mlx5 uses syscalls and switching the sign > > bit all over the place quickly became unmanageable. They are made negative > > when returning from public callbacks (except for kvargs by mistake). > > > > In short, we did run checkpatch, fixed a million warnings and other errors > > and left those on purpose, nothing to worry about. > > > > Yes, they are nothing to worry about, but at the same time, I fail to see why > most of them should not be fixed. Even if you are moving code, unless it's a whole > file it's not going to show up as a move in the diff, so some small changes > during the move can be ok. > > > > WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned' > > > #112: FILE: drivers/net/mlx5/mlx5_mr.c:65: > > > + unsigned mem_idx) > > > > This looks easily fixable. > > > > WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a > > > separate line > > > #288: FILE: drivers/net/mlx5/mlx5_mr.c:241: > > > + * pointer is valid. */ > > > > I also think this should be fixed. > > > > WARNING:USE_NEGATIVE_ERRNO: return of an errno should typically be > > > negative (ie: return -EINVAL) > > > #524: FILE: drivers/net/mlx5/mlx5_txq.c:265: > > > + return EINVAL; > > > > > > WARNING:LONG_LINE: line over 80 characters > > > #108: FILE: drivers/net/mlx5/mlx5_ethdev.c:1250: > > > + txq_ctrl->txq.stats.idx = > > > primary_txq->stats.idx; > > > > > > WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should > > > probably be static const char * const > > > #88: FILE: drivers/net/mlx5/mlx5.c:281: > > > + static const char *params[] = { > > > > > > ERROR:ASSIGN_IN_IF: do not use assignment in if condition > > > #218: FILE: drivers/net/mlx5/mlx5_rxtx.c:92: > > > + if (!ret || !(ret = ((*buf)[i] == magic[i]))) > > > > > > CHECK:SPACING: spaces preferred around that '&' (ctx:VxV) > > > #414: FILE: drivers/net/mlx5/mlx5_rxtx.c:625: > > > + (uintptr_t)&(*rxq->cqes)[rxq->cq_ci & > > > ^ > > > > > > WARNING:INDENTED_LABEL: labels should not be indented > > > #520: FILE: drivers/net/mlx5/mlx5_rxtx.c:789: > > > + skip: > This I also feel should be fixed too. Right but it won't prevent some of them to still show up anyway. We will take them into account in the future. Now, considering these warnings are minor and were already there before, do you think Nelio needs to spam the world with a v5? Looks like he forgot "v4" in the subject of his last cover letter as well. -- Adrien Mazarguil 6WIND