From: florian Date: Wed, 17 Mar 2021 15:24:04 +0000 (+0000) Subject: Split off init_ifaces from update_iface. init_ifaces discovers the X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=1bbf741cd250344ed5c1a069b3c7e46ee0536216;p=openbsd Split off init_ifaces from update_iface. init_ifaces discovers the state of the machine on startup using ioctl(2) and getifaddrs(3). We can then update this state with information provided by route messages. We still need getifaddrs(3) to check if the layer 2 address has changed. This simplifies error handling (what should we do if ioctl(2) fails?), reduces kernel round trips (no need to ask the kernel again for information RTM_IFINFO provided already) and prevents a theoretical race between RTM_IFINFO and getaddrinfo(3). In a fast link state UP -> DOWN -> UP transition RTM_IFINFO informs us that the link went down but we were not using this information but rather looked at getifaddrs(3) information which might see the link as already up again. We would then do nothing while we should try to get a new lease. By storing all interface information in the frontend process we can skip imsgs to the engine process if we get an RTM_IFINFO without relevant changes for us. --- diff --git a/sbin/dhcpleased/frontend.c b/sbin/dhcpleased/frontend.c index fbbeb63fbb5..57e60187b92 100644 --- a/sbin/dhcpleased/frontend.c +++ b/sbin/dhcpleased/frontend.c @@ -1,4 +1,4 @@ -/* $OpenBSD: frontend.c,v 1.5 2021/03/16 17:39:15 florian Exp $ */ +/* $OpenBSD: frontend.c,v 1.6 2021/03/17 15:24:04 florian Exp $ */ /* * Copyright (c) 2017, 2021 Florian Obser @@ -66,9 +66,7 @@ struct bpf_ev { struct iface { LIST_ENTRY(iface) entries; struct bpf_ev bpfev; - struct ether_addr hw_address; - uint32_t if_index; - int rdomain; + struct imsg_ifinfo ifinfo; int send_discover; uint32_t xid; struct in_addr requested_ip; @@ -79,8 +77,9 @@ struct iface { __dead void frontend_shutdown(void); void frontend_sig_handler(int, short, void *); -void update_iface(uint32_t, char*); +void update_iface(struct if_msghdr *); void frontend_startup(void); +void init_ifaces(void); void route_receive(int, short, void *); void handle_route_message(struct rt_msghdr *, struct sockaddr **); void get_rtaddrs(int, struct sockaddr *, struct sockaddr **); @@ -463,26 +462,49 @@ get_xflags(char *if_name) } void -update_iface(uint32_t if_index, char* if_name) +update_iface(struct if_msghdr *ifm) { struct iface *iface; - struct imsg_ifinfo imsg_ifinfo; + struct imsg_ifinfo ifinfo; struct ifaddrs *ifap, *ifa; - struct sockaddr_dl *sdl; - int flags, xflags; + uint32_t if_index; + int flags, xflags, found = 0; + char ifnamebuf[IF_NAMESIZE], *if_name; - if ((flags = get_flags(if_name)) == -1 || (xflags = - get_xflags(if_name)) == -1) - return; + if_index = ifm->ifm_index; + + flags = ifm->ifm_flags; + xflags = ifm->ifm_xflags; - if (!(xflags & IFXF_AUTOCONF4)) + iface = get_iface_by_id(if_index); + if_name = if_indextoname(if_index, ifnamebuf); + + if (if_name == NULL) { + if (iface != NULL) { + log_debug("interface with idx %d removed", if_index); + frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0, + &if_index, sizeof(if_index)); + remove_iface(if_index); + } return; + } - memset(&imsg_ifinfo, 0, sizeof(imsg_ifinfo)); + if (!(xflags & IFXF_AUTOCONF4)) { + if (iface != NULL) { + log_info("Removed autoconf flag from %s", if_name); + frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0, + &if_index, sizeof(if_index)); + remove_iface(if_index); + } + return; + } - imsg_ifinfo.if_index = if_index; - imsg_ifinfo.running = (flags & (IFF_UP | IFF_RUNNING)) == (IFF_UP | - IFF_RUNNING); + memset(&ifinfo, 0, sizeof(ifinfo)); + ifinfo.if_index = if_index; + ifinfo.link_state = ifm->ifm_data.ifi_link_state; + ifinfo.rdomain = ifm->ifm_tableid; + ifinfo.running = (flags & (IFF_UP | IFF_RUNNING)) == + (IFF_UP | IFF_RUNNING); if (getifaddrs(&ifap) != 0) fatal("getifaddrs"); @@ -495,17 +517,15 @@ update_iface(uint32_t if_index, char* if_name) switch(ifa->ifa_addr->sa_family) { case AF_LINK: { - struct if_data *if_data; + struct sockaddr_dl *sdl; - if_data = (struct if_data *)ifa->ifa_data; - imsg_ifinfo.link_state = if_data->ifi_link_state; - imsg_ifinfo.rdomain = if_data->ifi_rdomain; sdl = (struct sockaddr_dl *)ifa->ifa_addr; if (sdl->sdl_type != IFT_ETHER || sdl->sdl_alen != ETHER_ADDR_LEN) continue; - memcpy(imsg_ifinfo.hw_address.ether_addr_octet, - LLADDR(sdl), ETHER_ADDR_LEN); + memcpy(ifinfo.hw_address.ether_addr_octet, LLADDR(sdl), + ETHER_ADDR_LEN); + found = 1; goto out; } default: @@ -515,52 +535,127 @@ update_iface(uint32_t if_index, char* if_name) out: freeifaddrs(ifap); - iface = get_iface_by_id(if_index); - - if (iface != NULL) { - if (iface->rdomain != imsg_ifinfo.rdomain) { - iface->rdomain = imsg_ifinfo.rdomain; - if (iface->udpsock != -1) { - close(iface->udpsock); - iface->udpsock = -1; - } + if (!found) { + log_warnx("Could not find AF_LINK address for %s.", if_name); + if (iface != NULL) { + frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0, + &if_index, sizeof(if_index)); + remove_iface(if_index); } - } else { + return; + } + + if (iface == NULL) { if ((iface = calloc(1, sizeof(*iface))) == NULL) fatal("calloc"); - iface->if_index = if_index; - iface->rdomain = imsg_ifinfo.rdomain; iface->udpsock = -1; LIST_INSERT_HEAD(&interfaces, iface, entries); frontend_imsg_compose_main(IMSG_OPEN_BPFSOCK, 0, &if_index, sizeof(if_index)); + } else { + if (iface->ifinfo.rdomain != ifinfo.rdomain && + iface->udpsock != -1) { + close(iface->udpsock); + iface->udpsock = -1; + } } - memcpy(&iface->hw_address, &imsg_ifinfo.hw_address, - sizeof(iface->hw_address)); - - frontend_imsg_compose_main(IMSG_UPDATE_IF, 0, &imsg_ifinfo, - sizeof(imsg_ifinfo)); + if (memcmp(&iface->ifinfo, &ifinfo, sizeof(iface->ifinfo)) != 0) { + memcpy(&iface->ifinfo, &ifinfo, sizeof(iface->ifinfo)); + frontend_imsg_compose_main(IMSG_UPDATE_IF, 0, &iface->ifinfo, + sizeof(iface->ifinfo)); + } } void frontend_startup(void) { - struct if_nameindex *ifnidxp, *ifnidx; - if (!event_initialized(&ev_route)) fatalx("%s: did not receive a route socket from the main " "process", __func__); + init_ifaces(); event_add(&ev_route, NULL); +} + +void +init_ifaces(void) +{ + struct iface *iface; + struct imsg_ifinfo ifinfo; + struct if_nameindex *ifnidxp, *ifnidx; + struct ifaddrs *ifap, *ifa; + uint32_t if_index; + int flags, xflags; + char *if_name; if ((ifnidxp = if_nameindex()) == NULL) fatalx("if_nameindex"); - for(ifnidx = ifnidxp; ifnidx->if_index !=0 && ifnidx->if_name != NULL; - ifnidx++) - update_iface(ifnidx->if_index, ifnidx->if_name); + if (getifaddrs(&ifap) != 0) + fatal("getifaddrs"); + + for(ifnidx = ifnidxp; ifnidx->if_index != 0 && ifnidx->if_name != NULL; + ifnidx++) { + if_index = ifnidx->if_index; + if_name = ifnidx->if_name; + if ((flags = get_flags(if_name)) == -1) + continue; + if((xflags = get_xflags(if_name)) == -1) + continue; + if (!(xflags & IFXF_AUTOCONF4)) + continue; + + memset(&ifinfo, 0, sizeof(ifinfo)); + ifinfo.if_index = if_index; + ifinfo.link_state = -1; + ifinfo.running = (flags & (IFF_UP | IFF_RUNNING)) == + (IFF_UP | IFF_RUNNING); + + for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) { + if (strcmp(if_name, ifa->ifa_name) != 0) + continue; + if (ifa->ifa_addr == NULL) + continue; + + switch(ifa->ifa_addr->sa_family) { + case AF_LINK: { + struct if_data *if_data; + struct sockaddr_dl *sdl; + + sdl = (struct sockaddr_dl *)ifa->ifa_addr; + if (sdl->sdl_type != IFT_ETHER || + sdl->sdl_alen != ETHER_ADDR_LEN) + continue; + memcpy(ifinfo.hw_address.ether_addr_octet, + LLADDR(sdl), ETHER_ADDR_LEN); + + if_data = (struct if_data *)ifa->ifa_data; + ifinfo.link_state = if_data->ifi_link_state; + ifinfo.rdomain = if_data->ifi_rdomain; + goto out; + } + default: + break; + } + } + out: + if (ifinfo.link_state == -1) + /* no AF_LINK found */ + continue; + + if ((iface = calloc(1, sizeof(*iface))) == NULL) + fatal("calloc"); + iface->udpsock = -1; + memcpy(&iface->ifinfo, &ifinfo, sizeof(iface->ifinfo)); + LIST_INSERT_HEAD(&interfaces, iface, entries); + frontend_imsg_compose_main(IMSG_OPEN_BPFSOCK, 0, + &if_index, sizeof(if_index)); + frontend_imsg_compose_main(IMSG_UPDATE_IF, 0, &iface->ifinfo, + sizeof(iface->ifinfo)); + } + freeifaddrs(ifap); if_freenameindex(ifnidxp); } @@ -607,31 +702,11 @@ void handle_route_message(struct rt_msghdr *rtm, struct sockaddr **rti_info) { struct if_msghdr *ifm; - int xflags, if_index; - char ifnamebuf[IF_NAMESIZE], *if_name; switch (rtm->rtm_type) { case RTM_IFINFO: ifm = (struct if_msghdr *)rtm; - if_name = if_indextoname(ifm->ifm_index, ifnamebuf); - if (if_name == NULL) { - log_debug("RTM_IFINFO: lost if %d", ifm->ifm_index); - if_index = ifm->ifm_index; - frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0, - &if_index, sizeof(if_index)); - remove_iface(if_index); - } else { - xflags = get_xflags(if_name); - if (xflags == -1 || !(xflags & IFXF_AUTOCONF4)) { - log_debug("RTM_IFINFO: %s(%d) no(longer) " - "autoconf4", if_name, ifm->ifm_index); - if_index = ifm->ifm_index; - frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, - 0, &if_index, sizeof(if_index)); - } else { - update_iface(ifm->ifm_index, if_name); - } - } + update_iface(ifm); break; case RTM_PROPOSAL: if (rtm->rtm_priority == RTP_PROPOSAL_SOLICIT) { @@ -644,7 +719,6 @@ handle_route_message(struct rt_msghdr *rtm, struct sockaddr **rti_info) log_debug("unexpected RTM: %d", rtm->rtm_type); break; } - } #define ROUNDUP(a) \ @@ -685,7 +759,7 @@ bpf_receive(int fd, short events, void *arg) fatal("%s len == 0", __func__); memset(&imsg_dhcp, 0, sizeof(imsg_dhcp)); - imsg_dhcp.if_index = iface->if_index; + imsg_dhcp.if_index = iface->ifinfo.if_index; rem = len; p = iface->bpfev.buf; @@ -803,11 +877,11 @@ send_discover(struct iface *iface) } iface->send_discover = 0; - if_name = if_indextoname(iface->if_index, ifnamebuf); + if_name = if_indextoname(iface->ifinfo.if_index, ifnamebuf); log_debug("DHCPDISCOVER on %s", if_name == NULL ? "?" : if_name); - pkt_len = build_packet(DHCPDISCOVER, iface->xid, &iface->hw_address, - &iface->requested_ip, NULL); + pkt_len = build_packet(DHCPDISCOVER, iface->xid, + &iface->ifinfo.hw_address, &iface->requested_ip, NULL); bpf_send_packet(iface, dhcp_packet, pkt_len); } @@ -817,11 +891,12 @@ send_request(struct iface *iface) ssize_t pkt_len; char ifnamebuf[IF_NAMESIZE], *if_name; - if_name = if_indextoname(iface->if_index, ifnamebuf); + if_name = if_indextoname(iface->ifinfo.if_index, ifnamebuf); log_debug("DHCPREQUEST on %s", if_name == NULL ? "?" : if_name); - pkt_len = build_packet(DHCPREQUEST, iface->xid, &iface->hw_address, - &iface->requested_ip, &iface->server_identifier); + pkt_len = build_packet(DHCPREQUEST, iface->xid, + &iface->ifinfo.hw_address, &iface->requested_ip, + &iface->server_identifier); if (iface->dhcp_server.s_addr != INADDR_ANY) udp_send_packet(iface, dhcp_packet, pkt_len); else @@ -854,7 +929,8 @@ bpf_send_packet(struct iface *iface, uint8_t *packet, ssize_t len) int iovcnt = 0, i; memset(eh.ether_dhost, 0xff, sizeof(eh.ether_dhost)); - memcpy(eh.ether_shost, &iface->hw_address, sizeof(eh.ether_dhost)); + memcpy(eh.ether_shost, &iface->ifinfo.hw_address, + sizeof(eh.ether_dhost)); eh.ether_type = htons(ETHERTYPE_IP); iov[0].iov_base = &eh; iov[0].iov_len = sizeof(eh); @@ -912,7 +988,7 @@ get_iface_by_id(uint32_t if_index) struct iface *iface; LIST_FOREACH (iface, &interfaces, entries) { - if (iface->if_index == if_index) + if (iface->ifinfo.if_index == if_index) return (iface); }