Crash on reconfigure with kernel table

[Toke Høiland-Jørgensen]

While testing the babel sadr patch, I ran into this crash on reconfigure
of the kernel protocol:

Basically, start with this config:

router id 62.168.0.1;
debug protocols all;

protocol device {}

ipv6 sadr table tab1;

protocol kernel {
  ipv6 sadr {
table tab1;
export all;
import all;
  };
  learn yes;
}


Then change it to this (removing sadr from both table type and the
protocol spec):

router id 62.168.0.1;
debug protocols all;

protocol device {}

ipv6 table tab1;

protocol kernel {
  ipv6 {
table tab1;
export all;
import all;
  };
  learn yes;
}


And issue a 'configure' in birdc. This gives the following:


bird: kernel1: Scanning routing table
bird: Assertion 'f->addr_type == a->type' failed at nest/rt-fib.c:227

Program received signal SIGABRT, Aborted.
0x775bb860 in raise () from /usr/lib/libc.so.6
(gdb) bt
#0  0x775bb860 in raise () from /usr/lib/libc.so.6
#1  0x775bcec9 in abort () from /usr/lib/libc.so.6
#2  0x5560a455 in bug (msg=0x5561e0e0 "Assertion '%s' failed at 
%s:%d") at sysdep/unix/log.c:213
#3  0x555a4da6 in fib_find (f=0x5585fa60, a=0x5586a0a8) at 
nest/rt-fib.c:227
#4  0x555a5ae6 in fib_get (f=0x5585fa60, a=0x5586a0a8) at 
nest/rt-fib.c:278
#5  0x556079d0 in net_get (tab=0x5585fa50, addr=0x5586a0a8) at 
./nest/route.h:289
#6  0x55608204 in krt_learn_scan (p=0x5585f920, e=0x5586c290) 
at sysdep/unix/krt.c:319
#7  0x55608d12 in krt_got_route (p=0x5585f920, e=0x5586c290) at 
sysdep/unix/krt.c:640
#8  0x555ffd59 in nl_announce_route (s=0x7fffe290) at 
sysdep/linux/netlink.c:1423
#9  0x556004c7 in nl_parse_route (s=0x7fffe290, h=0x5585fe08) 
at sysdep/linux/netlink.c:1584
#10 0x55601013 in krt_do_scan (p=0x0) at sysdep/linux/netlink.c:1829
#11 0x55609397 in krt_scan (t=0x55869720) at sysdep/unix/krt.c:835
#12 0x55593667 in timers_fire (loop=0x558506a0 ) at 
lib/timer.c:235
#13 0x5560707e in io_loop () at sysdep/unix/io.c:2169
#14 0x5560c4d6 in main (argc=6, argv=0x7fffe4d8) at 
sysdep/unix/main.c:892


-Toke

[PATCH v3] babel: Add source-specific routing support

[Toke Høiland-Jørgensen]

This adds support for source-specific routing to the babel protocol. It
changes the protocol to support both NET_IP6 and NET_IP6_SADR channels
for IPv6 addresses, preferring the NET_IP6_SADR channel. If only a
NET_IP6 channel is configured, source-specific updates are ignored.
Otherwise, non-source-specific routes are simply treated as
source-specific routes with sadr prefix 0.

Signed-off-by: Toke Høiland-Jørgensen 
---
This version also works with plain ipv6 channels.

 proto/babel/babel.c   |  34 +++---
 proto/babel/babel.h   |  20 --
 proto/babel/packets.c | 173 --
 3 files changed, 206 insertions(+), 21 deletions(-)

diff --git a/proto/babel/babel.c b/proto/babel/babel.c
index aa7e8b68..59e350e3 100644
--- a/proto/babel/babel.c
+++ b/proto/babel/babel.c
@@ -1950,13 +1950,13 @@ babel_show_entries_(struct babel_proto *p UNUSED, 
struct fib *rtable)
   srcs++;
 
 if (e->valid)
