[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20111114.020749.1176229013272872343.davem@davemloft.net>
Date: Mon, 14 Nov 2011 02:07:49 -0500 (EST)
From: David Miller <davem@...emloft.net>
To: Wolfgang.Fritz@...mile.com
Cc: holger.brunck@...mile.com, netdev@...r.kernel.org,
shemminger@...ux-foundation.org, bridge@...ts.linux-foundation.org
Subject: Re: [PATCH] net, bridge: print log message after state changed
From: "Fritz, Wolfgang" <Wolfgang.Fritz@...mile.com>
Date: Mon, 14 Nov 2011 08:03:31 +0100
>
> Am Montag, den 14.11.2011, 00:37 -0500 schrieb David Miller:
>> From: Holger Brunck <holger.brunck@...mile.com>
>> Date: Thu, 10 Nov 2011 17:18:54 +0100
>>
>> > From: Wolfgang Fritz <wolfgang.fritz@...mile.com>
>> >
>> > Function br_log_state writes log message "... entering XXXX state" so it
>> > should be called after the state has changed and not before.
>> >
>> > Signed-off-by: Wolfgang Fritz <wolfgang.fritz@...mile.com>
>> > Signed-off-by: Holger Brunck <holger.brunck@...mile.com>
>>
>> "entering" can roughly mean "about to enter"
>>
>
> Exactly.
>
>> The message is therefore appropriately timed as far as I'm concerned.
>>
>
> It's not.
>
> Please test: create a bridge with STP disabled, add an interface to the
> bridge and set that interface down. You get the message "... entering
> forwarding state". That's wrong because it's "about to enter" disabled
> state some lines later.
>
> All other (4) calls to br_log_state are located after the state change,
> see for example br_stp_enable_port() just some lines above the patch.
I would never expect an "entering" message to print out after the event
happens, and in fact I'd _always_ want to see it beforehand so that if
the state change caused a crash or similar it'd be that much easier
to pinpoint the proper location.
I'm still not applying this. If the other log messages behave
differently, they are broken, so go fix those instead.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists