From f097f7659c7ff226a53c51673158e32fb69a6d21 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Mon, 9 Oct 2017 20:36:14 +0200 Subject: [PATCH] OSPF: Fix next hop calculation for PtP links in IPv4 OSPFv3-AF In such case, next hop has to be taken from Link-LSA like in broadcast case, not from neighbor source address like in other PtP cases. Also add some checks, comments and code cleanup. --- proto/ospf/rt.c | 115 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 82 insertions(+), 33 deletions(-) diff --git a/proto/ospf/rt.c b/proto/ospf/rt.c index f57925c3..b289d767 100644 --- a/proto/ospf/rt.c +++ b/proto/ospf/rt.c @@ -10,9 +10,7 @@ #include "ospf.h" -static void add_cand(list * l, struct top_hash_entry *en, - struct top_hash_entry *par, u32 dist, - struct ospf_area *oa, int i); +static void add_cand(struct ospf_area *oa, struct top_hash_entry *en, struct top_hash_entry *par, u32 dist, int i, uint lif, uint nif); static void rt_sync(struct ospf_proto *p); @@ -509,7 +507,7 @@ spfa_process_rt(struct ospf_proto *p, struct ospf_area *oa, struct top_hash_entr break; } - add_cand(&oa->cand, tmp, act, act->dist + rtl.metric, oa, i); + add_cand(oa, tmp, act, act->dist + rtl.metric, i, rtl.lif, rtl.nif); } } @@ -532,7 +530,7 @@ spfa_process_net(struct ospf_proto *p, struct ospf_area *oa, struct top_hash_ent for (i = 0; i < cnt; i++) { tmp = ospf_hash_find_rt(p->gr, oa->areaid, ln->routers[i]); - add_cand(&oa->cand, tmp, act, act->dist, oa, -1); + add_cand(oa, tmp, act, act->dist, -1, 0, 0); } } @@ -651,7 +649,8 @@ ospf_rt_spfa(struct ospf_area *oa) } static int -link_back(struct ospf_area *oa, struct top_hash_entry *en, struct top_hash_entry *par) +link_back(struct ospf_area *oa, struct top_hash_entry *en, + struct top_hash_entry *par, uint lif, uint nif) { struct ospf_proto *p = oa->po; struct ospf_lsa_rt_walk rtl; @@ -689,6 +688,10 @@ link_back(struct ospf_area *oa, struct top_hash_entry *en, struct top_hash_entry tmp = ospf_hash_find_net(p->gr, oa->areaid, rtl.id, rtl.nif); if (tmp == par) { + /* + * Note that there may be multiple matching Rt-fields if router 'en' + * have multiple interfaces to net 'par'. Perhaps we should do ECMP. + */ if (ospf_is_v2(p)) en->lb = ipa_from_u32(rtl.data); else @@ -700,7 +703,13 @@ link_back(struct ospf_area *oa, struct top_hash_entry *en, struct top_hash_entry case LSART_VLNK: case LSART_PTP: - /* Not necessary the same link, see RFC 2328 [23] */ + /* + * For OSPFv2, not necessary the same link, see RFC 2328 [23]. + * For OSPFv3, we verify that by comparing nif and lif fields. + */ + if (ospf_is_v3(p) && ((rtl.lif != nif) || (rtl.nif != lif))) + break; + tmp = ospf_hash_find_rt(p->gr, oa->areaid, rtl.id); if (tmp == par) return 1; @@ -1679,13 +1688,27 @@ inherit_nexthops(struct nexthop *pn) return pn && (ipa_nonzero(pn->gw) || !pn->iface); } +static inline ip_addr +link_lsa_lladdr(struct ospf_proto *p, struct top_hash_entry *en) +{ + struct ospf_lsa_link *link_lsa = en->lsa_body; + ip6_addr ll = link_lsa->lladdr; + + if (ip6_zero(ll)) + return IPA_NONE; + + return ospf_is_ip4(p) ? ipa_from_ip4(ospf3_6to4(ll)) : ipa_from_ip6(ll); +} + static struct nexthop * calc_next_hop(struct ospf_area *oa, struct top_hash_entry *en, - struct top_hash_entry *par, int pos) + struct top_hash_entry *par, int pos, uint lif, uint nif) { struct ospf_proto *p = oa->po; struct nexthop *pn = par->nhs; - struct ospf_iface *ifa; + struct top_hash_entry *link = NULL; + struct ospf_iface *ifa = NULL; + ip_addr nh = IPA_NONE; u32 rid = en->lsa.rt; /* 16.1.1. The next hop calculation */ @@ -1710,6 +1733,9 @@ calc_next_hop(struct ospf_area *oa, struct top_hash_entry *en, if (!ifa) return NULL; + if (ospf_is_v3(p) && (ifa->iface_id != lif)) + log(L_WARN "%s: Inconsistent interface ID %u/%u", p->p.name, ifa->iface_id, lif); + return new_nexthop(p, IPA_NONE, ifa->iface, ifa->ecmp_weight); } @@ -1720,14 +1746,44 @@ calc_next_hop(struct ospf_area *oa, struct top_hash_entry *en, if (!ifa) return NULL; + if (ospf_is_v3(p) && (ifa->iface_id != lif)) + log(L_WARN "%s: Inconsistent interface ID %u/%u", p->p.name, ifa->iface_id, lif); + if (ifa->type == OSPF_IT_VLINK) return new_nexthop(p, IPA_NONE, NULL, 0); - struct ospf_neighbor *m = find_neigh(ifa, rid); - if (!m || (m->state != NEIGHBOR_FULL)) - return NULL; + /* FIXME: On physical PtP links we may skip next-hop altogether */ - return new_nexthop(p, m->ip, ifa->iface, ifa->ecmp_weight); + if (ospf_is_v2(p) || ospf_is_ip6(p)) + { + /* + * In this case, next-hop is a source address from neighbor's packets. + * That is necessary for OSPFv2 and practical for OSPFv3 (as it works even + * if neighbor uses LinkLSASuppression), but does not work with OSPFv3-AF + * on IPv4 topology, where src is IPv6 but next-hop should be IPv4. + */ + struct ospf_neighbor *m = find_neigh(ifa, rid); + if (!m || (m->state != NEIGHBOR_FULL)) + return NULL; + + nh = m->ip; + } + else + { + /* + * Next-hop is taken from lladdr field of Link-LSA, based on Neighbor + * Iface ID (nif) field in our Router-LSA, which is just nbr->iface_id. + */ + link = ospf_hash_find(p->gr, ifa->iface_id, nif, rid, LSA_T_LINK); + if (!link) + return NULL; + + nh = link_lsa_lladdr(p, link); + if (ipa_zero(nh)) + return NULL; + } + + return new_nexthop(p, nh, ifa->iface, ifa->ecmp_weight); } /* The third case - bcast or nbma neighbor */ @@ -1754,21 +1810,14 @@ calc_next_hop(struct ospf_area *oa, struct top_hash_entry *en, * Next-hop is taken from lladdr field of Link-LSA, en->lb_id * is computed in link_back(). */ - struct top_hash_entry *lhe; - lhe = ospf_hash_find(p->gr, pn->iface->index, en->lb_id, rid, LSA_T_LINK); - - if (!lhe) + link = ospf_hash_find(p->gr, pn->iface->index, en->lb_id, rid, LSA_T_LINK); + if (!link) return NULL; - struct ospf_lsa_link *llsa = lhe->lsa_body; - - if (ip6_zero(llsa->lladdr)) + nh = link_lsa_lladdr(p, link); + if (ipa_zero(nh)) return NULL; - ip_addr nh = ospf_is_ip4(p) ? - ipa_from_ip4(ospf3_6to4(llsa->lladdr)) : - ipa_from_ip6(llsa->lladdr); - return new_nexthop(p, nh, pn->iface, pn->weight); } } @@ -1782,8 +1831,8 @@ calc_next_hop(struct ospf_area *oa, struct top_hash_entry *en, /* Add LSA into list of candidates in Dijkstra's algorithm */ static void -add_cand(list * l, struct top_hash_entry *en, struct top_hash_entry *par, - u32 dist, struct ospf_area *oa, int pos) +add_cand(struct ospf_area *oa, struct top_hash_entry *en, struct top_hash_entry *par, + u32 dist, int pos, uint lif, uint nif) { struct ospf_proto *p = oa->po; node *prev, *n; @@ -1813,10 +1862,10 @@ add_cand(list * l, struct top_hash_entry *en, struct top_hash_entry *par, return; /* We should check whether there is a reverse link from en to par, */ - if (!link_back(oa, en, par)) + if (!link_back(oa, en, par, lif, nif)) return; - struct nexthop *nhs = calc_next_hop(oa, en, par, pos); + struct nexthop *nhs = calc_next_hop(oa, en, par, pos, lif, nif); if (!nhs) { log(L_WARN "%s: Cannot find next hop for LSA (Type: %04x, Id: %R, Rt: %R)", @@ -1873,20 +1922,20 @@ add_cand(list * l, struct top_hash_entry *en, struct top_hash_entry *par, prev = NULL; - if (EMPTY_LIST(*l)) + if (EMPTY_LIST(oa->cand)) { - add_head(l, &en->cn); + add_head(&oa->cand, &en->cn); } else { - WALK_LIST(n, *l) + WALK_LIST(n, oa->cand) { act = SKIP_BACK(struct top_hash_entry, cn, n); if ((act->dist > dist) || ((act->dist == dist) && (act->lsa_type == LSA_T_RT))) { if (prev == NULL) - add_head(l, &en->cn); + add_head(&oa->cand, &en->cn); else insert_node(&en->cn, prev); added = 1; @@ -1897,7 +1946,7 @@ add_cand(list * l, struct top_hash_entry *en, struct top_hash_entry *par, if (!added) { - add_tail(l, &en->cn); + add_tail(&oa->cand, &en->cn); } } }