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

2016-10-07 Thread Matthias Schiffer
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.
> 
> However, the current "sleep 1" window - which the boot
> process waits for wifi devices to "settle down" - is too
> short to detect wifi devices for some routers.
> 
> For example, this can happen with USB WLAN devices on the
> WNDR4700. This is because the usb controller needs to load
> its firmware from UBI and initialize, before it can operate.
> 
> The issue can be seen on a BT HomeHub 5A as well as soon as
> the caldata are on an ubi volume. This is because the ath9k
> card has to be initialized by owl-loader first. Which has to
> wait for the firmware extraction script to retrieve the pci
> initialization values inside the caldata.
> 
> This patch moves the wifi detection to hotplug scripts.
> For mac80211, the wifi detection will now automatically
> run any time a "ieee80211" device is added. Likewise
> broadcom-wl's script checks for new "net" devices.
> 
> As for potential problems:
> - For devices with more than one wifi device, these scripts
>   will run multiple times during boot (concurrently).
>   For this reason, the script uses file locking to prevent
>   races during discovery. (i.e duplicated radio instances).
> 
> - The broadcom-wl hotplug script is suboptimal.
>   As it will run for any net devices (ethernet).
> 
> (- There might be non-wifi drivers that needed this delay)
> 
> All changes to base-files, mac80211 and broadcom-wl packages
> have been integrated into a single patch to play nice with
> git bisect.
> 
> Thanks to Martin Blumenstingl for helping with the implementation
> and testing of the patch.
> 
> Signed-off-by: Mathias Kresin 
> Signed-off-by: Christian Lamparter 
> ---
> 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.

Regards,
Matthias


