On Wed, Jun 22, 2016 at 11:30:05AM +0200, Adrien Mazarguil wrote:

On Wed, Jun 22, 2016 at 10:19:14AM +0100, Bruce Richardson wrote:

quoted

On Wed, Jun 22, 2016 at 10:20:54AM +0200, Adrien Mazarguil wrote:

quoted

On Tue, Jun 21, 2016 at 05:42:29PM +0100, Ferruh Yigit wrote:

quoted

On 6/21/2016 8:23 AM, Nelio Laranjeiro wrote:

quoted

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.
 

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

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.

After discussing this with Nelio (and cooling down a bit), we decided to
submit a v5 to address the above warnings, except false positives.

Then for the remaining stuff, namely the fact we use positive errno values
internally and existing code that does not conform DPDK coding rules, we
will submit another set of patches for 16.11 to address them all at once.

-- 
Adrien Mazarguil
6WIND