Hello, The attached patch fixes two (big-)endianess issues in OSPF which lead bird to stuck in EXSTART state. Patch tested on a PPC machine against a Cisco router. Regards, Rani diff -ru bird-1.0.7/proto/ospf/lsalib.c bird-1.0.7-debian/proto/ospf/lsalib.c --- bird-1.0.7/proto/ospf/lsalib.c 2003-08-14 10:13:14.000000000 +0200 +++ bird-1.0.7-debian/proto/ospf/lsalib.c 2003-09-02 03:54:22.000000000 +0200 @@ -7,6 +7,7 @@ */ #include "ospf.h" +#include <stdlib.h> void flush_lsa(struct top_hash_entry *en, struct ospf_area *oa) @@ -376,7 +377,11 @@ y = 510 - c0 - x; if (y > 255) y -= 255; +#ifdef CPU_LITTLE_ENDIAN chsum= x + (y << 8); +#else + chsum= y + (x << 8); +#endif h->checksum = chsum; return chsum; } diff -ru bird-1.0.7/proto/ospf/ospf.h bird-1.0.7-debian/proto/ospf/ospf.h --- bird-1.0.7/proto/ospf/ospf.h 2003-08-14 10:13:14.000000000 +0200 +++ bird-1.0.7-debian/proto/ospf/ospf.h 2003-09-02 03:14:10.000000000 +0200 @@ -178,10 +178,17 @@ }; struct immsb { +#ifdef CPU_LITTLE_ENDIAN u8 ms:1; u8 m:1; u8 i:1; u8 padding:5; +#else + u8 padding:5; + u8 i:1; + u8 m:1; + u8 ms:1; +#endif }; union imms { @@ -223,10 +230,17 @@ }; struct vebb { +#ifdef CPU_LITTLE_ENDIAN u8 b:1; u8 e:1; u8 v:1; u8 padding:5; +#else + u8 padding:5; + u8 v:1; + u8 e:1; + u8 b:1; +#endif }; union veb {
Rani Assaf <rani@paname.org> writes:
Hello,
The attached patch fixes two (big-)endianess issues in OSPF which lead bird to stuck in EXSTART state.
Patch tested on a PPC machine against a Cisco router.
I think that preffered way would be to use correct ntoh(3) library call while receiving LSA packet. Still doesn't matter, because BIRD doesn't support multiple OSPF areas :-( (as far as i know).
Regards, Rani
diff -ru bird-1.0.7/proto/ospf/lsalib.c bird-1.0.7-debian/proto/ospf/lsalib.c --- bird-1.0.7/proto/ospf/lsalib.c 2003-08-14 10:13:14.000000000 +0200 +++ bird-1.0.7-debian/proto/ospf/lsalib.c 2003-09-02 03:54:22.000000000 +0200 @@ -7,6 +7,7 @@ */
#include "ospf.h" +#include <stdlib.h>
void flush_lsa(struct top_hash_entry *en, struct ospf_area *oa) @@ -376,7 +377,11 @@ y = 510 - c0 - x; if (y > 255) y -= 255;
+#ifdef CPU_LITTLE_ENDIAN chsum= x + (y << 8); +#else + chsum= y + (x << 8); +#endif h->checksum = chsum; return chsum; } diff -ru bird-1.0.7/proto/ospf/ospf.h bird-1.0.7-debian/proto/ospf/ospf.h --- bird-1.0.7/proto/ospf/ospf.h 2003-08-14 10:13:14.000000000 +0200 +++ bird-1.0.7-debian/proto/ospf/ospf.h 2003-09-02 03:14:10.000000000 +0200 @@ -178,10 +178,17 @@ };
struct immsb { +#ifdef CPU_LITTLE_ENDIAN u8 ms:1; u8 m:1; u8 i:1; u8 padding:5; +#else + u8 padding:5; + u8 i:1; + u8 m:1; + u8 ms:1; +#endif };
union imms { @@ -223,10 +230,17 @@ };
struct vebb { +#ifdef CPU_LITTLE_ENDIAN u8 b:1; u8 e:1; u8 v:1; u8 padding:5; +#else + u8 padding:5; + u8 v:1; + u8 e:1; + u8 b:1; +#endif };
union veb {
-- ------------------------------------------------------------------------- David Rohleder davro@ics.muni.cz Institute of Computer Science, Masaryk University Brno, Czech Republic -------------------------------------------------------------------------
Hello!
I think that preffered way would be to use correct ntoh(3) library call while receiving LSA packet. Still doesn't matter, because BIRD doesn't support multiple OSPF areas :-( (as far as i know).
| +#ifdef CPU_LITTLE_ENDIAN | chsum= x + (y << 8); | +#else | + chsum= y + (x << 8); | +#endif | h->checksum = chsum; Yes, htons should be here. | +#ifdef CPU_LITTLE_ENDIAN | u8 ms:1; | u8 m:1; | u8 i:1; | u8 padding:5; | +#else | + u8 padding:5; | + u8 i:1; | + u8 m:1; | + u8 ms:1; | +#endif | }; I don't really understand this piece -- either it's a piece of packet as seen on the wire and then it must not depend on CPU endianity or it's already converted to host order somewhere else and then the conversion is wrong. I'd understand such a construction if it were in a union with a longer integer field, but it doesn't seem to be the case here. Have a nice fortnight -- Martin `MJ' Mares <mj@ucw.cz> http://atrey.karlin.mff.cuni.cz/~mj/ Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth Weather forecast for tonight: dark.
Hello, On Sat, Sep 06, 2003 at 11:44:24AM +0200, Martin Mares wrote:
| +#ifdef CPU_LITTLE_ENDIAN | u8 ms:1; | u8 m:1; | u8 i:1; | u8 padding:5; | +#else | + u8 padding:5; | + u8 i:1; | + u8 m:1; | + u8 ms:1; | +#endif | };
I don't really understand this piece -- either it's a piece of packet as seen on the wire and then it must not depend on CPU endianity or it's already
It's a piece of packet coming on the wire... The problem is that bit ordering in a bit structure depends on the endianess of the host (at least for GCC): Please try the following program on x86 and PPC (with and without the "#ifdef BIG_ENDIAN"): #include <stdio.h> union mes_bits { struct { #ifdef BIG_ENDIAN unsigned char toto:2; unsigned char titi:1; unsigned char tata:5; #else unsigned char tata:5; unsigned char titi:1; unsigned char toto:2; #endif }; unsigned char data; }; int main(void) { union mes_bits bits; bits.toto = 1; bits.titi = 0; bits.tata = 2; printf("bits.data: 0x%.2x\n", bits.data); return 0; } Regards, Rani
On Sat, 6 Sep 2003, Martin Mares wrote:
Hello!
I think that preffered way would be to use correct ntoh(3) library call while receiving LSA packet. Still doesn't matter, because BIRD doesn't support multiple OSPF areas :-( (as far as i know).
| +#ifdef CPU_LITTLE_ENDIAN | chsum= x + (y << 8); | +#else | + chsum= y + (x << 8); | +#endif | h->checksum = chsum;
Yes, htons should be here.
This was fixed some other way. Krzysztof Szuster had a good idea. The checksum is calculated in network's endianity.
| +#ifdef CPU_LITTLE_ENDIAN | u8 ms:1; | u8 m:1; | u8 i:1; | u8 padding:5; | +#else | + u8 padding:5; | + u8 i:1; | + u8 m:1; | + u8 ms:1; | +#endif | };
I don't really understand this piece -- either it's a piece of packet as seen on the wire and then it must not depend on CPU endianity or it's already converted to host order somewhere else and then the conversion is wrong. I'd understand such a construction if it were in a union with a longer integer field, but it doesn't seem to be the case here.
I cannot agree. The bit ordering in structures _depends_ on the endianity. Please look into /usr/include/netinet/ip.h for examples. This structure is in the network's endianity. (The same problem is also with "struct immsb".) Feela
Have a nice fortnight
Hello!
I cannot agree. The bit ordering in structures _depends_ on the endianity. Please look into /usr/include/netinet/ip.h for examples. This structure is in the network's endianity. (The same problem is also with "struct immsb".)
Sorry, I completely overlooked that these are bit fields, not just ordinary structure members. For bit fields the suggested approach is really the best one I know. Have a nice fortnight -- Martin `MJ' Mares <mj@ucw.cz> http://atrey.karlin.mff.cuni.cz/~mj/ Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth To avoid bugs in your room, just keep Windows closed.
On Fri, 5 Sep 2003, David Rohleder wrote:
Rani Assaf <rani@paname.org> writes:
Hello,
The attached patch fixes two (big-)endianess issues in OSPF which lead bird to stuck in EXSTART state.
Patch tested on a PPC machine against a Cisco router.
I think that preffered way would be to use correct ntoh(3) library call while receiving LSA packet. Still doesn't matter, because BIRD
It's not so easy, try to look into the code.
doesn't support multiple OSPF areas :-( (as far as i know).
How do the two things correlate?
Regards, Rani
diff -ru bird-1.0.7/proto/ospf/lsalib.c bird-1.0.7-debian/proto/ospf/lsalib.c --- bird-1.0.7/proto/ospf/lsalib.c 2003-08-14 10:13:14.000000000 +0200 +++ bird-1.0.7-debian/proto/ospf/lsalib.c 2003-09-02 03:54:22.000000000 +0200 @@ -7,6 +7,7 @@ */
#include "ospf.h" +#include <stdlib.h>
void flush_lsa(struct top_hash_entry *en, struct ospf_area *oa) @@ -376,7 +377,11 @@ y = 510 - c0 - x; if (y > 255) y -= 255;
+#ifdef CPU_LITTLE_ENDIAN chsum= x + (y << 8); +#else + chsum= y + (x << 8); +#endif h->checksum = chsum; return chsum; } diff -ru bird-1.0.7/proto/ospf/ospf.h bird-1.0.7-debian/proto/ospf/ospf.h --- bird-1.0.7/proto/ospf/ospf.h 2003-08-14 10:13:14.000000000 +0200 +++ bird-1.0.7-debian/proto/ospf/ospf.h 2003-09-02 03:14:10.000000000 +0200 @@ -178,10 +178,17 @@ };
struct immsb { +#ifdef CPU_LITTLE_ENDIAN u8 ms:1; u8 m:1; u8 i:1; u8 padding:5; +#else + u8 padding:5; + u8 i:1; + u8 m:1; + u8 ms:1; +#endif };
union imms { @@ -223,10 +230,17 @@ };
struct vebb { +#ifdef CPU_LITTLE_ENDIAN u8 b:1; u8 e:1; u8 v:1; u8 padding:5; +#else + u8 padding:5; + u8 v:1; + u8 e:1; + u8 b:1; +#endif };
union veb {
Ondrej Feela Filip <feela@network.cz> writes:
On Fri, 5 Sep 2003, David Rohleder wrote:
Rani Assaf <rani@paname.org> writes:
Hello,
The attached patch fixes two (big-)endianess issues in OSPF which lead bird to stuck in EXSTART state.
Patch tested on a PPC machine against a Cisco router.
I think that preffered way would be to use correct ntoh(3) library call while receiving LSA packet. Still doesn't matter, because BIRD
It's not so easy, try to look into the code.
doesn't support multiple OSPF areas :-( (as far as i know).
How do the two things correlate?
A bit, just to mention, that BIRD can not be used as ABR. -- ------------------------------------------------------------------------- David Rohleder davro@ics.muni.cz Institute of Computer Science, Masaryk University Brno, Czech Republic -------------------------------------------------------------------------
participants (4)
-
David Rohleder -
Martin Mares -
Ondrej Feela Filip -
Rani Assaf