[openib-general] Re: [PATCH] RFC Verbs: add support for transport specific verbs

[Fabian Tillier]

On 3/1/06, Michael S. Tsirkin <mst at mellanox.co.il> wrote:
> Quoting r. Fabian Tillier <ftillier at silverstorm.com>:
> > Subject: Re: [PATCH] RFC Verbs: add support for transport specific verbs
> >
> > On 2/28/06, Sean Hefty <mshefty at ichips.intel.com> wrote:
> > > Sean Hefty wrote:
> > > > Add support for transport specific extensions to the ib_device verbs.
> > > > Relocate process_mad as an IB specific verb.
> > > >
> > > > This provides a mechanism to add iWarp specific functionality, such as
> > > > the iWarp CM calls, to ib_device.
> > >
> > > Third proposal from Fab:
> > >
> > > struct rdma_device {
> > >        most everything
> > > };
> > >
> > > struct ib_device {
> > >        struct rdma_device dev;
> > >        IB specific operations;
> > > };
> > >
> > > Of course, this is a much larger change.
>
> And this still means we add complexity (need to verify device type)
> for calling transport-specific functions, which also has performance
> implications.

Can you quantify the performance penalty?  I would guess that for the
device specific operations, beside not being speed-path operations,
the overhead of checking the device type is likely lost in the noise
compared to the time it takes to actually perform the operation.

> > Perhaps, but it is probably the cleanest solution, allowing iWarp and
> > IB specific stuff to be totally independent with no header
> > interdependencies between the two transports.  It also follows the
> > existing precendent of how struct device is the first member of struct
> > ib_device.
>
> In linux we dont care about binary compatibility for kernel modules.
> So the fact that structure size changes because an extra callback is
> added does not introduce a dependency.

I never claimed we would maintain binary compatiblitly.  Just that
it's cleaner not to have a mish-mash of IB and iWarp.

> > While it is a larger change, I don't think it's particularily more
> > complicated or more difficult - it's just bigger.
>
> Its also less safe since we add downcasts.

I don't follow you here.  Any IB or iWarp specific calls can check
that the device passed in is of the right type.  You also don't need
to have any casts when calling neutral functions:

rdma_post_send( &ib_device->rdma_device, ... )

Maybe I missed your point here, but I don't buy this argument.

- Fab