From owner-svn-src-head@FreeBSD.ORG Mon Oct 10 05:11:58 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5B1021065672; Mon, 10 Oct 2011 05:11:58 +0000 (UTC) (envelope-from tomelite82@gmail.com) Received: from mail-yw0-f54.google.com (mail-yw0-f54.google.com [209.85.213.54]) by mx1.freebsd.org (Postfix) with ESMTP id BCC848FC1C; Mon, 10 Oct 2011 05:11:57 +0000 (UTC) Received: by ywp17 with SMTP id 17so6314894ywp.13 for ; Sun, 09 Oct 2011 22:11:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=D7hjLd/+IblJxYICu28IzCTaL4KQhaxHozA+zae0PUM=; b=GHRNJlR830st8u3r8Vk0eBhV/ogZoyXLQ1/yFvNWXuIYCJzbIsBCqg4HMpZU+BqEKK fzymXoQBpSO8k8bNFsZc3L6xjmu7GURwasv21SNZ2WMaG3lCnqqfQQm/b44ezdrCRjOA G/bKDEJkvAJO5zrPKJZOozi+0fGNdnrZwQzKY= MIME-Version: 1.0 Received: by 10.150.209.10 with SMTP id h10mr5006032ybg.52.1318223517032; Sun, 09 Oct 2011 22:11:57 -0700 (PDT) Sender: tomelite82@gmail.com Received: by 10.151.78.21 with HTTP; Sun, 9 Oct 2011 22:11:56 -0700 (PDT) In-Reply-To: <20111009194732.GB94905@glebius.int.ru> References: <201110031951.p93JpJLA025249@svn.freebsd.org> <20111009194732.GB94905@glebius.int.ru> Date: Sun, 9 Oct 2011 22:11:56 -0700 X-Google-Sender-Auth: mbeBQJQo2KNF0BpYfev7ohWw5kk Message-ID: From: Qing Li To: Gleb Smirnoff Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Oct 2011 05:11:58 -0000 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