Re: [Openhip-developers] [Openhip-cvs] SF.net SVN: openhip:[246] hip/trunk/src/util/hip_util.c

[S. D. M. <ma...@ni...>]

On Mon, Nov 19, 2012 at 8:57 AM, S. David Mattes <ma...@ni...> wrote:

> On Sun, Nov 18, 2012 at 5:23 PM, Tom Henderson <to...@to...> wrote:
>
>> On 11/14/2012 09:09 AM, sar...@us... wrote:
>>
>>> Revision: 246
>>>            http://openhip.svn.**sourceforge.net/openhip/?rev=**
>>> 246&view=rev<http://openhip.svn.sourceforge.net/openhip/?rev=246&view=rev>
>>> Author:   sarabito
>>> Date:     2012-11-14 17:09:51 +0000 (Wed, 14 Nov 2012)
>>> Log Message:
>>> -----------
>>> When comparing bits for cookie check, do not call ntoh64 prior to
>>> masking the bits.  Doing so was breaking compatibility on little vs. big
>>> endian systems (e.g. x86 to arm).
>>>
>>> Modified Paths:
>>> --------------
>>>      hip/trunk/src/util/hip_util.c
>>>
>>>
>> David,
>> I agree with this patch, but it breaks backward compatibility on i386;
>> after applying r246, I can no longer complete a base exchange with the test
>> hipserver.
>>
>> I also wonder whether there is a similar error in HIPL since we have been
>> interoperating with them successfully.
>>
>> We probably need to write a small unit test on compare_bits() and these
>> puzzle-oriented functions, and check what HIPL is doing.  If we convince
>> ourselves that there is a problem, we need to figure out the migration
>> strategy to correcting it.
>>
>> - Tom
>>
>>
>
> Hi Tom,
>
> You're right, and sorry our fix broke backward compatibility.  I'm also
> not convinced that our patch was the "correct" patch, just that it made the
> cross-arch puzzle check successful.  A unit test would be a great for this!
>
> What do you recommend in the near term?  Should we revert our patch in
> r246?  We can just apply to our local code base.
>
> Thank you,
> David
>

Hi Tom,

I thought about this a little more and did some additional testing and I
believe that applying the hton64 transformation is inappropriate in this
case.  I believe our patch did the correct thing to remove this
transformation.  In a test, I applied the hton64 transformation regardless
of the architecture (currently hton64 is a no-op for ARM), then
cross-architecture exchanges work.  This observation leads me to the
conclusion that the data has already been transformed into the correct host
byte-ordering.

It should be possible to preserve backward compatibility by transforming
the data with hton64 regardless of the host architecture, but this fix may
be fixing something broken by breaking it again, which, when the root cause
is finally corrected, there is yet another backward compatibility mess to
sort through.

So, we will wait for your guidance on this issue.

Thank you,
David