Date: Fri, 12 Aug 2011 15:08:42 +0200 From: Svatopluk Kraus <onwahe@gmail.com> To: "Li, Qing" <qing.li@bluecoat.com> Cc: Jeremiah Lott <jlott@averesystems.com>, Kevin Lo <kevlo@kevlo.org>, Andrew Boyer <aboyer@averesystems.com>, "freebsd-current@freebsd.org" <freebsd-current@freebsd.org> Subject: Re: [patch] Problem with two NIC on same NET (in_scrubprefix: err=17, new prefix add failed) Message-ID: <CAFHCsPWHFTtrzVpzj9vOV8kbhOE6tFUxEKkS1t9jTqBhO3FaYw@mail.gmail.com> In-Reply-To: <B143A8975061C446AD5E29742C5317230E549A@PWSVL-EXCMBX-01.internal.cacheflow.com> References: <CAFHCsPUpkr-vne%2B9cLnovUXDGeVrOzHrKp1YAub=TjJW_3aVtg@mail.gmail.com> <92B5D566-9816-4134-9358-2306D0F7DAFC@averesystems.com> <1312781293.2521.1.camel@srgsec> <CAFHCsPVM1iGehjt8AAiwhQvW81GqLjjf7SO_-OnWzYBadQ0jmw@mail.gmail.com> <1312960023.2614.12.camel@srgsec> <B143A8975061C446AD5E29742C5317230E16CD@PWSVL-EXCMBX-01.internal.cacheflow.com> <CAFHCsPVTVw4eMJCCfdUFrJ3zLHtD_9jW1zQKByd6=60croctnw@mail.gmail.com> <B143A8975061C446AD5E29742C5317230E549A@PWSVL-EXCMBX-01.internal.cacheflow.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
thank you very much for your review and related fixes.
On Fri, Aug 12, 2011 at 3:48 AM, Li, Qing <qing.li@bluecoat.com> wrote:
>> =A0I 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!
>>
>
> =A0 I hope the question does not offend you, but you do know the history
> =A0 behind IFA_RTSELF loopback route for each interface address, right ?
>
> =A0 The interface address loopback route is used for reaching the
> =A0 interface address within the system after the L2/L3 separation
> =A0 redesign, that's why "useloopback" setting is inapplicable.
>
> =A0 The check in various code paths may have a bit of consistency issue,
> =A0 but "useloopback" setting does not apply here.
In fact, I don't know the history, so you question is quite in place.
I always try to find more about problems that I'm solving. However,
sometimes it is not easy to find all things about an issue. Sometimes,
I miss the ideas behind to figure out the issue in clear view. That's
why I've started this discussion. So, maybe I'm not perfect, but I'm
trying to learn. ;))
>> 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.
>>
>
> =A0 The loopback route for the IFA should be installed unconditionally.
> =A0 So the check in in_ifinit() for "V_useloopback" needs to be removed.
It is clear now, but I don't know that when I was proposing the patch.
I saw "V_useloopback" in in_ifinit(), so I put it to my patch too.
>>> 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.
>>
>> 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.
>>
>
> =A0 You can hack the kernel however you see fit, but when you are
> =A0 ready for a patch commit, please provide sufficient context and
> =A0 problem description, and test cases whenever possible to make the
> =A0 code review process effective.
I understand.
> =A0 Again, my only point is, since these areas are core to the networking
> =A0 kernel, please test as many scenarios as possible, more than just you=
r
> =A0 specific setup. (I made this mistake myself sometimes.)
I undestand and once again, thanks,
Svata
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFHCsPWHFTtrzVpzj9vOV8kbhOE6tFUxEKkS1t9jTqBhO3FaYw>