On 11/17/2011 08:23 PM, David Lamparter wrote: > On Mon, Nov 14, 2011 at 02:07:49AM -0500, David Miller wrote: >>>> 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. > > There seems to be a misunderstanding here. The patch effectively does: > - br_log_state(p); > p->state = BR_STATE_DISABLED; > + br_log_state(p); > > and the issue it is trying to fix is not the timing but rather the code > printing the wrong (old, now-left) state. > Yes exactly this is the problem. We got confusing state messages in the current implementation. > I do agree that the log message should be printed before anything > happens, so, Holger, can you brew a patch that does that? > This can't be changed easily. Because in some situations we don't know which state we will enter at the entry point of the function. For example br_make_forwarding does something like if .. p->state = STATE_A else if ... p->state = STATE_B .... So my approach would be to change the log message from "port entering state" to "port entered state" and print it out after the state changed. Regards Holger _______________________________________________ Bridge mailing list Bridge@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/bridge
