Hi Bernard, > -----Original Message----- > From: Bernard Metzler <BMT@xxxxxxxxxxxxxx> > Sent: Sunday, March 10, 2019 4:41 PM > To: Parav Pandit <parav@xxxxxxxxxxxx> > Cc: Steve Wise <larrystevenwise@xxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx; > Leon Romanovsky <leonro@xxxxxxxxxxxx>; Yuval Shaia > <yuval.shaia@xxxxxxxxxx> > Subject: RE: [PATCH v5 00/13] SIW: Request for Comments > > 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.). > I didn't follow that part of the history, but it clearly proves the point that it was started incorrectly. With all the fancy code for doorbells, with mmap() call for QP, CQ, ground reality is, it doesn't even pass a developer's silly ib_write_bw today. And at that test shows 2Gbps bw on 64Gb RAM with 48 core cpus! > 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. > But that seems to be right approach to have modular code and reuse existing pieces. Leon, Yuval and others resonate that too. If existing pieces are inferior or mis fit, their technical details has to be called out. rdmavt improved and used library model to share code between two drivers. > If one asks for merging these days, one asks at first for a delay in getting siw > accepted. > Yuval and Leon wants to improve rxe in general and have desire to reuse rxe. They might be able to help you. Please check with them. > Over time, it might indeed be a good thing to merge parts. It should happen from beginning. > As said, I'd still like to investigate that. That will be very helpful. > 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. > It won't happen later once it is in tree. :-) > And let's be clear, lots of things between RoCE and iWarp are very different. All rest of the core components such as ib_core, addr, rdmacm are able to handle it two different transports. Cavium or qedr driver is dual protocol driver that support RoCE and iWarp both in single driver. > 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. > We would like to understand technically why rxe is mis fit for it, and why it cannot be improved. Lack of resources is probably not good reason. > 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. > >
