[openib-general] [PATCH] [CM] add OS administered service IDs

[Fab Tillier]

> From: Sean Hefty [mailto:sean.hefty at intel.com]
> Sent: Tuesday, June 07, 2005 10:50 AM
> 
> Index: core/cm.c
> ===================================================================
> --- core/cm.c	(revision 2563)
> +++ core/cm.c	(working copy)
> @@ -64,6 +64,7 @@ static struct ib_cm {
>  	struct list_head device_list;
>  	rwlock_t device_lock;
>  	struct rb_root listen_service_table;
> +	u64 listen_service_id;
>  	/* struct rb_root peer_service_table; todo: fix peer to peer */
>  	struct rb_root remote_qp_table;
>  	struct rb_root remote_id_table;
> @@ -718,14 +719,23 @@ int ib_cm_listen(struct ib_cm_id *cm_id,
>  	unsigned long flags;
>  	int ret = 0;
> 
> +	if ((service_id & IB_CM_ASSIGN_SERVICE_ID) == IB_CM_ASSIGN_SERVICE_ID &&
> +	    (service_id != IB_CM_ASSIGN_SERVICE_ID))
> +		return -EINVAL;
> +

This check only checks that the 2nd bit in the MSB of the SID is set.  You need
to check that the first byte is 0x02, which means you need a mask.  Something
like:

#define IB_CM_ASSIGN_SID_MASK __constant_cpu_to_be64(0xFF00000000000000ULL)

if ((service_id & IB_CM_ASSIGN_SID_MASK) == IB_CM_ASSIGN_SERVICE_ID &&
	(service_id != IB_CM_ASSIGN_SERVICE_ID))

>  	cm_id_priv = container_of(cm_id, struct cm_id_private, id);
>  	BUG_ON(cm_id->state != IB_CM_IDLE);
> 
>  	cm_id->state = IB_CM_LISTEN;
> -	cm_id->service_id = service_id;
> -	cm_id->service_mask = service_mask ? service_mask : ~0ULL;
> 
>  	spin_lock_irqsave(&cm.lock, flags);
> +	if (service_id == IB_CM_ASSIGN_SERVICE_ID) {
> +		cm_id->service_id = __cpu_to_be64(cm.listen_service_id++);
> +		cm_id->service_mask = ~0ULL;
> +	} else {
> +		cm_id->service_id = service_id;
> +		cm_id->service_mask = service_mask ? service_mask : ~0ULL;
> +	}

Should there be a check here for potential duplication?  I realize that the SID
is 64-bits, so it would take a very long time to wrap.  Also, just for good
measure, you should prevent cm.listen_service_id from exceeding
0x00FFFFFFFFFFFFF so that the upper byte is always 0x02 as required.

An alternative is to just use the cm_id as the service_id:
cm_id->service_id = (__cpu_to_be64(cm_id) & IB_CM_ASSING_SID_MASK) |
	IB_CM_ASSIGN_SERVICE_ID;

- Fab