[patch] Problem with two NIC on same NET (in_scrubprefix: err=17, new prefix add failed)

Svatopluk Kraus onwahe at gmail.com
Mon Aug 8 14:57:43 UTC 2011


Thanks for committing the fix.

I've continued with work on two NIC on same NET. Now, with
point-to-point interfaces too and I have more small fixes which I
submitted today:

http://www.freebsd.org/cgi/query-pr.cgi?pr=159600
http://www.freebsd.org/cgi/query-pr.cgi?pr=159601
http://www.freebsd.org/cgi/query-pr.cgi?pr=159602
http://www.freebsd.org/cgi/query-pr.cgi?pr=159603

I have one more related problem, but I'm not sure how complex the fix should be.

When an interface is marked down a network route is deleted (or
replaced) and a loopback route persists in routing table. It is OK.
However, when an interface is marked up again, then a network route is
installed unconditionally (but error is ignored) and a loopbak route
is deleted and added immediately and unconditionally too. IMHO, it is
not correct behaviour. I think that a second half of in_ifinit()
should be here starting by in_addprefix() call with some small or
bigger changes.

Maybe, adding network route and ignoring error could be OK, but
deleting loopback route should be done under IFA_RTSELF flag is set
condition (with existing route refcount check). V_useloopback should
be check before re-adding the route and existing route must be check
to evaluate refcount correctly. The proposed patch is attached.

However, I prefer to call in_addprefix() (which is static now) instead
of rtinit() and add some more checks from in_ifinit(). Can you (or
anyone) review the patch?

 Thanks once again,

    Svata


