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
_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw