On Fri, 19 May 2017 18:25:43 +0200 Ivan Vecera <cera@xxxxxxx> 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@xxxxxxxxxxxxx > Cc: sashok@xxxxxxxxxxxxxxxxxxx > Cc: stephen@xxxxxxxxxxxxxxxxxx > Cc: bridge@xxxxxxxxxxxxxxxxxxxxxxxxxx > Cc: lucien.xin@xxxxxxxxx > Cc: nikolay@xxxxxxxxxxxxxxxxxxx > Signed-off-by: Ivan Vecera <cera@xxxxxxx> 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?
