[patch] Problem with two NIC on same NET (in_scrubprefix:
err=17, new prefix add failed)
Svatopluk Kraus
onwahe at gmail.com
Fri Aug 12 13:08:45 UTC 2011
Hi,
thank you very much for your review and related fixes.
On Fri, Aug 12, 2011 at 3:48 AM, Li, Qing <qing.li at bluecoat.com> wrote:
>> 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!
>>
>
> 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.
>
> The check in various code paths may have a bit of consistency issue,
> 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.
>>
>
> The loopback route for the IFA should be installed unconditionally.
> 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.
>>
>
> You can hack the kernel however you see fit, but when you are
> ready for a patch commit, please provide sufficient context and
> problem description, and test cases whenever possible to make the
> code review process effective.
I understand.
> Again, my only point is, since these areas are core to the networking
> kernel, please test as many scenarios as possible, more than just your
> specific setup. (I made this mistake myself sometimes.)
I undestand and once again, thanks,
Svata
More information about the freebsd-current
mailing list