Hello tech@, claudio@ in particular,
this diff corrects the behavior of ospf6d when:
- vlan/vether/? interface is added with some inet6 address
- a configured interface is removed (please test with usb devs)
In both cases the daemon crashes.  Also, it contains various fixes
useful for debugging, cleans up and removes a possible memory leak.
The patched version adds all interfaces into iflist in all three
processes, which might be useful in the future when the user reloads
configuration.

Known problems:
- Crazy user types "for i = 1 to 1000 do ifconfig vlan$i create" and
  then destroy -> some interfaces get stuck and if_find says it can't
  find them. I couldn't find out the cause of this. An error prints.
- When you destroy a configured vlan interface, its routes won't
  withdraw. They are marked with DOWN flag, but then the parent sends
  IMSG_NETWORK_ADD, which leaves me with the question about purpose of
  IMSG_NETWORK_DEL. Any ideas? Or an elegant diff?

Desired behavior:
- remove if -> withdraw all its routes, but keep running
- add previously configured if -> announce all its routes back
- add unconfigured if -> get everyone to know about it without crashing

Comments? Corrections? Thanks in advance.
--
Martin Pelikan


Index: area.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/area.c,v
retrieving revision 1.4
diff -u -p -r1.4 area.c
--- area.c      28 Dec 2008 20:08:31 -0000      1.4
+++ area.c      26 Jan 2011 15:20:38 -0000
@@ -56,7 +56,6 @@ area_del(struct area *area)
 
        /* clean lists */
        while ((iface = LIST_FIRST(&area->iface_list)) != NULL) {
-               LIST_REMOVE(iface, entry);
                if_del(iface);
        }
 
Index: interface.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/interface.c,v
retrieving revision 1.15
diff -u -p -r1.15 interface.c
--- interface.c 20 Sep 2009 20:45:06 -0000      1.15
+++ interface.c 26 Jan 2011 15:20:38 -0000
@@ -27,6 +27,7 @@
 #include <net/if_types.h>
 #include <ctype.h>
 #include <err.h>
+#include <errno.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -205,7 +206,7 @@ if_new(u_short ifindex, char *ifname)
        struct iface            *iface;
 
        if ((iface = calloc(1, sizeof(*iface))) == NULL)
-               err(1, "if_new: calloc");
+               return (NULL);
 
        iface->state = IF_STA_DOWN;
 
@@ -259,7 +260,7 @@ if_del(struct iface *iface)
 {
        struct nbr      *nbr = NULL;
 
-       log_debug("if_del: interface %s", iface->name);
+       log_debug("if_del(pid %d): interface %s", getpid(), iface->name);
 
        /* revert the demotion when the interface is deleted */
        if ((iface->state & (IF_STA_MULTI | IF_STA_POINTTOPOINT)) == 0)
@@ -277,6 +278,11 @@ if_del(struct iface *iface)
                evtimer_del(&iface->lsack_tx_timer);
 
        ls_ack_list_clr(iface);
+
+       /* interfaces from config used to belong to some area */
+       if (iface->cflags & F_IFACE_CONFIGURED)
+               LIST_REMOVE(iface, entry);
+
        TAILQ_REMOVE(&iflist, iface, list);
        free(iface);
 }
