If we're adding an interface address that is already one of the interface's

aliases with the same netmask and destination, don't remove it and then
re-add exactly the same thing.

This means that static (non-sticky) routes that use the interface address
(or destination address) as a destination will not suddenly evaporate when
IPCP comes up (not unless the negotiated IPs have changed anyway).

1 files changed, 52 insertions, 33 deletions

diff --git a/usr.sbin/ppp/iface.c b/usr.sbin/ppp/iface.c
index 4451ced..67d2631 100644
--- a/usr.sbin/ppp/iface.c
+++ b/usr.sbin/ppp/iface.c
@@ -252,7 +252,7 @@ int
 iface_inAdd(struct iface *iface, struct in_addr ifa, struct in_addr mask,
             struct in_addr brd, int how)
 {
-  int slot, s, chg;
+  int slot, s, chg, nochange;
   struct ifaliasreq ifra;
   struct sockaddr_in *me, *peer, *msk;
   struct iface_addr *addr;
@@ -274,12 +274,6 @@ iface_inAdd(struct iface *iface, struct in_addr ifa, struct in_addr mask,
   }
   iface->in_addr = addr;
 
-  s = ID0socket(AF_INET, SOCK_DGRAM, 0);
-  if (s < 0) {
-    log_Printf(LogERROR, "iface_inAdd: socket(): %s\n", strerror(errno));
-    return 0;
-  }
-
   /*
    * We've gotta be careful here.  If we try to add an address with the
    * same destination as an existing interface, nothing will work.
@@ -288,9 +282,26 @@ iface_inAdd(struct iface *iface, struct in_addr ifa, struct in_addr mask,
    * There *may* be more than one - if the user has ``iface add''ed
    * stuff previously.
    */
+  nochange = 0;
+  s = -1;
   for (chg = 0; chg < iface->in_addrs; chg++) {
     if ((iface->in_addr[chg].brd.s_addr == brd.s_addr &&
          brd.s_addr != INADDR_BROADCAST) || chg == slot) {
+      /*
+       * If we've found an entry that exactly matches what we want to add,
+       * don't remove it and then add it again.  If we do, it's possible
+       * that the kernel will (correctly) ``tidy up'' any routes that use
+       * the IP number as a destination.
+       */
+      if (chg == slot && iface->in_addr[chg].mask.s_addr == mask.s_addr) {
+        nochange = 1;
+        continue;
+      }
+      if (s == -1 && (s = ID0socket(AF_INET, SOCK_DGRAM, 0)) == -1) {
+        log_Printf(LogERROR, "iface_inAdd: socket(): %s\n", strerror(errno));
+        return 0;
+      }
+
       memset(&ifra, '\0', sizeof ifra);
       strncpy(ifra.ifra_name, iface->name, sizeof ifra.ifra_name - 1);
       me = (struct sockaddr_in *)&ifra.ifra_addr;
@@ -325,35 +336,43 @@ iface_inAdd(struct iface *iface, struct in_addr ifa, struct in_addr mask,
     }
   }
 
-  memset(&ifra, '\0', sizeof ifra);
-  strncpy(ifra.ifra_name, iface->name, sizeof ifra.ifra_name - 1);
-  me = (struct sockaddr_in *)&ifra.ifra_addr;
-  msk = (struct sockaddr_in *)&ifra.ifra_mask;
-  peer = (struct sockaddr_in *)&ifra.ifra_broadaddr;
-  me->sin_family = msk->sin_family = peer->sin_family = AF_INET;
-  me->sin_len = msk->sin_len = peer->sin_len = sizeof(struct sockaddr_in);
-  me->sin_addr = ifa;
-  msk->sin_addr = mask;
-  peer->sin_addr = brd;
-
-  if (log_IsKept(LogDEBUG)) {
-    char buf[16];
-
-    strncpy(buf, inet_ntoa(brd), sizeof buf-1);
-    buf[sizeof buf - 1] = '\0';
-    log_Printf(LogDEBUG, "Add %s -> %s\n", inet_ntoa(ifa), buf);
+  if (!nochange) {
+    if (s == -1 && (s = ID0socket(AF_INET, SOCK_DGRAM, 0)) == -1) {
+      log_Printf(LogERROR, "iface_inAdd: socket(): %s\n", strerror(errno));
+      return 0;
+    }
+    memset(&ifra, '\0', sizeof ifra);
+    strncpy(ifra.ifra_name, iface->name, sizeof ifra.ifra_name - 1);
+    me = (struct sockaddr_in *)&ifra.ifra_addr;
+    msk = (struct sockaddr_in *)&ifra.ifra_mask;
+    peer = (struct sockaddr_in *)&ifra.ifra_broadaddr;
+    me->sin_family = msk->sin_family = peer->sin_family = AF_INET;
+    me->sin_len = msk->sin_len = peer->sin_len = sizeof(struct sockaddr_in);
+    me->sin_addr = ifa;
+    msk->sin_addr = mask;
+    peer->sin_addr = brd;
+
+    if (log_IsKept(LogDEBUG)) {
+      char buf[16];
+
+      strncpy(buf, inet_ntoa(brd), sizeof buf-1);
+      buf[sizeof buf - 1] = '\0';
+      log_Printf(LogDEBUG, "Add %s -> %s\n", inet_ntoa(ifa), buf);
+    }
+
+    /* An EEXIST failure w/ brd == INADDR_BROADCAST is ok (and works!) */
+    if (ID0ioctl(s, SIOCAIFADDR, &ifra) < 0 &&
+        (brd.s_addr != INADDR_BROADCAST || errno != EEXIST)) {
+      log_Printf(LogERROR, "iface_inAdd: ioctl(SIOCAIFADDR): %s: %s\n",
+                 inet_ntoa(ifa), strerror(errno));
+      ID0ioctl(s, SIOCDIFADDR, &ifra);	/* EEXIST ? */
+      close(s);
+      return 0;
+    }
   }
 
-  /* An EEXIST failure w/ brd == INADDR_BROADCAST is ok (and works!) */
-  if (ID0ioctl(s, SIOCAIFADDR, &ifra) < 0 &&
-      (brd.s_addr != INADDR_BROADCAST || errno != EEXIST)) {
-    log_Printf(LogERROR, "iface_inAdd: ioctl(SIOCAIFADDR): %s: %s\n",
-               inet_ntoa(ifa), strerror(errno));
-    ID0ioctl(s, SIOCDIFADDR, &ifra);	/* EEXIST ? */
+  if (s != -1)
     close(s);
-    return 0;
-  }
-  close(s);
 
   if (slot == iface->in_addrs) {
     /* We're adding a new interface address */