From owner-freebsd-current@FreeBSD.ORG Fri Aug 12 01:48:59 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4C233106564A for ; Fri, 12 Aug 2011 01:48:59 +0000 (UTC) (envelope-from qing.li@bluecoat.com) Received: from whisker.bluecoat.com (whisker.bluecoat.com [216.52.23.28]) by mx1.freebsd.org (Postfix) with ESMTP id 2627C8FC1B for ; Fri, 12 Aug 2011 01:48:58 +0000 (UTC) Received: from PWSVL-EXCHTS-01.internal.cacheflow.com ([10.2.2.122]) by whisker.bluecoat.com (8.14.2/8.14.2) with ESMTP id p7C1mtda021523 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Thu, 11 Aug 2011 18:48:55 -0700 (PDT) Received: from PWSVL-EXCMBX-01.internal.cacheflow.com ([fe80::15bc:12e2:4676:340f]) by PWSVL-EXCHTS-01.internal.cacheflow.com ([fe80::5c50:e2ba:8115:4223%20]) with mapi id 14.01.0289.001; Thu, 11 Aug 2011 18:48:50 -0700 From: "Li, Qing" To: Svatopluk Kraus , Qing Li Thread-Topic: [patch] Problem with two NIC on same NET (in_scrubprefix: err=17, new prefix add failed) Thread-Index: AQHMVzFioO1K/vmaTESFf9LzVajQjpUVvYtNgAEXuQCAAWtLwA== Date: Fri, 12 Aug 2011 01:48:49 +0000 Message-ID: References: <92B5D566-9816-4134-9358-2306D0F7DAFC@averesystems.com> <1312781293.2521.1.camel@srgsec> <1312960023.2614.12.camel@srgsec> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.2.2.106] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: Jeremiah Lott , Kevin Lo , Andrew Boyer , "freebsd-current@freebsd.org" Subject: RE: [patch] Problem with two NIC on same NET (in_scrubprefix: err=17, new prefix add failed) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Aug 2011 01:48:59 -0000 Hi, >=20 > I've started my work with not point-to-point interfaces and I've > found two problems. The first one -=20 > >=20 > When I've done more investigation, it looks similar to > http://svnweb.freebsd.org/base?view=3Drevision&revision=3D201543 > So, I propose the following patch. > I agree with your fix. As you've noted, I made the r201543 patch in IPv6 almost 2 years ago.=20 Turned out I had a note-to-self to verify if there are other similar=20 problems at the time but busy day job took me away ... > > The second one - submitted patch and description is bellow: >=20 > http://www.freebsd.org/cgi/query-pr.cgi?pr=3D159603 >=20 I agree with your fix. >=20 > I have no problem with loopback routes when I work with not > point-to-point interfaces as I can NOT set same source address on > them. However, if the interface is going down and up, then loopback > route is deleted without checking IFA_RTSELF flag (it must be > consistent! especially in kernel) and re-added regardless of > "useloopback" setting. So, at least, a loopback route is installed > even if useloopback is NOT allowed! >=20 I hope the question does not offend you, but you do know the history behind IFA_RTSELF loopback route for each interface address, right ? The interface address loopback route is used for reaching the interface address within the system after the L2/L3 separation redesign, that's why "useloopback" setting is inapplicable.=20 The check in various code paths may have a bit of consistency issue, but "useloopback" setting does not apply here. > > After that I've continued with point-to-point interfaces on same net, > i.e. I've work with un-numbered interfaces. Firstly, I could not set > parallel links on them. The fix is following and is already > commmitted: >=20 > http://www.freebsd.org/cgi/query-pr.cgi?pr=3D159600 >=20 I had a second look at it after some sleep, I agree with your fix. > > The bigger problem was with loopback routes on un-numbered > interfaces. In in_ifinit(), when un-numbered interface is setting > loopback route, then refcount on existing route is incremented and > IFA_RTSELF flags is set on its address. This is done if and only if > useloopback is set and interface is not IFF_LOOPBACK. It is OK. The > rest is hacked somehow and I don't know why. > The loopback route for the IFA should be installed unconditionally. So the check in in_ifinit() for "V_useloopback" needs to be removed. =20 > > In in_ifscrubprefix() which is used either when address is being > deleted or interface is going down, I found first inconsistence. > Refcount on existing route is decremented always (in both cases), but > the route is deleted only when address is being deleted. > That's by design. > > Futhermore, > IFA_RTSELF flag is NOT cleared when refcount is decremented! The fix > and behavour is following: >=20 > http://www.freebsd.org/cgi/query-pr.cgi?pr=3D159601 >=20 I agree with your fix. >=20 > In the view of this inconsistence, I understand a next one in > rip_ctlinput(). When interface is going up, then loopback route is > being deleted and re-added regardless of IFA_RTSELF flag and > useloopback setting. If un-numbered interfaces are used, then it > damages refcount on existing loopback route!! >=20 I will fix that. >=20 > If useloopback IS NOT set and IFA_RTSELF IS set, then loopback route > is deleted, but with correct refcount game. And if useloopback is SET > and IFA_RTSELF IS NOT set and interface IS NOT IFF_LOOPBACK, then > loopback route is added, but again with correct refcount game too. It > (with previous patch) should ensure IFA_RTSELF and loopback route > consistence. >=20 No, see above, the IFA_RTSELF route should be unconditionally. I agree with you about the consistency issue and will fix it. >> >> Unless you have a really good reason, other than code inspection, >> and have a set of test cases, please leave this code alone for now.=20 >=20 > I have good reason, but I can hack kernel just for me only in worse > scenario. However, I always try to minimalize the hacks count. >=20 You can hack the kernel however you see fit, but when you are=20 ready for a patch commit, please provide sufficient context and problem description, and test cases whenever possible to make the=20 code review process effective. >=20 > I understand, but I use my own DHCP client. Well, I try to look at it, > but maybe, someone else can test it. >=20 Again, my only point is, since these areas are core to the networking=20 kernel, please test as many scenarios as possible, more than just your=20 specific setup. (I made this mistake myself sometimes.) In any case, thank you very much for your fixes. -- Qing =20