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: