Missing interface route?

[Mouse <mouse%Rodents-Montreal.ORG@localhost>]

I found one of my systems failing to add an interface route upon
bringing up an interface under some circumstances.  After a bit of
digging, I tracked it down.  The same bug is present on 4.0.1 and 5.2,
so I thought I'd mention it here in case it's survived into versions
people here still care about.  (It is not present in 1.4T AFAICT; it's
not _that_ longstanding.)

My test case is is two IFF_BROADCAST interfaces: I bring up one as
10.0.0.1/30 and then the other as 10.0.0.99/24.  If the bug is present,
the first of those adds a route to 10.0.0.0/30 but the second does not
add any route to anywhere.  (It should add one to 10.0.0.0/24.)

If the first address is, say, 10.0.0.9/30 instead, it works fine.
Reading the code, I believe the case where it fails is when the result
of ANDing address with mask is the same for the two interfaces; the
code appears to think that in this case the interface route for one
interface is redundant with the interface route for the other, even
though this is not true if the masks differ.

In case the code is similar enough for it to be of help, here's the
change I made, which anyone who wants is welcome to pick up.  This is
for 4.0.1, where it fixes the issue without any ill effects I've
noticed so far.  I've verified that 5.2 has the bug but havn't yet
tried the change there (though the code reads so similarly I feel sure
it'll need, at worst, minor tweaks).

commit aa6d18f7f23dbdc2aa27a35f8903fcacf279a57d
Author: Mouse <mouse%Rodents-Montreal.ORG@localhost>
Date:   Fri Feb 21 17:20:59 2014 -0500

    When adding an interface route, test "do we have a route already?" more 
correctly.
    
    Without this fix, if I do
        ifconfig a 10.0.0.2/30
        ifconfig b 10.0.0.99/24
    the second one does not produce an interface route.

diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index 3a84103..37b4d66 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -990,23 +990,26 @@ static int
 in_addprefix(struct in_ifaddr *target, int flags)
 {
        struct in_ifaddr *ia;
-       struct in_addr prefix, mask, p;
+       struct in_addr prefix, mask, p, pmask;
        int error;
 
-       if ((flags & RTF_HOST) != 0)
+       if ((flags & RTF_HOST) != 0) {
                prefix = target->ia_dstaddr.sin_addr;
-       else {
+               mask.s_addr = ~0;
+       } else {
                prefix = target->ia_addr.sin_addr;
                mask = target->ia_sockmask.sin_addr;
                prefix.s_addr &= mask.s_addr;
        }
 
        TAILQ_FOREACH(ia, &in_ifaddrhead, ia_list) {
-               if (rtinitflags(ia))
+               if (rtinitflags(ia)) {
                        p = ia->ia_dstaddr.sin_addr;
-               else {
+                       pmask.s_addr = ~0;
+               } else {
                        p = ia->ia_addr.sin_addr;
-                       p.s_addr &= ia->ia_sockmask.sin_addr.s_addr;
+                       pmask = ia->ia_sockmask.sin_addr;
+                       p.s_addr &= pmask.s_addr;
                }
 
                if (prefix.s_addr != p.s_addr)
@@ -1018,7 +1021,8 @@ in_addprefix(struct in_ifaddr *target, int flags)
                 *
                 * XXX RADIX_MPATH implications here? -dyoung
                 */
-               if (ia->ia_flags & IFA_ROUTE)
+               if ( (ia->ia_flags & IFA_ROUTE) &&
+                    (mask.s_addr == pmask.s_addr) )
                        return 0;
        }
 

/~\ The ASCII                             Mouse
\ / Ribbon Campaign
 X  Against HTML                mouse%rodents-montreal.org@localhost
/ \ Email!           7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B