Date: Fri, 29 Oct 2010 10:19:48 -0700 From: Garrett Cooper <gcooper@FreeBSD.org> To: George Neville-Neil <gnn@neville-neil.com> Cc: arch@freebsd.org Subject: Re: RFC: ARP Packet Queues Message-ID: <AANLkTinO-vSUYLvg9R=OiZ-aYgZkFUNcAzfYf7vTBjNE@mail.gmail.com> In-Reply-To: <D92A9067-7305-42B6-8D15-D0A49A0542A4@neville-neil.com> References: <D92A9067-7305-42B6-8D15-D0A49A0542A4@neville-neil.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Oct 29, 2010 at 6:42 AM, George Neville-Neil <gnn@neville-neil.com> wrote: > Howdy, > > Please review the following patch against HEAD: > > http://people.freebsd.org/~gnn/head-arpqueue.diff > > This patch makes two changes to the ARP code: > > 1) It adds a sysctl configurable queue of packets that are held until an = ARP reply is received or > timed out. > > net.link.ether.inet.maxhold > > Having the queue addresses a problem in modern systems where programs tha= t use connectionless > protocols for communication will suffer from dropping many packets when t= hey start up or when > an ARP entry moves. > > 2) Makes the time we wait for an arp reply configurable via another sysct= l. > > net.link.ether.inet.wait > > The old, pre 8.0, ARP code would run the timer once per second. =A0The ne= w > ARP code sets a timeout of 20 seconds on each entry. =A0Neither value was= specified > in RFC 826. =A0As a matter of fact, RFC 826 had this to say about timeout= s: > > "It may be desirable to have table aging and/or timeouts. =A0The > implementation of these is outside the scope of this protocol." > > This new code does not change the default value of either the arpqueue (w= hich was > always 1 packet) nor does it change the new value of the ARP down timeout= . > > I have a different patch for 7, which I will propose after I can get this= in to > HEAD and MFC'd to 8. Hi George, I'm just curious so I took at look at the patch (although I'm not really good at reviewing this section of code). 1. This needs a tab after the #define for the macro: +#define V_arp_maxhold VNET(arp_maxhold) 2. Is the naming convention for these sysctls always net.link.ether imply arp, or is it another naming convention? 3. Is there a reason why packets_dropped is a signed quantity, i.e. int, not size_t, etc? The rest looks ok, but I could be missing some context, or a subtlety of some kind. I'll give this a shot on my new router box this weekend because it looks interesting. Thanks! -Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinO-vSUYLvg9R=OiZ-aYgZkFUNcAzfYf7vTBjNE>