On Thu, Jul 14, 2016 at 4:56 PM, Felix Fietkau <n...@nbd.name> wrote: > On 2016-07-14 16:53, Yousong Zhou wrote: >> On 14/07/2016, Felix Fietkau <n...@nbd.name> wrote: >>> On 2016-07-14 13:28, Hans Dedecker wrote: >>>> On Thu, Jul 14, 2016 at 12:01 PM, Yousong Zhou <yszhou4t...@gmail.com> >>>> wrote: >>>>> On 14 July 2016 at 16:14, Hans Dedecker <dedec...@gmail.com> wrote: >>>>>> Commit c6858766 adds teardown support when l3_dev link is lost >>>>>> especially for shell protocols >>>>>> that have no proto task. However shell protocols which have a proto task >>>>>> running like ppp will >>>>>> also be teared down which is not always the expected action. >>>>>> As an example the PPP daemon can be put into persist state trying to >>>>>> re-establish the link via >>>>>> a hold off mechanism which is not possible when the daemon is terminated >>>>>> by the proto shell >>>>>> teardown. >>>>>> Therefore restrict the teardown action for shell protocols having no >>>>>> proto task. >>>>>> >>>>> >>>>> How about adding an extra flag like managed-link, persistent-link, >>>>> on-demand-link? It looks to me doing teardown at link-down is more >>>>> common a case. >>>> Initially I was thinking about adding another flag like you propose >>>> but then I was doubting if the change in behavior for shell protocols >>>> having a proto task task was on purpose or not. In case of PPP and >>>> link failure you don't want an immediate restart by netifd in some >>>> cases (see https://github.com/lede-project/source/pull/200) as PPP >>>> daemon can take care of the link re-negotiation based on a holdoff >>>> timeout. >>>> Additionally if the wan link loses connectivity a link down >>>> notification will be received on the main device which will teardown >>>> the protocol. Anyway I'm open for suggestions which way to go forward. >>> Yousong, >>> >>> please provide some more details on where your commit c6858766 is >>> actually needed/useful. In all the use cases I can think of, handling >>> setup/teardown based on the l2 dev should be enough. >>> >>> - Felix >>> >> >> The issue them was that when l2tp-xxx went down, netifd has no proto >> task state to be notified of, and main_dev state seemed unchanged. If >> I remeber and understand the code correcly other pppd shell protos do >> teardown because of proto task event, not any device link state, and I >> thought it's reasonable and should not hurt to do an explicit >> teardown on link down. > It seems to me that we should do less magic here and make the behavior > opt-in via an explicit flag that needs to be enabled by the proto handler. > > - Felix Felix,
Currently the xl2tp protocol script registers itself as having no_proto_task which is used in proto_shell_task_finish to send a teardown when the shell is in setup state an having a proto task. I re-used the no_proto_task flag to send a teardown in case l3 dev link loss is detected. Do I understand it correctly you favour adding an explicit flag (to be set by the protocol script in this case l2tp) and thus decouple the teardown from the no_proto_task flag ? Hans _______________________________________________ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
