Re: [PATCH v5 01/13] iWarp wire packet format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux