Re: [patch] Problem with two NIC on same NET (in_scrubprefix:

Hi,

  thank you very much for your review and related fixes.

On Fri, Aug 12, 2011 at 3:48 AM, Li, Qing <[e-mail]> 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
_______________________________________________
[e-mail] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[e-mail]"