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

  Hi,

>=20
> I've started my work with not point-to-point interfaces and I've
> found two problems. The first one -=20
>
    <snip>
>=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.

   <snip>

>=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.

   <snip>

>=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.

   <snip>

>>
>> 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.

   <snip>

>=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




_______________________________________________
[e-mail] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[e-mail]"