[ofw] [PATCH] ND Provider: fix service ID byte ordering

[Fab Tillier]

Hefty, Sean wrote on Fri, 11 Feb 2011 at 16:53:43

>> Clearest is probably:
>> 
>>     strut _rdma_cm_sid
>>     {
>>         uint32_t zero;
>>         uint8_t is_rdma_cm;
>>         uint8_t prot;
>>         net16_t port;
>>     } * p_sid = &p_cm_req->service_id;
>>     
>>     p_sid->port = p_req->dst_port;
>>     p_sid->prot = p_req->prot;
>>     p_sid->is_rdma_cm = 1;
>>     p_sid->zero = 0;
> 
> This isn't bad, but I think the port field needs to be in host order, with the
> structure swapped after assignment.  Otherwise, I think the layout is
> backwards.

Ugh, this stuff makes my head hurt...  How about:

((USHORT*)&p_cm_req->service_id)[3] = p_req->dst_port;
((UCHAR*)&p_cm_req->service_id)[3] = p_req->prot;
((UCHAR*)&p_cm_req->service_id)[4] = 1;

>  Also, the structure is only required to align to a 32-bit boundary,
> which may also be an issue.

The structure is only used by reference, and is set to the address of an 8-byte aligned member of a structure.  It should work fine. :)

> This then leads to whether the structure should
> itself be contained in a union with a UINT64 (for swapping / alignment
> purposes) or defined in network order. Which, of course, makes things that
> much more complex...

I had intended to define the structure in network order (hence why the 'zero' field comes first.)  Maybe I botched it...

> your original patch is probably fine until then

Want me to wrap the #defines with CL_NTOH64, or not bother?

-Fab