[Bridge] [PATCH] bridge: fix "entering disabled state" logging

[Bridge] [PATCH] bridge: fix "entering disabled state" logging

[Paulius Zaleckas]

Now we have:
eth0: link down
br0: port 1(eth0) entering forwarding state

State should be logged *after* it was changed, not before.

Reported-by: Zilvinas Valinskas <zilvinas@wilibox.com>
Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>
---

 net/bridge/br_stp_if.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 19308e3..f494496 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -98,14 +98,13 @@ void br_stp_disable_port(struct net_bridge_port *p)
 	struct net_bridge *br = p->br;
 	int wasroot;
 
-	br_log_state(p);
-
 	wasroot = br_is_root_bridge(br);
 	br_become_designated_port(p);
 	p->state = BR_STATE_DISABLED;
 	p->topology_change_ack = 0;
 	p->config_pending = 0;
 
+	br_log_state(p);
 	br_ifinfo_notify(RTM_NEWLINK, p);
 
 	del_timer(&p->message_age_timer);

[PATCH] bridge: fix "entering disabled state" logging

[Paulius Zaleckas]

Now we have:
eth0: link down
br0: port 1(eth0) entering forwarding state

State should be logged *after* it was changed, not before.

Reported-by: Zilvinas Valinskas <zilvinas@wilibox.com>
Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>
---

 net/bridge/br_stp_if.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 19308e3..f494496 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -98,14 +98,13 @@ void br_stp_disable_port(struct net_bridge_port *p)
 	struct net_bridge *br = p->br;
 	int wasroot;
 
-	br_log_state(p);
-
 	wasroot = br_is_root_bridge(br);
 	br_become_designated_port(p);
 	p->state = BR_STATE_DISABLED;
 	p->topology_change_ack = 0;
 	p->config_pending = 0;
 
+	br_log_state(p);
 	br_ifinfo_notify(RTM_NEWLINK, p);
 
 	del_timer(&p->message_age_timer);

Re: [Bridge] [PATCH] bridge: fix "entering disabled state" logging

[Paulius Zaleckas]

On 03/06/2012 04:22 PM, Paulius Zaleckas wrote:
> Now we have:
> eth0: link down
> br0: port 1(eth0) entering forwarding state
>
> State should be logged *after* it was changed, not before.

The funny thing is that it was introduced:
2010-05-16 	stephen hemminger
bridge: change console message interface
28a16c97963d3bc36a2c192859f6d8025ef2967a

and no one noticed since then :D

Re: [PATCH] bridge: fix "entering disabled state" logging

[Paulius Zaleckas]

On 03/06/2012 04:22 PM, Paulius Zaleckas wrote:
> Now we have:
> eth0: link down
> br0: port 1(eth0) entering forwarding state
>
> State should be logged *after* it was changed, not before.

The funny thing is that it was introduced:
2010-05-16 	stephen hemminger
bridge: change console message interface
28a16c97963d3bc36a2c192859f6d8025ef2967a

and no one noticed since then :D

Re: [Bridge] [PATCH] bridge: fix "entering disabled state" logging

[Stephen Hemminger]

On Tue, 06 Mar 2012 16:34:54 +0200
Paulius Zaleckas <paulius.zaleckas@gmail.com> wrote:

> On 03/06/2012 04:22 PM, Paulius Zaleckas wrote:
> > Now we have:
> > eth0: link down
> > br0: port 1(eth0) entering forwarding state
> >
> > State should be logged *after* it was changed, not before.
> 
> The funny thing is that it was introduced:
> 2010-05-16 	stephen hemminger
> bridge: change console message interface
> 28a16c97963d3bc36a2c192859f6d8025ef2967a
> 
> and no one noticed since then :D

The message could be corrected to have correct verb tense. s/entering/entered/

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

Re: [PATCH] bridge: fix "entering disabled state" logging

[Stephen Hemminger]

On Tue, 06 Mar 2012 16:34:54 +0200
Paulius Zaleckas <paulius.zaleckas@gmail.com> wrote:

