Subject: Re: [PATCH 10/16] batman-adv: use CRC32C instead of CRC16 in TT code

From: Antonio Quartulli <[email-masked]>
Date: Wed, 9 Oct 2013 19:05:57 +0200

On Wed, Oct 09, 2013 at 05:19:44PM +0100, David Laight wrote:
> > On Wed, Oct 09, 2013 at 04:49:53PM +0100, David Laight wrote:
> > > > > Are you really generating CRC32 of a pile of ethernet MAC addresses
> > > > > and the XORing the CRC together?
> > > > > That gives the same answer as XORing together the MAC addresses and
> > > > > then doing a CRC of the final value.
> > > >
> > > > I was not sure about this since the CRC32 is not a linear operation. However
> > > > this routine is not on the fast path, so we can also live with this order.
> > >
> > > All CRC are linear.
> > > Because '(a + b) mod c' is the same as '((a mod c) + (b mod c)) mod c'.
> > >
> > > The CRC of a buffer is the XOR of the CRCs generated for each '1' bit.
> > > The CRC for each bit depends on how far it is from the end of the buffer.
> > 
> > In our tables we cannot make any assumption about the order of the entries: the
> > node whom generated the table may store the entries in a different order from
> > what we have got.
> > This is why I did not implemented it as a simple CRC of the
> > whole the GlobalTable/buffer but I CRC'd each MAC+VID on its own.
> > 
> ...
> > 
> > Assuming what I said above (that we cannot make assumptions on the order of the
> > entries), what is your suggestion?
> 
> I'm not sure what you are using this CRC for.
> If you are trying to use it to check that the two tables match - so a
> full update isn't needed then it just won't work.

No, we have a sequence number for this purpose.

The CRC is only used to check that (after having applied all the needed changes)
the content of the table is really the same as the one owned by whom generated
it.

Therefore the CRC is practically used only to check that the node memory has not
been corrupted and I can hardly imagine that such a case can lead to the same
CRC as the expected one (of course, probability is not zero, but that's because
it is an hash function..).

The conclusion is that we can live with this patch :-)

Thank you anyway for your explanation. This will surely help in case we decide
to change/improve it.

Regards,

-- 
Antonio Quartulli