On Mon, Aug 8, 2011 at 7:28 AM, Kevin Lo <kevlo at freebsd.org> wrote:
> Hi Andrew,
>
> I just committed Svatopluk's fix to HEAD, thanks!
>
>        Kevin
>
> On Wed, 2011-08-03 at 11:11 -0400, Andrew Boyer wrote:
>> We found and fixed a similar issue with an identical patch.  It has been working fine for us under stable/8.
>>
>> Unfortunately I am weeks and weeks behind on pushing fixes back to the tree, so you had to duplicate the work.  If it can be committed (and MFC'd to 8, please) it would save others the trouble.
>>
>> -Andrew
>>
>> On Aug 3, 2011, at 10:51 AM, Svatopluk Kraus wrote:
>>
>> > I have two NIC on same NET (both are up). If a NIC which installs
>> > network route is going down then an error happens during network route
>> > replacement (in_scrubprefix: err=17, new prefix add failed).
>> >
>> >  I've done a little bit investigation. In rtinit1(), before
>> > rtrequest1_fib() is called, info.rti_flags is initialized by flags
>> > (function argument) or-ed with ifa->ifa_flags. Both NIC has a loopback
>> > route to itself, so IFA_RTSELF is set on ifa(s). As IFA_RTSELF is
>> > defined by RTF_HOST, rtrequest1_fib() is called with RTF_HOST flag
>> > even if netmask is not NULL. Consequently, netmask is set to zero in
>> > rtrequest1_fib(), and request to add network route is changed under
>> > hands to request to add host route. It is the reason of logged info
>> > and my problem.
>> >
>> >  When I've done more investigation, it looks similar to
>> > http://svnweb.freebsd.org/base?view=revision&revision=201543. So, I
>> > propose the following patch.
>> >
>> > Index: sys/net/route.c
>> > ===================================================================
>> > --- sys/net/route.c (revision 224635)
>> > +++ sys/net/route.c (working copy)
>> > @@ -1478,7 +1478,7 @@
>> >              */
>> >             bzero((caddr_t)&info, sizeof(info));
>> >             info.rti_ifa = ifa;
>> > -           info.rti_flags = flags | ifa->ifa_flags;
>> > +           info.rti_flags = flags | (ifa->ifa_flags & ~IFA_RTSELF);
>> >             info.rti_info[RTAX_DST] = dst;
>> >             /*
>> >              * doing this for compatibility reason
>> >
>> >
>> >  Is the patch sufficient?
>> >
>> >      Svata
>> > _______________________________________________
>> > freebsd-current at freebsd.org mailing list
>> > http://lists.freebsd.org/mailman/listinfo/freebsd-current
>> > To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
>>
>> --------------------------------------------------
>> Andrew Boyer  aboyer at averesystems.com
>>
>>
>>
>>
>> _______________________________________________
>> freebsd-current at freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-current
>> To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
>
>
>
-------------- next part --------------
Index: sys/netinet/raw_ip.c
===================================================================
--- sys/netinet/raw_ip.c	(revision 224705)
+++ sys/netinet/raw_ip.c	(working copy)
@@ -761,17 +761,66 @@
 		    || (ifp->if_flags & IFF_POINTOPOINT))
 			flags |= RTF_HOST;
 
-		err = ifa_del_loopback_route((struct ifaddr *)ia, sa);
-		if (err == 0)
-			ia->ia_flags &= ~IFA_RTSELF;
+		/*
+		 * Try to install our prefix. The prefix already can be
+		 * installed by another interface, so error can be ignored.
+		 */
 
 		err = rtinit(&ia->ia_ifa, RTM_ADD, flags);
 		if (err == 0)
 			ia->ia_flags |= IFA_ROUTE;
 
+		/*
+		 * Installed loopback route isn't deleted when interface
+		 * is going down. So, here only check V_useloopback flag
+		 * and act according to it.
+		 */
+
+		if (!V_useloopback) {
+			if (ia->ia_flags & IFA_RTSELF) {
+				struct route ia_ro;
+				int freeit = 0;
+
+				bzero(&ia_ro, sizeof(ia_ro));
+				ia_ro.ro_dst = *sa;
+				rtalloc_ign_fib(&ia_ro, 0, 0);
+				if ((ia_ro.ro_rt != NULL) && (ia_ro.ro_rt->rt_ifp != NULL) &&
+				    (ia_ro.ro_rt->rt_ifp == V_loif)) {
+					RT_LOCK(ia_ro.ro_rt);
+					if (ia_ro.ro_rt->rt_refcnt <= 1)
+						freeit = 1;
+					else
+						RT_REMREF(ia_ro.ro_rt);
+				RTFREE_LOCKED(ia_ro.ro_rt);
+				}
+				if (freeit) {
+					err = ifa_del_loopback_route((struct ifaddr *)ia,
+				       				      sa);
+					if (err == 0)
+						ia->ia_flags &= ~IFA_RTSELF;
+				}
+			}
+		}
+		else if (!(ia->ia_flags & IFA_RTSELF) &&
+			 !(ifp->if_flags & IFF_LOOPBACK)) {
+			struct route ia_ro;
+
+			bzero(&ia_ro, sizeof(ia_ro));
+			*((struct sockaddr_in *)(&ia_ro.ro_dst)) = ia->ia_addr;
+			rtalloc_ign_fib(&ia_ro, 0, 0);
+			if ((ia_ro.ro_rt != NULL) && (ia_ro.ro_rt->rt_ifp != NULL) &&
+			    (ia_ro.ro_rt->rt_ifp == V_loif)) {
+				RT_LOCK(ia_ro.ro_rt);
+				RT_ADDREF(ia_ro.ro_rt);
+				RTFREE_LOCKED(ia_ro.ro_rt);
+			} else
 		err = ifa_add_loopback_route((struct ifaddr *)ia, sa);
+
 		if (err == 0)
 			ia->ia_flags |= IFA_RTSELF;
+			if (ia_ro.ro_rt != NULL)
+				RTFREE(ia_ro.ro_rt);
+		}
 
 		ifa_free(&ia->ia_ifa);
 		break;


More information about the freebsd-current mailing list