[prev in list] [next in list] [prev in thread] [next in thread]
List: openbsd-misc
Subject: Re: ospf6d problem when a route already exists with a different nexthop
From: Manuel Guesdon <ml+openbsd.misc () oxymium ! net>
Date: 2012-09-29 13:09:27
Message-ID: 20120929150927.656edeb0 () alpha ! oxymium ! net
[Download RAW message or body]
Sorry, here is the patch:
diff -u ospf6d.uptodate/kroute.c ospf6d.patch1/kroute.c
--- ospf6d.uptodate/kroute.c Thu Sep 20 15:25:33 2012
+++ ospf6d.patch1/kroute.c Thu Sep 27 18:01:37 2012
@@ -59,6 +59,8 @@
int kr_redist_eval(struct kroute *, struct rroute *);
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 *,
@@ -140,18 +142,102 @@
}
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 (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");
+ log_debug("kr_update_fib: before: %s%s",
+ log_in6addr(&kn->r.nexthop),
+ i == krcount ? " (deleted)" : "");
+ }
+ }
+
+ /*
+ * now add or change the route
+ */
+ for (i = 0; i < krcount; i++) {
+ /* nexthop within 127/8 -> ignore silently */
+ if (kr && IN6_IS_ADDR_LOOPBACK(&kr->r.nexthop))
+ continue;
+
+ if (action == RTM_ADD && kr) {
+ for (kn = kr; kn != NULL; kn = kn->next) {
+ if (IN6_ARE_ADDR_EQUAL(&kn->r.nexthop,&kroute[i].nexthop))
+ break;
+ }
+
+ log_debug("kr_update_fib: after : %s%s",
+ log_in6addr(&kroute[i].nexthop),
+ kn == NULL ? " (added)" : "");
+
+ 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;
@@ -170,79 +256,43 @@
* - 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 (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_remove(kr) == -1)
+ return (-1);
- if (kroute_insert(kr) == -1)
- free(kr);
- } else if (kr) {
- kr->r.nexthop = kroute->nexthop;
- kr->r.scope = kroute->scope;
- }
-
return (0);
}
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);
}
@@ -257,6 +307,7 @@
kr_fib_couple(void)
{
struct kroute_node *kr;
+ struct kroute_node *kn;
if (kr_state.fib_sync == 1) /* already coupled */
return;
@@ -265,7 +316,9 @@
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");
}
@@ -274,13 +327,16 @@
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)
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;
@@ -1062,7 +1118,7 @@
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;
diff -u ospf6d.uptodate/ospf6d.c ospf6d.patch1/ospf6d.c
--- ospf6d.uptodate/ospf6d.c Thu Sep 20 15:25:33 2012
+++ ospf6d.patch1/ospf6d.c Thu Sep 20 22:27:10 2012
@@ -422,7 +422,7 @@
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)
@@ -444,7 +444,9 @@
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;
diff -u ospf6d.uptodate/ospf6d.h ospf6d.patch1/ospf6d.h
--- ospf6d.uptodate/ospf6d.h Thu Sep 20 15:25:33 2012
+++ ospf6d.patch1/ospf6d.h Thu Sep 20 22:27:10 2012
@@ -526,7 +526,7 @@
/* 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);
diff -u ospf6d.uptodate/rde.c ospf6d.patch1/rde.c
--- ospf6d.uptodate/rde.c Thu Sep 20 15:25:33 2012
+++ ospf6d.patch1/rde.c Thu Sep 27 17:51:59 2012
@@ -869,28 +869,36 @@
void
rde_send_change_kroute(struct rt_node *r)
{
+ int krcount = 0;
struct kroute kr;
struct rt_nexthop *rn;
+ struct ibuf *wbuf;
- TAILQ_FOREACH(rn, &r->nexthop, entry) {
- if (!rn->invalid)
- break;
+ if ((wbuf = imsg_create(&iev_main->ibuf, IMSG_KROUTE_CHANGE, 0, 0,
+ sizeof(kr))) == NULL) {
+ return;
}
- if (!rn)
- 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;
+ TAILQ_FOREACH(rn, &r->nexthop, entry) {
+ if (rn->invalid)
+ continue;
+ krcount++;
- imsg_compose_event(iev_main, IMSG_KROUTE_CHANGE, 0, 0, -1,
- &kr, sizeof(kr));
+ 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 (krcount == 0)
+ fatalx("rde_send_change_kroute: no valid nexthop found");
+ imsg_close(&iev_main->ibuf, wbuf);
+ imsg_event_add(iev_main);
}
void
On Thu, 27 Sep 2012 19:09:58 +0200
Manuel Guesdon <ml+openbsd.misc@oxymium.net> wrote:
>| On Thu, 20 Sep 2012 18:00:26 +0200
>| Claudio Jeker <cjeker@diehard.n-r-g.com> wrote:
>|
>| >| On Thu, Sep 20, 2012 at 05:19:53PM +0200, Manuel Guesdon wrote:
>| >| > Hi,
>| >| >
>| >| > After checking cvs tree, it seems that ospf6d isn't following changes done in
>| >| > ospfd.
>| >| >
>| >| > Is someone working on updating ospf6d to add changes done in ospfd ? (or may
>| >| > be it's not the best way to do ?).
>| >| >
>| >| > If it's a good idea to do it, I can try. Do you have some advice ? Should I
>| >| > try to apply a big diff between ospfd initial (i.e. 2007 version) and current
>| >| > state ? Or is there a way to retrieve each patch made to ospfd between 2007
>| >| > and now ?
>| >| >
>| >| > A last question: ospf6d initial version was created by copying and modifying
>| >| > ospfd files; I presume it was done this way instead of having same code with
>| >| > #ifdef mecanism for good reasons. After 5 years of evolution, does these
>| >| > reasons still appear beoing valid (I just ask, I haven't sufficient knowledge
>| >| > to give an answer).
>| >| >
>| >|
>| >| OSPFv2 and OSPFv3 are similar but still to different to have a common
>| >| source. We try to sync parts between the two daemons from time to time but
>| >| in some parts the behaviour is to different so that syncing is almost
>| >| impossible.
>|
>| OK.
>|
>| >| I may sound like a broken record but we accept diffs. So if you think
>| >| there are commits in ospfd that need to be synced over we will have a look
>| >| at them.
>|
>| Here is a patch adapted from ospfd patch of "Tue Sep 25 11:25:41 2007
>| UTC" (the one of version 1.52 of kroute.c):
>| <<
>| Last missing piece in the equal cost multipath support for ospfd.
>| Send all possible nexthops to the parent process and correctly sync
>| the RIB, FIB and kernel routing table. Based on initial work by pyr@.
>| OK pyr@ norby@
>| PS: don't forget that you need to enable multipath support via a sysctl
>| >>
>|
>| It seems to solve my problem but not "perfectly".
>| When starting ospf6d with the best link between 2 hosts down, fib contains
>| 2 other routes comme from 2 other hosts (these 2 routes have equal cost).
>| When the link came UP, these 2 routes are removed and replaced by the best
>| route; that's alright.
>| Next when the link goes down, the 2 alternative routes are well added in fib
>| but the precedent best route is still in the fib (I see it with route -n -v
>| show |grep TargetIP). May be an important point: TargetIP is an IPv6 on lo1.
>|
>| I can't find why; if you have any idea... I have a test network so I can make
>| test easily...
>|
>|
>| Manuel
>|
>|
--
Cordialement,
Manuel Guesdon
--
______________________________________________________________________
Manuel Guesdon - OXYMIUM
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic