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

[Hefty, Sean]

> Doing it in host order becomes this ugly combination of nested swap calls:
> 
>     p_cm_req->service_id = RtlUlonglongByteSwap(
>         RtlUlonglongByteSwap( IB_REQ_CM_RDMA_SID_PREFIX ) +

Defining the SID_PREFIX in host order helps some.  Having shorter names for ntoh and hton functionality instead of "RtlUlonglongByteSwap" would help even more. :P

>         ((UINT64)p_req->prot) << 16 +
>         RtlUshortByteSwap( p_req->dst_port )
>         );
> 
> Or
> 
>     p_cm_req->service_id = IB_REQ_CM_RDMA_SID_PREFIX +
>         RtlUlonglongByteSwap( ((UINT64)p_req->prot) << 16 +
> RtlUshortByteSwap( p_req->dst_port ) );
> 
> Is that any easier to understand/maintain?
> 
> 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.  Also, the structure is only required to align to a 32-bit boundary, which may also be an issue.  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 guess if Windows ever supported a big-endian architecture, we wouldn't have much choice in the matter, and we'd be force to just swap things around.  Maybe Nokia will put IB cards into their phones?

your original patch is probably fine until then