-  cli_msg(-1025, "%-24N %-23lR %6u %5u %7u %7u",
+  cli_msg(-1025, "%-50N %-23lR %6u %5u %7u %7u",
  e->n.addr, e->router_id, e->metric, e->seqno, rts, srcs);
 else if (r = e->selected)
-  cli_msg(-1025, "%-24N %-23lR %6u %5u %7u %7u",
+  cli_msg(-1025, "%-50N %-23lR %6u %5u %7u %7u",
  e->n.addr, r->router_id, r->metric, r->seqno, rts, srcs);
 else
-  cli_msg(-1025, "%-24N %-23s %6s %5s %7u %7u",
+  cli_msg(-1025, "%-50N %-23s %6s %5s %7u %7u",
  e->n.addr, "", "-", "-", rts, srcs);
   }
   FIB_WALK_END;
@@ -1975,7 +1975,7 @@ babel_show_entries(struct proto *P)
   }
 
   cli_msg(-1025, "%s:", p->p.name);
-  cli_msg(-1025, "%-24s %-23s %6s %5s %7s %7s",
+  cli_msg(-1025, "%-50s %-23s %6s %5s %7s %7s",
  "Prefix", "Router ID", "Metric", "Seqno", "Routes", "Sources");
 
   babel_show_entries_(p, >ip4_rtable);
@@ -1994,7 +1994,7 @@ babel_show_routes_(struct babel_proto *p UNUSED, struct 
fib *rtable)
 {
   char c = (r == e->selected) ? '*' : (r->feasible ? '+' : ' ');
   btime time = r->expires ? r->expires - current_time() : 0;
-  cli_msg(-1025, "%-24N %-25I %-10s %5u %c %5u %7t",
+  cli_msg(-1025, "%-50N %-25I %-10s %5u %c %5u %7t",
  e->n.addr, r->next_hop, r->neigh->ifa->ifname,
  r->metric, c, r->seqno, MAX(time, 0));
 }
@@ -2015,7 +2015,7 @@ babel_show_routes(struct proto *P)
   }
 
   cli_msg(-1025, "%s:", p->p.name);
-  cli_msg(-1025, "%-24s %-25s %-9s %6s F %5s %7s",
+  cli_msg(-1025, "%-50s %-25s %-9s %6s F %5s %7s",
  "Prefix", "Nexthop", "Interface", "Metric", "Seqno", "Expires");
 
   babel_show_routes_(p, >ip4_rtable);
@@ -2187,9 +2187,14 @@ babel_init(struct proto_config *CF)
 {
   struct proto *P = proto_new(CF);
   struct babel_proto *p = (void *) P;
+  struct channel_config *ip6_chan = proto_cf_find_channel(CF, NET_IP6_SADR);
+
+  /* Only want one of IP6 or IP6 SADR */
+  if (!ip6_chan)
+ip6_chan = proto_cf_find_channel(CF, NET_IP6);
 
   proto_configure_channel(P, >ip4_channel, proto_cf_find_channel(CF, 
NET_IP4));
-  proto_configure_channel(P, >ip6_channel, proto_cf_find_channel(CF, 
NET_IP6));
+  proto_configure_channel(P, >ip6_channel, ip6_chan);
 
   P->if_notify = babel_if_notify;
   P->rt_notify = babel_rt_notify;
@@ -2207,10 +2212,11 @@ babel_start(struct proto *P)
 {
   struct babel_proto *p = (void *) P;
   struct babel_config *cf = (void *) P->cf;
+  u8 ip6_type = p->ip6_channel ? p->ip6_channel->net_type : NET_IP6_SADR;
 
   fib_init(>ip4_rtable, P->pool, NET_IP4, sizeof(struct babel_entry),
   OFFSETOF(struct babel_entry, n), 0, babel_init_entry);
-  fib_init(>ip6_rtable, P->pool, NET_IP6, sizeof(struct babel_entry),
+  fib_init(>ip6_rtable, P->pool, ip6_type, sizeof(struct babel_entry),
   OFFSETOF(struct babel_entry, n), 0, babel_init_entry);
 
   init_list(>interfaces);
@@ -2258,11 +2264,19 @@ babel_reconfigure(struct proto *P, struct proto_config 
*CF)
 {
   struct babel_proto *p = (void *) P;
   struct babel_config *new = (void *) CF;
+  struct channel_config *ip6_chan;
 
   TRACE(D_EVENTS, "Reconfiguring");
 
+  /* Only want one of IP6 or IP6 SADR */
+  if (!(ip6_chan = proto_cf_find_channel(CF, NET_IP6_SADR)))
+ip6_chan = proto_cf_find_channel(CF, NET_IP6);
+
+  if (ip6_chan && p->ip6_channel && ip6_chan->net_type != 
p->ip6_channel->net_type)
+return 0;
+
   if (!proto_configure_channel(P, >ip4_channel, proto_cf_find_channel(CF, 
NET_IP4)) ||
-  !proto_configure_channel(P, >ip6_channel, proto_cf_find_channel(CF, 
NET_IP6)))
+  !proto_configure_channel(P, >ip6_channel, ip6_chan))
 return 0;
 
   p->p.cf = CF;
@@ -2280,7 +2294,7 @@ struct protocol proto_babel = {
   .template =  "babel%d",
   .attr_class =EAP_BABEL,
   .preference =DEF_PREF_BABEL,
-  .channel_mask =  NB_IP,
+  .channel_mask =  (NB_IP | NB_IP6_SADR),
   .proto_size =sizeof(struct babel_proto),
   .config_size =   

[PATCH] Add cscope Makefile target

[Toke Høiland-Jørgensen]

For those who prefer cscope to etags

Signed-off-by: Toke Høiland-Jørgensen 
---
 .gitignore  | 1 +
 Makefile.in | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 0dcebfd1..3b734f49 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,3 +12,4 @@
 /configure
 /sysdep/autoconf.h.in
 /sysdep/autoconf.h.in~
+/cscope.*
diff --git a/Makefile.in b/Makefile.in
index fdd5e6c7..c8168bbe 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -58,7 +58,7 @@ endif
 docgoals := docs userdocs progdocs
 testgoals := check test tests tests_run
 cleangoals := clean distclean testsclean
-.PHONY: all daemon cli $(docgoals) $(testgoals) $(cleangoals) tags
+.PHONY: all daemon cli $(docgoals) $(testgoals) $(cleangoals) tags cscope
 all: daemon cli
 
 daemon: $(daemon)
@@ -162,6 +162,9 @@ endif
 tags:
cd $(srcdir) ; etags -lc `find $(dirs) -name *.[chY]`
 
+cscope:
+   cd $(srcdir) ; find $(dirs) -name *.[chY] > cscope.files ; cscope -b
+
 # Install
 
 install: all
-- 
2.16.1

Re: [PATCH 1/2] Add IP6_SADR support to Bird Core

[Ondrej Zajicek]

On Mon, Feb 05, 2018 at 03:33:25PM +0100, Toke Høiland-Jørgensen wrote:
> Will fix and resubmit. What about the issue with 'learn'? :)

Fixed in 28b3b551222ab58456a067a9be4790824cdbb60e

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

Re: bird 2.0.1: SIGSEGV in nexthop_size

[Ondrej Zajicek]

On Mon, Jan 29, 2018 at 01:38:17PM +0100, Svenne Krap wrote:
> Hi,
> 
> We downgraded to version 1.6.3 as we are in a hurry to get to production.
> 
> We can still simulate the old environment, so I will try the patches...
> 
> It will probably not be today or tomorrow.

Hi, the issue noticed by Toke Hoiland-Jorgensen is most likely the cause
of failed assertion messages in log, so no need to run the test with the
second patch. You could use the attached patch to fix the second issue.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."
commit 28b3b551222ab58456a067a9be4790824cdbb60e
Author: Ondrej Zajicek (work) 
Date:   Tue Feb 6 16:08:45 2018 +0100

KRT: Fix IPv6 route learn

Internal table used for route learn was created with non-matching net
type for IPv6 kernel proto.

Thanks to Toke Hoiland-Jorgensen for the bugreport

diff --git a/bird.conf b/bird.conf
index 410f190c..e383c934 100644
--- a/bird.conf
+++ b/bird.conf
@@ -22,13 +22,14 @@ protocol direct {
 
 # Feed routes to kernel FIB
 protocol kernel {
-	ipv4 { export all; };
-#	learn;			# Learn all routes from the kernel
+	ipv4 { export all; import all; };
+	learn;			# Learn all routes from the kernel
 #	scan time 10;		# Scan kernel tables every 10 seconds
 }
 
 protocol kernel {
-	ipv6;
+	ipv6 { import all; };
+	learn;
 }
 
 # Static route feed
diff --git a/nest/route.h b/nest/route.h
index 43d7d696..1c86110b 100644
--- a/nest/route.h
+++ b/nest/route.h
@@ -282,7 +282,7 @@ void rt_preconfig(struct config *);
 void rt_commit(struct config *new, struct config *old);
 void rt_lock_table(rtable *);
 void rt_unlock_table(rtable *);
-void rt_setup(pool *, rtable *, char *, struct rtable_config *);
+void rt_setup(pool *, rtable *, struct rtable_config *);
 static inline net *net_find(rtable *tab, const net_addr *addr) { return (net *) fib_find(>fib, addr); }
 static inline net *net_find_valid(rtable *tab, const net_addr *addr)
 { net *n = net_find(tab, addr); return (n && rte_is_valid(n->routes)) ? n : NULL; }
diff --git a/nest/rt-table.c b/nest/rt-table.c
index 6ee261e8..784f6cfb 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -1599,22 +1599,19 @@ rt_event(void *ptr)
 }
 
 void
-rt_setup(pool *p, rtable *t, char *name, struct rtable_config *cf)
+rt_setup(pool *p, rtable *t, struct rtable_config *cf)
 {
   bzero(t, sizeof(*t));
-  t->name = name;
+  t->name = cf->name;
   t->config = cf;
-  t->addr_type = cf ? cf->addr_type : NET_IP4;
+  t->addr_type = cf->addr_type;
   fib_init(>fib, p, t->addr_type, sizeof(net), OFFSETOF(net, n), 0, NULL);
   init_list(>channels);
 
-  if (cf)
-{
-  t->rt_event = ev_new(p);
-  t->rt_event->hook = rt_event;
-  t->rt_event->data = t;
-  t->gc_time = current_time();
-}
+  t->rt_event = ev_new(p);
+  t->rt_event->hook = rt_event;
+  t->rt_event->data = t;
+  t->gc_time = current_time();
 }
 
 /**
@@ -2090,7 +2087,7 @@ rt_commit(struct config *new, struct config *old)
   {
 	rtable *t = mb_alloc(rt_table_pool, sizeof(struct rtable));
 	DBG("\t%s: created\n", r->name);
-	rt_setup(rt_table_pool, t, r->name, r);
+	rt_setup(rt_table_pool, t, r);
 	add_tail(_tables, >n);
 	r->table = t;
   }
diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c
index e7bd79e3..a3cbb336 100644
--- a/sysdep/unix/krt.c
+++ b/sysdep/unix/krt.c
@@ -506,7 +506,13 @@ static void
 krt_learn_init(struct krt_proto *p)
 {
   if (KRT_CF->learn)
-rt_setup(p->p.pool, >krt_table, "Inherited", NULL);
+  {
+struct rtable_config *cf = mb_allocz(p->p.pool, sizeof(struct rtable_config));
+cf->name = "Inherited";
+cf->addr_type = p->p.net_type;
+
+rt_setup(p->p.pool, >krt_table, cf);
+  }
 }
 
 static void

Re: [PATCH 1/2] Add IP6_SADR support to Bird Core

[Ondrej Zajicek]

On Tue, Feb 06, 2018 at 04:57:43PM +0100, Toke Høiland-Jørgensen wrote:
> What, we have to be nice to the users now? ;)
> 
> Fair enough, I'll add support for both types of channels. Should it be
> possible to connect the same instance to both an ipv6 and and ipv6_sadr
> channel type, or is it OK to enforce that there's only one of those
> active?

It is OK to enforce that there is only one of those active.
How non-SADR-aware Babel should handle SADR routes? Is it
mandatory sub-TLV, so they are ignored; or optional sub-TLV,
so they are handled as regular routes?

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

Re: [PATCH 1/2] Add IP6_SADR support to Bird Core

[Toke Høiland-Jørgensen]

Ondrej Zajicek  writes:

> On Tue, Feb 06, 2018 at 04:38:59PM +0100, Toke Høiland-Jørgensen wrote:
>> > I did not check 2/2 yet, but i think it should support both regular
>> > ipv6 and ipv6 SADR channels/tables, not just the SADR ones.
>> 
>> Hmm, well, my assumption was that since sadr tables can hold normal ipv6
>> routes as well, the pipe hack you were talking about would be sufficient
>> to support both?
>
> It could, but that would be cumbersome for users that just want
> regular routes. The pipe hack is more for setups that integrate SADR
> routes from Babel with other (non-SADR) protocols. Simple setups
> should keep simple configs.

What, we have to be nice to the users now? ;)

Fair enough, I'll add support for both types of channels. Should it be
possible to connect the same instance to both an ipv6 and and ipv6_sadr
channel type, or is it OK to enforce that there's only one of those
active?

-Toke

Re: [PATCH 1/2] Add IP6_SADR support to Bird Core

[Toke Høiland-Jørgensen]

Ondrej Zajicek  writes:

> On Tue, Feb 06, 2018 at 05:20:25PM +0100, Toke Høiland-Jørgensen wrote:
>> It's mandatory, otherwise you can get routing loops. See
>> https://tools.ietf.org/html/draft-ietf-babel-source-specific-03#section-6
>> 
>> I figure the packet parser needs to be aware of whether it is configured
>> to support SADR routes and drop all source-specific updates if not...
>
> OK, so there is no need to have separate option, it could be handled
> just by which channel type is defined.

Yeah, that was my thought. Will add that :)

-Toke

Re: Bird 1.6.3 filters breaking down on reconfigure.

[Ondrej Zajicek]

On Mon, Feb 05, 2018 at 11:05:05PM +0100, Roelf "rewbycraft" Wichertjes wrote:
> Hello everyone,
> 
> I am experiencing a problem where upon issuing "birdc configure" (or "birdc6
> configure") some strange things happen.
> ...
> A lot of these issues sound like bird simply not properly re-applying
> filters after they have changed.
> 
> Both problems can be worked around be restarting the affected protocols.
> However this is not a viable long-term solution as flapping bgp sessions (or
> just flat out restarting routers when a lot of protocols exhibit these
> issues)


Hello

It seems that we have some fixed bugs in master branch that could be
this issue. Could you try build BIRD from GIT repository (master branch)?

Unfortunately, with concentration on 2.0.0 we neglected to release
version 1.6.4 earlier.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

Re: [PATCH 1/2] Add IP6_SADR support to Bird Core

[Toke Høiland-Jørgensen]

Ondrej Zajicek  writes:

> On Tue, Feb 06, 2018 at 04:57:43PM +0100, Toke Høiland-Jørgensen wrote:
>> What, we have to be nice to the users now? ;)
>> 
>> Fair enough, I'll add support for both types of channels. Should it be
>> possible to connect the same instance to both an ipv6 and and ipv6_sadr
>> channel type, or is it OK to enforce that there's only one of those
>> active?
>
> It is OK to enforce that there is only one of those active. How
> non-SADR-aware Babel should handle SADR routes? Is it mandatory
> sub-TLV, so they are ignored; or optional sub-TLV, so they are handled
> as regular routes?

It's mandatory, otherwise you can get routing loops. See
https://tools.ietf.org/html/draft-ietf-babel-source-specific-03#section-6

I figure the packet parser needs to be aware of whether it is configured
to support SADR routes and drop all source-specific updates if not...

-Toke

Re: [PATCH 1/2] Add IP6_SADR support to Bird Core

[Toke Høiland-Jørgensen]

Ondrej Zajicek  writes:

> On Mon, Feb 05, 2018 at 03:33:25PM +0100, Toke Høiland-Jørgensen wrote:
>> Will fix and resubmit. What about the issue with 'learn'? :)
>
> Fixed in 28b3b551222ab58456a067a9be4790824cdbb60e

Great, thanks!

Do you want me to submit a new version of the patch series with that
part removed, or are you OK with just dropping it when you merge?
Assuming you don't have any more issues you want me to fix, of course ;)

-Toke

Re: [PATCH 1/2] Add IP6_SADR support to Bird Core

[Ondrej Zajicek]

On Tue, Feb 06, 2018 at 05:20:25PM +0100, Toke Høiland-Jørgensen wrote:
> It's mandatory, otherwise you can get routing loops. See
> https://tools.ietf.org/html/draft-ietf-babel-source-specific-03#section-6
> 
> I figure the packet parser needs to be aware of whether it is configured
> to support SADR routes and drop all source-specific updates if not...

OK, so there is no need to have separate option, it could be handled
just by which channel type is defined.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

Re: [PATCH 1/2] Add IP6_SADR support to Bird Core

[Ondrej Zajicek]

On Tue, Feb 06, 2018 at 04:38:59PM +0100, Toke Høiland-Jørgensen wrote:
> > I did not check 2/2 yet, but i think it should support both regular
> > ipv6 and ipv6 SADR channels/tables, not just the SADR ones.
> 
> Hmm, well, my assumption was that since sadr tables can hold normal ipv6
> routes as well, the pipe hack you were talking about would be sufficient
> to support both?

It could, but that would be cumbersome for users that just want regular
routes. The pipe hack is more for setups that integrate SADR routes from
Babel with other (non-SADR) protocols. Simple setups should keep simple
configs.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

Re: Crash on reconfigure with kernel table

[Ondrej Zajicek]

On Tue, Feb 06, 2018 at 08:50:52PM +0100, Toke Høiland-Jørgensen wrote:
> While testing the babel sadr patch, I ran into this crash on reconfigure
> of the kernel protocol:

Hi

Likely it is reconfiguration of table tab1 (changing net type) that
does not work. You could try to rename it during net type change.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."