From: Stephen Hemminger <stephen@networkplumber•org>
To: Ivan Vecera <cera@cera•cz>
Cc: lucien.xin@gmail•com, nikolay@cumulusnetworks•com,
netdev@vger•kernel.org, bridge@lists•linux-foundation.org,
davem@davemloft•net
Subject: Re: [PATCH net] bridge: fix hello and hold timers starting/stopping
Date: Fri, 19 May 2017 09:38:16 -0700 [thread overview]
Message-ID: <20170519093816.2b556a1b@xeon-e3> (raw)
In-Reply-To: <20170519162543.31670-1-cera@cera.cz>
On Fri, 19 May 2017 18:25:43 +0200
Ivan Vecera <cera@cera•cz> wrote:
> Current bridge code incorrectly handles starting/stopping of hello and
> hold timers during STP enable/disable.
>
> 1. Timers are stopped in br_stp_start() during NO_STP->USER_STP
> transition. This is not correct as the timers are stopped in NO_STP
> case.
>
> 2. Timers are started in br_stp_stop() during USER_STP->NO_STP transition.
> This is not also correct as the timers should be stopped in NO_STP
> state.
>
> 3. Timers are NOT stopped in br_stp_stop() during KERNEL_STP->NO_STP
> transition. They should be stopped as they are running in KERNEL_STP
> state and should not run in NO_STP case.
>
> The patch is a follow-up for "bridge: start hello_timer when enabling
> KERNEL_STP in br_stp_start" patch from Xin Long.
>
> Cc: davem@davemloft•net
> Cc: sashok@cumulusnetworks•com
> Cc: stephen@networkplumber•org
> Cc: bridge@lists•linux-foundation.org
> Cc: lucien.xin@gmail•com
> Cc: nikolay@cumulusnetworks•com
> Signed-off-by: Ivan Vecera <cera@cera•cz>
Overall, this looks correct but the wording of commit message
is too terse.
It would be better to add a more complete description of the impact
of this from a user's point of view. I am concerned that this
might have other side effects.
For example, what is the sequence of commands to validated this.
What is the impact, should this go to stable?
next prev parent reply other threads:[~2017-05-19 16:38 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-19 16:25 [PATCH net] bridge: fix hello and hold timers starting/stopping Ivan Vecera
2017-05-19 16:38 ` Stephen Hemminger [this message]
2017-05-19 16:51 ` Xin Long
2017-05-19 16:55 ` Ivan Vecera
2017-05-19 16:55 ` Nikolay Aleksandrov
2017-05-19 17:06 ` Ivan Vecera
2017-05-19 17:26 ` Xin Long
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=20170519093816.2b556a1b@xeon-e3 \
--to=stephen@networkplumber$(echo .)org \
--cc=bridge@lists$(echo .)linux-foundation.org \
--cc=cera@cera$(echo .)cz \
--cc=davem@davemloft$(echo .)net \
--cc=lucien.xin@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nikolay@cumulusnetworks$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox