Re: [PATCH] [batman] Add padding around allocation debugger structures

On Friday 29 May 2009 16:00:40 Nathan Wharton wrote:
> >  <at> Nathan: Could you let me know if these patches work for you ? If so I'll
> > commit them.
> >
> > Regards,
> > Marek
>
> I set /proc/cpu/alignment to 4 (raise bus error) and I get a bus error:
>
> Program received signal SIGBUS, Bus error.
> list_add_tail (new=0x29368, head=0x28819) at list-batman.c:68
> 68              __list_add( new, head->prev, (struct list_head *)head );
> (gdb) l
> 63       * Insert a new entry before the specified head.
> 64       * This is useful for implementing queues.
> 65       */
> 66      void list_add_tail( struct list_head *new, struct
> list_head_first *head ) {
> 67
> 68              __list_add( new, head->prev, (struct list_head *)head );
> 69
> 70              head->prev = new;
> 71
> 72      }
Have you added the patches per hand? At this moment no patch I've made 
available in trunk. As you have run it with gdb, can you please append a full 
backtrace?

Best regards,
	Sven

Re: [PATCH] [batman] Add padding around allocation debugger structures

On Mon, Jun 1, 2009 at 11:44 AM, Sven Eckelmann <sven.eckelmann@...> wrote:
> On Friday 29 May 2009 16:00:40 Nathan Wharton wrote:
>> >  <at> Nathan: Could you let me know if these patches work for you ? If so I'll
>> > commit them.
>> >
>> > Regards,
>> > Marek
>>
>> I set /proc/cpu/alignment to 4 (raise bus error) and I get a bus error:
>>
>> Program received signal SIGBUS, Bus error.
>> list_add_tail (new=0x29368, head=0x28819) at list-batman.c:68
>> 68              __list_add( new, head->prev, (struct list_head *)head );
>> (gdb) l
>> 63       * Insert a new entry before the specified head.
>> 64       * This is useful for implementing queues.
>> 65       */
>> 66      void list_add_tail( struct list_head *new, struct
>> list_head_first *head ) {
>> 67
>> 68              __list_add( new, head->prev, (struct list_head *)head );
>> 69
>> 70              head->prev = new;
>> 71
>> 72      }
> Have you added the patches per hand? At this moment no patch I've made
> available in trunk. As you have run it with gdb, can you please append a full
> backtrace?
>
> Best regards,

Re: [PATCH] [batman] Add padding around allocation debugger structures

On Monday 01 June 2009 20:03:43 Nathan Wharton wrote:
> I had to copy the patches out of the e-mail.
>
> Here is the back trace:
> #0  list_add_tail (new=0x29bf0, head=0x298c9) at list-batman.c:68
> #1  0x0000ee7c in _hna_global_add (orig_node=0x29f80,
> hna_element=0x29ba8) at hna.c:371
> #2  0x0000f160 in hna_global_add (orig_node=0x29f80, new_hna=<value
> optimized out>, new_hna_len=<value optimized out>)
>     at hna.c:529
> #3  0x000099c8 in update_routes (orig_node=0x29f80,
> neigh_node=0x2a080, hna_recv_buff=0xbead1591 "\n\002\001",
>     hna_buff_len=10) at batman.c:377
> #4  0x0000c730 in update_orig (orig_node=0x29f80, in=0xbead157f,
> neigh=167772673, if_incoming=0x27678,
>     hna_recv_buff=0xbead1591 "\n\002\001", hna_buff_len=-16723,
> is_duplicate=0 '\0', curr_time=3199014207)
>     at originator.c:227
> #5  0x0000a7e0 in batman () at batman.c:956
> #6  0x000148d4 in main (argc=14, argv=0xbead1e14) at posix/posix.c:629
>
> Looks like debugMalloc didn't return an aligned value for head.  I'll
> step through that and see what I see.
Ok, I think I see the problem. The malloc returned a valid aligned adress. 
list_add_tail will get a pointer to an element in hna_global_entry. This 
structure is packed and all operations on it should be non-alignment safe. If 
you look at it further you will notice that orig_list is at position 9 
(assuming 4 bytes for a pointer) - which will not be aligned to 4 bytes of 
course.....
And here comes the problem: the compiler will only do the safe operations on 

Re: [PATCH] [batman] Add padding around allocation debugger structures

On Mon, Jun 1, 2009 at 2:35 PM, Sven Eckelmann <sven.eckelmann@...> wrote:
> Ok, I think I see the problem. The malloc returned a valid aligned adress.
> list_add_tail will get a pointer to an element in hna_global_entry. This
> structure is packed and all operations on it should be non-alignment safe. If
> you look at it further you will notice that orig_list is at position 9
> (assuming 4 bytes for a pointer) - which will not be aligned to 4 bytes of
> course.....
> And here comes the problem: the compiler will only do the safe operations on
> non-aligned data if it knows that it is not alignent. Since a cast is done by
> calling list_add_tail it will not know that this parameter is not aligned and
> the non-alignment bug will occur.
>
> So my question to marek: Is it really needed to have "struct hna_global_entry"
> packed in hna.h:57? If not then we should remove it and this problem should be
> gone. And what is with "struct hna_element".
>
> Thank you for your work, Nathan :)

