<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 2, 2015 at 10:31 AM, Felix Fietkau <span dir="ltr"><<a href="mailto:nbd@openwrt.org" target="_blank">nbd@openwrt.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 2015-09-01 15:53, Hans Dedecker wrote:<br>
><br>
><br>
> On Tue, Sep 1, 2015 at 2:49 PM, Felix Fietkau <<a href="mailto:nbd@openwrt.org">nbd@openwrt.org</a><br>
> <mailto:<a href="mailto:nbd@openwrt.org">nbd@openwrt.org</a>>> wrote:<br>
><br>
>     On 2015-09-01 14:43, Hans Dedecker wrote:<br>
>     > The function set_state disable is not called for external devices<br>
>     in device_release<br>
>     > which means for external vlan/macvlan devices they won't be deleted.<br>
>     > As a result of this the set_state enable call for external devices<br>
>     by device_claim fails<br>
>     > as vlan/macvlan devices cannot be created since the device already<br>
>     exists in the kernel.<br>
>     > Therefore move the external device check from device_set_state to<br>
>     device_claim so<br>
>     > external vlan/macvlan devices are not created again and can also<br>
>     be external.<br>
>     Why/how do vlan/macvlan devices become external?<br>
><br>
> This use case is driven by an external application which is adding a<br>
> vlan device to the bridge.<br>
> Initially the vlan device is created as an internal device but not added<br>
> to the bridge; later added by an external application via ubus to the<br>
> bridge. In this scenario we hit the issue when doing network reload the<br>
> vlan device is not anymore an active member of the bridge as vlan<br>
> set_state enable was called which failed.<br>
> This is a bit similar to a wireless device which has uci device config;<br>
> initially it's considered as an internal device by netifd when loading<br>
> the config but becomes an external device when the wireless logic adds<br>
> it to the bridge.<br>
The main point of dev->external is to declare that a device is fully<br>
managed externally, and netifd is not supposed to bring it up or down.<br>
Maybe a better fix would be to allow devices to be added dynamically<br>
without forcing dev->external on them (or just prevent that flag from<br>
being added for vlan devices).<br></blockquote><div>Devices can be added dynamically without being forced external via the function interface_handle_link; so that is covered. </div><div>Regarding the external flag being set for vlan devices; would it be acceptable the external flag can only be set  for simple devices in device_get ? This is the only place where non simple devices can be set external.</div><div>My intention with this patch was also to line up the external flag check when the state is altered; in device_release the external flag is checked although the same check is done in set_device_state (which would eventually be called by the set_state callback function). So I thought to add the same external flag check in device_claim to line up both functions.</div><div><br></div><div>Hans</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
- Felix<br>
</font></span></blockquote></div><br></div></div>