Date: Sun, 9 Oct 2011 22:11:56 -0700 From: Qing Li <qingli@freebsd.org> To: Gleb Smirnoff <glebius@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, bz@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r225947 - head/sys/netinet Message-ID: <CAGnGRd%2BUfDYb=3y=8DoeArT7Qp2S1rUyTbQMTjD6cgybyoMKhA@mail.gmail.com> In-Reply-To: <20111009194732.GB94905@glebius.int.ru> References: <201110031951.p93JpJLA025249@svn.freebsd.org> <20111009194732.GB94905@glebius.int.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Gleb, > > On Mon, Oct 03, 2011 at 07:51:19PM +0000, Qing Li wrote: > Q> Author: qingli > Q> Date: Mon Oct =A03 19:51:18 2011 > Q> New Revision: 225947 > Q> URL: http://svn.freebsd.org/changeset/base/225947 > Q> > Q> Log: > Q> =A0 A system may have multiple physical interfaces, all of which are o= n the > Q> =A0 same prefix. Since a single route entry is installed for the prefi= x > Q> =A0 (without RADIX_MPATH), incoming packets on the interfaces that are= not > Q> =A0 associated with the prefix route may trigger an error message abou= t > Q> =A0 unable to allocation LLE entry, and fails L2. This patch makes sur= e a > Q> =A0 valid route is present in the system, and allow the aforementioned > Q> =A0 condition to exist and treats as valid. > Q> > Q> =A0 Reviewed by: =A0 =A0 =A0 bz > Q> =A0 MFC after: 5 days > > =A0this commit together with r225946 makes the in_lltable_rtcheck() > quite difficult to understand. > > =A0What confuses me most, is that in lines 1435-1445 you are > assigning error to a positive value, BUT proceeding further > with function. > This is what was there before (meaning returning error immediately), but I guess a couple of folks felt it looked a bit cluttered. This is mostly due to the fact the "RTFREE_LOCKED()" operation has to be performed before returning. > > Well, after third review it is clear, that > next if() case would definitely be true, and you would proceed > with return. But that is difficult to see from first glance. > Not so, only for an indirect prefix route. > > I'd suggest to remove error variable, return immediately in > all error cases, and also the RTF_GATEWAY check can be shifted up, > since it is the most simple and the most usual to be true. > No, the RTF_GATEWAY check cannot be shifted up because if we did that, the (indirect host route, with destination matching the gateway IP) would never be executed, if when that set of conditions are true, which i= s allowed and the reason for the patch in the first place. > > Also, in this commit you really do not need the __DECONST hacks. > Okay, I must had a temporary short circuit. I will fix that. Thanks --Qing
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGnGRd%2BUfDYb=3y=8DoeArT7Qp2S1rUyTbQMTjD6cgybyoMKhA>