-----linux-rdma-owner@xxxxxxxxxxxxxxx wrote: ----- >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx> >From: "Leon Romanovsky" >Sent by: linux-rdma-owner@xxxxxxxxxxxxxxx >Date: 03/03/2019 01:43PM >Cc: linux-rdma@xxxxxxxxxxxxxxx >Subject: Re: [PATCH v5 00/13] SIW: Request for Comments > >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 Leon, Let me try that. I hope next time it will avoid the mess I got. Thanks Bernard. > >> >> 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" removed by Bernard Metzler/Zurich/IBM]
