Re: [PATCH 0/3] New firewire stack

Stefan Richter
2006-12-05T18:50:26Z

Kristian Høgsberg wrote:
> David Miller wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> DO NOT USE BITFIELDS FOR DATA ON THE WIRE !!!

Actually we do so in some places of the existing FireWire drivers.
Didn't go wrong so far. :-)

>>> Where do you handle endianness ? (no need to shout for
>>> that one).
>>>
>>> (Or in general, do not use bitfields period ....)
>>
>> Yes, this is a show stopper, the endianness and
>> word-size/endian testing should have been done before
>> submission.
>
> I guess my mistake here was to present it as a patch submission. I
> acknowledged in my cover letter that it wasn't feature complete and I'm
> not pushing for inclusion just yet. I'm very much aware of the point
> that when replacing a subsystem like this, the new code has to be as
> good as the old code. In that respect, the patches I posted are lacking
> in other areas (isochronous streaming is the big one) that will take
> more work to fix than just making it work on big-endian and 64-bit
> architectures. It's still a work in progress.
[...]

That's right; there are a few in-kernel features and, much more
importantly, userspace ABIs missing before Kristian's stack could
replace the old one.

The good news is that the ABIs are either hidden behind userspace
libraries or are deprecated and to be phased out next year anyway.

The bad news is that the old stack is internally not as cleanly modular
as it was initially targeted to be. This makes it difficult to replace
the stack in parts, particularly where isochronous protocols are
concerned. Maybe this becomes a non-issue once the old ABIs were
removed; although I suppose that Kristian would like to see his stack in
wider use much earlier than that. The asynchronous stuff (what's left of
it besides sbp2) should be easy to move over.
--
Stefan Richter
-=====-=-==- ==-- --=-=