[LEDE-DEV] [PATCH netifd 3/3] interface-event: Don't dequeue hotplug event in case of interface reload event

[Felix Fietkau]

On 2016-09-14 14:44, Hans Dedecker wrote:
> On Wed, Sep 14, 2016 at 1:15 PM, Felix Fietkau <nbd at nbd.name> wrote:
>> On 2016-09-13 14:33, Hans Dedecker wrote:
>>> Dropping hotplug event in case of interface reload results into hotplug scripts
>>> not being being run for the interface and thus external actors not being informed
>>> about the actual state of the interface.
>>>
>>> This is clearly visible if the interface auto parameter is set to disabled for
>>> multiple interfaces resulting into no hotplug down event for all interfaces.
>>>
>>> Therefore don't flush the interface hotplug queue in case an interface reload
>>> event is observed.
>> I have some doubt about this. What use is the reload event to hotplug
>> scripts? As far as I can tell there is no guarantee that the interface
>> is in a well defined state when this event is emitted. For reloads that
>> imply an up/down cycle, the regular events should suffice. For reloads
>> that affect the state without making the interface go through up/down,
>> we could maybe have a separate event. Am I missing something?
> Problem is related to the interface reload event flushing the pending
> event in the interface hotplug queue.
> Assume there are three interfaces configured A, B and C; for each of
> them we set the auto parameter to 0 followed by a network reload.
> This will bring the interfaces in reload config state; each interface
> will be brought down resulting into an IFEV_DOWN event.
> As a result for interface A a hotplug event with action ifdown will be
> launched; for interfaces B and C the hotplug event will be queued as
> hotplug event for interface A is running.
> Once interfaces B and C are marked as down in netifd the function
> interface_do_reload will launch the IFEV_RELOAD event resulting into
> drop of the queued hotplug event of both interface B and C in the
> function interface_event_cb (hotplug event for interface A is still
> running at that moment)
> Thus hotplug scripts have seen an ifdown event for interface A but not
> for interface B and C; meaning state driven hotplug actions for
> interface B and C are not performed.
> Therefore I removed the flushing of the hotplug queue based on the
> reload event which fixes the above described problem;  did also tests
> in other scenarios where a down/up is triggered by reload config and
> did not observe any issues.
It seems that I misunderstood the code. Thanks for the explanation, it
makes sense to me now.

- Felix