Re: [PATCH 1/9] batman-adv: add UNICAST_4ADDR packet type

2 messages, 2 authors, 2012-11-04 · open the first message on its own page

Re: [PATCH 1/9] batman-adv: add UNICAST_4ADDR packet type

From: Sven Eckelmann <hidden>
Date: 2012-11-04 10:29:29

On Saturday 03 November 2012 15:22:26 David Miller wrote:
[...]
Your packet layouts are very poorly designed and I want you to stop
and think seriously about things before extending things further.
You are right about the packet design/layout. But I also have some problems 
with the following statement. Maybe you can help me to resolve it.
All of this __packed stuff is a serious problem.

It means that on RISC system, fields such as your 32-bit sequence
number, will be read and written using byte loads and stores.

This is terrible.

Instead, design the structures so that they are full filled out to
at least 4 byte boundaries, so that they and the contents after
them, are 4 byte aligned too.

Then you won't need to mark all of your packet header structs
with __packed, and therefore the compiler can use full 32-bit
loads and stores to access 32-bit fields.
Ok, lets assume batman-adv has everything 4 byte aligned and we are running on 
a machine without unaligned r/w access. The machine may issues bus errors and 
so on.

Now also assume following really unusual situation: We get our data from a 
ethernet driver and the skb stores the ethernet header. The start of the 
ethernet header is perfectly aligned (4 or even 16 byte boundary aligned). The 
the header is 14/18 byte long (6 byte src, 6 byte dst, 2 byte ethertype and 
maybe 4 byte vlan). Now the payload starts only on a 2 byte boundary -> it is 
never 4 byte boundary aligned. A 32 bit read now causes different variations 
of problems (reminder: bus error).

This brings me to the question: Why should the "32 bit align everything 
relative to the start of the struct" approach help to resolve the situation 
for the access of 32 bit data structure members? May I am missing some 
information?

Kind regards,
	Sven

Re: [B.A.T.M.A.N.] [PATCH 1/9] batman-adv: add UNICAST_4ADDR packet type

From: David Miller <davem@davemloft.net>
Date: 2012-11-04 17:26:13

From: Sven Eckelmann <sven@narfation.org>
Date: Sun, 04 Nov 2012 11:29:29 +0100
Now also assume following really unusual situation: We get our data from a 
ethernet driver and the skb stores the ethernet header. The start of the 
ethernet header is perfectly aligned (4 or even 16 byte boundary aligned). The 
the header is 14/18 byte long (6 byte src, 6 byte dst, 2 byte ethertype and 
maybe 4 byte vlan). Now the payload starts only on a 2 byte boundary -> it is 
never 4 byte boundary aligned. A 32 bit read now causes different variations 
of problems (reminder: bus error).
Every ethernet driver must provide the networking stack with a modulo 2
aligned ethernet header, which makes sure that everything after the
ethernet header is at least 4 byte aligned.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help