Mailing List Archive

Hi list,



I was very excited when I discovered "Quagga" last week (I new only
Zebra and isisd). Then, I decided to test its IS-IS engine in front of
Ciscos and Junipers. Unfortunately, there were problems to establish
adjacencies and to correctly exchange LSPs. So, I checked your code and
propose a few patches below fixing these points:



- suppress undeclared command handlers for isisd to compile;

- change the ISIS Hello interval from 1 to 10 (cisco's default value) -
cosmetic;

- suppress a bad "#if 0 #endif" block for isis to be activated on
startup - medium;

- error in same subnet comparison (the previous algorithm only worked
when netmask % 8 == 0 - serious;

- error in password checking (a bug in the matching function and an
other one for SNPs handling - by default on IOS there's no password
transmission/checking in SNPs - serious;

- password memorization (to store and retrieve it from the config file
without to retype it each time youu start isisd)- medium;

- LSP recognition of the ISIS Graceful Restart LSP (not implemented yet)
- cosmetic ;

- Error in the checksum calculation. The previous algorithm could
produce a bad checksum if the 2 complement's vs 1 complement's
adaptation was required. It is the algorithm I've implemented on
Ethereal and successfully tested with thousands of Ciscos LSPs -
serious.



I don't know whether you prefer attached or "embedded" patches. I
propose the latter but of course, I can provide different files if it's
better for the list maintainer.



Best regards

Laurent





------------------------------------------------------------------------
-------------



diff -uwb ../quagga/isisd/isis_circuit.c isisd/isis_circuit.c

--- ../quagga/isisd/isis_circuit.c Thu Jan 29 16:57:29 2004

+++ isisd/isis_circuit.c Wed Jan 28 09:38:23 2004

@@ -2148,9 +2149,10 @@



install_element (INTERFACE_NODE, &isis_hello_cmd);

install_element (INTERFACE_NODE, &no_isis_hello_cmd);



+#if 0

install_element (INTERFACE_NODE, &ip_address_cmd);

install_element (INTERFACE_NODE, &no_ip_address_cmd);

+#endif



install_element (INTERFACE_NODE, &csnp_interval_cmd);

install_element (INTERFACE_NODE, &no_csnp_interval_cmd);





------------------------------------------------------------------------
-------------





diff -uwb ../quagga/isisd/isis_constants.h isisd/isis_constants.h

--- ../quagga/isisd/isis_constants.h Thu Jan 29 16:57:29 2004

+++ isisd/isis_constants.h Wed Jan 28 12:06:44 2004

@@ -65,7 +65,8 @@

#define ISIS_LEVEL1 1

#define ISIS_LEVEL2 2



-#define HELLO_INTERVAL 1

+/* HELLO INTERVAL should be set to 10 instead of 1 */

+#define HELLO_INTERVAL 10

#define HELLO_MINIMAL HELLO_INTERVAL

#define HELLO_MULTIPLIER 3

#define DEFAULT_PRIORITY 64





------------------------------------------------------------------------
-------------





diff -uwb ../quagga/isisd/isis_main.c isisd/isis_main.c

--- ../quagga/isisd/isis_main.c Thu Jan 29 16:57:31 2004

+++ isisd/isis_main.c Thu Jan 29 13:38:48 2004

@@ -303,11 +303,10 @@



/* parse config file */

/* this is needed three times! because we have interfaces before the
areas */

+ /* Please, do not comment out any vty_read_config call!! */

vty_read_config (config_file, config_current, config_default);

-#if 0

vty_read_config (config_file, config_current, config_default);

vty_read_config (config_file, config_current, config_default);

-#endif

/* demonize */

if (daemon_mode)

daemon (0, 0);





------------------------------------------------------------------------
-------------







diff -uwb ../quagga/isisd/isis_pdu.c isisd/isis_pdu.c

--- ../quagga/isisd/isis_pdu.c Thu Jan 29 16:57:32 2004

+++ isisd/isis_pdu.c Thu Jan 29 12:27:09 2004

@@ -106,7 +106,7 @@

ip_same_subnet (struct prefix_ipv4 *ip1, struct in_addr *ip2) {

u_char *addr1, *addr2;

- int shift, offset;

+ int shift, offset, offsetloop;

int len;



addr1 = (u_char *) &ip1->prefix.s_addr;

@@ -114,10 +114,10 @@

len = ip1->prefixlen;



shift = len % PNBBY;

- offset = len / PNBBY;

+ offsetloop = offset = len / PNBBY;



- while (offset--) {

- if (addr1[offset] != addr2[offset]) {

+ while (offsetloop--) {

+ if (addr1[offsetloop] != addr2[offsetloop]) {

return 0;

}

}

@@ -177,8 +177,9 @@

}

switch (one->type) {

case ISIS_PASSWD_TYPE_CLEARTXT:

- if (one->len != theother->len)

+ if (one->len != theother->len) {

return 1; /* Auth fail () - passwd len mismatch */

+ }

return memcmp (one->passwd, theother->passwd, one->len);

break;

default:

@@ -715,7 +716,6 @@

goto out;

}





adj = isis_adj_lookup (hdr.source_id, circuit->u.bc.adjdb[level -
1]);

if (!adj) {

/*

@@ -1263,6 +1263,13 @@

return retval;

}



+#if 0

+ /* FIXME: Authentication in LSPs does not mean authentication in
SNPs...

+ * In fact by default IOS only deals with LSPs authentication!!

+ * To force authentication in SNPs, one

+ * must specify the 'authenticate snp' command after 'area-password
WORD' or

+ * 'domain-password WORD'. This command is not supported for the
moment.

+ */

(level == 1) ? (passwd = &circuit->area->area_passwd) :

(passwd = &circuit->area->domain_passwd);

if (passwd->type) {

@@ -1274,6 +1281,7 @@

return ISIS_OK;

}

}

+#endif



/* debug isis snp-packets */

if (isis->debugs & DEBUG_SNP_PACKETS) {





------------------------------------------------------------------------
-------------





diff -uwb ../quagga/isisd/isis_tlv.c isisd/isis_tlv.c

--- ../quagga/isisd/isis_tlv.c Thu Jan 29 16:57:33 2004

+++ isisd/isis_tlv.c Thu Jan 29 12:18:48 2004

@@ -419,6 +419,7 @@

#endif

if (*expected & TLVFLAG_AUTH_INFO) {

tlvs->auth_info.type = *pnt;

+ tlvs->auth_info.len = length-1;

pnt++;

memcpy (tlvs->auth_info.passwd, pnt, length - 1);

pnt += length - 1;

@@ -653,6 +654,22 @@



break;



+ case GRACEFUL_RESTART:

+
/*+-------+-------+-------+-------+-------+-------+-------+-------+

+ *| Reserved | SA | RA | RR
| 1

+
*+-------+-------+-------+-------+-------+-------+-------+-------+

+ *| Remaining Time
| 2

+
*+---------------------------------------------------------------+

+ *| Restarting Neighbor ID (If known)
| 0-8

+
*+---------------------------------------------------------------+

+ */

+ *found |= TLVFLAG_GRACEFUL_RESTART;

+ if (*expected & TLVFLAG_GRACEFUL_RESTART) {

+ /* FIXME: make this work */

+ }

+ pnt += length;

+ break;

+

default:

zlog_warn ("ISIS-TLV (%s): unsupported TLV type %d, length %d",

areatag,

@@ -801,7 +818,7 @@

{

u_char value[255];

u_char *pos = value;

- pos++;

+ *pos++ = ISIS_PASSWD_TYPE_CLEARTXT;

memcpy (pos, auth_value, auth_len);



return add_tlv (AUTH_INFO, auth_len + 1, value, stream);





------------------------------------------------------------------------
-------------





diff -uwb ../quagga/isisd/isis_tlv.h isisd/isis_tlv.h

--- ../quagga/isisd/isis_tlv.h Thu Jan 29 16:57:33 2004

+++ isisd/isis_tlv.h Wed Jan 28 15:38:09 2004

@@ -27,7 +27,7 @@

/*

* Structures found in TLV's.

* this header is fully complient with

- * draft-ietf-isis-wg-tlv-codepoints-02.txt

+ * RFC 3359

1. TLV Codepoints reserved

____________________________________________________

Name Value IIH LSP SNP Status

@@ -43,6 +43,7 @@

LSP Entries 9 n n y ISO 10589

Authentication 10 y y y ISO 10589

Opt. Checksum 12 y n y IETF-draft

+ Lucent Proprietary 66 n y n

LSPBufferSize 14 n y n ISO 10589 Rev 2 Draft

TE IIS Neigh. 22 n y n IETF-draft

DECnet Phase IV 42 y n n DEC (ancient)

@@ -87,6 +88,7 @@

#define TE_ROUTER_ID 134

#define TE_IPV4_REACHABILITY 135

#define DYNAMIC_HOSTNAME 137

+#define GRACEFUL_RESTART 211

#define IPV6_REACHABILITY 236

#define IPV6_ADDR 232

#define WAY3_HELLO 240

@@ -236,6 +238,7 @@

#define TLVFLAG_3WAY_HELLO (1<<18)

#define TLVFLAG_TE_ROUTER_ID (1<<19)

#define TLVFLAG_CHECKSUM (1<<20)

+#define TLVFLAG_GRACEFUL_RESTART (1<<21)



void init_tlvs (struct tlvs *tlvs, uint32_t expected);

void free_tlvs (struct tlvs *tlvs);





------------------------------------------------------------------------
-------------





diff -uwb ../quagga/isisd/isisd.c isisd/isisd.c

--- ../quagga/isisd/isisd.c Thu Jan 29 16:57:34 2004

+++ isisd/isisd.c Thu Jan 29 11:45:07 2004

@@ -1114,7 +1114,7 @@

}

area->domain_passwd.len = (u_char)len;

area->domain_passwd.type = ISIS_PASSWD_TYPE_CLEARTXT;

- strncpy (area->domain_passwd.passwd, argv[0], 255);

+ strcpy(area->domain_passwd.passwd, argv[0]);



return CMD_SUCCESS;

}

@@ -1912,6 +1912,16 @@

write ++;

}

}

+ if (area->area_passwd.len > 0) {

+ vty_out(vty, " area-password %s%s",

+ area->area_passwd.passwd, VTY_NEWLINE);

+ write++;

+ }

+ if (area->domain_passwd.len > 0) {

+ vty_out(vty, " domain-password %s%s",

+ area->domain_passwd.passwd, VTY_NEWLINE);

+ write++;

+ }

#ifdef TOPOLOGY_GENERATE

/* seems we save the whole command line here */

if (area->top_params) {





------------------------------------------------------------------------
-------------





diff -uwb ../quagga/isisd/iso_checksum.c isisd/iso_checksum.c

--- ../quagga/isisd/iso_checksum.c Thu Jan 29 16:57:34 2004

+++ isisd/iso_checksum.c Wed Jan 28 14:30:08 2004

@@ -42,7 +42,6 @@

* Verifies that the checksum is correct.

* Return 0 on correct and 1 on invalid checksum.

* Based on Annex C.4 of ISO/IEC 8473

- * FIXME: Check for overflow

*/



int

@@ -52,7 +51,7 @@

u_int32_t c0;

u_int32_t c1;

u_int16_t checksum;

- int i;

+ int i, partial_len;



p = buffer;

checksum = 0;

@@ -77,7 +76,10 @@

c0 = 0;

c1 = 0;



- for (i = 0; i < len; i++) {

+

+ while (len) {

+ partial_len = MIN(len, 5803);

+ for (i = 0; i < partial_len; i++) {

c0 = c0 + *(p++);

c1 += c0;

}

@@ -85,6 +87,9 @@

c0 = c0 % 255;

c1 = c1 % 255;



+ len -= partial_len;

+ }

+

if ( c0 == 0 && c1 == 0)

return 0;



@@ -96,9 +101,6 @@

* Creates the checksum. *csum points to the position of the checksum
in the

* PDU.

* Based on Annex C.4 of ISO/IEC 8473

- * we will not overflow until about length of 6000,

- * which is the answer to (255+255n)*n/2 > 2^32

- * so if we have a length of over 5000 we will return zero (for now)

*/

#define FIXED_CODE

u_int16_t

@@ -113,26 +115,23 @@

u_int32_t c1;

u_int16_t checksum;

u_int16_t *csum;

- int i;

+ int i, init_len, partial_len;



checksum = 0;

-

/*

* Zero the csum in the packet.

*/

csum = (u_int16_t*)(buffer + n);

*(csum) = checksum;



- /* for the limitation of our implementation */

- if (len > 5000) {

- return 0;

- }

-

p = buffer;

c0 = 0;

c1 = 0;

+ init_len = len;



- for (i = 0; i < len; i++) {

+ while (len != 0) {

+ partial_len = MIN(len, 5803);

+ for (i = 0; i < partial_len; i++) {

c0 = c0 + *(p++);

c1 += c0;

}

@@ -140,24 +139,25 @@

c0 = c0 % 255;

c1 = c1 % 255;



- mul = (len - n)*(c0);

+ len -=partial_len;

+ }

+

+ mul = (init_len - n)*(c0);



#ifdef FIXED_CODE

x = mul - c0 - c1;

y = c1 - mul - 1;



- if ( y >= 0 ) y++;

+ if ( y > 0 ) y++;

if ( x < 0 ) x--;



x %= 255;

y %= 255;



if (x == 0) x = 255;

- if (y == 0) y = 255;

-

- x &= 0x00FF;

+ if (y == 0) y = 1;



- checksum = ((y << 8) | x);

+ checksum = (y << 8) | (x & 0xFF);



#else

x = mul - c0 - c1;