[OpenWrt-Devel] [PATCH 2/2] netifd: Don't call set_state for external device in device_claim

[Hans Dedecker]

On Wed, Sep 2, 2015 at 10:31 AM, Felix Fietkau <nbd at openwrt.org> wrote:

> On 2015-09-01 15:53, Hans Dedecker wrote:
> >
> >
> > On Tue, Sep 1, 2015 at 2:49 PM, Felix Fietkau <nbd at openwrt.org
> > <mailto:nbd at openwrt.org>> 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.

Hans

>
> - Felix
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20150902/987d863c/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel