On Thursday, January 24, 2013 21:38:58 Antonio Quartulli wrote: > On Thu, Jan 24, 2013 at 09:36:11PM +0800, Marek Lindner wrote: > > On Thursday, January 24, 2013 05:53:52 Antonio Quartulli wrote: > > > > It might make sense though to check for different types of addresses > > > > that are invalid for ARP (zeronet, loopback, multicast, etc.), but I > > > > wanted to keep the patch as simple as possible. If you think these > > > > should be filtered as well, I'll prepare a v2. > > > > > > in distributed-arp-table.c:784 you can already find these checks ;) > > > > If most of the checking is done in batadv_arp_get_type() why not moving > > these checks there too ? That would allow to have all checks in the same > > place ? > > 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
