[openib-general][PATCH][RFC]: CMA header

[Guy German]

Sean Hefty wrote:
>> typedef void (*ib_cma_event_handler)(enum ib_cma_event event, void 
>> *context, const void *private_data);
>> typedef void (*ib_cma_listen_handler)(void *cma_id, struct ib_device 
>> *device, void *private_data, void *context);
> 
> 
> I think we can merge these two handlers.  We do not want to pass back 
> struct ib_device* to a caller.  The device needs to be associated with 
> the cma_id up front.

The listen handler passes also the new cma_id. Do you think it is better to 
merge them and pass null in this field in the "active side" case? (or maybe pass 
the same cma_id back in the handler)...

>> int ib_cma_get_device(struct sockaddr *remote_address,
>>               enum ib_qos qos, struct ib_device **device, u8 *port);
> 
> 
> I don't believe that we can support this function and still work with 
> device removal.

I agree that this is an open issue, at the moment, that's why I did not 
implement it yet. I will try to open a discussion on it, in a different thread.

>> int ib_cma_connect(struct ib_cma_conn *cma_conn, void **cma_id);
> 
> Creating the cma_id inside this call, rather than using a separate call 
> means that the user must be able to handle a connection request callback 
> before the cma_id is known.  I.e. a callback can occur before this call 
> returns.  (In fact, the entire connection could be established, data 
> transfered, and disconnected before this call returns.)  It may be 
> easier to have a separate call to allocate the cma_id that records the 
> context and event handler.

That's a good point. I wanted to save the cma consumer the trouble of creating 
and destroying cma_id's, and thought the cma can do it for him, but I agree that 
this can be problematic. I will change that.

>> int ib_cma_listen(struct ib_device *device, struct sockaddr *address,
>>           __be64 service_id, void *context,
>>           ib_cma_listen_handler cm_listen_handler,
>>           void **cma_id);
> 
> 
> Same issue as above.

Point taken.

>> int ib_cma_destroy(void *cma_id);
> 
> Why not have this apply to both active and passive sides?  Will this 
> interface support peer to peer connections?  If so, then we may not want 
> to distinguish between active and passive at this level.

True. This will have to apply to the active side as well, after we add 
ib_cma_create.


Thanks,
Guy