On Fri, Mar 01, 2019 at 09:16:32AM +0000, Bernard Metzler wrote: > > > --- > Bernard Metzler, PhD > Tech. Leader High Performance I/O, Principal Research Staff > IBM Zurich Research Laboratory > Saeumerstrasse 4 > CH-8803 Rueschlikon, Switzerland > +41 44 724 8605 > > -----"Leon Romanovsky" <leon@xxxxxxxxxx> wrote: ----- > > >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx> > >From: "Leon Romanovsky" <leon@xxxxxxxxxx> > >Date: 02/28/2019 08:23PM > >Cc: linux-rdma@xxxxxxxxxxxxxxx > >Subject: Re: [PATCH v5 00/13] SIW: Request for Comments > > > >On Thu, Feb 28, 2019 at 02:28:41PM +0000, Bernard Metzler wrote: > >> -----"Leon Romanovsky" <leon@xxxxxxxxxx> wrote: ----- > >> > >> >To: "Bernard Metzler" <bmt@xxxxxxxxxxxxxx> > >> >From: "Leon Romanovsky" <leon@xxxxxxxxxx> > >> >Date: 02/28/2019 01:33PM > >> >Cc: linux-rdma@xxxxxxxxxxxxxxx > >> >Subject: Re: [PATCH v5 00/13] SIW: Request for Comments > >> > > >> >On Tue, Feb 19, 2019 at 11:08:50AM +0100, Bernard Metzler wrote: > >> >> This patch set contributes version 5 of the SoftiWarp > >> >> driver, as originally introduced to the list Oct 6th, 2017. > >> >> SoftiWarp (siw) implements the iWarp RDMA protocol over > >> >> kernel TCP sockets. The driver integrates with the > >> >> linux-rdma framework. > >> >> > >> >> In response to the various helpful feedback, we fixed (besides > >> >> other small fixes) the following issues: > >> >> > >> >> 1. The debugfs logic got completely removed. There is no > >> >> debugfs entry for siw anymore. We will interate with rdmatool > >> >> in the future. > >> >> > >> >> 2. The driver adheres to the device and object management as > >> >> recently proposed by Jason Gunthorpe. This includes the new > >> >> protection domain management. > >> >> > >> >> 3. An entry to linux MAINTAINERS for siw was added. > >> >> > >> >> 4. The packet length at MPA protocol level got restricted > >> >> to physical MTU size: to interoperate with hardware iWarp > >> >> devices such as Chelsio T5/T6, GSO usage got unselected. > >> >> It can be selcted with a global parameter 'gso_seg_limit'. > >> >> Setting 'gso_seg_limit = 0' makes full use of GSO if > >advertised > >> >> by the TCP socket. This almost doubles bandwidth for bulk > >data > >> >> transfers in a pure siw <-> siw setting, but does not > >> >interoperate > >> >> wth some hardware RNIC's. > >> >> > >> >> 5. An 'SPDX-License-Identifier' got added to all files > >> >> > >> >> 6. The user interface as defined in > >include/uapi/rdma/siwa_user.h > >> >> got cleaned up. > >> >> > >> >> 7. Useless redefinition of functions for kernel reference count > >> >> reading got removed. > >> >> > >> >> > >> >> The driver continues to make use of rdma-netdev extensions to > >> >> add and delete links, as proposed by Steve Wise. > >> >> > >> >> The driver continues to rely on the iWarp port mapper extensions > >> >> for software RDMA devices, as proposed by Steve Wise. > >> >> > >> >> We maintain a snapshot of the current code at > >> >> https://github.com/zrlio/softiwarp-for-linux-rdma.git > >> >> within branch 'siw-for-rdma-next-v5'. > >> >> This branch includes the latest netlink and portmapper patches > >from > >> >> Steve as well as the latest device and object management > >> >> from Jason. > >> >> > >> >> The matching siw user library is maintained at > >> >> https://github.com/zrlio/softiwarp-user-for-linux-rdma.git. > >> >> It is based on rdma-core, and extended with Steve's patches > >> >> to both rdma netlink and portmapper. The relevant branch > >> >> name is 'siw-for-rdma-next'. > >> >> > >> >> Since siw may generate work completion notifications from a > >> >> kthread context, performance of kernel siw clients like NVMeF > >> >> depends on the following pending kernel patch (included in > >> >> https://github.com/zrlio/softiwarp-for-linux-rdma.git): > >> >> https://www.spinics.net/lists/linux-rdma/msg75081.html > >> >> > >> >> As always, we'd highly appreciate your code review. Thanks > >> >> very much for your time! > >> >> > >> >> Bernard > >> > > >> >Bernard, > >> > > >> >Can you please post your fixed series so we will be able to > >continue > >> >review? > >> > > >> > >> Leon, > >> > >> I do my best as time permits to bring along a next version > >> end of this week. > >> I try to follow all advises, even if I end up in battles > >> between applying clang-format and checkpatch, which > >> yields partially contradicting results, which I then manually > >> fix, hoping it still makes us all happy. > > > >Very strange, I'm running all my patches through clang-formatter > >and saw only one difference between final result and checkpatch > >expectation - space in the loops. That error is ignored nowadays. > > > Yes, but I really ran into that. clang-formt even sometimes inserts > me white spaces at the beginning of a line (within enum definitions), > between '*' and variable name, between function name and opening > parenthesis '(', it even inserts goto labels... Is there any style > file available which fits our needs (I tried without explicit style, > and -style=file (which picks the .clang-format from the root of > the kernel sources)? For me it worked almost out of the box. I installed clang together with git-clang-format, downloaded clang-format.py from the web and put this snippet into .vimrc: " Format line in INSERT mode or buffer in VISUAL mode to clang format map <C-F> :pyf $HOME/.vim/clang-format.py<cr> imap <C-F> <c-o>:pyf $HOME/.vim/clang-format.py<cr> Now Ctrl-F formats me the source code according to kernel .clang-format file. Thanks > > Many thanks for your help, > Bernard. > > >> > >> I hope we then focus on functional/correctness, and keep > >> style aside a little: I will have it clang-format formatted > >> applying styles from .clang-format of the kernel plus fixes of > >> that silly output to match with kernels checkpatch). Not to > >> mention the potential result of checkpatch, sparse, clang > >> or other tools applied to the given rdma-core files and other > >> drivers. ;) > > > >This sounds like a tedious task, but it actually catches a lot of > >errors, this is why it is better to see them fixed before actual > >reviewing. > > > >> > >> > >> Best, > >> Bernard. > >> > > > [attachment "signature.asc" removed by Bernard Metzler/Zurich/IBM] >
Attachment:
signature.asc
Description: PGP signature
