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

[Sean Hefty]

Guy German 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.

> /**
>  * ib_cma_get_device - Returns the device and port to be used according 
>  *   to the destination ip address (this can be detemined according 
>  *   to the local routing table). Call this function before 
>  *   creating the qp. If using link-local IPv6 addresses
>  *   there is no need to call this function.
>  * @remote_address - The destination address for connection
>  * @qos - desired quality of service
>  * @device - The device to use (output)
>  * @port - port to use (output)
>  *
>  */
> 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.

> /**
>  * ib_cma_connect - this is the connect request function, called by 
>  *   the active side. The consumer registers an upcall that will be 
>  *   initiated by the cma with an appropriate connection event 
>  *   notification (established/rejected/disconnected etc)
>  * @cma_conn: This structure contains the following connection parameters:
>  *   @qp: qp for establishing the connection
>  *   @qp_attr: only relevant attributes are used
>  *   @dst_ip: destination ip address
>  *   @service_id: destination service id (port)
>  *   @context: context to be returned in the callback
>  *   @cma_event_handler: the upcall function for the active side
>  *   @private_data: private data to be received at the listener upcall
>  *   @private_data_len: private data length (max 255)
>  *   @qos: Quality os service for the rc
>  *   @connect_flags: default or multipath connection
>  * @cma_id: This returned handle is a void* (different in ib and iwarp)
>  *   in ib - it is pointer to struct cma_context.
>  */
> 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.


> /**
>  * ib_cma_listen - this function is called by the passive side. It is
>  *   listening on a the specified port (ib service id) for incomming 
>  *   connection requests
>  * @device:
>  * @address:
>  * @service_id: service id (port) to listen on
>  * @context: user context to be returned in the callback
>  * @cm_listen_handler: the listen callback 
>  * @cma_id: cma handle for the passive side
>  */
> 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.

> /**
>  * ib_cma_destroy - this functionis is called on the passive side, to 
>  *   stop listenning on a certain sevice id
>  * @cma_id: the same cma handle received when ib_cma_sid_listen was called
>  */
> 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.


- Sean