> ---
>  package/base-files/files/etc/init.d/boot |  9 -
>  .../files/etc/hotplug.d/net/00-broadcom-wifi-detect  | 16 
> 
>  package/kernel/mac80211/Makefile |  2 ++
>  package/kernel/mac80211/files/mac80211.hotplug   | 16 
> 
>  4 files changed, 34 insertions(+), 9 deletions(-)
>  create mode 100644 
> package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect
>  create mode 100644 package/kernel/mac80211/files/mac80211.hotplug
> 
> diff --git a/package/base-files/files/etc/init.d/boot 
> b/package/base-files/files/etc/init.d/boot
> index 904f7db..1d61f2f 100755
> --- a/package/base-files/files/etc/init.d/boot
> +++ b/package/base-files/files/etc/init.d/boot
> @@ -38,15 +38,6 @@ boot() {
>  
>   /sbin/kmodloader
>  
> - # allow wifi modules time to settle
> - sleep 1
> -
> - /sbin/wifi detect > /tmp/wireless.tmp
> - [ -s /tmp/wireless.tmp ] && {
> - cat /tmp/wireless.tmp >> /etc/config/wireless
> - }
> - rm -f /tmp/wireless.tmp
> -
>   /bin/config_generate
>   uci_apply_defaults
>   
> diff --git 
> a/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect 
> b/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect
> new file mode 100644
> index 000..cd7af8b
> --- /dev/null
> +++ 
> b/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +unlock() {
> + lock -u /tmp/wireless.lock
> +}
> +
> +[ "${ACTION}" = "add" ] && {
> + trap unlock EXIT
> + lock /tmp/wireless.lock
> +
> + /sbin/wifi detect > /tmp/wireless.tmp
> +
> + [ -s /tmp/wireless.tmp ] && cat /tmp/wireless.tmp >> 
> /etc/config/wireless
> +
> + rm -f /tmp/wireless.tmp
> +}
> diff --git a/package/kernel/mac80211/Makefile 
> b/package/kernel/mac80211/Makefile
> index 91c9362..f320326 100644
> --- a/package/kernel/mac80211/Makefile
> +++ b/package/kernel/mac80211/Makefile
> @@ -1732,6 +1732,8 @@ define KernelPackage/cfg80211/install
>   $(INSTALL_DIR) $(1)/lib/wifi $(1)/lib/netifd/wireless
>   $(INSTALL_DATA) ./files/lib/wifi/mac80211.sh $(1)/lib/wifi
>   $(INSTALL_BIN) ./files/lib/netifd/wireless/mac80211.sh 
> $(1)/lib/netifd/wireless
> + $(INSTALL_DIR) $(1)/etc/hotplug.d/ieee80211
> + $(INSTALL_DATA) ./files/mac80211.hotplug 
> $(1)/etc/hotplug.d/ieee80211/00-wifi-detect
>  endef
>  
>  define 

[LEDE-DEV] [PATCH] apm821xx: replace recovery image for the MBL with initramfs

2016-10-07 Thread Christian Lamparter
The patch "images: bump default rootfs size to 256 MB"
a1f83bad606411a561e8e60110c71232b1a28aa2 caused a crash
during boot for the recovery images. This is because
both variants of the MyBook Live only have 256MB of RAM
and for the recovery option, the ext4 rootfs was simply
stored in the RAMDISK.

This patch replaces recovery image for the MBL with an
initramfs kernel.

In order to boot the initramfs (for recovery or development):

0. copy the initramfs and device tree into tftp's server directory
   # cp *-initramfs-kernel.bin to /tftp-server/mbl.bin
   # cp *-ext4-kernel.dtb to /tftp-server/fdt.bin

1. Connect the MyBook Live (Duo) serial port.
   (Warning! Use a 3.3v level shifter).

2. Hit Enter during u-boot and insert these three lines:
   # setenv serverip 192.168.1.254; setenv ipaddr 192.168.1.1;
   # tftp ${kernel_addr_r} mbl.bin; tftp ${fdt_addr_r} fdt.bin
   # run addtty addmisc; bootm ${kernel_addr_r} - ${fdt_addr_r}

   Where 192.168.1.254 is your TFTP server.

Signed-off-by: Christian Lamparter 
---
 target/linux/apm821xx/image/Makefile   | 22 +-
 .../linux/apm821xx/image/mbl_gen_recovery_tar.sh   | 92 --
 target/linux/apm821xx/sata/config-default  |  5 --
 target/linux/apm821xx/sata/target.mk   |  2 +-
 4 files changed, 3 insertions(+), 118 deletions(-)
 delete mode 100644 target/linux/apm821xx/image/mbl_gen_recovery_tar.sh

diff --git a/target/linux/apm821xx/image/Makefile 
b/target/linux/apm821xx/image/Makefile
index 607df06..fb1705e 100644
--- a/target/linux/apm821xx/image/Makefile
+++ b/target/linux/apm821xx/image/Makefile
@@ -196,24 +196,6 @@ define Build/hdd-img
$(if $(CONFIG_TARGET_IMAGES_GZIP),gzip -9n -c $@ > $(BIN_DIR)/$(notdir 
$@).gz)
 endef
 
-define Build/uRamdisk
-   $(STAGING_DIR_HOST)/bin/mkimage \
-   -A powerpc -T ramdisk -C gzip \
-   -n "$(DEVICE_NAME) rootfs" \
-   -d $@ $@.new
-   mv $@.new $@
-endef
-
-define Build/recovery-tar
-   sh ./mbl_gen_recovery_tar.sh \
-   --profile $(DEVICE_PROFILE) \
-   --dtb $(IMAGE_KERNEL).dtb \
-   --dtbname $(DEVICE_DTB) \
-   --kernel $(IMAGE_KERNEL) \
-   --rootfs $@ \
-   $@
-endef
-
 define Build/export-dtb
cp $(IMAGE_KERNEL).dtb $@
 endef
@@ -223,13 +205,13 @@ define Device/MyBookLiveDefault
   BLOCKSIZE := 1k
   DTB_SIZE := 16384
   KERNEL := kernel-bin | dtb | gzip | uImage gzip
+  KERNEL_INITRAMFS := kernel-bin | dtb | gzip | uImage gzip
   BOOT_SIZE := 8
-  IMAGES := rootfs.img recovery.tar kernel.dtb
+  IMAGES := rootfs.img kernel.dtb
   DEVICE_DTB := apollo3g.dtb
   FILESYSTEMS := ext4
   IMAGE/kernel.dtb := export-dtb
   IMAGE/rootfs.img := boot-script | boot-img | hdd-img
-  IMAGE/recovery.tar := append-rootfs | gzip | uRamdisk | recovery-tar
 endef
 
 define Device/MyBookLiveSingle
diff --git a/target/linux/apm821xx/image/mbl_gen_recovery_tar.sh 
b/target/linux/apm821xx/image/mbl_gen_recovery_tar.sh
deleted file mode 100644
index f871aef..000
--- a/target/linux/apm821xx/image/mbl_gen_recovery_tar.sh
+++ /dev/null
@@ -1,92 +0,0 @@
-#!/bin/sh
-
-# based on scripts/sysupgrade-nand.sh
-
-profile=""
-dtb=""
-dtbname=""
-kernel=""
-rootfs=""
-outfile=""
-err=""
-
-while [ "$1" ]; do
-   case "$1" in
-   "--profile")
-   profile="$2"
-   shift
-   shift
-   continue
-   ;;
-   "--dtb")
-   dtb="$2"
-   shift
-   shift
-   continue
-   ;;
-   "--dtbname")
-   dtbname="$2"
-   shift
-   shift
-   continue
-   ;;
-   "--kernel")
-   kernel="$2"
-   shift
-   shift
-   continue
-   ;;
-   "--rootfs")
-   rootfs="$2"
-   shift
-   shift
-   continue
-   ;;
-   *)
-   if [ ! "$outfile" ]; then
-   outfile=$1
-   shift
-   continue
-   else
-   shift
-   continue
-   fi
-   ;;
-   esac
-done
-
-if [ -z "$profile" -o ! -r "$dtb" -o ! -r "$kernel" -o ! -r "$rootfs" -o ! 
"$outfile" ]; then
-   echo "syntax: $0 [--profile profilename] [--dtb dtbimage] [--dtbname 
dtbname] [--kernel kernelimage] [--rootfs rootfs] out"
-   exit 1
-fi
-
-tmpdir="$( mktemp -d 2> /dev/null )"
-if [ -z "$tmpdir" ]; then
-   # try OSX signature
-   tmpdir="$( mktemp -t 'roottmp' -d )"
-fi
-
-if [ -z "$tmpdir" ]; then
-   exit 1
-fi
-
-mkdir -p "${tmpdir}/${profile}"
-[ -z "${dtb}" ] || cp "${dtb}" "${tmpdir}/${profile}/${dtbname}"
-[ -z "${rootfs}" ] || cp "${rootfs}" "${tmpdir}/${profile}/uRamdisk"
-[ -z "${kernel}" ] || cp "${kernel}" 

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

