Hi,
our QA reports issues with the ospf6d since the kernel uses more multipath
routes.
It exits after certain topology changes with:
rde_send_change_kroute: no valid nexthop found
Since the kernel uses more multipath routes, the lack of multipath support in
ospf6d became a problem.
The attached patch ports the multipath support from ospfd to ospf6d.
It bases on the following ospfd commits:
cvs diff -D "2007-09-24" -D "2007-09-26"
cvs diff -r1.65 -r1.67 kroute.c
A similar patch was suggested by Manues Guesdon a couple of years ago.
This patch doesn't fix all problems I am seeing, but it improves the situation
a lot. A second patch porting priority support will follow and fix further
issues. I decided to split my fixes into two parts to make review easier.
Thanks & Regards
Florian
Index: kroute.c
===================================================================
RCS file: /openbsd/src/usr.sbin/ospf6d/kroute.c,v
retrieving revision 1.50
diff -u -p -r1.50 kroute.c
--- kroute.c 27 Dec 2016 17:18:56 -0000 1.50
+++ kroute.c 11 May 2017 20:48:49 -0000
@@ -59,6 +59,8 @@ void kr_redist_remove(struct kroute_node
int kr_redist_eval(struct kroute *, struct kroute *);
void kr_redistribute(struct kroute_node *);
int kroute_compare(struct kroute_node *, struct kroute_node *);
+int kr_change_fib(struct kroute_node *, struct kroute *, int, int);
+int kr_delete_fib(struct kroute_node *);
struct kroute_node *kroute_find(const struct in6_addr *, u_int8_t);
struct kroute_node *kroute_matchgw(struct kroute_node *,
@@ -141,18 +143,105 @@ kr_init(int fs)
}
int
-kr_change(struct kroute *kroute)
+kr_change_fib(struct kroute_node *kr, struct kroute *kroute, int krcount,
+ int action)
+{
+ int i;
+ struct kroute_node *kn, *nkn;
+
+ if (action == RTM_ADD) {
+ /*
+ * First remove all stale multipath routes.
+ * This step must be skipped when the action is RTM_CHANGE
+ * because it is already a single path route that will be
+ * changed.
+ */
+ for (kn = kr; kn != NULL; kn = nkn) {
+ for (i = 0; i < krcount; i++) {
+ if (kn->r.scope == kroute[i].scope &&
+ IN6_ARE_ADDR_EQUAL(&kn->r.nexthop,
+ &kroute[i].nexthop))
+ break;
+ }
+ nkn = kn->next;
+ if (i == krcount) {
+ /* stale route */
+ if (kr_delete_fib(kn) == -1)
+ log_warnx("kr_delete_fib failed");
+ /*
+ * if head element was removed we need to adjust
+ * the head
+ */
+ if (kr == kn)
+ kr = nkn;
+ }
+ }
+ }
+
+ /*
+ * now add or change the route
+ */
+ for (i = 0; i < krcount; i++) {
+ /* nexthop ::1 -> ignore silently */
+ if (IN6_IS_ADDR_LOOPBACK(&kroute[i].nexthop))
+ continue;
+
+ if (action == RTM_ADD && kr) {
+ for (kn = kr; kn != NULL; kn = kn->next) {
+ if (kn->r.scope == kroute[i].scope &&
+ IN6_ARE_ADDR_EQUAL(&kn->r.nexthop,
+ &kroute[i].nexthop))
+ break;
+ }
+
+ if (kn != NULL)
+ /* nexthop already present, skip it */
+ continue;
+ } else
+ /* modify first entry */
+ kn = kr;
+
+ /* send update */
+ if (send_rtmsg(kr_state.fd, action, &kroute[i]) == -1)
+ return (-1);
+
+ /* create new entry unless we are changing the first entry */
+ if (action == RTM_ADD)
+ if ((kn = calloc(1, sizeof(*kn))) == NULL)
+ fatal(NULL);
+
+ kn->r.prefix = kroute[i].prefix;
+ kn->r.prefixlen = kroute[i].prefixlen;
+ kn->r.nexthop = kroute[i].nexthop;
+ kn->r.scope = kroute[i].scope;
+ kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED;
+ kn->r.ext_tag = kroute[i].ext_tag;
+ rtlabel_unref(kn->r.rtlabel); /* for RTM_CHANGE */
+ kn->r.rtlabel = kroute[i].rtlabel;
+
+ if (action == RTM_ADD)
+ if (kroute_insert(kn) == -1) {
+ log_debug("kr_update_fib: cannot insert %s",
+ log_in6addr(&kn->r.nexthop));
+ free(kn);
+ }
+ action = RTM_ADD;
+ }
+ return (0);
+}
+
+int
+kr_change(struct kroute *kroute, int krcount)
{
struct kroute_node *kr;
int action = RTM_ADD;
kroute->rtlabel = rtlabel_tag2id(kroute->ext_tag);
- if ((kr = kroute_find(&kroute->prefix, kroute->prefixlen)) !=
- NULL) {
- if (!(kr->r.flags & F_KERNEL))
- action = RTM_CHANGE;
- else { /* a non-ospf route already exists. not a problem */
+ kr = kroute_find(&kroute->prefix, kroute->prefixlen);
+ if (kr != NULL) {
+ if (kr->r.flags & F_KERNEL) {
+ /* a non-ospf route already exists. not a problem */
if (!(kr->r.flags & F_BGPD_INSERTED)) {
do {
kr->r.flags |= F_OSPFD_INSERTED;
@@ -171,48 +260,30 @@ kr_change(struct kroute *kroute)
* - zero out ifindex (this is no longer relevant)
*/
action = RTM_CHANGE;
- kr->r.flags = kroute->flags | F_OSPFD_INSERTED;
- kr->r.ifindex = 0;
- rtlabel_unref(kr->r.rtlabel);
- kr->r.ext_tag = kroute->ext_tag;
- kr->r.rtlabel = kroute->rtlabel;
- }
+ } else if (kr->next == NULL) /* single path OSPF route */
+ action = RTM_CHANGE;
}
- /* nexthop within 127/8 -> ignore silently */
- if (kr && IN6_IS_ADDR_LOOPBACK(&kr->r.nexthop))
- return (0);
+ return (kr_change_fib(kr, kroute, krcount, action));
+}
- /*
- * Ingnore updates that did not change the route.
- * Currently only the nexthop can change.
- */
- if (kr && kr->r.scope == kroute->scope &&
- IN6_ARE_ADDR_EQUAL(&kr->r.nexthop, &kroute->nexthop))
+int
+kr_delete_fib(struct kroute_node *kr)
+{
+ if (!(kr->r.flags & F_OSPFD_INSERTED))
+ return 0;
+
+ if (kr->r.flags & F_KERNEL) {
+ /* remove F_OSPFD_INSERTED flag, route still exists in kernel */
+ kr->r.flags &= ~F_OSPFD_INSERTED;
return (0);
+ }
- if (send_rtmsg(kr_state.fd, action, kroute) == -1)
+ if (send_rtmsg(kr_state.fd, RTM_DELETE, &kr->r) == -1)
return (-1);
- if (action == RTM_ADD) {
- if ((kr = calloc(1, sizeof(struct kroute_node))) == NULL) {
- log_warn("kr_change");
- return (-1);
- }
- kr->r.prefix = kroute->prefix;
- kr->r.prefixlen = kroute->prefixlen;
- kr->r.nexthop = kroute->nexthop;
- kr->r.scope = kroute->scope;
- kr->r.flags = kroute->flags | F_OSPFD_INSERTED;
- kr->r.ext_tag = kroute->ext_tag;
- kr->r.rtlabel = kroute->rtlabel;
-
- if (kroute_insert(kr) == -1)
- free(kr);
- } else if (kr) {
- kr->r.nexthop = kroute->nexthop;
- kr->r.scope = kroute->scope;
- }
+ if (kroute_remove(kr) == -1)
+ return (-1);
return (0);
}
@@ -220,30 +291,19 @@ kr_change(struct kroute *kroute)
int
kr_delete(struct kroute *kroute)
{
- struct kroute_node *kr;
+ struct kroute_node *kr, *nkr;
if ((kr = kroute_find(&kroute->prefix, kroute->prefixlen)) ==
NULL)
return (0);
- if (!(kr->r.flags & F_OSPFD_INSERTED))
- return (0);
-
- if (kr->r.flags & F_KERNEL) {
- /* remove F_OSPFD_INSERTED flag, route still exists in kernel */
- do {
- kr->r.flags &= ~F_OSPFD_INSERTED;
- kr = kr->next;
- } while (kr);
- return (0);
+ while (kr != NULL) {
+ nkr = kr->next;
+ if (kr_delete_fib(kr) == -1)
+ return (-1);
+ kr = nkr;
}
- if (send_rtmsg(kr_state.fd, RTM_DELETE, kroute) == -1)
- return (-1);
-
- if (kroute_remove(kr) == -1)
- return (-1);
-
return (0);
}
@@ -258,6 +318,7 @@ void
kr_fib_couple(void)
{
struct kroute_node *kr;
+ struct kroute_node *kn;
if (kr_state.fib_sync == 1) /* already coupled */
return;
@@ -266,7 +327,9 @@ kr_fib_couple(void)
RB_FOREACH(kr, kroute_tree, &krt)
if (!(kr->r.flags & F_KERNEL))
- send_rtmsg(kr_state.fd, RTM_ADD, &kr->r);
+ for (kn = kr; kn != NULL; kn = kn->next) {
+ send_rtmsg(kr_state.fd, RTM_ADD, &kn->r);
+ }
log_info("kernel routing table coupled");
}
@@ -275,13 +338,17 @@ void
kr_fib_decouple(void)
{
struct kroute_node *kr;
+ struct kroute_node *kn;
if (kr_state.fib_sync == 0) /* already decoupled */
return;
- RB_FOREACH(kr, kroute_tree, &krt)
+ RB_FOREACH(kr, kroute_tree, &krt) {
if (!(kr->r.flags & F_KERNEL))
- send_rtmsg(kr_state.fd, RTM_DELETE, &kr->r);
+ for (kn = kr; kn != NULL; kn = kn->next) {
+ send_rtmsg(kr_state.fd, RTM_DELETE, &kn->r);
+ }
+ }
kr_state.fib_sync = 0;
@@ -932,7 +999,7 @@ send_rtmsg(int fd, int action, struct kr
bzero(&hdr, sizeof(hdr));
hdr.rtm_version = RTM_VERSION;
hdr.rtm_type = action;
- hdr.rtm_flags = RTF_UP;
+ hdr.rtm_flags = RTF_UP|RTF_MPATH;
hdr.rtm_priority = RTP_OSPF;
if (action == RTM_CHANGE)
hdr.rtm_fmask = RTF_REJECT|RTF_BLACKHOLE;
Index: ospf6d.c
===================================================================
RCS file: /openbsd/src/usr.sbin/ospf6d/ospf6d.c,v
retrieving revision 1.32
diff -u -p -r1.32 ospf6d.c
--- ospf6d.c 3 Sep 2016 10:25:36 -0000 1.32
+++ ospf6d.c 11 May 2017 20:48:49 -0000
@@ -389,7 +389,7 @@ main_dispatch_rde(int fd, short event, v
struct imsgbuf *ibuf = &iev->ibuf;
struct imsg imsg;
ssize_t n;
- int shut = 0;
+ int count, shut = 0;
if (event & EV_READ) {
if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
@@ -413,7 +413,9 @@ main_dispatch_rde(int fd, short event, v
switch (imsg.hdr.type) {
case IMSG_KROUTE_CHANGE:
- if (kr_change(imsg.data))
+ count = (imsg.hdr.len - IMSG_HEADER_SIZE) /
+ sizeof(struct kroute);
+ if (kr_change(imsg.data, count))
log_warn("main_dispatch_rde: error changing "
"route");
break;
Index: ospf6d.h
===================================================================
RCS file: /openbsd/src/usr.sbin/ospf6d/ospf6d.h,v
retrieving revision 1.31
diff -u -p -r1.31 ospf6d.h
--- ospf6d.h 27 Dec 2016 17:18:56 -0000 1.31
+++ ospf6d.h 11 May 2017 20:48:49 -0000
@@ -532,7 +532,7 @@ u_int16_t iso_cksum(void *, u_int16_t,
/* kroute.c */
int kr_init(int);
-int kr_change(struct kroute *);
+int kr_change(struct kroute *, int);
int kr_delete(struct kroute *);
void kr_shutdown(void);
void kr_fib_couple(void);
Index: rde.c
===================================================================
RCS file: /openbsd/src/usr.sbin/ospf6d/rde.c,v
retrieving revision 1.69
diff -u -p -r1.69 rde.c
--- rde.c 27 Dec 2016 17:18:56 -0000 1.69
+++ rde.c 11 May 2017 20:48:49 -0000
@@ -878,28 +878,37 @@ rde_router_id(void)
void
rde_send_change_kroute(struct rt_node *r)
{
+ int krcount = 0;
struct kroute kr;
struct rt_nexthop *rn;
+ struct ibuf *wbuf;
+
+ if ((wbuf = imsg_create(&iev_main->ibuf, IMSG_KROUTE_CHANGE, 0, 0,
+ sizeof(kr))) == NULL) {
+ return;
+ }
TAILQ_FOREACH(rn, &r->nexthop, entry) {
- if (!rn->invalid)
- break;
+ if (rn->invalid)
+ continue;
+ krcount++;
+
+ bzero(&kr, sizeof(kr));
+ kr.prefix = r->prefix;
+ kr.nexthop = rn->nexthop;
+ if (IN6_IS_ADDR_LINKLOCAL(&rn->nexthop) ||
+ IN6_IS_ADDR_MC_LINKLOCAL(&rn->nexthop))
+ kr.scope = rn->ifindex;
+ kr.ifindex = rn->ifindex;
+ kr.prefixlen = r->prefixlen;
+ kr.ext_tag = r->ext_tag;
+ imsg_add(wbuf, &kr, sizeof(kr));
}
- if (!rn)
+ if (krcount == 0)
fatalx("rde_send_change_kroute: no valid nexthop found");
- bzero(&kr, sizeof(kr));
- kr.prefix = r->prefix;
- kr.nexthop = rn->nexthop;
- if (IN6_IS_ADDR_LINKLOCAL(&rn->nexthop) ||
- IN6_IS_ADDR_MC_LINKLOCAL(&rn->nexthop))
- kr.scope = rn->ifindex;
- kr.ifindex = rn->ifindex;
- kr.prefixlen = r->prefixlen;
- kr.ext_tag = r->ext_tag;
-
- imsg_compose_event(iev_main, IMSG_KROUTE_CHANGE, 0, 0, -1,
- &kr, sizeof(kr));
+ imsg_close(&iev_main->ibuf, wbuf);
+ imsg_event_add(iev_main);
}
void
