All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, xemul@sw.ru, devel@openvz.org,
	bridge@lists.osdl.org
Subject: Re: [Bridge] [BRIDGE] Unaligned access on IA64 when comparing ethernet addresses
Date: Tue, 17 Apr 2007 23:24:36 +0200	[thread overview]
Message-ID: <46253B14.8030401@cosmosbay.com> (raw)
In-Reply-To: <20070417.140941.41631883.davem@davemloft.net>

David Miller a écrit :
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Tue, 17 Apr 2007 13:37:23 -0700
> 
>> The previous patch relied on the bridge id being aligned by
>> the compiler (which happens as a side effect). So please use
>> this instead.
>>
>> compare_ether_addr() implicitly requires that the addresses
>> passed are 2-bytes aligned in memory.
>>
>> This is not true for br_stp_change_bridge_id() and
>> br_stp_recalculate_bridge_id() in which one of the addresses
>> is unsigned char *, and thus may not be 2-bytes aligned.
>>
>> Signed-off-by: Evgeny Kravtsunov <emkravts@openvz.org>
>> Signed-off-by: Kirill Korotaev <dev@openvz.org>
>> Signed-off-by: Pavel Emelianov <xemul@openvz.org>
>> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> bridge_id would be aligned by luck, because it is composed of char's
> there is no explicit reason it should be aligned on at least an
> unsigned short boundary.
> 
> I like the other patch much better, it provided explicit alignment and
> is guarenteed to get rid of the problem.
> 
> Indeed, I wrote a test program on 32-bit Sparc to validate this:
> 
> struct bridge_id {
> 	unsigned char a[6];
> 	unsigned char b[6];
> };
> 
> extern void bar(unsigned char *, unsigned char *);
> 
> void foo(void)
> {
> 	unsigned char a;
> 	struct bridge_id b;
> 
> 	bar(&b.a[0], &a);
> }
> 
> foo() gets compiled like this:
> 
> foo:
> 	save	%sp, -120, %sp
> 	add	%fp, -21, %o0
> 	call	bar, 0
> 	 add	%fp, -9, %o1
> 	jmp	%i7+8
> 	 restore
> 
> See?  The bridge_id (passed in via %o0) is on an odd byte boundary
> on the stack.
> 
> So your patch isn't fixing the bug at all.
> 
> I'm going to apply the original patch, because that one will
> actually fix the problem and was actually tested on a system
> that saw the problem.

I suspect you missed part of Stephen patch :

(maybe some mailer problem...)

--- linux-2.6.orig/net/bridge/br_private.h	2007-04-17
13:26:48.000000000 -0700 +++ linux-2.6/net/bridge/br_private.h
2007-04-17 13:30:29.000000000 -0700 @@ -36,7 +36,7 @@
  {
  	unsigned char	prio[2];
  	unsigned char	addr[6];
-};
+} __attribute__((aligned(8)));

  struct mac_addr
  {


WARNING: multiple messages have this Message-ID (diff)
From: Eric Dumazet <dada1@cosmosbay.com>
To: David Miller <davem@davemloft.net>
Cc: shemminger@linux-foundation.org, xemul@sw.ru,
	netdev@vger.kernel.org, bridge@lists.osdl.org, devel@openvz.org
Subject: Re: [BRIDGE] Unaligned access on IA64 when comparing ethernet addresses
Date: Tue, 17 Apr 2007 23:24:36 +0200	[thread overview]
Message-ID: <46253B14.8030401@cosmosbay.com> (raw)
In-Reply-To: <20070417.140941.41631883.davem@davemloft.net>

David Miller a écrit :
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Tue, 17 Apr 2007 13:37:23 -0700
> 
>> The previous patch relied on the bridge id being aligned by
>> the compiler (which happens as a side effect). So please use
>> this instead.
>>
>> compare_ether_addr() implicitly requires that the addresses
>> passed are 2-bytes aligned in memory.
>>
>> This is not true for br_stp_change_bridge_id() and
>> br_stp_recalculate_bridge_id() in which one of the addresses
>> is unsigned char *, and thus may not be 2-bytes aligned.
>>
>> Signed-off-by: Evgeny Kravtsunov <emkravts@openvz.org>
>> Signed-off-by: Kirill Korotaev <dev@openvz.org>
>> Signed-off-by: Pavel Emelianov <xemul@openvz.org>
>> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> bridge_id would be aligned by luck, because it is composed of char's
> there is no explicit reason it should be aligned on at least an
> unsigned short boundary.
> 
> I like the other patch much better, it provided explicit alignment and
> is guarenteed to get rid of the problem.
> 
> Indeed, I wrote a test program on 32-bit Sparc to validate this:
> 
> struct bridge_id {
> 	unsigned char a[6];
> 	unsigned char b[6];
> };
> 
> extern void bar(unsigned char *, unsigned char *);
> 
> void foo(void)
> {
> 	unsigned char a;
> 	struct bridge_id b;
> 
> 	bar(&b.a[0], &a);
> }
> 
> foo() gets compiled like this:
> 
> foo:
> 	save	%sp, -120, %sp
> 	add	%fp, -21, %o0
> 	call	bar, 0
> 	 add	%fp, -9, %o1
> 	jmp	%i7+8
> 	 restore
> 
> See?  The bridge_id (passed in via %o0) is on an odd byte boundary
> on the stack.
> 
> So your patch isn't fixing the bug at all.
> 
> I'm going to apply the original patch, because that one will
> actually fix the problem and was actually tested on a system
> that saw the problem.

I suspect you missed part of Stephen patch :

(maybe some mailer problem...)

--- linux-2.6.orig/net/bridge/br_private.h	2007-04-17
13:26:48.000000000 -0700 +++ linux-2.6/net/bridge/br_private.h
2007-04-17 13:30:29.000000000 -0700 @@ -36,7 +36,7 @@
  {
  	unsigned char	prio[2];
  	unsigned char	addr[6];
-};
+} __attribute__((aligned(8)));

  struct mac_addr
  {


  reply	other threads:[~2007-04-17 21:24 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-17 11:49 [Bridge] [BRIDGE] Unaligned access on IA64 when comparing ethernet addresses Pavel Emelianov
2007-04-17 11:49 ` Pavel Emelianov
2007-04-17 19:31 ` [Bridge] " David Miller
2007-04-17 19:31   ` David Miller
2007-04-17 19:55   ` [Bridge] " Stephen Hemminger
2007-04-17 19:55     ` Stephen Hemminger
2007-04-17 20:09     ` [Bridge] " David Miller
2007-04-17 20:09       ` David Miller
2007-04-17 20:37       ` [Bridge] " Stephen Hemminger
2007-04-17 20:37         ` Stephen Hemminger
2007-04-17 21:09         ` [Bridge] " David Miller
2007-04-17 21:09           ` David Miller
2007-04-17 21:24           ` Eric Dumazet [this message]
2007-04-17 21:24             ` Eric Dumazet
2007-04-17 21:27             ` [Bridge] " David Miller
2007-04-17 21:27               ` David Miller
2007-04-18  6:43         ` [Bridge] " Pavel Emelianov
2007-04-18  6:43           ` Pavel Emelianov
2007-04-18  8:28           ` [Bridge] " David Miller
2007-04-18  8:28             ` David Miller
2007-04-18  8:37             ` [Bridge] " Pavel Emelianov
2007-04-18  8:37               ` Pavel Emelianov
2007-04-18 14:44             ` [Bridge] " Stephen Hemminger
2007-04-18 14:44               ` Stephen Hemminger
2007-04-18 20:04               ` [Bridge] " David Miller
2007-04-18 20:04                 ` David Miller
2007-04-19 14:14                 ` Eric Dumazet
2007-04-19 14:14                   ` Eric Dumazet
2007-04-19 18:18                   ` Stephen Hemminger
2007-04-19 18:18                     ` Stephen Hemminger
2007-04-19 20:01                   ` [BRIDGE] " David Miller
2007-04-19 20:01                     ` David Miller
2007-04-19 20:29                     ` Eric Dumazet
2007-04-19 20:29                       ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46253B14.8030401@cosmosbay.com \
    --to=dada1@cosmosbay.com \
    --cc=bridge@lists.osdl.org \
    --cc=davem@davemloft.net \
    --cc=devel@openvz.org \
    --cc=netdev@vger.kernel.org \
    --cc=xemul@sw.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.