On Monday, May 7, 2018 3:55:59 AM HKT Leonardo Mörlein wrote:
if (!vlan)return false;
if (!vlan) {/* Due to a bug, some originators send TT* announcements for empty vlans. As the receiving* nodes will ignore those empty vlans (they do not* add a batadv_orig_node_vlan into the transglobal* for the originating node), crc check will fail* here. To circumvent this issue, we skip the* verification for the vlan if the crc is* equal to 0x00000000.*/if (tt_vlan_tmp->crc == 0x00000000)continue;elsereturn false;}
There are some issues with this approach:
* As you might be aware, a CRC of 0x00000000 is not an invalid or uninitialized CRC. That means it is conceivable the CRC over the translation table entries and flags results in a CRC of 0x00000000 without any bugs being present. Applying this patch would lead to introducing a bug in these conditions.
* The second concern is about how to deal with 'the bug' (referring to your patch comment above). In your case 'the bug' results in missing TT entries for a given VLAN plus a CRC of 0x00000000. What if the CRC was 0x00000001 due to some other bug ? Then you're check would totally miss the mark and we're back at the beginning. If we wish to deal with such misbehavior on a scale where we assume the code should be able to deal with garbage values in some or all fields a different approach is needed. Relying on a magic number (CRC 0x00000000) is as bad as the current behavior (assuming each VLAN has an entry).
Shortly, I will send a patch to prevent sending an empty VLAN around. Please note that this patch still is more of a band-aid. We still don't know how the empty VLAN came into being in the first place. Plus, that patch won't protect against garbage packets which certainly requires more effort.
Cheers, Marek