When one is connected to a network, suspends or hibernates, moves to a different network and wakes up one ends up with ip addresses from both networks and things probably go sideways. There is a good chance that source address selection picks the wrong IP.
One common suggestion is that slaacd should just delete old addresses when it sees a new prefix announced. I'm afraid that's too simplistic. I'm aware of networks where two routers announce two different prefixes. We would flip flop between them. claudio@ recently suggested to delete addresses on link state change. I'm kinda worried that there might be still scenarios where we would flip flop between prefixes. So I hit the literature, but I couldn't find anything in the RFCs or in drafts on how to handle this. The stance in the RFCs is, if pltime > 0 the address is good, that's it. I was toying with the idea to solve this in the kernel, namely in the address selection algorithm and found this in RFC 6724: 2. Added Rule 5.5 to allow choosing a source address from a prefix advertised by the chosen next-hop for a given destination. This allows better connectivity in the presence of BCP 38 [RFC2827] ingress filtering and egress filtering. Previously, RFC 3484 had issues with multiple egress networks reached via the same interface, as discussed in [RFC5220]. But that seems difficult to implement. We don't track the router for the prefix in the kernel. I note that my jesus laptop deletes old addresses. But I have no idea what heuristic they employ. So, how about we steer the address selection away from old addresses on link state change? If the link goes down deprecate all addresses by setting the pltime to 0. The addresses are still valid, but will not be used for new connections. When the link comes back up we send a solicitation. If we are in a new network we get new addresses, the old addresses are deprecated and will no longer be used. Upper layers should take care of terminating existing connections with the old addresses. Thoughts? Tests? OKs? p.s. if someone knows of some documentation (RFC, draft, some writing what other implementations do) I'd be very interested. diff --git engine.c engine.c index 961e1b115b6..b035165e4e5 100644 --- engine.c +++ engine.c @@ -211,6 +211,7 @@ struct slaacd_iface { struct ether_addr hw_address; struct sockaddr_in6 ll_address; uint8_t soiikey[SLAACD_SOIIKEY_LEN]; + int link_state; LIST_HEAD(, radv) radvs; LIST_HEAD(, address_proposal) addr_proposals; LIST_HEAD(, dfr_proposal) dfr_proposals; @@ -227,6 +228,7 @@ void send_interface_info(struct slaacd_iface *, pid_t); void engine_showinfo_ctl(struct imsg *, uint32_t); void debug_log_ra(struct imsg_ra *); int in6_mask2prefixlen(struct in6_addr *); +void deprecate_all_proposals(struct slaacd_iface *); #endif /* SMALL */ struct slaacd_iface *get_slaacd_iface_by_id(uint32_t); void remove_slaacd_iface(uint32_t); @@ -574,6 +576,7 @@ engine_dispatch_main(int fd, short event, void *bula) int shut = 0; #ifndef SMALL struct imsg_addrinfo imsg_addrinfo; + struct imsg_link_state imsg_link_state; struct address_proposal *addr_proposal = NULL; size_t i; #endif /* SMALL */ @@ -772,6 +775,27 @@ engine_dispatch_main(int fd, short event, void *bula) LIST_INSERT_HEAD(&iface->addr_proposals, addr_proposal, entries); + break; + case IMSG_UPDATE_LINK_STATE: + if (imsg.hdr.len != IMSG_HEADER_SIZE + + sizeof(imsg_link_state)) + fatal("%s: IMSG_UPDATE_LINK_STATE wrong " + "length: %d", __func__, imsg.hdr.len); + + memcpy(&imsg_link_state, imsg.data, + sizeof(imsg_link_state)); + + iface = get_slaacd_iface_by_id( + imsg_link_state.if_index); + if (iface == NULL) + break; + if (iface->link_state != imsg_link_state.link_state) { + iface->link_state = imsg_link_state.link_state; + if (iface->link_state == LINK_STATE_DOWN) + deprecate_all_proposals(iface); + else + start_probe(iface); + } break; #endif /* SMALL */ default: @@ -953,6 +977,19 @@ engine_showinfo_ctl(struct imsg *imsg, uint32_t if_index) break; } } +void +deprecate_all_proposals(struct slaacd_iface *iface) +{ + struct address_proposal *addr_proposal; + + log_debug("%s: iface: %d", __func__, iface->if_index); + + LIST_FOREACH (addr_proposal, &iface->addr_proposals, entries) { + addr_proposal->pltime = 0; + configure_address(addr_proposal); + addr_proposal->state = PROPOSAL_NEARLY_EXPIRED; + } +} #endif /* SMALL */ struct slaacd_iface* diff --git frontend.c frontend.c index 96d69b8b661..6be2d504f12 100644 --- frontend.c +++ frontend.c @@ -507,6 +507,7 @@ update_autoconf_addresses(uint32_t if_index, char* if_name) struct ifaddrs *ifap, *ifa; struct in6_addrlifetime *lifetime; struct sockaddr_in6 *sin6; + struct imsg_link_state imsg_link_state; time_t t; int xflags; @@ -520,12 +521,20 @@ update_autoconf_addresses(uint32_t if_index, char* if_name) get_lladdr(if_name, &imsg_addrinfo.hw_address, &imsg_addrinfo.ll_address); + memset(&imsg_link_state, 0, sizeof(imsg_link_state)); + imsg_link_state.if_index = if_index; + if (getifaddrs(&ifap) != 0) fatal("getifaddrs"); for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) { if (strcmp(if_name, ifa->ifa_name) != 0) continue; + + if (ifa->ifa_addr->sa_family == AF_LINK) + imsg_link_state.link_state = + ((struct if_data *)ifa->ifa_data)->ifi_link_state; + if (ifa->ifa_addr->sa_family != AF_INET6) continue; sin6 = (struct sockaddr_in6 *)ifa->ifa_addr; @@ -591,6 +600,12 @@ update_autoconf_addresses(uint32_t if_index, char* if_name) } freeifaddrs(ifap); + + log_debug("%s: %s link state down? %s", __func__, if_name, + imsg_link_state.link_state == LINK_STATE_DOWN ? "yes" : "no"); + + frontend_imsg_compose_main(IMSG_UPDATE_LINK_STATE, 0, + &imsg_link_state, sizeof(imsg_link_state)); } #endif /* SMALL */ diff --git slaacd.c slaacd.c index 6778659fa10..0e4f8d38148 100644 --- slaacd.c +++ slaacd.c @@ -393,6 +393,7 @@ main_dispatch_frontend(int fd, short event, void *bula) int shut = 0; #ifndef SMALL struct imsg_addrinfo imsg_addrinfo; + struct imsg_link_state imsg_link_state; int verbose; #endif /* SMALL */ @@ -438,6 +439,16 @@ main_dispatch_frontend(int fd, short event, void *bula) main_imsg_compose_engine(IMSG_UPDATE_ADDRESS, 0, &imsg_addrinfo, sizeof(imsg_addrinfo)); break; + case IMSG_UPDATE_LINK_STATE: + if (imsg.hdr.len != IMSG_HEADER_SIZE + + sizeof(imsg_link_state)) + fatal("%s: IMSG_UPDATE_LINK_STATE wrong " + "length: %d", __func__, imsg.hdr.len); + memcpy(&imsg_link_state, imsg.data, + sizeof(imsg_link_state)); + main_imsg_compose_engine(IMSG_UPDATE_LINK_STATE, 0, + &imsg_link_state, sizeof(imsg_link_state)); + break; #endif /* SMALL */ case IMSG_UPDATE_IF: if (imsg.hdr.len != IMSG_HEADER_SIZE + diff --git slaacd.h slaacd.h index 910be91e687..8ffd42ad6c0 100644 --- slaacd.h +++ slaacd.h @@ -55,6 +55,7 @@ enum imsg_type { IMSG_CTL_SHOW_INTERFACE_INFO_DFR_PROPOSAL, IMSG_CTL_END, IMSG_UPDATE_ADDRESS, + IMSG_UPDATE_LINK_STATE, #endif /* SMALL */ IMSG_CTL_SEND_SOLICITATION, IMSG_SOCKET_IPC, @@ -167,6 +168,11 @@ struct imsg_addrinfo { uint32_t vltime; uint32_t pltime; }; + +struct imsg_link_state { + uint32_t if_index; + int link_state; +}; #endif /* SMALL */ struct imsg_ifinfo { -- I'm not entirely sure you are real.