Hi On 11/03/2016 08:40, Ryota Ozaki wrote: > This proposal separates Layer 2 nexthop caches (ARP > and NDP entries) from the routing table and instead > stores them in each interface. This change obsoletes > the concept of cloning and cloned routes; nexthop > caches won't be bound to any routes.
Nice work!!! > > Here is a patch (tl;tr): > http://www.netbsd.org/~ozaki-r/separate-nexthop-caches.diff I'll try and review this in detail, but I'm really pushed for time right now so might be a while :( Here's a few thoughts though: nd6_is_addr_neighbor() needs to test for RTF_CONNECTED. The comment (which you retained) in that function even says this, but no check is actually made anymore. This should be fixed. > - RTF_CLONING and RTF_CLONED are obsolete > - Keep the definitions to not break package > builds I think we should just drop (by this i mean comment not, not physically removed from the header) the RTF_CLONING and RTF_CLONED defines. FreeBSD has already done this, and I hate having useless defines for removed functonality as the header is then effectively lying about the system we are compiling for. Packages should just #ifdef around this (dhcpcd already does this for example). Also, I'm pretty sure that these flags would be relatively unused by packages. +#define RTF_CONNECTED 0x100 /* host address route */ /* hosts on this route are neighbours */ Is probably a better description for the purpose of the flag. > - RTM_RESOLVE and RTF_XSORELVE are obsolete > - The definitions remain to not break > package builds (may not be needed) > - RTF_LLINFO is obsolete > - The definition remains Disagree again for the same reasons above. > - arp/ndp -d don't remove interface addresses > - They were removed (unexpectedly?) I hope you don't mean they remove the address from the interface :) Do you mean they no longer remove the address reported by arp/ndp for the hardware address? If so, I think we should try and retain that functionality if possible (I actually use it!) > - ARP entries that are created by arp ... temp > can be overwritten now Unsure what you mean here, can you expand on your description please? Roy