> On 03/06/2012 04:22 PM, Paulius Zaleckas wrote:
> > Now we have:
> > eth0: link down
> > br0: port 1(eth0) entering forwarding state
> >
> > State should be logged *after* it was changed, not before.
> 
> The funny thing is that it was introduced:
> 2010-05-16 	stephen hemminger
> bridge: change console message interface
> 28a16c97963d3bc36a2c192859f6d8025ef2967a
> 
> and no one noticed since then :D

The message could be corrected to have correct verb tense. s/entering/entered/

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

Re: [Bridge] [PATCH] bridge: fix "entering disabled state" logging

[David Miller]

From: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Date: Tue, 06 Mar 2012 16:34:54 +0200

> On 03/06/2012 04:22 PM, Paulius Zaleckas wrote:
>> Now we have:
>> eth0: link down
>> br0: port 1(eth0) entering forwarding state
>>
>> State should be logged *after* it was changed, not before.
> 
> The funny thing is that it was introduced:
> 2010-05-16 	stephen hemminger
> bridge: change console message interface
> 28a16c97963d3bc36a2c192859f6d8025ef2967a
> 
> and no one noticed since then :D

No, someone noticed, you're not the first person to propose this
patch and it was rejected just like your's will be.

Re: [PATCH] bridge: fix "entering disabled state" logging

[David Miller]

From: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Date: Tue, 06 Mar 2012 16:34:54 +0200

> On 03/06/2012 04:22 PM, Paulius Zaleckas wrote:
>> Now we have:
>> eth0: link down
>> br0: port 1(eth0) entering forwarding state
>>
>> State should be logged *after* it was changed, not before.
> 
> The funny thing is that it was introduced:
> 2010-05-16 	stephen hemminger
> bridge: change console message interface
> 28a16c97963d3bc36a2c192859f6d8025ef2967a
> 
> and no one noticed since then :D

No, someone noticed, you're not the first person to propose this
patch and it was rejected just like your's will be.

Re: [Bridge] [PATCH] bridge: fix "entering disabled state" logging

[David Miller]

From: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Date: Tue, 06 Mar 2012 16:22:19 +0200

> Now we have:
> eth0: link down
> br0: port 1(eth0) entering forwarding state
> 
> State should be logged *after* it was changed, not before.
> 
> Reported-by: Zilvinas Valinskas <zilvinas@wilibox.com>
> Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>

This is intentional, this was a discussion about this.

"Entering" means "about to" therefore we say it before it happens.

Re: [PATCH] bridge: fix "entering disabled state" logging

[David Miller]

From: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Date: Tue, 06 Mar 2012 16:22:19 +0200

> Now we have:
> eth0: link down
> br0: port 1(eth0) entering forwarding state
> 
> State should be logged *after* it was changed, not before.
> 
> Reported-by: Zilvinas Valinskas <zilvinas@wilibox.com>
> Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>

This is intentional, this was a discussion about this.

"Entering" means "about to" therefore we say it before it happens.

Re: [Bridge] [PATCH] bridge: fix "entering disabled state" logging

[Paulius Zaleckas]

On 03/06/2012 09:33 PM, David Miller wrote:
> From: Paulius Zaleckas<paulius.zaleckas@gmail.com>
> Date: Tue, 06 Mar 2012 16:22:19 +0200
>
>> Now we have:
>> eth0: link *down*
>> br0: port 1(eth0) entering *forwarding* state
>>
>> State should be logged *after* it was changed, not before.
>>
>> Reported-by: Zilvinas Valinskas<zilvinas@wilibox.com>
>> Signed-off-by: Paulius Zaleckas<paulius.zaleckas@gmail.com>
>
> This is intentional, this was a discussion about this.
>
> "Entering" means "about to" therefore we say it before it happens.

You have missed the whole point here... please look at dmesg output, I 
have bolded what you should pay attention to. It should say "entering 
disabled state" instead of "entering forwarding state".

However I do agree with you and Stephen that it should say "entered" and 
not "entering". I will send you patch changing this.

I will also resend this patch with better description.

Re: [PATCH] bridge: fix "entering disabled state" logging

[Paulius Zaleckas]

On 03/06/2012 09:33 PM, David Miller wrote:
> From: Paulius Zaleckas<paulius.zaleckas@gmail.com>
> Date: Tue, 06 Mar 2012 16:22:19 +0200
>
>> Now we have:
>> eth0: link *down*
>> br0: port 1(eth0) entering *forwarding* state
>>
>> State should be logged *after* it was changed, not before.
>>
>> Reported-by: Zilvinas Valinskas<zilvinas@wilibox.com>
>> Signed-off-by: Paulius Zaleckas<paulius.zaleckas@gmail.com>
>
> This is intentional, this was a discussion about this.
>
> "Entering" means "about to" therefore we say it before it happens.

You have missed the whole point here... please look at dmesg output, I 
have bolded what you should pay attention to. It should say "entering 
disabled state" instead of "entering forwarding state".

However I do agree with you and Stephen that it should say "entered" and 
not "entering". I will send you patch changing this.

I will also resend this patch with better description.

Re: [Bridge] [PATCH] bridge: fix "entering disabled state" logging

[David Lamparter]

[-- Attachment #1: Type: text/plain, Size: 1248 bytes --]

On Wed, Mar 07, 2012 at 10:04:04AM +0200, Paulius Zaleckas wrote:
> On 03/06/2012 09:33 PM, David Miller wrote:
> > From: Paulius Zaleckas<paulius.zaleckas@gmail.com>
> > Date: Tue, 06 Mar 2012 16:22:19 +0200
> >
> >> Now we have:
> >> eth0: link *down*
> >> br0: port 1(eth0) entering *forwarding* state
> >>
> >> State should be logged *after* it was changed, not before.
> >>
> >> Reported-by: Zilvinas Valinskas<zilvinas@wilibox.com>
> >> Signed-off-by: Paulius Zaleckas<paulius.zaleckas@gmail.com>
> >
> > This is intentional, this was a discussion about this.
> >
> > "Entering" means "about to" therefore we say it before it happens.
> 
> You have missed the whole point here... please look at dmesg output, I 
> have bolded what you should pay attention to. It should say "entering 
> disabled state" instead of "entering forwarding state".

To make this even more clear so it's not dismissed another time:

it says "entering forwarding state" when it is actually entering
disabled state. Because the print references the value before the
change. Which is changed to disabled just after.

Whatever the tense of the verb, it prints the state that's just being
left instead of the one being entered.


-David

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 230 bytes --]

Re: [PATCH] bridge: fix "entering disabled state" logging

[David Lamparter]

[-- Attachment #1: Type: text/plain, Size: 1248 bytes --]

On Wed, Mar 07, 2012 at 10:04:04AM +0200, Paulius Zaleckas wrote:
> On 03/06/2012 09:33 PM, David Miller wrote:
> > From: Paulius Zaleckas<paulius.zaleckas@gmail.com>
> > Date: Tue, 06 Mar 2012 16:22:19 +0200
> >
> >> Now we have:
> >> eth0: link *down*
> >> br0: port 1(eth0) entering *forwarding* state
> >>
> >> State should be logged *after* it was changed, not before.
> >>
> >> Reported-by: Zilvinas Valinskas<zilvinas@wilibox.com>
> >> Signed-off-by: Paulius Zaleckas<paulius.zaleckas@gmail.com>
> >
> > This is intentional, this was a discussion about this.
> >
> > "Entering" means "about to" therefore we say it before it happens.
> 
> You have missed the whole point here... please look at dmesg output, I 
> have bolded what you should pay attention to. It should say "entering 
> disabled state" instead of "entering forwarding state".

To make this even more clear so it's not dismissed another time:

it says "entering forwarding state" when it is actually entering
disabled state. Because the print references the value before the
change. Which is changed to disabled just after.

Whatever the tense of the verb, it prints the state that's just being
left instead of the one being entered.


-David

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 230 bytes --]