On Mon, Oct 26, 2015 at 04:40:12PM +0100, Claudio Jeker wrote:
> ospfd has some issues with self-originated networks and building summary
> entries for those in case the router is an ABR (area border router).
> This diff should hopefully fix all of the troubles. It changes a bit the
> way we do nexthop calculation in the SPF/rib calculation to make sure we
> handle self-originated networks correctly. As a side-effect it should also
> remove the behaviour where ospfd added a OSPF route for all those
> self-originated routes. Also the way we track active areas is changed to
> be actually the way it should be.
>
> Please test this on all ospfd setups and check for issues.
I guess nobody is using ospfd anymore. Anyway here is an updated diff that
fixes an issue with stub networks announced by routers connected via a P2P
link. I'm running this on our ABR router at work now and plan to commit
this in the next days.
--
:wq Claudio
Index: area.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/area.c,v
retrieving revision 1.9
diff -u -p -r1.9 area.c
--- area.c 7 Jan 2009 21:16:36 -0000 1.9
+++ area.c 23 Oct 2015 14:22:24 -0000
@@ -94,19 +94,24 @@ area_find(struct ospfd_conf *conf, struc
}
void
-area_track(struct area *area, int state)
+area_track(struct area *area)
{
- int old = area->active;
+ int old = area->active;
+ struct iface *iface;
- if (state & NBR_STA_FULL)
- area->active++;
- else if (area->active == 0)
- fatalx("area_track: area already inactive");
- else
- area->active--;
+ area->active = 0;
+ LIST_FOREACH(iface, &area->iface_list, entry) {
+ if (iface->state & IF_STA_DOWN)
+ continue;
+ area->active = 1;
+ break;
+ }
- if (area->active == 0 || old == 0)
+ if (area->active != old) {
+ ospfe_imsg_compose_rde(IMSG_AREA_CHANGE, area->id.s_addr, 0,
+ &area->active, sizeof(area->active));
ospfe_demote_area(area, old == 0);
+ }
}
int
@@ -116,7 +121,7 @@ area_border_router(struct ospfd_conf *co
int active = 0;
LIST_FOREACH(area, &conf->area_list, entry)
- if (area->active > 0)
+ if (area->active)
active++;
return (active > 1);
Index: interface.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v
retrieving revision 1.79
diff -u -p -r1.79 interface.c
--- interface.c 27 Sep 2015 17:31:50 -0000 1.79
+++ interface.c 23 Oct 2015 14:22:24 -0000
@@ -136,8 +136,10 @@ if_fsm(struct iface *iface, enum iface_e
if (new_state != 0)
iface->state = new_state;
- if (iface->state != old_state)
+ if (iface->state != old_state) {
+ area_track(iface->area);
orig_rtr_lsa(iface->area);
+ }
if (old_state & (IF_STA_MULTI | IF_STA_POINTTOPOINT) &&
(iface->state & (IF_STA_MULTI | IF_STA_POINTTOPOINT)) == 0)
Index: neighbor.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/neighbor.c,v
retrieving revision 1.46
diff -u -p -r1.46 neighbor.c
--- neighbor.c 17 Jan 2013 10:07:56 -0000 1.46
+++ neighbor.c 23 Oct 2015 14:22:24 -0000
@@ -204,7 +204,6 @@ nbr_fsm(struct nbr *nbr, enum nbr_event
* neighbor changed from/to FULL
* originate new rtr and net LSA
*/
- area_track(nbr->iface->area, nbr->state);
orig_rtr_lsa(nbr->iface->area);
if (nbr->iface->state & IF_STA_DR)
orig_net_lsa(nbr->iface);
Index: ospfd.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
retrieving revision 1.92
diff -u -p -r1.92 ospfd.h
--- ospfd.h 27 Sep 2015 17:31:50 -0000 1.92
+++ ospfd.h 26 Oct 2015 13:37:36 -0000
@@ -104,6 +104,7 @@ enum imsg_type {
IMSG_NEIGHBOR_CAPA,
IMSG_NETWORK_ADD,
IMSG_NETWORK_DEL,
+ IMSG_AREA_CHANGE,
IMSG_DD,
IMSG_DD_END,
IMSG_DD_BADLSA,
@@ -499,6 +500,7 @@ struct ctl_rt {
enum dst_type d_type;
u_int8_t flags;
u_int8_t prefixlen;
+ u_int8_t connected;
};
struct ctl_sum {
@@ -530,7 +532,7 @@ struct demote_msg {
struct area *area_new(void);
int area_del(struct area *);
struct area *area_find(struct ospfd_conf *, struct in_addr);
-void area_track(struct area *, int);
+void area_track(struct area *);
int area_border_router(struct ospfd_conf *);
u_int8_t area_ospf_options(struct area *);
Index: ospfe.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfe.c,v
retrieving revision 1.91
diff -u -p -r1.91 ospfe.c
--- ospfe.c 27 Sep 2015 17:31:50 -0000 1.91
+++ ospfe.c 23 Oct 2015 14:22:24 -0000
@@ -996,9 +996,9 @@ orig_rtr_lsa(struct area *area)
oeconf->border = border;
orig_rtr_lsa_all(area);
}
-
if (oeconf->border)
lsa_rtr.flags |= OSPF_RTR_B;
+
/* TODO set V flag if a active virtual link ends here and the
* area is the transit area for this link. */
if (virtual)
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde.c,v
retrieving revision 1.97
diff -u -p -r1.97 rde.c
--- rde.c 14 Mar 2015 02:22:09 -0000 1.97
+++ rde.c 26 Oct 2015 15:30:35 -0000
@@ -298,11 +298,6 @@ rde_dispatch_imsg(int fd, short event, v
if (nbr == NULL)
break;
- if (state != nbr->state &&
- (nbr->state & NBR_STA_FULL ||
- state & NBR_STA_FULL))
- area_track(nbr->area, state);
-
nbr->state = state;
if (nbr->state & NBR_STA_FULL)
rde_req_list_free(nbr);
@@ -315,6 +310,19 @@ rde_dispatch_imsg(int fd, short event, v
break;
nbr->capa_options = *(u_int8_t *)imsg.data;
break;
+ case IMSG_AREA_CHANGE:
+ if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(state))
+ fatalx("invalid size of OE request");
+
+ LIST_FOREACH(area, &rdeconf->area_list, entry) {
+ if (area->id.s_addr == imsg.hdr.peerid)
+ break;
+ }
+ if (area == NULL)
+ break;
+ memcpy(&state, imsg.data, sizeof(state));
+ area->active = state;
+ break;
case IMSG_DB_SNAPSHOT:
nbr = rde_nbr_find(imsg.hdr.peerid);
if (nbr == NULL)
@@ -771,6 +779,9 @@ rde_send_change_kroute(struct rt_node *r
TAILQ_FOREACH(rn, &r->nexthop, entry) {
if (rn->invalid)
continue;
+ if (rn->connected)
+ /* skip self-originated routes */
+ continue;
krcount++;
bzero(&kr, sizeof(kr));
@@ -780,8 +791,12 @@ rde_send_change_kroute(struct rt_node *r
kr.ext_tag = r->ext_tag;
imsg_add(wbuf, &kr, sizeof(kr));
}
- if (krcount == 0)
- fatalx("rde_send_change_kroute: no valid nexthop found");
+ if (krcount == 0) {
+ /* no valid nexthop or self originated, so remove */
+ ibuf_free(wbuf);
+ rde_send_delete_kroute(r);
+ return;
+ }
imsg_close(&iev_main->ibuf, wbuf);
imsg_event_add(iev_main);
}
@@ -1352,6 +1367,9 @@ rde_summary_update(struct rt_node *rte,
/* first check if we actually need to announce this route */
if (!(rte->d_type == DT_NET || rte->flags & OSPF_RTR_E))
return;
+ /* route is invalid, lsa_remove_invalid_sums() will do the cleanup */
+ if (rte->cost >= LS_INFINITY)
+ return;
/* never create summaries for as-ext LSA */
if (rte->p_type == PT_TYPE1_EXT || rte->p_type == PT_TYPE2_EXT)
return;
@@ -1363,16 +1381,17 @@ rde_summary_update(struct rt_node *rte,
return;
/* nexthop check, nexthop part of area -> no summary */
TAILQ_FOREACH(rn, &rte->nexthop, entry) {
+ if (rn->invalid)
+ continue;
nr = rt_lookup(DT_NET, rn->nexthop.s_addr);
if (nr && nr->area.s_addr == area->id.s_addr)
continue;
break;
}
- if (rn == NULL) /* all nexthops belong to this area */
+ if (rn == NULL)
+ /* all nexthops belong to this area or are invalid */
return;
- if (rte->cost >= LS_INFINITY)
- return;
/* TODO AS border router specific checks */
/* TODO inter-area network route stuff */
/* TODO intra-area stuff -- condense LSA ??? */
Index: rde_lsdb.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde_lsdb.c,v
retrieving revision 1.49
diff -u -p -r1.49 rde_lsdb.c
--- rde_lsdb.c 14 Aug 2013 20:16:09 -0000 1.49
+++ rde_lsdb.c 26 Oct 2015 11:58:32 -0000
@@ -119,10 +119,6 @@ vertex_nexthop_add(struct vertex *dst, s
{
struct v_nexthop *vn;
- if (nexthop == 0)
- /* invalid nexthop, skip it */
- return;
-
if ((vn = calloc(1, sizeof(*vn))) == NULL)
fatal("vertex_nexthop_add");
Index: rde_spf.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde_spf.c,v
retrieving revision 1.75
diff -u -p -r1.75 rde_spf.c
--- rde_spf.c 18 Sep 2012 18:58:56 -0000 1.75
+++ rde_spf.c 15 Nov 2015 19:11:49 -0000
@@ -22,6 +22,7 @@
#include <arpa/inet.h>
#include <err.h>
#include <stdlib.h>
+#include <string.h>
#include "ospfd.h"
#include "ospf.h"
@@ -64,14 +65,19 @@ spf_calc(struct area *area)
/* initialize SPF tree */
if ((v = spf_root = lsa_find_area(area, LSA_TYPE_ROUTER,
- rde_router_id(), rde_router_id())) == NULL)
+ rde_router_id(), rde_router_id())) == NULL) {
/* empty area because no interface is active */
return;
+ }
area->transit = 0;
spf_root->cost = 0;
w = NULL;
+ /* make sure the spf root has a nexthop */
+ vertex_nexthop_clear(spf_root);
+ vertex_nexthop_add(spf_root, spf_root, 0);
+
/* calculate SPF tree */
do {
/* loop links */
@@ -159,8 +165,7 @@ spf_calc(struct area *area)
} while (v != NULL);
/* spf_dump(area); */
- log_debug("spf_calc: area %s calculated",
- inet_ntoa(area->id));
+ log_debug("spf_calc: area %s calculated", inet_ntoa(area->id));
area->num_spf_calc++;
start_spf_timer();
@@ -182,7 +187,7 @@ rt_calc(struct vertex *v, struct area *a
switch (v->type) {
case LSA_TYPE_ROUTER:
/* stub networks */
- if (v->cost >= LS_INFINITY || TAILQ_EMPTY(&v->nexthop))
+ if (v->cost >= LS_INFINITY)
return;
for (i = 0; i < lsa_num_links(v); i++) {
@@ -211,7 +216,7 @@ rt_calc(struct vertex *v, struct area *a
adv_rtr, PT_INTRA_AREA, DT_RTR, v->lsa->data.rtr.flags, 0);
break;
case LSA_TYPE_NETWORK:
- if (v->cost >= LS_INFINITY || TAILQ_EMPTY(&v->nexthop))
+ if (v->cost >= LS_INFINITY)
return;
addr.s_addr = htonl(v->ls_id) & v->lsa->data.net.mask;
@@ -245,7 +250,7 @@ rt_calc(struct vertex *v, struct area *a
v->cost = w->cost +
(ntohl(v->lsa->data.sum.metric) & LSA_METRIC_MASK);
- if (v->cost >= LS_INFINITY || TAILQ_EMPTY(&v->nexthop))
+ if (v->cost >= LS_INFINITY)
return;
adv_rtr.s_addr = htonl(v->adv_rtr);
@@ -751,7 +756,7 @@ rt_nexthop_add(struct rt_node *r, struct
rn->adv_rtr.s_addr = adv_rtr.s_addr;
rn->connected = (type == LSA_TYPE_NETWORK &&
- vn->prev == spf_root);
+ vn->prev == spf_root) || (vn->nexthop.s_addr == 0);
rn->invalid = 0;
r->invalid = 0;
@@ -768,7 +773,7 @@ rt_nexthop_add(struct rt_node *r, struct
rn->adv_rtr.s_addr = adv_rtr.s_addr;
rn->uptime = now.tv_sec;
rn->connected = (type == LSA_TYPE_NETWORK &&
- vn->prev == spf_root);
+ vn->prev == spf_root) || (vn->nexthop.s_addr == 0);
rn->invalid = 0;
r->invalid = 0;
@@ -823,20 +828,23 @@ rt_dump(struct in_addr area, pid_t pid,
fatalx("rt_dump: invalid RIB type");
}
+ bzero(&rtctl, sizeof(rtctl));
+ rtctl.prefix.s_addr = r->prefix.s_addr;
+ rtctl.area.s_addr = r->area.s_addr;
+ rtctl.cost = r->cost;
+ rtctl.cost2 = r->cost2;
+ rtctl.p_type = r->p_type;
+ rtctl.d_type = r->d_type;
+ rtctl.flags = r->flags;
+ rtctl.prefixlen = r->prefixlen;
+
TAILQ_FOREACH(rn, &r->nexthop, entry) {
if (rn->invalid)
continue;
- rtctl.prefix.s_addr = r->prefix.s_addr;
+ rtctl.connected = rn->connected;
rtctl.nexthop.s_addr = rn->nexthop.s_addr;
- rtctl.area.s_addr = r->area.s_addr;
rtctl.adv_rtr.s_addr = rn->adv_rtr.s_addr;
- rtctl.cost = r->cost;
- rtctl.cost2 = r->cost2;
- rtctl.p_type = r->p_type;
- rtctl.d_type = r->d_type;
- rtctl.flags = r->flags;
- rtctl.prefixlen = r->prefixlen;
rtctl.uptime = now.tv_sec - rn->uptime;
rde_imsg_compose_ospfe(IMSG_CTL_SHOW_RIB, 0, pid,
@@ -854,9 +862,6 @@ rt_update(struct in_addr prefix, u_int8_
struct rt_node *rte;
struct rt_nexthop *rn;
int better = 0, equal = 0;
-
- if (vnh == NULL || TAILQ_EMPTY(vnh)) /* XXX remove */
- fatalx("rt_update: invalid nexthop");
if ((rte = rt_find(prefix.s_addr, prefixlen, d_type)) == NULL) {
if ((rte = calloc(1, sizeof(struct rt_node))) == NULL)
