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

[Guy German]

Roland Dreier wrote:
> This isn't horrible, 
Cheers ;)

> but you seem to have ignored most of the
> discussion from last month:

I did not implement yet the functions that were controversial (i.e. 
ib_cma_get_device and ib_cma_get_src_ip). I think it is more productive to 
discuss those issues over a preliminary implementation.

>     > int ib_cma_get_device(struct sockaddr *remote_address,
>     > 		      enum ib_qos qos, struct ib_device **device, u8 *port);
> 
> How are you dealing with hotplug and object lifetime issues here?

I did not fully understand why the user can't deal with hot unplug, if he follows:
- register as client (implements add and removal functions)
- creates a list of devices
- get a device from the cma (request by ip)
- check the device is valid and appears in its list
- apply a locking/synchronization mechanism between the removal function and the 
calls to the verbs.

I guess I am missing something, so I will start a new thread on that issue.

>     > int ib_cma_get_src_ip(void *cma_id, ib_cma_addr_handler addr_handler,
>     > 		      void *context);
> 
> There's no point in making this asynchronous, since we're putting the
> source/dest information in the CM REQ private data.

Do we have to wait to an ibta approval to do this or can this be implemented 
right away, in this fashion ?

>     > int ib_cma_create_qp(struct ib_pd *pd, u8 port, struct ib_qp **qp,
>     > 		     struct ib_qp_init_attr *init_attr);
> 
> What's the point of this function?

This function (_in the current implementation_) is sort of "application note" on 
what cma consumers need to do. It forces the consumer to use a qp that was 
modified to init after creation. Its only 25 lines long, so I thought I can 
leave it there even if one chooses not to use it.
If we choose to change the implementation of device retrieval this function 
might be useful.

>     > 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);
> 
> A minor point, but there's no need for the service_id parameter.
> We'll just use the sin_port (or sin6_port) member of the sockaddr.
> Similarly struct ib_cma_conn doesn't need the service_id member either.

iSER, today, uses sid of 64bit, other ULP's might use 64bit sid's too, why limit 
them to 16 bit ?

Thanks,
Guy