[dpdk-dev] [PATCH v3 00/25] Refactor mlx5 to improve performance

Adrien Mazarguil adrien.mazarguil at 6wind.com
Wed Jun 22 10:20:54 CEST 2016


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.

> WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
> #112: FILE: drivers/net/mlx5/mlx5_mr.c:65:
> +       unsigned mem_idx)
> 
> 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. */
> 
> 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:

-- 
Adrien Mazarguil
6WIND


More information about the dev mailing list