new ARP code review

Qing Li qingli at speakeasy.net
Mon Jun 18 18:18:26 UTC 2007


> 
> i agree that the timing is a bit tight for inclusion, especially 
> because the work dates back to 2004 if not before, and i think Qing 
> Li took over development at least two years ago - not a great track 
> record in terms of dedication to the work. I'd rather not see it 
> rushed in :)
>

   Not sure how to respond to your comment here ...

   I emailed to net@ and developers@ for review after I put in the support
   for IPv6, and made the new functions generic more than two years ago. I
   received one full review from Gleb and a partial review from Andre. And 
   that patch has been sitting there in my home directory on
   people.freebsd.org/~qingli ever since. The very last patch I put there is 
   dated April 19, 2005 (for the then -current). This time around, I got two
   other reviews, and that's it. 

   I'm certainly open for any suggestion on how to get more reviews
   from the community. And let me know if you have any other specific 
   work items that you want done so you don't feel being rushed.

> 
> the splitting is exactly the goal of this work and is by design.
> The mapping between the L3 and L2 addresses has nothing to do with
> the IP route lookup, and it should be elsewhere (namely, in the hash
> table or whatever data structure is appropriate).
> 
> Eventually, with this structure you can do the route lookup
> only when you need to find the next hop (e.g. when a route
> changes etc.) and just the much-cheaper L3-L2 map in other cases.
> 
> Even if the current implementation keeps doing both, this change
> is a step towards a separation of the two functions and enabling
> more cleanup in the code.
> 
> I hope you don't disagree on the design. As for actual performance,
> we may pay something, as we did if you compare 4.x and 6.x/7.x,
> but then the opportunities for parallelization, reduction of
> contention and further code simplifications are well worth it.
> 

   The current code necessary for creating ARP entries through 
   arp_rtrequest(), and the subsequent call paths are convoluted and 
   difficult to understand. The same approach was imported in the ND6 code.
   This work has eliminated these types of code and the logic flows much
   better. 

   A couple of people raised the two-lookup performance issue, but 
   "Do you agree in principle ..." is exactly the kind of reviews I was
   hoping for, but received none so far. This was the gating issue 
   for me for proceeding further two years ago and remains so today.

   -- Qing









More information about the freebsd-net mailing list