2016-10-07 Thread Jo-Philipp Wich
Hi,

> - The broadcom-wl hotplug script is suboptimal.
>   As it will run for any net devices (ethernet).

I think you can restrict the hotplugging for broadcom-wl to "wl[0-9]"
net devices, this should avoid some extraneous invocations.

Otherwise,
Acked-by: Jo-Philipp Wich 

~ Jo

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


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

2016-10-07 Thread Christian Lamparter
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.

However, the current "sleep 1" window - which the boot
process waits for wifi devices to "settle down" - is too
short to detect wifi devices for some routers.

For example, this can happen with USB WLAN devices on the
WNDR4700. This is because the usb controller needs to load
its firmware from UBI and initialize, before it can operate.

The issue can be seen on a BT HomeHub 5A as well as soon as
the caldata are on an ubi volume. This is because the ath9k
card has to be initialized by owl-loader first. Which has to
wait for the firmware extraction script to retrieve the pci
initialization values inside the caldata.

This patch moves the wifi detection to hotplug scripts.
For mac80211, the wifi detection will now automatically
run any time a "ieee80211" device is added. Likewise
broadcom-wl's script checks for new "net" devices.

As for potential problems:
- For devices with more than one wifi device, these scripts
  will run multiple times during boot (concurrently).
  For this reason, the script uses file locking to prevent
  races during discovery. (i.e duplicated radio instances).

