From e2ae08694e45b2a127c9d741e41dee4b14c2964d Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 28 Nov 2018 16:43:17 +0100 Subject: [PATCH] Nest: Do not hard-reset interface when preferred address is changed Modify protocols to use preferred address change notification instead on depending on hard-reset of interfaces in that case, and remove hard-reset in that case. This avoids issue when e.g. IPv6 protocol restarts interface when IPv4 preferred address changed (as hard-reset is unavoidable and common for whole iface). The patch also fixes a bug when removing last address does not send preferred address change notification. --- nest/iface.c | 23 ++++++++++------------- nest/iface.h | 12 ++++++++---- proto/babel/babel.c | 43 +++++++++++++++++++++++++++++-------------- proto/radv/radv.c | 18 +++++++++--------- proto/rip/rip.c | 19 +++++++++---------- 5 files changed, 65 insertions(+), 50 deletions(-) diff --git a/nest/iface.c b/nest/iface.c index 23a82ac5..c1966ac6 100644 --- a/nest/iface.c +++ b/nest/iface.c @@ -526,32 +526,28 @@ if_recalc_preferred(struct iface *i) } } - if (a4 != i->addr4) + if ((a4 != i->addr4) || (i->flags & IF_LOST_ADDR4)) { if_set_preferred(&i->addr4, a4); change |= IF_CHANGE_ADDR4; } - if (a6 != i->addr6) + if ((a6 != i->addr6) || (i->flags & IF_LOST_ADDR6)) { if_set_preferred(&i->addr6, a6); change |= IF_CHANGE_ADDR6; } - if (ll != i->llv6) + if ((ll != i->llv6) || (i->flags & IF_LOST_LLV6)) { if_set_preferred(&i->llv6, ll); change |= IF_CHANGE_LLV6; } - i->flags &= ~IF_NEEDS_RECALC; + i->flags &= ~(IF_NEEDS_RECALC | IF_LOST_ADDR4 | IF_LOST_ADDR6 | IF_LOST_LLV6); - /* - * FIXME: There should be proper notification instead of iface restart: - * if_notify_change(change, i) - */ if (change) - if_change_flags(i, i->flags | IF_TMP_DOWN); + if_notify_change(change, i); } void @@ -643,11 +639,12 @@ ifa_delete(struct ifa *a) * We unlink deleted preferred address and mark for recalculation. * FIXME: This could break if we make iface scan non-atomic, as * protocols still could use the freed address until they get - * if_notify from preferred route recalculation. + * if_notify from preferred route recalculation. We should fix and + * simplify this in the future by having struct ifa refcounted */ - if (b == i->addr4) i->addr4 = NULL; - if (b == i->addr6) i->addr6 = NULL; - if (b == i->llv6) i->llv6 = NULL; + if (b == i->addr4) { i->addr4 = NULL; i->flags |= IF_LOST_ADDR4; } + if (b == i->addr6) { i->addr6 = NULL; i->flags |= IF_LOST_ADDR6; } + if (b == i->llv6) { i->llv6 = NULL; i->flags |= IF_LOST_LLV6; } i->flags |= IF_NEEDS_RECALC; } diff --git a/nest/iface.h b/nest/iface.h index bc241501..0eb277cd 100644 --- a/nest/iface.h +++ b/nest/iface.h @@ -73,12 +73,15 @@ struct iface { */ -#define IF_JUST_CREATED 0x10000000 /* Send creation event as soon as possible */ -#define IF_TMP_DOWN 0x20000000 /* Temporary shutdown due to interface reconfiguration */ -#define IF_UPDATED 0x40000000 /* Iface touched in last scan */ +#define IF_JUST_CREATED 0x10000000 /* Send creation event as soon as possible */ +#define IF_TMP_DOWN 0x20000000 /* Temporary shutdown due to interface reconfiguration */ +#define IF_UPDATED 0x40000000 /* Iface touched in last scan */ #define IF_NEEDS_RECALC 0x80000000 /* Preferred address recalculation is needed */ +#define IF_LOST_ADDR4 0x01000000 /* Preferred address was deleted, notification needed */ +#define IF_LOST_ADDR6 0x02000000 +#define IF_LOST_LLV6 0x04000000 -#define IA_UPDATED IF_UPDATED /* Address touched in last scan */ +#define IA_UPDATED IF_UPDATED /* Address touched in last scan */ /* Interface change events */ @@ -93,6 +96,7 @@ struct iface { #define IF_CHANGE_SYSDEP 0x800 #define IF_CHANGE_TOO_MUCH 0x40000000 /* Used internally */ +#define IF_CHANGE_UPDOWN (IF_CHANGE_UP | IF_CHANGE_DOWN) #define IF_CHANGE_PREFERRED (IF_CHANGE_ADDR4 | IF_CHANGE_ADDR6 | IF_CHANGE_LLV6) void if_init(void); diff --git a/proto/babel/babel.c b/proto/babel/babel.c index 12c9a474..b4e42a9a 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -846,8 +846,7 @@ babel_send_route_request(struct babel_proto *p, struct babel_entry *e, struct ba { union babel_msg msg = {}; - TRACE(D_PACKETS, "Sending route request for %N to %I", - e->n.addr, n->addr); + TRACE(D_PACKETS, "Sending route request for %N to %I", e->n.addr, n->addr); msg.type = BABEL_TLV_ROUTE_REQUEST; net_copy(&msg.route_request.net, e->n.addr); @@ -861,8 +860,7 @@ babel_send_wildcard_request(struct babel_iface *ifa) struct babel_proto *p = ifa->proto; union babel_msg msg = {}; - TRACE(D_PACKETS, "Sending wildcard route request on %s", - ifa->ifname); + TRACE(D_PACKETS, "Sending wildcard route request on %s", ifa->ifname); msg.type = BABEL_TLV_ROUTE_REQUEST; msg.route_request.full = 1; @@ -1515,6 +1513,21 @@ babel_iface_update_state(struct babel_iface *ifa) babel_iface_stop(ifa); } +static void +babel_iface_update_addr4(struct babel_iface *ifa) +{ + struct babel_proto *p = ifa->proto; + + ip_addr addr4 = ifa->iface->addr4 ? ifa->iface->addr4->ip : IPA_NONE; + ifa->next_hop_ip4 = ipa_nonzero(ifa->cf->next_hop_ip4) ? ifa->cf->next_hop_ip4 : addr4; + + if (ipa_zero(ifa->next_hop_ip4) && p->ip4_channel) + log(L_WARN "%s: Missing IPv4 next hop address for %s", p->p.name, ifa->ifname); + + if (ifa->up) + babel_iface_kick_timer(ifa); +} + static void babel_iface_update_buffers(struct babel_iface *ifa) { @@ -1624,15 +1637,21 @@ babel_if_notify(struct proto *P, unsigned flags, struct iface *iface) { struct babel_proto *p = (void *) P; struct babel_config *cf = (void *) P->cf; + struct babel_iface *ifa = babel_find_iface(p, iface); if (iface->flags & IF_IGNORE) return; - if (flags & IF_CHANGE_UP) + /* Add, remove or restart interface */ + if (flags & (IF_CHANGE_UPDOWN | IF_CHANGE_LLV6)) { - struct babel_iface_config *ic = (void *) iface_patt_find(&cf->iface_list, iface, NULL); + if (ifa) + babel_remove_iface(p, ifa); - /* we only speak multicast */ + if (!(iface->flags & IF_UP)) + return; + + /* We only speak multicast */ if (!(iface->flags & IF_MULTICAST)) return; @@ -1640,22 +1659,18 @@ babel_if_notify(struct proto *P, unsigned flags, struct iface *iface) if (!iface->llv6) return; + struct babel_iface_config *ic = (void *) iface_patt_find(&cf->iface_list, iface, NULL); if (ic) babel_add_iface(p, iface, ic); return; } - struct babel_iface *ifa = babel_find_iface(p, iface); - if (!ifa) return; - if (flags & IF_CHANGE_DOWN) - { - babel_remove_iface(p, ifa); - return; - } + if (flags & IF_CHANGE_ADDR4) + babel_iface_update_addr4(ifa); if (flags & IF_CHANGE_MTU) babel_iface_update_buffers(ifa); diff --git a/proto/radv/radv.c b/proto/radv/radv.c index a381f737..12d90823 100644 --- a/proto/radv/radv.c +++ b/proto/radv/radv.c @@ -330,13 +330,19 @@ radv_if_notify(struct proto *P, unsigned flags, struct iface *iface) { struct radv_proto *p = (struct radv_proto *) P; struct radv_config *cf = (struct radv_config *) (P->cf); + struct radv_iface *ifa = radv_iface_find(p, iface); if (iface->flags & IF_IGNORE) return; - if (flags & IF_CHANGE_UP) + /* Add, remove or restart interface */ + if (flags & (IF_CHANGE_UPDOWN | IF_CHANGE_LLV6)) { - struct radv_iface_config *ic = (void *) iface_patt_find(&cf->patt_list, iface, NULL); + if (ifa) + radv_iface_remove(ifa); + + if (!(iface->flags & IF_UP)) + return; /* Ignore non-multicast ifaces */ if (!(iface->flags & IF_MULTICAST)) @@ -346,22 +352,16 @@ radv_if_notify(struct proto *P, unsigned flags, struct iface *iface) if (!iface->llv6) return; + struct radv_iface_config *ic = (void *) iface_patt_find(&cf->patt_list, iface, NULL); if (ic) radv_iface_new(p, iface, ic); return; } - struct radv_iface *ifa = radv_iface_find(p, iface); if (!ifa) return; - if (flags & IF_CHANGE_DOWN) - { - radv_iface_remove(ifa); - return; - } - if ((flags & IF_CHANGE_LINK) && (iface->flags & IF_LINK_UP)) radv_iface_notify(ifa, RA_EV_INIT); } diff --git a/proto/rip/rip.c b/proto/rip/rip.c index 2838d425..612b6f92 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -746,35 +746,34 @@ rip_if_notify(struct proto *P, unsigned flags, struct iface *iface) { struct rip_proto *p = (void *) P; struct rip_config *cf = (void *) P->cf; + struct rip_iface *ifa = rip_find_iface(p, iface); if (iface->flags & IF_IGNORE) return; - if (flags & IF_CHANGE_UP) + /* Add, remove or restart interface */ + if (flags & (IF_CHANGE_UPDOWN | (rip_is_v2(p) ? IF_CHANGE_ADDR4 : IF_CHANGE_LLV6))) { - struct rip_iface_config *ic = (void *) iface_patt_find(&cf->patt_list, iface, NULL); + if (ifa) + rip_remove_iface(p, ifa); + + if (!(iface->flags & IF_UP)) + return; /* Ignore ifaces without appropriate address */ if (rip_is_v2(p) ? !iface->addr4 : !iface->llv6) return; + struct rip_iface_config *ic = (void *) iface_patt_find(&cf->patt_list, iface, NULL); if (ic) rip_add_iface(p, iface, ic); return; } - struct rip_iface *ifa = rip_find_iface(p, iface); - if (!ifa) return; - if (flags & IF_CHANGE_DOWN) - { - rip_remove_iface(p, ifa); - return; - } - if (flags & IF_CHANGE_MTU) rip_iface_update_buffers(ifa);