@@ -728,8 +734,8 @@ if_join_group(struct iface *iface, struc
                mreq.ipv6mr_multiaddr = *addr;
                mreq.ipv6mr_interface = iface->ifindex;
 
-               if (setsockopt(iface->fd, IPPROTO_IPV6, IPV6_JOIN_GROUP,
-                   &mreq, sizeof(mreq)) < 0) {
+               if (iface->fd > 0 && setsockopt(iface->fd, IPPROTO_IPV6,
+                   IPV6_JOIN_GROUP, &mreq, sizeof(mreq)) < 0) {
                        log_warn("if_join_group: error IPV6_JOIN_GROUP, "
                            "interface %s address %s", iface->name,
                            log_in6addr(addr));
@@ -762,12 +768,15 @@ if_leave_group(struct iface *iface, stru
                mreq.ipv6mr_multiaddr = *addr;
                mreq.ipv6mr_interface = iface->ifindex;
 
-               if (setsockopt(iface->fd, IPPROTO_IPV6, IPV6_LEAVE_GROUP,
-                   (void *)&mreq, sizeof(mreq)) < 0) {
-                       log_warn("if_leave_group: error IPV6_LEAVE_GROUP, "
-                           "interface %s address %s", iface->name,
-                           log_in6addr(addr));
-                       return (-1);
+               if (iface->fd > 0 && setsockopt(iface->fd, IPPROTO_IPV6,
+                   IPV6_LEAVE_GROUP, (void *)&mreq, sizeof(mreq)) < 0) {
+                       /* the interface might be already gone */
+                       if (errno != ENXIO) {
+                               log_warn("if_leave_group: error "
+                                   "IPV6_LEAVE_GROUP, interface %s address %s",
+                                   iface->name, log_in6addr(addr));
+                               return (-1);
+                       }
                }
                break;
        case IF_TYPE_POINTOMULTIPOINT:
@@ -790,9 +799,14 @@ if_set_mcast(struct iface *iface)
        case IF_TYPE_BROADCAST:
                if (setsockopt(iface->fd, IPPROTO_IPV6, IPV6_MULTICAST_IF,
                    &iface->ifindex, sizeof(iface->ifindex)) < 0) {
-                       log_debug("if_set_mcast: error setting "
-                           "IP_MULTICAST_IF, interface %s", iface->name);
-                       return (-1);
+                       /* the interface might be already gone */
+                       if (errno != ENXIO) {
+                               log_debug("if_set_mcast: error setting "
+                                   "IP_MULTICAST_IF, interface %s",
+                                   iface->name);
+                               return (-1);
+                       }
+                       return (~F_IFACE_AVAIL);
                }
                break;
        case IF_TYPE_POINTOMULTIPOINT:
Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/kroute.c,v
retrieving revision 1.29
diff -u -p -r1.29 kroute.c
--- kroute.c    14 Oct 2010 07:38:05 -0000      1.29
+++ kroute.c    26 Jan 2011 15:20:39 -0000
@@ -973,15 +973,22 @@ if_announce(void *msg)
                if ((iface = if_new(ifan->ifan_index, ifan->ifan_name)) == NULL)
                        fatal("if_announce failed");
                iface->cflags |= F_IFACE_AVAIL;
+               main_imsg_compose_rde(IMSG_IFADD, 0,
+                   iface, sizeof(struct iface));
+               main_imsg_compose_ospfe(IMSG_IFADD, 0,
+                   iface, sizeof(struct iface));
                break;
        case IFAN_DEPARTURE:
-               iface = if_find(ifan->ifan_index);
-               if (iface->cflags & F_IFACE_CONFIGURED) {
-                       main_imsg_compose_rde(IMSG_IFDELETE, 0,
-                           &iface->ifindex, sizeof(iface->ifindex));
-                       main_imsg_compose_ospfe(IMSG_IFDELETE, 0,
-                           &iface->ifindex, sizeof(iface->ifindex));
+               if ((iface = if_find(ifan->ifan_index)) == NULL)  {
+                       log_warn("someone announced departure of non-existing "
+                           "interface %s - possible race condition?",
+                           ifan->ifan_name);
+                       return;
                }
+               main_imsg_compose_rde(IMSG_IFDELETE, 0,
+                   &iface->ifindex, sizeof(iface->ifindex));
+               main_imsg_compose_ospfe(IMSG_IFDELETE, 0,
+                   &iface->ifindex, sizeof(iface->ifindex));
                if_del(iface);
                break;
        }
Index: ospf6d.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
retrieving revision 1.21
diff -u -p -r1.21 ospf6d.c
--- ospf6d.c    22 Aug 2010 21:15:25 -0000      1.21
+++ ospf6d.c    26 Jan 2011 15:20:39 -0000
@@ -325,12 +325,12 @@ check_child(pid_t pid, const char *pname
 
        if (waitpid(pid, &status, WNOHANG) > 0) {
                if (WIFEXITED(status)) {
-                       log_warnx("lost child: %s exited", pname);
+                       log_warnx("lost child %d: %s exited", pid, pname);
                        return (1);
                }
                if (WIFSIGNALED(status)) {
-                       log_warnx("lost child: %s terminated; signal %d",
-                           pname, WTERMSIG(status));
+                       log_warnx("lost child %d: %s terminated; signal %d",
+                           pid, pname, WTERMSIG(status));
                        return (1);
                }
        }
Index: ospfe.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/ospfe.c,v
retrieving revision 1.33
diff -u -p -r1.33 ospfe.c
--- ospfe.c     22 Aug 2010 20:27:52 -0000      1.33
+++ ospfe.c     26 Jan 2011 15:20:39 -0000
@@ -53,6 +53,7 @@ struct ospfd_conf     *oeconf = NULL, *nconf
 struct imsgev          *iev_main;
 struct imsgev          *iev_rde;
 int                     oe_nofib;
+extern TAILQ_HEAD(, iface) iflist;
 
 /* ARGSUSED */
 void
@@ -203,15 +204,13 @@ ospfe_shutdown(void)
 
        /* stop all interfaces and remove all areas */
        while ((area = LIST_FIRST(&oeconf->area_list)) != NULL) {
-               LIST_FOREACH(iface, &area->iface_list, entry) {
-                       if (if_fsm(iface, IF_EVT_DOWN)) {
-                               log_debug("error stopping interface %s",
-                                   iface->name);
-                       }
-               }
                LIST_REMOVE(area, entry);
                area_del(area);
        }
+       /* interfaces not in config file */
+       while ((iface = TAILQ_FIRST(&iflist)) != TAILQ_END(&iflist)) {
+               if_del(iface);
+       }
 
        close(oeconf->ospf_socket);
 
@@ -285,30 +284,31 @@ ospfe_dispatch_main(int fd, short event,
 
                        iface = if_find(ifp->ifindex);
                        if (iface == NULL)
-                               fatalx("interface lost in ospfe");
+                               fatalx("IFINFO interface lost in ospfe");
                        iface->flags = ifp->flags;
                        iface->linkstate = ifp->linkstate;
                        iface->nh_reachable = ifp->nh_reachable;
 
                        if (iface->nh_reachable) {
                                if_fsm(iface, IF_EVT_UP);
-                               log_warnx("interface %s up", iface->name);
+                               log_warnx("ospfe: interface %s up",
+                                   iface->name);
                        } else {
                                if_fsm(iface, IF_EVT_DOWN);
-                               log_warnx("interface %s down", iface->name);
+                               log_warnx("ospfe: interface %s down",
+                                   iface->name);
                        }
                        break;
                case IMSG_IFADD:
-                       if ((iface = malloc(sizeof(struct iface))) == NULL)
-                               fatal(NULL);
-                       memcpy(iface, imsg.data, sizeof(struct iface));
-
-                       LIST_INIT(&iface->nbr_list);
-                       TAILQ_INIT(&iface->ls_ack_list);
-                       RB_INIT(&iface->lsa_tree);
+                       ifp = imsg.data;
+                       if ((iface = if_new(ifp->ifindex, ifp->name)) == NULL)
+                               fatalx("IFADD in ospfe");
 
-                       area = area_find(oeconf, iface->area_id);
-                       LIST_INSERT_HEAD(&area->iface_list, iface, entry);
+                       /* ifaces from config belong to some area */
+                       if ((iface->cflags & F_IFACE_CONFIGURED) &&
+                           (area = area_find(oeconf, iface->area_id)) != NULL)
+                               LIST_INSERT_HEAD(&area->iface_list, iface,
+                                   entry);
                        break;
                case IMSG_IFDELETE:
                        if (imsg.hdr.len != IMSG_HEADER_SIZE +
@@ -318,9 +318,8 @@ ospfe_dispatch_main(int fd, short event,
                        memcpy(&ifindex, imsg.data, sizeof(ifindex));
                        iface = if_find(ifindex);
                        if (iface == NULL)
-                               fatalx("interface lost in ospfe");
+                               fatalx("IFDELETE interface lost in ospfe");
 
-                       LIST_REMOVE(iface, entry);
                        if_del(iface);
                        break;
                case IMSG_IFADDRNEW:
Index: packet.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/packet.c,v
retrieving revision 1.11
diff -u -p -r1.11 packet.c
--- packet.c    31 Dec 2010 21:22:42 -0000      1.11
+++ packet.c    26 Jan 2011 15:20:40 -0000
@@ -98,10 +98,13 @@ send_packet(struct iface *iface, void *p
 
        /* set outgoing interface for multicast traffic */
        if (IN6_IS_ADDR_MULTICAST(dst))
-               if (if_set_mcast(iface) == -1) {
+               switch (if_set_mcast(iface)) {
+               case -1:
                        log_warn("send_packet: error setting multicast "
                            "interface, %s", iface->name);
                        return (-1);
+               case ~F_IFACE_AVAIL:
+                       return (0);
                }
 
        if (sendto(iface->fd, pkt, len, MSG_DONTROUTE, (struct sockaddr *)&sa6,
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/rde.c,v
retrieving revision 1.50
diff -u -p -r1.50 rde.c
--- rde.c       22 Aug 2010 20:55:10 -0000      1.50
+++ rde.c       26 Jan 2011 15:20:40 -0000
@@ -89,6 +89,7 @@ struct imsgev         *iev_ospfe;
 struct imsgev          *iev_main;
 struct rde_nbr         *nbrself;
 struct lsa_tree                 asext_tree;
+extern TAILQ_HEAD(, iface) iflist;
 
 /* ARGSUSED */
 void
@@ -208,15 +209,21 @@ void
 rde_shutdown(void)
 {
        struct area     *a;
+       struct iface    *iface;
 
        stop_spf_timer(rdeconf);
        cand_list_clr();
        rt_clear();
 
+       /* remove all areas and interfaces within */
        while ((a = LIST_FIRST(&rdeconf->area_list)) != NULL) {
                LIST_REMOVE(a, entry);
                area_del(a);
        }
+       /* interfaces not in config file */
+       while ((iface = TAILQ_FIRST(&iflist)) != TAILQ_END(&iflist)) {
+               if_del(iface);
+       }
        rde_nbr_free();
 
        msgbuf_clear(&iev_ospfe->ibuf.w);
@@ -585,8 +592,17 @@ rde_dispatch_imsg(int fd, short event, v
                        ifp = imsg.data;
 
                        iface = if_find(ifp->ifindex);
-                       if (iface == NULL)
-                               fatalx("interface lost in rde");
+                       if (iface == NULL) {
+                               /*
+                                * Race condition.
+                                * When ospfe sends us IFINFO that interface
+                                * is down, we might've already got IFDELETE
+                                * from parent earlier. 
+                                */
+                               log_debug("IFINFO interface %s gone in rde",
+                                   ifp->name);
+                               break;
+                       }
                        iface->flags = ifp->flags;
                        iface->linkstate = ifp->linkstate;
                        iface->nh_reachable = ifp->nh_reachable;
@@ -625,7 +641,7 @@ rde_dispatch_parent(int fd, short event,
 {
        static struct area      *narea;
        struct area             *area;
-       struct iface            *iface;
+       struct iface            *iface, *ifp;
        struct ifaddrchange     *ifc;
        struct iface_addr       *ia, *nia;
        struct imsg              imsg;
@@ -708,20 +724,19 @@ rde_dispatch_parent(int fd, short event,
                                rde_send_change_kroute(rn);
                        else
                                /* should not happen */
-                               imsg_compose_event(iev_main, 
IMSG_KROUTE_DELETE, 0,
-                                   0, -1, &kr, sizeof(kr));
+                               imsg_compose_event(iev_main, IMSG_KROUTE_DELETE,
+                                   0, 0, -1, &kr, sizeof(kr));
                        break;
                case IMSG_IFADD:
-                       if ((iface = malloc(sizeof(struct iface))) == NULL)
-                               fatal(NULL);
-                       memcpy(iface, imsg.data, sizeof(struct iface));
-
-                       LIST_INIT(&iface->nbr_list);
-                       TAILQ_INIT(&iface->ls_ack_list);
-                       RB_INIT(&iface->lsa_tree);
+                       ifp = imsg.data;
+                       if ((iface = if_new(ifp->ifindex, ifp->name)) == NULL)
+                               fatalx("IFADD in rde");
 
-                       area = area_find(rdeconf, iface->area_id);
-                       LIST_INSERT_HEAD(&area->iface_list, iface, entry);
+                       /* ifaces from config belong to area */
+                       if ((iface->cflags & F_IFACE_CONFIGURED) &&
+                           (area = area_find(rdeconf, iface->area_id)) != NULL)
+                               LIST_INSERT_HEAD(&area->iface_list, iface,
+                                   entry);
                        break;
                case IMSG_IFDELETE:
                        if (imsg.hdr.len != IMSG_HEADER_SIZE +
@@ -731,9 +746,8 @@ rde_dispatch_parent(int fd, short event,
                        memcpy(&ifindex, imsg.data, sizeof(ifindex));
                        iface = if_find(ifindex);
                        if (iface == NULL)
-                               fatalx("interface lost in ospfe");
+                               fatalx("IFDELETE interface lost in rde");
 
-                       LIST_REMOVE(iface, entry);
                        if_del(iface);
                        break;
                case IMSG_IFADDRNEW:

Reply via email to