-----linux-rdma-owner@xxxxxxxxxxxxxxx wrote: ----- >To: "Jason Gunthorpe" <jgg@xxxxxxxx> >From: "Bernard Metzler" >Sent by: linux-rdma-owner@xxxxxxxxxxxxxxx >Date: 02/27/2019 02:50PM >Cc: linux-rdma@xxxxxxxxxxxxxxx >Subject: Re: [PATCH v5 01/13] iWarp wire packet format > >-----"Jason Gunthorpe" <jgg@xxxxxxxx> wrote: ----- > >>To: "Bernard Metzler" <bmt@xxxxxxxxxxxxxx> >>From: "Jason Gunthorpe" <jgg@xxxxxxxx> >>Date: 02/22/2019 12:58AM >>Cc: linux-rdma@xxxxxxxxxxxxxxx >>Subject: Re: [PATCH v5 01/13] iWarp wire packet format >> >>On Tue, Feb 19, 2019 at 11:08:51AM +0100, Bernard Metzler wrote: >>> +/* >>> + * DDP/RDMAP Hdr bits & fields >>> + */ >>> +enum { >>> + DDP_FLAG_TAGGED = cpu_to_be16(0x8000), >>> + DDP_FLAG_LAST = cpu_to_be16(0x4000), >>> + DDP_MASK_RESERVED = cpu_to_be16(0x3C00), >>> + DDP_MASK_VERSION = cpu_to_be16(0x0300), >>> + RDMAP_MASK_VERSION = cpu_to_be16(0x00C0), >>> + RDMAP_MASK_RESERVED = cpu_to_be16(0x0030), >>> + RDMAP_MASK_OPCODE = cpu_to_be16(0x000f) >>> +}; >>> + >>> +static inline u8 __ddp_version(struct iwarp_ctrl *ctrl) >>> +{ >>> + return (u8)(be16_to_cpu(ctrl->ddp_rdmap_ctrl & DDP_MASK_VERSION) >>>> 8); >>> +}; >> >>Is this driver sprase clean for endian-ness conversions? > >I think so, I checked for PPC <-> x86 > > >> >>Why write it like the above? >> >> (be16_to_cpu(ctrl->ddp_rdmap_ctrl) & DDP_MASK_VERSION) >> 8 >> >>? >Yes, looks silly. It should have same byte order on both sides. >Maybe it just works, since both cpu_to_beXX and beXX_to_cpu are >the same on tested machines. Will fix that. > I re-checked the above. It seems to be correct. __ddp_version() gets the DDP version out of a wire formatted header. DDP_MASK_VERSION is in wire format as well. We finally translate it into host byte order with be16_to_cpu() In an attempt to make things more clear, I change the name of that 'getter' functions into __ddp_get_version() etc. So we have now 'get' and 'set' function. 'get' functions get from NBO and return translated into HBO, 'set' functions transform HBO values into NBO and set them in the packet header. >> >>Also no redundent casts please, the u8 does not need to be explicit >>here. There are lots of them in this code >> > >Right, will fix that. > > >>Jason >> >> > >