[LEDE-DEV] [PATCH v1 1/2] base-files, mac80211, broadcom-wl: plug-and-play wifi detection

[Christian Lamparter]

On Saturday, October 8, 2016 6:04:09 PM CEST Matthias Schiffer wrote:
> On 10/08/2016 05:41 PM, Christian Lamparter wrote:
> > On Friday, October 7, 2016 8:29:30 PM CEST Matthias Schiffer wrote:
> >> On 10/07/2016 08:10 PM, Christian Lamparter wrote:
> >>> Currently, the wifi detection script is executed as part of
> >>> the (early) boot process. Pluggable wifi USB devices, which
> >>> are inserted at a later time are not automatically
> >>> detected and therefore they don't show up in LuCI.
> >>>
> >>> A user has to deal with wifi detection manually, or restart
> >>> the router.
> >>>
> >>> [...]
> >>> ---
> >>> We would like to hear, if these changes work with broadcom-wl.
> >>> (Felix removed the hostap, so this isn't included anymore).
> >>>
> >>> trap and lock are part of the default busybox setup.
> >>
> >> Hi,
> >> it would be great to remove the direct write of /etc/config/wireless
> >> completely, as it won't lock against other users of UCI that modify the
> >> wireless config. IMO, it should just use UCI to modify the configuration as
> >> everything else does.
> > 
> 
> > Well, What's the situation with ECE and configd? I didn't want to touch it
> > since you plan to move away from the config files and replace them with
> > json. 
> 
> There isn't a concrete plan to integrate ECE with LEDE yet (there are still
> some TODOs, and it will need to be discussed further with the LEDE
> community). It will provide a bidirectional UCI mapping; this means as long
> as the uci CLI and similar tools are used, most things should just continue
> work with ECE.
Ok. But how would the /etc/config/wireless have worked? As it just produced a
file and bypassed all the uci CLI and similar tools. Did you setup file
notifications to check if something as modified in /etc/config/ run uci to
import it or sth. like that? (Because that was why I contacted you earlier :) )
 
> > Note: the "> /dev/null" for uci calls were added just in case someone still
> > has the old /etc/init.d/boot and to not write garbage into /e/c/wireless.
> > 
> > Note2: I've also changed the "plug-and-play wifi" patch and removed the
> > /tmp/wireless.tmp step. But we still need proper locking. 
> > (That said, I would like to move the locking to the mac80211.sh / broadcom.sh
> > detect functions, is everyone fine with that?)
> 
> Makes sense to me (maybe we can further factor out common parts of
> mac80211.sh and broadcom.sh?).
I don't think that's within the scope of this series :-D. Also, I don't have
any  broadcom devices... 

> Note that you don't need any locking in the hotplug handlers anyways, they are
> always run sequentially.
I moved the locking to detect_mac80211 and detect_broadcom. 
A user might still want to run "wifi detect" (out of habit?) and that
could race with hotplug event .

As for procd:
Can you tell me where this serialization happens or how it works?

I've looked into this earlier and found that procd uses the hotplug-call
in hotplug.json to run all the scripts under /etc/hotplug.d/%SUBSYSTEMS%.

I've looked up handle_exec (which is used to run hotplug-call)
and found execvp [0]. The thing with execvp is that it fully replaces
the current thread with the new program it is supposed to execute
(it never returns, unless it fails before it jumps to the new program).
So procd needs to fork() before it can run hotplug-call and this is
done in queue_next [1].

However, if procd is supposed to serialize these calls, it will
need to use some sort of wait() or waitpid() to wait for the
forked processes to finish. Since there is no wait()/waitpid() in 
there, the hotplug-call can run concurrently.

(I know that the event messages from the kernel to procd are indeed
serialized by the netlink message interface.)

so, what is wrong here?

Regards,
Christian

[0] <https://git.lede-project.org/?p=project/procd.git;a=blob;f=plug/hotplug.c;h=85959155634f1a6a9ada11c7045601885a0abad8;hb=72f63807b3644ef7b8ab9025a02d7f509f39ad14#l204>

[1] <https://git.lede-project.org/?p=project/procd.git;a=blob;f=plug/hotplug.c;h=85959155634f1a6a9ada11c7045601885a0abad8;hb=72f63807b3644ef7b8ab9025a02d7f509f39ad14#l347>