- The broadcom-wl hotplug script is suboptimal.
  As it will run for any net devices (ethernet).

(- There might be non-wifi drivers that needed this delay)

All changes to base-files, mac80211 and broadcom-wl packages
have been integrated into a single patch to play nice with
git bisect.

Thanks to Martin Blumenstingl for helping with the implementation
and testing of the patch.

Signed-off-by: Mathias Kresin 
Signed-off-by: Christian Lamparter 
---
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.
---
 package/base-files/files/etc/init.d/boot |  9 -
 .../files/etc/hotplug.d/net/00-broadcom-wifi-detect  | 16 
 package/kernel/mac80211/Makefile |  2 ++
 package/kernel/mac80211/files/mac80211.hotplug   | 16 
 4 files changed, 34 insertions(+), 9 deletions(-)
 create mode 100644 
package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect
 create mode 100644 package/kernel/mac80211/files/mac80211.hotplug

diff --git a/package/base-files/files/etc/init.d/boot 
b/package/base-files/files/etc/init.d/boot
index 904f7db..1d61f2f 100755
--- a/package/base-files/files/etc/init.d/boot
+++ b/package/base-files/files/etc/init.d/boot
@@ -38,15 +38,6 @@ boot() {
 
/sbin/kmodloader
 
-   # allow wifi modules time to settle
-   sleep 1
-
-   /sbin/wifi detect > /tmp/wireless.tmp
-   [ -s /tmp/wireless.tmp ] && {
-   cat /tmp/wireless.tmp >> /etc/config/wireless
-   }
-   rm -f /tmp/wireless.tmp
-
/bin/config_generate
uci_apply_defaults

diff --git 
a/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect 
b/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect
new file mode 100644
index 000..cd7af8b
--- /dev/null
+++ b/package/kernel/broadcom-wl/files/etc/hotplug.d/net/00-broadcom-wifi-detect
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+unlock() {
+   lock -u /tmp/wireless.lock
+}
+
+[ "${ACTION}" = "add" ] && {
+   trap unlock EXIT
+   lock /tmp/wireless.lock
+
+   /sbin/wifi detect > /tmp/wireless.tmp
+
+   [ -s /tmp/wireless.tmp ] && cat /tmp/wireless.tmp >> 
/etc/config/wireless
+
+   rm -f /tmp/wireless.tmp
+}
diff --git a/package/kernel/mac80211/Makefile b/package/kernel/mac80211/Makefile
index 91c9362..f320326 100644
--- a/package/kernel/mac80211/Makefile
+++ b/package/kernel/mac80211/Makefile
@@ -1732,6 +1732,8 @@ define KernelPackage/cfg80211/install
$(INSTALL_DIR) $(1)/lib/wifi $(1)/lib/netifd/wireless
$(INSTALL_DATA) ./files/lib/wifi/mac80211.sh $(1)/lib/wifi
$(INSTALL_BIN) ./files/lib/netifd/wireless/mac80211.sh 
$(1)/lib/netifd/wireless
+   $(INSTALL_DIR) $(1)/etc/hotplug.d/ieee80211
+   $(INSTALL_DATA) ./files/mac80211.hotplug 
$(1)/etc/hotplug.d/ieee80211/00-wifi-detect
 endef
 
 define KernelPackage/ipw2100/install
diff --git a/package/kernel/mac80211/files/mac80211.hotplug 
b/package/kernel/mac80211/files/mac80211.hotplug
new file mode 100644
index 000..cd7af8b
--- /dev/null
+++ b/package/kernel/mac80211/files/mac80211.hotplug
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+unlock() {
+   lock -u /tmp/wireless.lock
+}
+
+[ "${ACTION}" = "add" ] && {
+   trap unlock EXIT
+   lock /tmp/wireless.lock
+
+   /sbin/wifi detect > /tmp/wireless.tmp
+
+   [ -s /tmp/wireless.tmp ] && cat /tmp/wireless.tmp >> 

[LEDE-DEV] [PATCH] ltq-ptm: Support 1508-byte MTU for RFC4638

2016-10-07 Thread David Woodhouse
Tested with VDSL on TP-Link WD8970, I see full 1500-byte PPP data
frames, which end up being 1526 byte Ethernet frames (including
Ethernet+VLAN headers) on the wire.

Fixes: FS#210

Signed-off-by: David Woodhouse 

diff --git a/package/kernel/lantiq/ltq-ptm/src/ifxmips_ptm_adsl.c 
b/package/kernel/lantiq/ltq-ptm/src/ifxmips_ptm_adsl.c
index dc75b12..b096b0a 100644
--- a/package/kernel/lantiq/ltq-ptm/src/ifxmips_ptm_adsl.c
+++ b/package/kernel/lantiq/ltq-ptm/src/ifxmips_ptm_adsl.c
@@ -128,6 +128,7 @@ static int ptm_stop(struct net_device *);
   static unsigned int ptm_poll(int, unsigned int);
   static int ptm_napi_poll(struct napi_struct *, int);
 static int ptm_hard_start_xmit(struct sk_buff *, struct net_device *);
+static int ptm_change_mtu(struct net_device *, int);
 static int ptm_ioctl(struct net_device *, struct ifreq *, int);
 static void ptm_tx_timeout(struct net_device *);
 
@@ -246,7 +247,7 @@ static struct net_device_ops g_ptm_netdev_ops = {
 .ndo_start_xmit  = ptm_hard_start_xmit,
 .ndo_validate_addr   = eth_validate_addr,
 .ndo_set_mac_address = eth_mac_addr,
-.ndo_change_mtu  = eth_change_mtu,
+.ndo_change_mtu  = ptm_change_mtu,
 .ndo_do_ioctl= ptm_ioctl,
 .ndo_tx_timeout  = ptm_tx_timeout,
 };
@@ -451,6 +452,15 @@ PTM_HARD_START_XMIT_FAIL:
 return NETDEV_TX_OK;
 }
 
+static int ptm_change_mtu(struct net_device *dev, int mtu)
+{
+   /* Allow up to 1508 bytes, for RFC4638 */
+if (mtu < 68 || mtu > ETH_DATA_LEN + 8)
+return -EINVAL;
+dev->mtu = mtu;
+return 0;
+}
+
 static int ptm_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 {
 int ndev;
diff --git a/package/kernel/lantiq/ltq-ptm/src/ifxmips_ptm_vdsl.c 
b/package/kernel/lantiq/ltq-ptm/src/ifxmips_ptm_vdsl.c
index 41464e6..bc27c27 100644
--- a/package/kernel/lantiq/ltq-ptm/src/ifxmips_ptm_vdsl.c
+++ b/package/kernel/lantiq/ltq-ptm/src/ifxmips_ptm_vdsl.c
@@ -74,6 +74,7 @@ static int ptm_stop(struct net_device *);
   static unsigned int ptm_poll(int, unsigned int);
   static int ptm_napi_poll(struct napi_struct *, int);
 static int ptm_hard_start_xmit(struct sk_buff *, struct net_device *);
+static int ptm_change_mtu(struct net_device *, int);
 static int ptm_ioctl(struct net_device *, struct ifreq *, int);
 static void ptm_tx_timeout(struct net_device *);
 
@@ -114,7 +115,7 @@ static struct net_device_ops g_ptm_netdev_ops = {
 .ndo_start_xmit  = ptm_hard_start_xmit,
 .ndo_validate_addr   = eth_validate_addr,
 .ndo_set_mac_address = eth_mac_addr,
-.ndo_change_mtu  = eth_change_mtu,
+.ndo_change_mtu  = ptm_change_mtu,
 .ndo_do_ioctl= ptm_ioctl,
 .ndo_tx_timeout  = ptm_tx_timeout,
 };
@@ -358,6 +359,15 @@ PTM_HARD_START_XMIT_FAIL:
 return 0;
 }
 
+static int ptm_change_mtu(struct net_device *dev, int mtu)
+{
+   /* Allow up to 1508 bytes, for RFC4638 */
+if (mtu < 68 || mtu > ETH_DATA_LEN + 8)
+return -EINVAL;
+dev->mtu = mtu;
+return 0;
+}
+
 static int ptm_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 {
 ASSERT(dev == g_net_dev[0], "incorrect device");
-- 
dwmw2

smime.p7s
Description: S/MIME cryptographic signature
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] ubus cli: wait_for: fix race causing false timeouts

2016-10-07 Thread Zefir Kurtisi
On 10/07/2016 02:15 PM, Alexandru Ardelean wrote:
> On Fri, Oct 7, 2016 at 3:09 PM, Felix Fietkau  wrote:
>> Instead of introducing yet another timer, wouldn't it also be possible
>> to close this race window by registering the event handler before
>> attempting the lookup?
>>
>> - Felix
>>
>> ___
>> Lede-dev mailing list
>> Lede-dev@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/lede-dev
> 
> I've also seen this race.
> I tried something like this:
> https://github.com/commodo/ubus/commit/8c3986caaa7cd2c12f2b8907ceea54c5bdce3bd2
> 
> But never got around to doing much testing to see if the race goes
> away completely.
> So, I never pushed it upstream.
> 
> @Zefir, maybe you could try it ?
> 
> Thanks
> Alex
> 
Hi Alex,

your assumption is right, that's the root cause for the random timeouts.

Unfortunately, it is hard to provide a positive proof, since for me the effect
went away when I added some logging in between.

My patch made it disappear, but of course what Felix suggests and you already
implemented is the better approach. I'll take your commit instead and test it.
>From looking at the changes it should do, but to get some confidence it will 
>take
some time.


Cheers,
Zefir

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] ubus cli: wait_for: fix race causing false timeouts

