Re: [PATCH 0/3] New firewire stack

Kristian Høgsberg
2006-12-05T06:24:17Z

Benjamin Herrenschmidt wrote:
> On Tue, 2006-12-05 at 00:22 -0500, Kristian Høgsberg wrote:
>> Hi,
>>
>> I'm announcing an alternative firewire stack that I've been working on
>> the last few weeks. I'm aiming to implement feature parity with the
>> current firewire stack, but not necessarily interface compatibility.
>> For now, I have the low-level OHCI driver done, the mid-level
>> transaction logic done, and the SBP-2 (storage) driver is basically
>> done. What's missing is a streaming interface (in progress) to allow
>> reception and transmission of isochronous data and a userspace
>> interface for controlling devices (much like raw1394 or libusb for
>> usb). I'm working out of this git repository:
>
> A very very very quick look at the code shows that:
>
> - It looks nice / clear

Great, good to hear.

> - It's horribly broken in at least two area :
>
> DO NOT USE BITFIELDS FOR DATA ON THE WIRE !!!
>
> and
>
> Where do you handle endianness ? (no need to shout for
> that one).

Well, the code isn't big-endian safe yet, but the only place where I expect to
have to fix this is in fw-ohci.c. I need to figure out how I want to set up
the OHCI controller to this - it has a couple of bits to control this. All
data outside the low-level driver is cpu-endian, with the exception of payload
data. IEEE1394 doesn't specify an endianness for the payload data, even
though most protocols use big-endian. Some protocols have a mix of
byte-arrays and be32 words (eg SBP-2) so it's up to the protocol to byteswap
its data as appropriate.

> (Or in general, do not use bitfields period ....)
>
> bitfields format is not guaranteed, and is not endian consistent.

Ok... I was planning to make big-endian versions of the structs so that the
endian issue would be solved. But if the bit layout is not consistent, I
guess bitfields are useless for wire formats. I didn't know that though, I
thought the C standard specified that the compiler should allocate bits out of
a word using the lower bits first. Is the problem that it allocates them out
of a 64-bit word on 64-bit platforms?

cheers,
Kristian