It works!

This is on my W500 Thinkpad with a Motorola V195 modem via usb.
I'm happy.  I do hope this gets into 4.6.

--STeve Andre'

On Wednesday 01 July 2009 12:23:20 Todd T. Fries wrote:
> Guys,
>
> I tested this and it seems ppp in the tree is busted to the point of not
> working without this diff.
>
> If you use ppp please test current snaps to confirm it is busted then
> apply claudio's diff below and test again.
>
> If you use ppp and do not test, do not be surprised if it does not work in
> the next release.
>
> Thanks,
>
> Penned by Claudio Jeker on 20090630 17:05.28, we have:
> | So ppp(8) did insane routing message handling in its sysctl handlers. The
> | worst thing about them are that their actually not needed and better
> | replaced with libc functions (getifaddrs and if_nametoindex).
> |
> | This diff is not haevily tested (my last ppp usage is years ago) so I'm
> | hopeing people with ppp(8) issues could give this a whirl and see if it
> | fixes the problems.
> | --
> |
> | :wq Claudio
> |
> | Index: ppp/arp.c
> | ===================================================================
> | RCS file: /cvs/src/usr.sbin/ppp/ppp/arp.c,v
> | retrieving revision 1.15
> | diff -u -p -r1.15 arp.c
> | --- ppp/arp.c       6 May 2008 06:34:10 -0000       1.15
> | +++ ppp/arp.c       30 Jun 2009 14:52:53 -0000
> | @@ -38,6 +38,7 @@
> |  #include <sys/un.h>
> |
> |  #include <errno.h>
> | +#include <ifaddrs.h>
> |  #include <stdio.h>
> |  #include <stdlib.h>
> |  #include <string.h>
> | @@ -229,93 +230,58 @@ int
> |  arp_EtherAddr(int s, struct in_addr ipaddr, struct sockaddr_dl *hwaddr,
> |                int verbose)
> |  {
> | -  int mib[6], skip;
> | -  size_t needed;
> | -  char *buf, *ptr, *end;
> | -  struct if_msghdr *ifm;
> | -  struct ifa_msghdr *ifam;
> | -  struct sockaddr_dl *dl;
> | -  struct sockaddr *sa[RTAX_MAX];
> | -
> | -  mib[0] = CTL_NET;
> | -  mib[1] = PF_ROUTE;
> | -  mib[2] = 0;
> | -  mib[3] = 0;
> | -  mib[4] = NET_RT_IFLIST;
> | -  mib[5] = 0;
> | -
> | -  if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) {
> | -    log_Printf(LogERROR, "arp_EtherAddr: sysctl: estimate: %s\n",
> | -              strerror(errno));
> | -    return 0;
> | -  }
> | -
> | -  if ((buf = malloc(needed)) == NULL)
> | -    return 0;
> | +  struct sockaddr_dl *dl = NULL;
> | +  struct ifaddrs *ifa, *ifap;
> | +  int skip = 1;
> |
> | -  if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) {
> | -    free(buf);
> | +  if (getifaddrs(&ifap) != 0) {
> | +    log_Printf(LogERROR, "arp_EtherAddr: getifaddrs: %s\n",
> | strerror(errno)); return 0;
> |    }
> | -  end = buf + needed;
> |
> | -  ptr = buf;
> | -  while (ptr < end) {
> | -    ifm = (struct if_msghdr *)ptr;         /* On if_msghdr */
> | -    if (ifm->ifm_type != RTM_IFINFO)
> | -      break;
> | -    ptr += ifm->ifm_msglen;
> | -    if (ifm->ifm_version != RTM_VERSION)
> | -      continue;
> | -    dl = (struct sockaddr_dl *)(ifm + 1);  /* Single _dl at end */
> | -    skip = (ifm->ifm_flags & (IFF_UP | IFF_BROADCAST | IFF_POINTOPOINT |
> | +  for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
> | +    if (ifa->ifa_addr->sa_family == AF_LINK) {
> | +      dl = (struct sockaddr_dl *)ifa->ifa_addr;
> | +      skip = (ifa->ifa_flags & (IFF_UP | IFF_BROADCAST | IFF_POINTOPOINT
> | | IFF_NOARP | IFF_LOOPBACK)) != (IFF_UP | IFF_BROADCAST); -    while (ptr
> | < end) {
> | -      ifam = (struct ifa_msghdr *)ptr;     /* Next ifa_msghdr (alias) */
> | -      if (ifam->ifam_type != RTM_NEWADDR)  /* finished ? */
> | -        break;
> | -      ptr += ifam->ifam_msglen;
> | -      if (ifam->ifam_version != RTM_VERSION)
> | -        continue;
> | -      if (skip || (ifam->ifam_addrs & (RTA_NETMASK|RTA_IFA)) !=
> | -          (RTA_NETMASK|RTA_IFA))
> | -        continue;
> | -      /* Found a candidate.  Do the addresses match ? */
> | -      if (log_IsKept(LogDEBUG) &&
> | -          ptr == (char *)ifm + ifm->ifm_msglen + ifam->ifam_msglen)
> | -        log_Printf(LogDEBUG, "%.*s interface is a candidate for
> | proxy\n", -                  dl->sdl_nlen, dl->sdl_data);
> | -
> | -      iface_ParseHdr(ifam, sa);
> | -
> | -      if (sa[RTAX_IFA]->sa_family == AF_INET) {
> | -        struct sockaddr_in *ifa, *netmask;
> | -
> | -        ifa = (struct sockaddr_in *)sa[RTAX_IFA];
> | -        netmask = (struct sockaddr_in *)sa[RTAX_NETMASK];
> | -
> | -        if (log_IsKept(LogDEBUG)) {
> | -          char a[16];
> | -
> | -          strncpy(a, inet_ntoa(netmask->sin_addr), sizeof a - 1);
> | -          a[sizeof a - 1] = '\0';
> | -          log_Printf(LogDEBUG, "Check addr %s, mask %s\n",
> | -                     inet_ntoa(ifa->sin_addr), a);
> | -        }
> | -
> | -        if ((ifa->sin_addr.s_addr & netmask->sin_addr.s_addr) ==
> | -            (ipaddr.s_addr & netmask->sin_addr.s_addr)) {
> | -          log_Printf(verbose ? LogPHASE : LogDEBUG,
> | -                     "Found interface %.*s for %s\n", dl->sdl_nlen,
> | -                     dl->sdl_data, inet_ntoa(ipaddr));
> | -          memcpy(hwaddr, dl, dl->sdl_len);
> | -          free(buf);
> | -          return 1;
> | -        }
> | +      continue;
> | +    }
> | +    if (skip)
> | +      /* Skip unusable interface */
> | +      continue;
> | +
> | +    /* Found a candidate.  Do the addresses match ? */
> | +    if (log_IsKept(LogDEBUG))
> | +      log_Printf(LogDEBUG, "%.*s interface is a candidate for proxy\n",
> | +                dl->sdl_nlen, dl->sdl_data);
> | +
> | +    if (ifa->ifa_addr->sa_family == AF_INET) {
> | +      struct sockaddr_in *addr, *netmask;
> | +
> | +      addr = (struct sockaddr_in *)ifa->ifa_addr;
> | +      netmask = (struct sockaddr_in *)ifa->ifa_netmask;
> | +
> | +      if (log_IsKept(LogDEBUG)) {
> | +        char a[16];
> | +
> | +        strncpy(a, inet_ntoa(netmask->sin_addr), sizeof a - 1);
> | +        a[sizeof a - 1] = '\0';
> | +        log_Printf(LogDEBUG, "Check addr %s, mask %s\n",
> | +                   inet_ntoa(addr->sin_addr), a);
> | +      }
> | +
> | +      if ((addr->sin_addr.s_addr & netmask->sin_addr.s_addr) ==
> | +          (ipaddr.s_addr & netmask->sin_addr.s_addr)) {
> | +        log_Printf(verbose ? LogPHASE : LogDEBUG,
> | +                   "Found interface %.*s for %s\n", dl->sdl_nlen,
> | +                   dl->sdl_data, inet_ntoa(ipaddr));
> | +        memcpy(hwaddr, dl, dl->sdl_len);
> | +        freeifaddrs(ifap);
> | +        return 1;
> |        }
> |      }
> |    }
> | -  free(buf);
> | +  freeifaddrs(ifap);
> |
> |    return 0;
> |  }
> | Index: ppp/iface.c
> | ===================================================================
> | RCS file: /cvs/src/usr.sbin/ppp/ppp/iface.c,v
> | retrieving revision 1.28
> | diff -u -p -r1.28 iface.c
> | --- ppp/iface.c     25 Jun 2009 15:59:28 -0000      1.28
> | +++ ppp/iface.c     30 Jun 2009 14:54:08 -0000
> | @@ -44,6 +44,7 @@
> |  #include <sys/un.h>
> |
> |  #include <errno.h>
> | +#include <ifaddrs.h>
> |  #include <string.h>
> |  #include <stdarg.h>
> |  #include <stdio.h>
> | @@ -90,114 +91,64 @@ static const struct in6_addr in6mask128
> |  struct iface *
> |  iface_Create(const char *name)
> |  {
> | -  int mib[6], maxtries, err;
> | -  size_t needed, namelen;
> | -  char *buf, *ptr, *end;
> | -  struct if_msghdr *ifm;
> | -  struct ifa_msghdr *ifam;
> | +  size_t namelen;
> |    struct sockaddr_dl *dl;
> | -  struct sockaddr *sa[RTAX_MAX];
> | +  struct ifaddrs *ifap, *ifa;
> |    struct iface *iface;
> |    struct iface_addr *addr;
> |
> | -  mib[0] = CTL_NET;
> | -  mib[1] = PF_ROUTE;
> | -  mib[2] = 0;
> | -  mib[3] = 0;
> | -  mib[4] = NET_RT_IFLIST;
> | -  mib[5] = 0;
> | -
> | -  maxtries = 20;
> | -  err = 0;
> | -  do {
> | -    if (maxtries-- == 0 || (err && err != ENOMEM)) {
> | -      fprintf(stderr, "iface_Create: sysctl: %s\n", strerror(err));
> | -      return NULL;
> | -    }
> | -
> | -    if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) {
> | -      fprintf(stderr, "iface_Create: sysctl: estimate: %s\n",
> | -                strerror(errno));
> | -      return NULL;
> | -    }
> | -
> | -    if ((buf = (char *)malloc(needed)) == NULL) {
> | -      fprintf(stderr, "iface_Create: malloc failed: %s\n",
> | strerror(errno)); -      return NULL;
> | -    }
> | -
> | -    if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) {
> | -      err = errno;
> | -      free(buf);
> | -      buf = NULL;
> | -    }
> | -  } while (buf == NULL);
> | +  if (getifaddrs(&ifap) != 0) {
> | +    fprintf(stderr, "iface_Create: getifaddrs: %s\n", strerror(errno));
> | +    return NULL;
> | +  }
> |
> | -  ptr = buf;
> | -  end = buf + needed;
> |    iface = NULL;
> |    namelen = strlen(name);
> |
> | -  while (ptr < end && iface == NULL) {
> | -    ifm = (struct if_msghdr *)ptr;                 /* On if_msghdr */
> | -    if (ifm->ifm_version != RTM_VERSION)
> | +  for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
> | +    if (strcmp(name, ifa->ifa_name))
> |        continue;
> | -    if (ifm->ifm_type != RTM_IFINFO)
> | -      break;
> | -    dl = (struct sockaddr_dl *)(ifm + 1);          /* Single _dl at end */
> | -    if (dl->sdl_nlen == namelen && !strncmp(name, dl->sdl_data,
> | namelen)) { +    if (ifa->ifa_addr->sa_family == AF_LINK) {
> | +      dl = (struct sockaddr_dl *)ifa->ifa_addr;
> |        iface = (struct iface *)malloc(sizeof *iface);
> |        if (iface == NULL) {
> |          fprintf(stderr, "iface_Create: malloc: %s\n", strerror(errno));
> | +        freeifaddrs(ifap);
> |          return NULL;
> |        }
> |        iface->name = strdup(name);
> | -      iface->index = ifm->ifm_index;
> | -      iface->flags = ifm->ifm_flags;
> | +      iface->index = if_nametoindex(name);
> | +      iface->flags = ifa->ifa_flags;
> |        iface->mtu = 0;
> |        iface->addrs = 0;
> |        iface->addr = NULL;
> |      }
> | -    ptr += ifm->ifm_msglen;                                /* First 
> ifa_msghdr */
> | -    for (; ptr < end; ptr += ifam->ifam_msglen) {
> | -      ifam = (struct ifa_msghdr *)ptr;                     /* Next if 
> address */
> | -
> | -      if (ifam->ifam_type != RTM_NEWADDR)          /* finished this if */
> | -        break;
> | -      if (ifm->ifm_version != RTM_VERSION)
> | -        continue;
> | -
> | -      if (iface != NULL && ifam->ifam_addrs & RTA_IFA) {
> | -        /* Found a configured interface ! */
> | -        iface_ParseHdr(ifam, sa);
> |
> | -        if (sa[RTAX_IFA] && (sa[RTAX_IFA]->sa_family == AF_INET
> | +    if (ifa->ifa_addr->sa_family == AF_INET
> |  #ifndef NOINET6
> | -                             || sa[RTAX_IFA]->sa_family == AF_INET6
> | +        || ifa->ifa_addr->sa_family == AF_INET6
> |  #endif
> | -                             )) {
> | -          /* Record the address */
> | +       ) {
> | +      /* Record the address */
> |
> | -          addr = (struct iface_addr *)
> | -            realloc(iface->addr, (iface->addrs + 1) * sizeof
> | iface->addr[0]); -          if (addr == NULL)
> | -            break;
> | -          iface->addr = addr;
> | -
> | -          addr += iface->addrs;
> | -          iface->addrs++;
> | -
> | -          ncprange_setsa(&addr->ifa, sa[RTAX_IFA], sa[RTAX_NETMASK]);
> | -          if (sa[RTAX_BRD])
> | -            ncpaddr_setsa(&addr->peer, sa[RTAX_BRD]);
> | -          else
> | -            ncpaddr_init(&addr->peer);
> | -        }
> | -      }
> | +      addr = (struct iface_addr *)
> | +        realloc(iface->addr, (iface->addrs + 1) * sizeof
> | iface->addr[0]); +      if (addr == NULL)
> | +        break;
> | +      iface->addr = addr;
> | +
> | +      addr += iface->addrs;
> | +      iface->addrs++;
> | +
> | +      ncprange_setsa(&addr->ifa, ifa->ifa_addr, ifa->ifa_netmask);
> | +      if (ifa->ifa_broadaddr)
> | +        ncpaddr_setsa(&addr->peer, ifa->ifa_broadaddr);
> | +      else
> | +        ncpaddr_init(&addr->peer);
> |      }
> |    }
> |
> | -  free(buf);
> | +  freeifaddrs(ifap);
> |
> |    return iface;
> |  }
> | @@ -700,20 +651,4 @@ iface_Show(struct cmdargs const *arg)
> |    }
> |
> |    return 0;
> | -}
> | -
> | -void
> | -iface_ParseHdr(struct ifa_msghdr *ifam, struct sockaddr *sa[RTAX_MAX])
> | -{
> | -  char *wp;
> | -  int rtax;
> | -
> | -  wp = (char *)(ifam + 1);
> | -
> | -  for (rtax = 0; rtax < RTAX_MAX; rtax++)
> | -    if (ifam->ifam_addrs & (1 << rtax)) {
> | -      sa[rtax] = (struct sockaddr *)wp;
> | -      wp += ROUNDUP(sa[rtax]->sa_len);
> | -    } else
> | -      sa[rtax] = NULL;
> |  }
> | Index: ppp/iface.h
> | ===================================================================
> | RCS file: /cvs/src/usr.sbin/ppp/ppp/iface.h,v
> | retrieving revision 1.8
> | diff -u -p -r1.8 iface.h
> | --- ppp/iface.h     19 Aug 2001 23:22:17 -0000      1.8
> | +++ ppp/iface.h     30 Jun 2009 14:51:03 -0000
> | @@ -62,4 +62,3 @@ extern int iface_Show(struct cmdargs con
> |  extern int iface_SetFlags(const char *, int);
> |  extern int iface_ClearFlags(const char *, int);
> |  extern void iface_Destroy(struct iface *);
> | -extern void iface_ParseHdr(struct ifa_msghdr *, struct sockaddr
> | *[RTAX_MAX]); Index: ppp/route.c
> | ===================================================================
> | RCS file: /cvs/src/usr.sbin/ppp/ppp/route.c,v
> | retrieving revision 1.37
> | diff -u -p -r1.37 route.c
> | --- ppp/route.c     25 Jun 2009 15:59:28 -0000      1.37
> | +++ ppp/route.c     30 Jun 2009 14:03:36 -0000
> | @@ -204,113 +204,15 @@ static int route_nifs = -1;
> |  const char *
> |  Index2Nam(int idx)
> |  {
> | -  /*
> | -   * XXX: Maybe we should select() on the routing socket so that we can
> | -   *      notice interfaces that come & go (PCCARD support).
> | -   *      Or we could even support a signal that resets these so that
> | -   *      the PCCARD insert/remove events can signal ppp.
> | -   */
> | -  static char **ifs;               /* Figure these out once */
> | -  static int debug_done;   /* Debug once */
> | +  static char ifname[IF_NAMESIZE];
> | +  char *ifn;
> |
> | -  if (idx > route_nifs || (idx > 0 && ifs[idx-1] == NULL)) {
> | -    int mib[6], have, had;
> | -    size_t needed;
> | -    char *buf, *ptr, *end;
> | -    struct sockaddr_dl *dl;
> | -    struct if_msghdr *ifm;
> | +  ifn = if_indextoname(idx, ifname);
> |
> | -    if (ifs) {
> | -      free(ifs);
> | -      ifs = NULL;
> | -      route_nifs = 0;
> | -    }
> | -    debug_done = 0;
> | -
> | -    mib[0] = CTL_NET;
> | -    mib[1] = PF_ROUTE;
> | -    mib[2] = 0;
> | -    mib[3] = 0;
> | -    mib[4] = NET_RT_IFLIST;
> | -    mib[5] = 0;
> | -
> | -    if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) {
> | -      log_Printf(LogERROR, "Index2Nam: sysctl: estimate: %s\n",
> | -                 strerror(errno));
> | -      return NumStr(idx, NULL, 0);
> | -    }
> | -    if ((buf = malloc(needed)) == NULL)
> | -      return NumStr(idx, NULL, 0);
> | -    if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) {
> | -      free(buf);
> | -      return NumStr(idx, NULL, 0);
> | -    }
> | -    end = buf + needed;
> | -
> | -    have = 0;
> | -    for (ptr = buf; ptr < end; ptr += ifm->ifm_msglen) {
> | -      ifm = (struct if_msghdr *)ptr;
> | -      if (ifm->ifm_version != RTM_VERSION)
> | -        continue;
> | -      if (ifm->ifm_type != RTM_IFINFO)
> | -        continue;
> | -      dl = (struct sockaddr_dl *)(ifm + 1);
> | -      if (ifm->ifm_index > 0) {
> | -        if (ifm->ifm_index > have) {
> | -          char **newifs;
> | -
> | -          had = have;
> | -          have = ifm->ifm_index + 5;
> | -          if (had)
> | -            newifs = (char **)realloc(ifs, sizeof(char *) * have);
> | -          else
> | -            newifs = (char **)calloc(sizeof(char *), have);
> | -          if (!newifs) {
> | -            log_Printf(LogDEBUG, "Index2Nam: %s\n", strerror(errno));
> | -            route_nifs = 0;
> | -            if (ifs) {
> | -              free(ifs);
> | -              ifs = NULL;
> | -            }
> | -            free(buf);
> | -            return NumStr(idx, NULL, 0);
> | -          }
> | -          ifs = newifs;
> | -          memset(ifs + had, '\0', sizeof(char *) * (have - had));
> | -        }
> | -        if (ifs[ifm->ifm_index-1] == NULL) {
> | -          ifs[ifm->ifm_index-1] = (char *)malloc(dl->sdl_nlen+1);
> | -          if (ifs[ifm->ifm_index-1] == NULL)
> | -       log_Printf(LogDEBUG, "Skipping interface %d: Out of memory\n",
> | -                  ifm->ifm_index);
> | -     else {
> | -       memcpy(ifs[ifm->ifm_index-1], dl->sdl_data, dl->sdl_nlen);
> | -       ifs[ifm->ifm_index-1][dl->sdl_nlen] = '\0';
> | -       if (route_nifs < ifm->ifm_index)
> | -         route_nifs = ifm->ifm_index;
> | -     }
> | -        }
> | -      } else if (log_IsKept(LogDEBUG))
> | -        log_Printf(LogDEBUG, "Skipping out-of-range interface %d!\n",
> | -                  ifm->ifm_index);
> | -    }
> | -    free(buf);
> | -  }
> | -
> | -  if (log_IsKept(LogDEBUG) && !debug_done) {
> | -    int f;
> | -
> | -    log_Printf(LogDEBUG, "Found the following interfaces:\n");
> | -    for (f = 0; f < route_nifs; f++)
> | -      if (ifs[f] != NULL)
> | -        log_Printf(LogDEBUG, " Index %d, name \"%s\"\n", f+1, ifs[f]);
> | -    debug_done = 1;
> | -  }
> | -
> | -  if (idx < 1 || idx > route_nifs || ifs[idx-1] == NULL)
> | +  if (idx < 1 || ifn == NULL)
> |      return NumStr(idx, NULL, 0);
> |
> | -  return ifs[idx-1];
> | +  return ifn;
> |  }
> |
> |  void

Reply via email to