You are welcome, thanks for your help.

The crashing of batgat on unloading turns out to be socket 4306 not
being ready to be reused yet.
I worked around this by using:
batmand -c -r 0 ; sleep 1 ; batmand -c -g 11000
and
batmand -c -g 0 ; sleep 1 ; batmand -c -r 2

Marek helped figure that out on irc.

Re: [PATCH] [batman] Add padding around allocation debugger structures

On Tuesday 02 June 2009 03:35:07 Sven Eckelmann wrote:
> So my question to marek: Is it really needed to have "struct
> hna_global_entry" packed in hna.h:57? If not then we should remove it and
> this problem should be gone. And what is with "struct hna_element".

The first 5 bytes of both structs are used as base for the hash index. If the 
compiler changes the order or something similar it might not work.

Regards,
Marek

Implementation of BATMAN routing metric on NS2

_______________________________________________
B.A.T.M.A.N mailing list
B.A.T.M.A.N@...
https://lists.open-mesh.net/mm/listinfo/b.a.t.m.a.n

Re: Implementation of BATMAN routing metric on NS2

On Tuesday 02 June 2009 09:20:02 Siphamandla Nxumalo wrote:
> I would like to know if there is any implementation of BATMAN roting metric
> that has been done on NS2.
Not as I would know. My information is taken from
 https://lists.open-mesh.net/pipermail/b.a.t.m.a.n/2009-April/002545.html
and other question on this list and irc.

Regards,
	Sven

_______________________________________________
B.A.T.M.A.N mailing list
B.A.T.M.A.N@...
https://lists.open-mesh.net/mm/listinfo/b.a.t.m.a.n

Re: Add padding around allocation debugger structures

On Tuesday 02 June 2009 06:36:41 Marek Lindner wrote:
> On Tuesday 02 June 2009 03:35:07 Sven Eckelmann wrote:
> > So my question to marek: Is it really needed to have "struct
> > hna_global_entry" packed in hna.h:57? If not then we should remove it and
> > this problem should be gone. And what is with "struct hna_element".
>
> The first 5 bytes of both structs are used as base for the hash index. If
> the compiler changes the order or something similar it might not work.
Ok, then it should be safe to force the alignment of the pointers in 
hna_global_entry. Everything else seems to be much more complicated and 
doesn't create much cleaner code.

Best regards,
	Sven

_______________________________________________
B.A.T.M.A.N mailing list
B.A.T.M.A.N@...
https://lists.open-mesh.net/mm/listinfo/b.a.t.m.a.n

[PATCH] [batman] Align pointers in hna list elements

Architectures like SuperARM or Xscale needs aligned data for multi-byte
operations. GCC can create instructions sequences for packed data, but
must know that something will not be aligned. Since list_add will
operate on untyped data over void-pointers it cannot know that
hna_global_entry is packed and will create only a fast and unsafe
version for load and store operations.
It is only important for the first 5 bytes of hna_global_entry to be
packed we can force these elements to be aligned without changing
the relative addresses of the first bytes.

Signed-off-by: Sven Eckelmann <sven.eckelmann@...>
---
 batman/hna.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/batman/hna.h b/batman/hna.h
index 6063324..3e7049e 100644
--- a/batman/hna.h
+++ b/batman/hna.h
 <at>  <at>  -58,8 +58,8  <at>  <at>  struct hna_global_entry
 {
 	uint32_t addr;
 	uint8_t netmask;
-	struct orig_node *curr_orig_node;
-	struct list_head_first orig_list;
+	struct orig_node *curr_orig_node ALIGN_WORD;
+	struct list_head_first orig_list ALIGN_WORD;
 } __attribute__((packed));

 struct hna_orig_ptr
--

-- 
1.6.3.1

Re: [PATCH] [batman] Align pointers in hna list elements

On Tue, Jun 2, 2009 at 12:56 PM, Sven Eckelmann <sven.eckelmann@...> wrote:
> Architectures like SuperARM or Xscale needs aligned data for multi-byte
> operations. GCC can create instructions sequences for packed data, but
> must know that something will not be aligned. Since list_add will
> operate on untyped data over void-pointers it cannot know that
> hna_global_entry is packed and will create only a fast and unsafe
> version for load and store operations.
> It is only important for the first 5 bytes of hna_global_entry to be
> packed we can force these elements to be aligned without changing
> the relative addresses of the first bytes.
>

It looks good here.  I am running this combined with the previous 3
patches with cpu/alignment set to bus error on problems.