From owner-svn-src-all@FreeBSD.ORG Mon Oct 10 17:05:49 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 68C77106566C; Mon, 10 Oct 2011 17:05:49 +0000 (UTC) (envelope-from bz@freebsd.org) Received: from mx1.sbone.de (mx1.sbone.de [IPv6:2a01:4f8:130:3ffc::401:25]) by mx1.freebsd.org (Postfix) with ESMTP id EDE9A8FC08; Mon, 10 Oct 2011 17:05:48 +0000 (UTC) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:31::2013:587]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id C537F25D3899; Mon, 10 Oct 2011 17:05:47 +0000 (UTC) Received: from content-filter.sbone.de (content-filter.sbone.de [IPv6:fde9:577b:c1a9:31::2013:2742]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id DEC28BD3C44; Mon, 10 Oct 2011 17:05:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:31::2013:587]) by content-filter.sbone.de (content-filter.sbone.de [fde9:577b:c1a9:31::2013:2742]) (amavisd-new, port 10024) with ESMTP id MkCXODx92tQX; Mon, 10 Oct 2011 17:05:44 +0000 (UTC) Received: from orange-en1.sbone.de (orange-en1.sbone.de [IPv6:fde9:577b:c1a9:31:cabc:c8ff:fecf:e8e3]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id B0892BD3C3B; Mon, 10 Oct 2011 17:05:44 +0000 (UTC) Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: "Bjoern A. Zeeb" In-Reply-To: Date: Mon, 10 Oct 2011 17:05:43 +0000 Content-Transfer-Encoding: quoted-printable Message-Id: References: <201110031951.p93JpJLA025249@svn.freebsd.org> <20111009194732.GB94905@glebius.int.ru> <20111010061313.GC94905@FreeBSD.org> To: Qing Li X-Mailer: Apple Mail (2.1084) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Gleb Smirnoff , src-committers@freebsd.org Subject: Re: svn commit: r225947 - head/sys/netinet X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Oct 2011 17:05:49 -0000 On 10. Oct 2011, at 08:44 , Qing Li wrote: Hey, > Okay, now I know what's confusing you ... it's that bug I introduced = :-) >=20 > The 2nd "if()" check on the RTF_GATEWAY flag should have been > an "else if()". >=20 > In a nutshell, the logic is any indirect route should fail the check, > except for that special host route. >=20 > Attached is the rework of that part of the patch, with some cleaning = up > per your suggestion. >=20 > Thank you very much for catching that bug. >=20 > --Qing >=20 >=20 >> Q> > Well, after third review it is clear, that >> Q> > next if() case would definitely be true, and you would proceed >> Q> > with return. But that is difficult to see from first glance. >> Q> >> Q> Not so, only for an indirect prefix route. >> Q> >> Q> > I'd suggest to remove error variable, return immediately in >> Q> > all error cases, and also the RTF_GATEWAY check can be shifted = up, >> Q> > since it is the most simple and the most usual to be true. >> Q> > >> Q> >> Q> No, the RTF_GATEWAY check cannot be shifted up because if we did >> Q> that, the (indirect host route, with destination matching the = gateway IP) >> Q> would never be executed, if when that set of conditions are = true, which is >> Q> allowed and the reason for the patch in the first place. >>=20 >> Can you elaborate on that please? As far as I see, any rtentry that = has >> RTF_GATEWAY would return with EINVAL. The first if() clause doesn't >> do any actual processing, only checking flags and memcmp()ing. The = third >> clause either. The error is never reset to 0. So, I don't see any >> difference in returning EINVAL for RTF_GATEWAY immediately or later >> after other checks. I am a bit confused by this entire thing; it seems it's only parts of = what I had looked at initially but maybe the commit was split due to = follow-ups on an early change. I am however happy that some things I had mentioned initially are now = being addressed in the cleanup patch. I have not checked the logic changes on = it however. Thanks, Bjoern --=20 Bjoern A. Zeeb You have to have visions! Stop bit received. Insert coin for new address family.=