On 01/24/2013 02:47 PM, Marek Lindner wrote:
> On Thursday, January 24, 2013 21:38:58 Antonio Quartulli wrote:
>>
>> I thought the same, but in batadv_arp_get_type() we have a general check
>> that discards wrong/bogus ARP request.
>>
>> Here instead we are filtering "correct" ARP requests that DAT should not
>> handle.
> 
> What is the difference except for the naming ? In both cases we don't want 
> these packets to be handled by DAT. 
> 
> Feel free to move these extra validation checks into a separate function that 
> gets called from batadv_arp_get_type() if you wish to emphasize the difference 
> between the types of checks. Having all checks in the same place will help to 
> avoid overlooking things later (as already happened).
> 
> Cheers,
> Marek
> 

In my opinion, the DAT should handle the sane one of source and
destination if one of them is sane and the other is bogus. So I would
maybe rather move all the checks to batadv_dat_entry_add()? There it
will only catch the add case though, not the lookup case...

At least a check for ff:ff:ff:ff:ff:ff should be added to maint as soon
as possible, as such entries were actually overwriting correct DAT
entries on my test node (and maybe even preventing ARP resolution as the
DAT node answered with these instead of the actual addresses).

Matthias