On Wed, Oct 09, 2013 at 03:11:08PM +0100, David Laight wrote:
CRC32C has to be preferred to CRC16 because of its possible HW native support and because of the reduced collision probability. With this change the Translation Table component now uses CRC32C to compute the local and global table checksum.
...
-/* Calculates the checksum of the local table of a given orig_node */ -static uint16_t batadv_tt_global_crc(struct batadv_priv *bat_priv, +/**
- batadv_tt_global_crc - calculates the checksum of the local table belonging
- to the given orig_node
- @bat_priv: the bat priv with all the soft interface information
- */
+static uint32_t batadv_tt_global_crc(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node)
...
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -1435,27 +1437,24 @@ static uint16_t batadv_tt_global_crc(struct batadv_priv *bat_priv, orig_node)) continue;
...
crc ^= crc32c(0, tt_common->addr, ETH_ALEN);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.
So it gives you almost no protection against corruption at all.
The corruption check is for the entire global table. The global table is the union of the local tables of all the other nodes (each node has a local and a global table).
The resulting value (the xor of the CRCs) is then compared to the value sent by whom originated this piece of global table.
In this way each batman-adv node is sure to have the entries that the node really generated in its local table.
(sorry for using the word table several times..)
Does this clarify? or have I misunderstood your objection?
...
-/* Calculates the checksum of the local table */ -static uint16_t batadv_tt_local_crc(struct batadv_priv *bat_priv) +/**
- batadv_tt_local_crc - calculates the checksum of the local table
- @bat_priv: the bat priv with all the soft interface information
- */
+static uint32_t batadv_tt_local_crc(struct batadv_priv *bat_priv) {
...
This looks like a clone of the previous routine. Surely you can avoid the code duplication.
Some parts are the same, true. But this was already like this. We can surely try to improve it later on with another patch.
Regards,