Mailing List Archive

quagga bug fixes
I ran into some problems while trying to get quagga up and running
on Solaris, specifically when I have the line "interface ip.tun0" in zebra.conf,
start up zebra (zebra will create an interface structure for ip.tun0 with index
-1) and then plumb/add the ip.tun0 interface (after zebra is runninga),
I found that:
- ifm_read does a lookup by index, which is misleading for the situation
above.
- if_cmp_func can run into an infinite loop if ifp1->name == ifp2->name
- listnode_add_sort can't correctly handle the case when list->cmp returns 0
- the problems described in [quagga-dev 225]

Here's a patch (attached) that I believe will address the problems.

--Sowmini

===================================================================
RCS file: lib/if.c,v
retrieving revision 1.11
diff -u -r1.11 lib/if.c
--- /tmp/T013aarv Mon Sep 8 13:03:58 2003
+++ lib/if.c Mon Sep 8 12:56:42 2003
@@ -59,7 +59,7 @@
p1 = ifp1->name;
p2 = ifp2->name;

- while (1) {
+ while (*p1 && *p2) {
/* look up to any number */
l1 = strcspn(p1, "0123456789");
l2 = strcspn(p2, "0123456789");
@@ -91,6 +91,11 @@
/* numbers were equal, lets do it again..
(it happens with name like "eth123.456:789") */
}
+ if (*p1)
+ return 1;
+ if (*p2)
+ return -1;
+ return 0;
}

/* Create new interface structure. */
===================================================================
RCS file: lib/linklist.c,v
retrieving revision 1.2
diff -u -r1.2 lib/linklist.c
--- /tmp/T0T4ayrv Mon Sep 8 13:03:58 2003
+++ lib/linklist.c Mon Sep 8 12:56:51 2003
@@ -87,15 +87,17 @@
struct listnode *n;
struct listnode *new;

- new = listnode_new ();
- new->data = val;

if (list->cmp)
{
for (n = list->head; n; n = n->next)
{
+ if ((*list->cmp) (val, n->data) == 0)
+ return;
if ((*list->cmp) (val, n->data) < 0)
{
+ new = listnode_new ();
+ new->data = val;
new->next = n;
new->prev = n->prev;

@@ -110,6 +112,8 @@
}
}

+ new = listnode_new ();
+ new->data = val;
new->prev = list->tail;

if (list->tail)
===================================================================
RCS file: ripd/rip_interface.c,v
retrieving revision 1.10
diff -u -r1.10 ripd/rip_interface.c
--- /tmp/T0H5aWrv Mon Sep 8 13:03:58 2003
+++ ripd/rip_interface.c Mon Sep 8 12:57:04 2003
@@ -700,6 +700,7 @@
zlog_info ("connected address %s/%d is added",
inet_ntoa (p->u.prefix4), p->prefixlen);

+ rip_enable_apply(ifc->ifp);
/* Check if this prefix needs to be redistributed */
rip_apply_address_add(ifc);

===================================================================
RCS file: zebra/kernel_socket.c,v
retrieving revision 1.8
diff -u -r1.8 zebra/kernel_socket.c
--- /tmp/T0Q6aisv Mon Sep 8 13:03:58 2003
+++ zebra/kernel_socket.c Mon Sep 8 12:56:23 2003
@@ -48,7 +48,13 @@
#ifdef HAVE_SA_LEN
#define WRAPUP(X) ROUNDUP(((struct sockaddr *)(X))->sa_len)
#else
-#define WRAPUP(X) ROUNDUP(sizeof (struct sockaddr))
+#define WRAPUP(X) \
+ (((struct sockaddr *)(X))->sa_family == AF_INET ? \
+ ROUNDUP(sizeof(struct sockaddr_in)):\
+ (((struct sockaddr *)(X))->sa_family == AF_INET6 ? \
+ ROUNDUP(sizeof(struct sockaddr_in6)) : \
+ (((struct sockaddr *)(X))->sa_family == AF_LINK ? \
+ ROUNDUP(sizeof(struct sockaddr_dl)) : sizeof(struct sockaddr))))
#endif /* HAVE_SA_LEN */

/* Routing socket message types. */
@@ -201,15 +207,55 @@
int
ifm_read (struct if_msghdr *ifm)
{
- struct interface *ifp;
+ struct interface *ifp = NULL;
struct sockaddr_dl *sdl = NULL;
+ char ifname[IFNAMSIZ];

+#ifdef SUNOS_5
+ int i;
+ struct sockaddr *sa;
+ u_char *cp = (u_char *)(ifm + 1);
+
+ /*
+ * if_msghdr_t on 64 bit kernels in Solaris 9 and earlier versions
+ * is 12 bytes larger than the 32 bit version, so make adjustment
+ * here.
+ */
+ sa = (struct sockaddr *)cp;
+ if (sa->sa_family == AF_UNSPEC)
+ cp = cp + 12;
+
+ for (i = 1; i != 0; i <<= 1)
+ {
+ if (i & ifm->ifm_addrs)
+ {
+ sa = (struct sockaddr *)cp;
+ cp += WRAPUP(sa);
+ if (i & RTA_IFP)
+ {
+ sdl = (struct sockaddr_dl *)sa;
+ break;
+ }
+ }
+ }
+#else
sdl = (struct sockaddr_dl *)(ifm + 1);
+#endif

- /* Use sdl index. */
- ifp = if_lookup_by_index (ifm->ifm_index);
+ /*
+ * Check if ifp already exists. If the interface has already been specified
+ * in the conf file, but is just getting created, we would have an
+ * entry in the iflist with incomplete data (e.g., ifindex == -1),
+ * so we lookup on name.
+ */
+ if (sdl != NULL)
+ {
+ bcopy(sdl->sdl_data, ifname, sdl->sdl_nlen);
+ ifname[sdl->sdl_nlen] = '\0';
+ ifp = if_lookup_by_name (ifname);
+ }

- if (ifp == NULL)
+ if ((ifp == NULL) || (ifp->ifindex == -1))
{
/* Check interface's address.*/
if (! (ifm->ifm_addrs & RTA_IFP))
@@ -219,7 +265,8 @@
return -1;
}

- ifp = if_create (sdl->sdl_data, sdl->sdl_nlen);
+ if (ifp == NULL)
+ ifp = if_create (sdl->sdl_data, sdl->sdl_nlen);

ifp->ifindex = ifm->ifm_index;
ifp->flags = ifm->ifm_flags;
Re: quagga bug fixes [ In reply to ]
On Mon, 8 Sep 2003 sowmini.varadhan@sun.com wrote:

> I ran into some problems while trying to get quagga up and running
> on Solaris, specifically when I have the line "interface ip.tun0"
> in zebra.conf, start up zebra (zebra will create an interface
> structure for ip.tun0 with index -1) and then plumb/add the ip.tun0
> interface (after zebra is runninga), I found that:

> - ifm_read does a lookup by index, which is misleading for the situation
> above.

> - if_cmp_func can run into an infinite loop if ifp1->name == ifp2->name

ouch.

> - listnode_add_sort can't correctly handle the case when list->cmp returns 0

ouch yes.

> - the problems described in [quagga-dev 225]

ok. will apply lib/if.c and lib/linklist.c now, will look at [225]
and reply again.

> Here's a patch (attached) that I believe will address the problems.

thank you very much!

> --Sowmini

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
A rock store eventually closed down; they were taking too much for granite.