On 2015-09-02 13:27, Hans Dedecker wrote: > > > On Wed, Sep 2, 2015 at 10:31 AM, Felix Fietkau <[email protected] > <mailto:[email protected]>> wrote: > > On 2015-09-01 15:53, Hans Dedecker wrote: > > > > > > On Tue, Sep 1, 2015 at 2:49 PM, Felix Fietkau <[email protected] > <mailto:[email protected]> > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > On 2015-09-01 14:43, Hans Dedecker wrote: > > > The function set_state disable is not called for external > devices > > in device_release > > > which means for external vlan/macvlan devices they won't be > deleted. > > > As a result of this the set_state enable call for external > devices > > by device_claim fails > > > as vlan/macvlan devices cannot be created since the device > already > > exists in the kernel. > > > Therefore move the external device check from > device_set_state to > > device_claim so > > > external vlan/macvlan devices are not created again and can also > > be external. > > Why/how do vlan/macvlan devices become external? > > > > This use case is driven by an external application which is adding a > > vlan device to the bridge. > > Initially the vlan device is created as an internal device but not > added > > to the bridge; later added by an external application via ubus to the > > bridge. In this scenario we hit the issue when doing network > reload the > > vlan device is not anymore an active member of the bridge as vlan > > set_state enable was called which failed. > > This is a bit similar to a wireless device which has uci device > config; > > initially it's considered as an internal device by netifd when loading > > the config but becomes an external device when the wireless logic adds > > it to the bridge. > The main point of dev->external is to declare that a device is fully > managed externally, and netifd is not supposed to bring it up or down. > Maybe a better fix would be to allow devices to be added dynamically > without forcing dev->external on them (or just prevent that flag from > being added for vlan devices). > > Devices can be added dynamically without being forced external via the > function interface_handle_link; so that is covered. > 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. > 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. Okay, seems I didn't look at the context closely enough. I think this patch makes sense after all, and I will apply it.
- Felix _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
