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.
