Re: Giving love to ospf6d

2015-12-20 Thread Denis Fondras
Hello,

Here is the second iteration of the patch. It fixes a crash and it is a
refactoring.

Denis

Index: area.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/area.c,v
retrieving revision 1.4
diff -u -p -r1.4 area.c
--- area.c  28 Dec 2008 20:08:31 -  1.4
+++ area.c  20 Dec 2015 20:38:00 -
@@ -57,7 +57,6 @@ area_del(struct area *area)
/* clean lists */
while ((iface = LIST_FIRST(>iface_list)) != NULL) {
LIST_REMOVE(iface, entry);
-   if_del(iface);
}
 
while ((n = LIST_FIRST(>nbr_list)) != NULL)
Index: interface.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/interface.c,v
retrieving revision 1.22
diff -u -p -r1.22 interface.c
--- interface.c 27 Sep 2015 17:31:50 -  1.22
+++ interface.c 20 Dec 2015 20:38:00 -
@@ -170,8 +170,7 @@ int
 if_init(void)
 {
TAILQ_INIT();
-
-   return (fetchifs(0));
+   return (0);
 }
 
 /* XXX using a linked list should be OK for now */
Index: ospf6d.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
retrieving revision 1.29
diff -u -p -r1.29 ospf6d.c
--- ospf6d.c5 Dec 2015 13:12:41 -   1.29
+++ ospf6d.c20 Dec 2015 20:38:01 -
@@ -597,6 +597,7 @@ ospf_reload(void)
 {
struct area *area;
struct ospfd_conf   *xconf;
+   struct iface*iface;
 
if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL)
return (-1);
@@ -605,10 +606,16 @@ ospf_reload(void)
if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1)
return (-1);
 
-   /* send areas, interfaces happen out of band */
+   /* send areas & interfaces */
LIST_FOREACH(area, >area_list, entry) {
if (ospf_sendboth(IMSG_RECONF_AREA, area, sizeof(*area)) == -1)
return (-1);
+
+   LIST_FOREACH(iface, >iface_list, entry) {
+   if (ospf_sendboth(IMSG_RECONF_IFACE, iface,
+   sizeof(*iface)) == -1)
+   return (-1);
+   }
}
 
if (ospf_sendboth(IMSG_RECONF_END, NULL, 0) == -1)
Index: ospf6d.h
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
retrieving revision 1.29
diff -u -p -r1.29 ospf6d.h
--- ospf6d.h27 Sep 2015 17:31:50 -  1.29
+++ ospf6d.h20 Dec 2015 20:38:01 -
@@ -121,6 +121,7 @@ enum imsg_type {
IMSG_ABR_DOWN,
IMSG_RECONF_CONF,
IMSG_RECONF_AREA,
+   IMSG_RECONF_IFACE,
IMSG_RECONF_END,
IMSG_DEMOTE
 };
