[rds-devel] future/20081031

[Andy Grover]

Or Gerlitz wrote:
>> So please take a look. Let's not worry about the massive code 
>> duplication I've just introduced for now
> May I ask what is the reasoning beyond this IB/iWARP code separation
>  which as you say resulted in massive code duplication?

Didn't I say to ignore it? sigh :)

> So far other ULPs which support both (e.g open-MPI, rNFS, Lustre and 
> soon iSer) use the same rdma transport code for both IB and iWARP 
> with some internal minor branching in the code based on the 
> differences between the two. Moving forward I believe this can create
>  maintainship / support headache... also probably be a show stopper 
> for mainline inclusion.

First, we have a number of concerns about iWARP compared to IB. These
include lack of support for rdma over loopback, send completion
semantics, and error reporting semantics that may require changes in the
semantics of the RDS user interface. Having a clean separation allows us
to assess and work on these issues with no chance of affecting IB
support, which is extremely important.

Second, the iwarp patch added a struct of function pointers to handle IB
using FMR, iWARP using fastreg (rds_ib_mr_pool_ops). We already have one
struct of function pointers (struct rds_transport) so there's no need
for another -- mr_pool_ops's functions should be merged into struct
rds_transport.

Third, I see maintaining two modules of size X as easier than
maintaining one module of size 2X. I needed to get this out there so Jon
could build on top of it, but this is a "future" release. It will not be
submitted anywhere in its current condition. Once we have ib and iwarp
transports both working then we can start pulling duplicate
functionality up above the rds_transport interface, so that the two
modules end up with only non-duplicated ib- and iwarp-specific code.

Fourth, RDS transports were close to being modules before, but the fact
they weren't meant that some things that should've been cleaned up by
the transport (e.g. connections) were actually cleaned up by rds on
rmmod. Making them separate modules revealed this immediately, so I like
it for keeping the code properly abstracted. If this poses political
issues then we can alter the makefile to just compile-in the transports
to ib_rds.ko.

> I understand the possible frustration in Oracle resulted from the 
> iwarp patches breaking the IB code, a possible way to solve this in 
> the future is conducting extensive review along with some testing of 
> proposed patches (specifically by the submitter and the 
> maintainer...)

Going this way was always the plan -- that snafu just made me want to
get there faster :)

> Also, adding an "RDS:" prefix to the change-log subject line of 
> patches would be very much helpful to distinguish RDS from non-RDS 
> ones when examining the git patch log ...

This is a dev snapshot tree, so I'll be cleaning up the patch comments
when it gets more mature.

Regards -- Andy