Date: Thu, 11 Aug 2005 10:45:37 +0200 From: Jeremie Le Hen <jeremie@le-hen.org> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet if_ether.c Message-ID: <20050811084537.GV45385@obiwan.tataz.chchile.org> In-Reply-To: <200508110825.j7B8PnTf027445@repoman.freebsd.org> References: <200508110825.j7B8PnTf027445@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Gleb, On Thu, Aug 11, 2005 at 08:25:48AM +0000, Gleb Smirnoff wrote: > glebius 2005-08-11 08:25:48 UTC > > FreeBSD src repository > > Modified files: > sys/netinet if_ether.c > Log: > o Fix a race between three threads: output path, > incoming ARP packet and route request adding/removing > ARP entries. The root of the problem is that > struct llinfo_arp was accessed without any locks. > To close race we will use locking provided by > rtentry, that references this llinfo_arp: > - Make arplookup() return a locked rtentry. > - In arpresolve() hold the lock provided by > rt_check()/arplookup() until the end of function, > covering all accesses to the rtentry itself and > llinfo_arp it refers to. > - In in_arpinput() do not drop lock provided by > arplookup() during first part of the function. > - Simplify logic in the first part of in_arpinput(), > removing one level of indentation. > - In the second part of in_arpinput() hold rtentry > lock while copying address. > > o Fix a condition when route entry is destroyed, while > another thread is contested on its lock: > - When storing a pointer to rtentry in llinfo_arp list, > always add a reference to this rtentry, to prevent > rtentry being destroyed via RTM_DELETE request. > - Remove this reference when removing entry from > llinfo_arp list. > > o Further cleanup of arptimer(): > - Inline arptfree() into arptimer(). > - Use official queue(3) way to pass LIST. > - Hold rtentry lock while reading its structure. > - Do not check that sdl_family is AF_LINK, but > assert this. > > Reviewed by: sam > Stress test: http://www.holm.cc/stress/log/cons141.html > Stress test: http://people.freebsd.org/~pho/stress/log/cons144.html Do you plan to MFC this to RELENG_6 (and maybe RELENG_5) ? Thanks for correcting this. Regards, -- Jeremie Le Hen < jeremie at le-hen dot org >< ttz at chchile dot org >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050811084537.GV45385>