[LEDE-DEV] [OpenWrt-Devel] [PATCH netifd 2/3] interface: Only teardown interfaces having no proto task when l3_dev link lost

[Hans Dedecker]

On Thu, Jul 14, 2016 at 4:56 PM, Felix Fietkau <nbd at nbd.name> wrote:
> On 2016-07-14 16:53, Yousong Zhou wrote:
>> On 14/07/2016, Felix Fietkau <nbd at nbd.name> wrote:
>>> On 2016-07-14 13:28, Hans Dedecker wrote:
>>>> On Thu, Jul 14, 2016 at 12:01 PM, Yousong Zhou <yszhou4tech at gmail.com>
>>>> wrote:
>>>>> On 14 July 2016 at 16:14, Hans Dedecker <dedeckeh at 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