2016-10-07 Thread Alexandru Ardelean
On Fri, Oct 7, 2016 at 3:09 PM, Felix Fietkau  wrote:
> On 2016-10-07 13:57, Zefir Kurtisi wrote:
>> In ubus_cli_wait_for() there is a critical section between
>> initially checking for the requested services and the
>> following handling of 'ubus.object.add' events.
>>
>> In our system we let procd (re)start services and synchronize
>> inter-service dependencies by using 'ubus wait_for' in the
>> initscripts' service_started() functions. There we observe
>> that 'wait_for' randomly is waiting for the full timeout
>> and returning UBUS_STATUS_TIMEOUT, even if the service it
>> is waiting for is already up and running.
>>
>> This happens when the service is started in the critical
>> section mentioned above. This commit adds periodic lookup
>> for the requested services while waiting for the 'add' event
>> and with that fixes the observed failure.
>>
>> Signed-off-by: Zefir Kurtisi 
> Instead of introducing yet another timer, wouldn't it also be possible
> to close this race window by registering the event handler before
> attempting the lookup?
>
> - Felix
>
> ___
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev

I've also seen this race.
I tried something like this:
https://github.com/commodo/ubus/commit/8c3986caaa7cd2c12f2b8907ceea54c5bdce3bd2

But never got around to doing much testing to see if the race goes
away completely.
So, I never pushed it upstream.

@Zefir, maybe you could try it ?

Thanks
Alex

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] ubus cli: wait_for: fix race causing false timeouts

2016-10-07 Thread Felix Fietkau
On 2016-10-07 13:57, Zefir Kurtisi wrote:
> In ubus_cli_wait_for() there is a critical section between
> initially checking for the requested services and the
> following handling of 'ubus.object.add' events.
> 
> In our system we let procd (re)start services and synchronize
> inter-service dependencies by using 'ubus wait_for' in the
> initscripts' service_started() functions. There we observe
> that 'wait_for' randomly is waiting for the full timeout
> and returning UBUS_STATUS_TIMEOUT, even if the service it
> is waiting for is already up and running.
> 
> This happens when the service is started in the critical
> section mentioned above. This commit adds periodic lookup
> for the requested services while waiting for the 'add' event
> and with that fixes the observed failure.
> 
> Signed-off-by: Zefir Kurtisi 
Instead of introducing yet another timer, wouldn't it also be possible
to close this race window by registering the event handler before
attempting the lookup?

- Felix

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev