On Thu, May 17, 2018 at 06:52:57PM +0530, Potnuri Bharat Teja wrote:
> > > + case FW_RI_WRITE_IMMEDIATE:
> > > + wc->opcode = IB_WC_RECV_RDMA_WITH_IMM;
> > > + wc->ex.imm_data = CQE_IMM_DATA(&cqe);
> >
> > wc->ex.imm_data is a be32
> >
> > While the struct here is:
> >
> > > + struct {
> > > + __be32 mo;
> > > + __be32 msn;
> > > + __u64 imm_data;
> > > + } imm_data_rcqe;
> >
> > Is that right?
> >
> > This seems like a great way to make an endianess oopsie, ie it
> > probably doesn't work on BE.
> >
> > The struct should define imm_data as a be32 at the right offset.
> >
> Agreed.
> I beleive, immd_data field is not supposed to be BE. It is opaque to the
> driver and iwarp protocol, perhaps its made BE to help interop
> between BE
It is made BE just to make sparse more useful - really the rule is
'the value is not in cpu-endian' and standard convention is to use it
as a BE value, technically LE would be OK as well.
> and LE machines. Moreover iWARP protocol defines immediate data to be 64b.
> So that is why the WR has it as a __u64.
Ah, that causes troubles then :)
> Another concern here is, RDMA stack has it defined as __be32, I am not sure if
> it is an IB limitation.
It is.
You need to define how interop works, exactly which 32 bits on the
wire of the 64 bit imm_data will be represented by the 32 bit IB
WR/WC?
> One possible way to fix this would be to change the __u64 to __be64 as iWARP
> spec recommends it to be 64. and the force cast it from ib WR to iw_cxgb4 WR.
No, that isn't good enough, it still will get the wrong bytes during
truncation to 32 bits.
You need to define it as
union iwarp_imm_data {
struct {
u32 reserved;
be32 ib_imm_data32;
};
be64 imm_data64;
}
And probably in some common header with a nice comment since all iwarp
drivers will need this.
Assuming you decide that the last 4 bytes of the imm_data should be
used as the 32 bits for IB (this is consistent with a BE
representation)
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
