Mailing List Archive: PATCH: fix rip_peer_list

PATCH: fix rip_peer_list_cmp()

Nov 19, 2003, 3:46 PM

Post #1 of 3 (690 views)

Hello,

Around the 0.96.3 relase of quagga the listnode_add_sort() function from
the lib/linklist.c file was changed. Now, it has one additional check:
+ if ((*list->cmp) (val, n->data) == 0)
+ return;

Current rip_peer_list_cmp() function looks like this:

int
rip_peer_list_cmp (struct rip_peer *p1, struct rip_peer *p2)
{
return htonl (p1->addr.s_addr) > htonl (p2->addr.s_addr);
}

So, if first RIP packet comes from for example 192.168.0.3 then peers
with lower IPs (ex. 192.168.0.2, 192.168.0.1) will not be added. This
patch fix this, changing the rip_peer_list_cmp() function to one which
really compares p1 and p2, like other "cmp" functions:

diff -Nur quagga-0.96.4-orig/ripd/rip_peer.c quagga-0.96.4/ripd/rip_peer.c
--- quagga-0.96.4-orig/ripd/rip_peer.c 2002-12-13 21:15:30.000000000 +0100
+++ quagga-0.96.4/ripd/rip_peer.c 2003-11-19 23:25:44.000000000 +0100
@@ -200,7 +200,13 @@
int
rip_peer_list_cmp (struct rip_peer *p1, struct rip_peer *p2)
{
- return htonl (p1->addr.s_addr) > htonl (p2->addr.s_addr);
+ if (htonl (p1->addr.s_addr) > htonl (p2->addr.s_addr))
+ return 1;
+
+ if (htonl (p1->addr.s_addr) < htonl (p2->addr.s_addr))
+ return -1;
+
+ return 0;
}

void

With this patch "show ip rip status" works now as expected, without
missing peers.

Best regards,

Krzysztof Olêdzki

Re: PATCH: fix rip_peer_list_cmp() [ In reply to ]

jardin at 6wind

Nov 20, 2003, 1:01 AM

Post #2 of 3 (665 views)

Permalink

If you are right, there is the same bug with
int
ripng_peer_list_cmp (struct ripng_peer *p1, struct ripng_peer *p2)
{
return addr6_cmp(&p1->addr, &p2->addr) > 0;
}

Why was listnode_add_sort() changed ?

Vincent

PS:
/***
* 1 if A > B
* 0 if A = B
* -1 if A < B
**/
static inline int
addr6_cmp(struct in6_addr *A, struct in6_addr *B) {

#ifndef s6_addr32
#define s6_addr32 __u6_addr.__u6_addr32
#endif /*s6_addr32*/

#define a(i) A->s6_addr32[i]
#define b(i) B->s6_addr32[i]

if (a(3) > b(3))
return 1;
else if ((a(3) == b(3)) && (a(2) > b(2)))
return 1;
else if ((a(3) == b(3)) && (a(2) == b(2)) && (a(1) > b(1)))
return 1;
else if ((a(3) == b(3)) && (a(2) == b(2)) && (a(1) == b(1)) && (a(0) >
b(0)))
return 1;

if ((a(3) == b(3)) && (a(2) == b(2)) && (a(1) == b(1)) && (a(0) ==
b(0)))
return 0;

return -1;
}

On Wed, 19 Nov 2003, Krzysztof Oledzki wrote:

> Hello,
>
> Around the 0.96.3 relase of quagga the listnode_add_sort() function from
> the lib/linklist.c file was changed. Now, it has one additional check:
> + if ((*list->cmp) (val, n->data) == 0)
> + return;
>
> Current rip_peer_list_cmp() function looks like this:
>
> int
> rip_peer_list_cmp (struct rip_peer *p1, struct rip_peer *p2)
> {
> return htonl (p1->addr.s_addr) > htonl (p2->addr.s_addr);
> }
>
> So, if first RIP packet comes from for example 192.168.0.3 then peers
> with lower IPs (ex. 192.168.0.2, 192.168.0.1) will not be added. This
> patch fix this, changing the rip_peer_list_cmp() function to one which
> really compares p1 and p2, like other "cmp" functions:
>
> diff -Nur quagga-0.96.4-orig/ripd/rip_peer.c quagga-0.96.4/ripd/rip_peer.c
> --- quagga-0.96.4-orig/ripd/rip_peer.c 2002-12-13 21:15:30.000000000 +0100
> +++ quagga-0.96.4/ripd/rip_peer.c 2003-11-19 23:25:44.000000000 +0100
> @@ -200,7 +200,13 @@
> int
> rip_peer_list_cmp (struct rip_peer *p1, struct rip_peer *p2)
> {
> - return htonl (p1->addr.s_addr) > htonl (p2->addr.s_addr);
> + if (htonl (p1->addr.s_addr) > htonl (p2->addr.s_addr))
> + return 1;
> +
> + if (htonl (p1->addr.s_addr) < htonl (p2->addr.s_addr))
> + return -1;
> +
> + return 0;
> }
>
> void
>
> With this patch "show ip rip status" works now as expected, without
> missing peers.
>
> Best regards,
>
> Krzysztof Olêdzki

Re: PATCH: fix rip_peer_list_cmp() [ In reply to ]

paul at clubi

Nov 20, 2003, 1:41 AM

Post #3 of 3 (675 views)

Permalink

On Thu, 20 Nov 2003, Vincent.Jardin wrote:

> If you are right, there is the same bug with
> int
> ripng_peer_list_cmp (struct ripng_peer *p1, struct ripng_peer *p2)
> {
> return addr6_cmp(&p1->addr, &p2->addr) > 0;
> }

ah, the > 0 needs removing.

> Why was listnode_add_sort() changed ?

See [quagga-dev 227], it didnt check for the case where comparator
returned 0 (equal).

> Vincent

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
He that would govern others, first should be the master of himself.

Mailing List Archive

Mailing List Archive

Attached Files: