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

[Sean Hefty]

Fab Tillier wrote:
>>+	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))

Good catch - I'll fix this.

>>+	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.

You'd need to call listen something like 1,000,000 times a second for over 
2000 years before you'd even exhaust the OS administered IDs.

- Sean