Hi Parav, I understand your point of having some common driver infrastructure between rxe and siw. I remember I was even proposing rxe folks looking into that when rxe emerged after siw was already prototyped and open sourced (yes, siw was first out there). There was not much interest from rxe folks at that point in time, which is OK and understood (always cool to write something new and better from scratch, who wants iWarp, etc.). At the current stage of the project, it is rather contra-productive to completely refactor siw to match with rxe, since I do not have the resources to rip apart the whole thing, and I am not yet convinced it makes sense. If one asks for merging these days, one asks at first for a delay in getting siw accepted. Over time, it might indeed be a good thing to merge parts. As said, I'd still like to investigate that. But let the sometimes intentionally different concepts of these two drivers co-exist for some time out there, and let's draw conclusions of what to make common later. And let's be clear, lots of things between RoCE and iWarp are very different. Not only wire protocol and lower interfaces, but also semantically. These things will stay different by specification, and an efficient implementation of those things will likely still go it's own ways. Thanks Bernard. -----"Parav Pandit" <parav@xxxxxxxxxxxx> wrote: ----- >To: "Steve Wise" <larrystevenwise@xxxxxxxxx> >From: "Parav Pandit" <parav@xxxxxxxxxxxx> >Date: 03/10/2019 08:06PM >Cc: "Bernard Metzler" <bmt@xxxxxxxxxxxxxx>, >"linux-rdma@xxxxxxxxxxxxxxx" <linux-rdma@xxxxxxxxxxxxxxx>, "Leon >Romanovsky" <leonro@xxxxxxxxxxxx>, "Yuval Shaia" ><yuval.shaia@xxxxxxxxxx> >Subject: RE: [PATCH v5 00/13] SIW: Request for Comments > >Hi Steve, > >From: Steve Wise <larrystevenwise@xxxxxxxxx> >Sent: Sunday, March 10, 2019 1:17 PM >To: Parav Pandit <parav@xxxxxxxxxxxx> >Cc: Bernard Metzler <bmt@xxxxxxxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx; >Leon Romanovsky <leonro@xxxxxxxxxxxx>; Yuval Shaia ><yuval.shaia@xxxxxxxxxx> >Subject: Re: [PATCH v5 00/13] SIW: Request for Comments > >Hey Parav, > >On Sun, Mar 10, 2019 at 12:45 PM Parav Pandit ><mailto:parav@xxxxxxxxxxxx> wrote: >Hi Bernard, > >> -----Original Message----- >> From: mailto:linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma- >> mailto:owner@xxxxxxxxxxxxxxx> On Behalf Of Bernard Metzler >> Sent: Tuesday, February 19, 2019 4:09 AM >> To: mailto:linux-rdma@xxxxxxxxxxxxxxx >> Cc: Bernard Metzler <mailto:bmt@xxxxxxxxxxxxxx> >> Subject: [PATCH v5 00/13] SIW: Request for Comments >> >> 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. >> > >Great to see another software driver after rdma_rxe. >rdma_rxe is improved incrementally by several people. > >rdma_rxe and siw have lot in common as below. >1. qp, mr, cq, srq resource allocation >2. state machine handling for qp, mr, srq > >The iwarp qp state machine is different. > >3. user interface for post_send, recv, cq poll, notification > >> There are semantic differences as well. Two I can think of are the >meaning behind a send completion for IB vs iWARP, and the >> requirement that sink MRs for an iwarp rdma read must use an rkey >with REMOTE_WRITE. > >[Parav] These are internals to take care at transport level. Doesn't >deserve a different driver for it. > >>> 4. user interface for resource creation qp, cq, mr, srq etc >resources >> The user interface is already common for all rdma drivers. >ib_create_qp(), ib_create_cq(), etc.. > >[Parav] user interface at code level between user and kernel level. >siw needs to reuse rdma_rxe library too, instead of creating new user >library and kernel code for handling all of it. > >> 5. data path handling for invalidate, send with invalidate etc >> 6. netlink life cycle commands for iwarp rdma devices > >>> What differs between them is - skb processing (roce) vs sockets >(iwarp). > >> And the fact that each siw connection is its own socket and tcp >connection, vs a single tunneled UDP socket for all RoCE >"connections". >[Parav] This is transport internals, that should be treated at lowest >level. >This part of the transport layer is different between them. >Hence please create transport callbacks to do transport specific >work. > >So please reuse the rdma_rxe driver, refactor it to accommodate the >need to iwarp. > >> I don't agree with this. Soft iWARP is different enough to let it >stand on its own, certainly for the first inclusion into Linux. >[Parav] It doesn't matter first or second inclusion. >It is second inclusion of a sw rdma driver that can possibly leverage >existing rdma rxe. > > > Also, the integration with the rdma-cm would have to be >refactored, i think, since there are two very different paths through >the CM for >> RoCE vs iWARP. >[Parav] So that CM handling can be different. QP, MR states are still >handled in common way. >Rdma cm path is just additional pieces, and its fine to include it in >rdma_rxe when iwarp interfaces are created using netlink. >modify_qp_is_ok() had link layer check that Kamal removed, and that >can be pulled back easily. > >> I can see the two sharing some of the data structures and methods, >but to ask this now basically asks for a rewrite of both rxe and siw, >and > that is unreasonable, in my opinion. > >[Parav] why it is unreasonable? >Leon already quoted " it is much more easier and exciting to write >something new instead of fixing already existing piece of code. >Luckily enough, kernel community doesn't allow new code without >proving that old code is not possible to fix." >Though it was on different thread, it equally applies here to reuse >and refactor existing driver. > >So refactoring rdma_rxe should be possible. >Yuval also likes to enhance rxe, so you might get help from him as >well to do so. > >Not doing refactor is not a good idea. >rdmavt is good example sharing common code between two drivers. > >> Let siw evolve incrementally, as rxe has. >Sure, it can involve incrementally inside the rxe. > >Some of the in-reply is messed up. I tried but couldn't fix all the >email text. >Steve, >Please fix the gmail client for text-only replies. >