Index: ospfe.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospfe.c,v
retrieving revision 1.47
diff -u -p -r1.47 ospfe.c
--- ospfe.c 5 Dec 2015 13:12:41 -   1.47
+++ ospfe.c 20 Dec 2015 20:38:02 -
@@ -250,6 +250,7 @@ ospfe_dispatch_main(int fd, short event,
static struct area  *narea;
struct area *area;
struct iface*iface, *ifp;
+   struct iface*niface;
struct ifaddrchange *ifc;
struct iface_addr   *ia, *nia;
struct imsg  imsg;
@@ -388,6 +389,19 @@ ospfe_dispatch_main(int fd, short event,
RB_INIT(>lsa_tree);
 
LIST_INSERT_HEAD(>area_list, narea, entry);
+   break;
+   case IMSG_RECONF_IFACE:
+   if ((niface = malloc(sizeof(struct iface))) == NULL)
+   fatal(NULL);
+
+   memcpy(niface, imsg.data, sizeof(struct iface));
+
+   LIST_INIT(>nbr_list);
+   TAILQ_INIT(>ls_ack_list);
+   RB_INIT(>lsa_tree);
+
+   narea = area_find(nconf, niface->area_id);
+   LIST_INSERT_HEAD(>iface_list, niface, entry);
break;
case IMSG_RECONF_END:
if ((oeconf->flags & OSPFD_FLAG_STUB_ROUTER) !=
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
retrieving revision 1.27
diff -u -p -r1.27 parse.y
--- parse.y 20 Nov 2014 05:51:20 -  1.27
+++ parse.y 20 Dec 2015 20:38:02 -
@@ -924,6 +924,7 @@ parse_config(char *filename, int opts)
LIST_INIT(>area_list);
LIST_INIT(>cand_list);
SIMPLEQ_INIT(>redist_list);
+   fetchifs(0);
 
yyparse();
errors = file->errors;
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/rde.c,v
retrieving revision 1.65
diff -u -p -r1.65 rde.c
--- 

Giving love to ospf6d

2015-12-15 Thread Denis Fondras
Hello,

I want to use ospf6d but it is a bit rough around the edge.
A simple "ospf6ctl reload" makes it crash ! Come on, this is far from the
expected OpenBSD quality :p

I made a patch that make it crash less often. I still stumble on some "lost
interface" or "unknown interface" from time to time, lsa addresses are
often duplicated but it is better (from my point of view).

Concerning the duplicated lsa, this is what I get before reload :
orig_link_lsa: interface rl0
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::

And after reload :
orig_link_lsa: interface rl0
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: interface rl0
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: interface rl0
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::

The fix would be (I think, I am still investigating) to check for existing LSA
before adding a new one in ospfe_dispatch_main() (ospfe.c) when handling
IMSG_IFADDRNEW. Perhaps use a tree instead of a list ?

Would someone help me to give some love to ospf6d to improve it and make it
usable ?

Denis

Index: interface.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/interface.c,v
retrieving revision 1.22
diff -u -p -r1.22 interface.c
--- interface.c 27 Sep 2015 17:31:50 -  1.22
+++ interface.c 15 Dec 2015 16:40:33 -
@@ -170,8 +170,7 @@ int
 if_init(void)
 {
TAILQ_INIT();
-
-   return (fetchifs(0));
+   return (0);
 }
 
 /* XXX using a linked list should be OK for now */
Index: ospf6d.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
retrieving revision 1.29
diff -u -p -r1.29 ospf6d.c
--- ospf6d.c5 Dec 2015 13:12:41 -   1.29
+++ ospf6d.c15 Dec 2015 16:40:34 -
@@ -597,6 +597,7 @@ ospf_reload(void)
 {
struct area *area;
struct ospfd_conf   *xconf;
+   struct iface*iface;
 
if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL)
return (-1);
@@ -605,10 +606,16 @@ ospf_reload(void)
if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1)
return (-1);
 
-   /* send areas, interfaces happen out of band */
+   /* send areas & interfaces */
LIST_FOREACH(area, >area_list, entry) {
if (ospf_sendboth(IMSG_RECONF_AREA, area, sizeof(*area)) == -1)
return (-1);
+
+   LIST_FOREACH(iface, >iface_list, entry) {
+   if (ospf_sendboth(IMSG_RECONF_IFACE, iface,
+   sizeof(*iface)) == -1)
+   return (-1);
+   }
}
 
if (ospf_sendboth(IMSG_RECONF_END, NULL, 0) == -1)
Index: ospf6d.h
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
retrieving revision 1.29
diff -u -p -r1.29 ospf6d.h
--- ospf6d.h27 Sep 2015 17:31:50 -  1.29
+++ ospf6d.h15 Dec 2015 16:40:34 -
@@ -121,6 +121,7 @@ enum imsg_type {
IMSG_ABR_DOWN,
IMSG_RECONF_CONF,
IMSG_RECONF_AREA,
+   IMSG_RECONF_IFACE,
IMSG_RECONF_END,
IMSG_DEMOTE
 };
Index: ospfe.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospfe.c,v
retrieving revision 1.47
diff -u -p -r1.47 ospfe.c
--- ospfe.c 5 Dec 2015 13:12:41 -   1.47
+++ ospfe.c 15 Dec 2015 16:40:34 -
@@ -250,6 +250,7 @@ ospfe_dispatch_main(int fd, short event,
static struct area  *narea;
struct area *area;
struct iface*iface, *ifp;
+   struct iface*niface;
struct ifaddrchange *ifc;
struct iface_addr   *ia, *nia;
struct imsg  imsg;
@@ -388,6 +389,19 @@ ospfe_dispatch_main(int fd, short event,
RB_INIT(>lsa_tree);
 
LIST_INSERT_HEAD(>area_list, narea, entry);
+   break;
+   case IMSG_RECONF_IFACE:
+   if ((niface = malloc(sizeof(struct iface))) == NULL)
+   fatal(NULL);
+
+   memcpy(niface, imsg.data,