From 2dac1bcdc969a940fa34396db89ae3f1f2a3073f Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Wed, 22 Jul 2020 09:19:55 +0200 Subject: [PATCH 1/5] mwan3: fix race condition on mwan3 restart This adjustment of the locks fixes the race condition when a mwan3 hotplug script and a mwan3 command are running at the same time. Signed-off-by: Florian Eckert --- net/mwan3/files/etc/hotplug.d/iface/15-mwan3 | 14 +++++++----- net/mwan3/files/etc/hotplug.d/iface/16-mwan3 | 11 +++++++--- .../files/etc/hotplug.d/iface/16-mwan3-user | 16 ++++++++++++-- net/mwan3/files/usr/sbin/mwan3 | 22 ++++++++++++++----- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/net/mwan3/files/etc/hotplug.d/iface/15-mwan3 b/net/mwan3/files/etc/hotplug.d/iface/15-mwan3 index 5bfbd2462..db2dc237a 100644 --- a/net/mwan3/files/etc/hotplug.d/iface/15-mwan3 +++ b/net/mwan3/files/etc/hotplug.d/iface/15-mwan3 @@ -12,19 +12,24 @@ if [ "$ACTION" == "ifup" ]; then [ -n "$DEVICE" ] || exit 3 fi +mwan3_lock "$ACTION" "$INTERFACE" config_load mwan3 config_get_bool enabled globals 'enabled' '0' -[ ${enabled} -gt 0 ] || exit 0 +[ ${enabled} -gt 0 ] || { + mwan3_unlock "$ACTION" "$INTERFACE" + exit 0 +} -mwan3_lock "$ACTION" "$INTERFACE" mwan3_init mwan3_set_connected_iptables mwan3_set_custom_ipset -mwan3_unlock "$ACTION" "$INTERFACE" config_get enabled $INTERFACE enabled 0 config_get initial_state $INTERFACE initial_state "online" -[ "$enabled" == "1" ] || exit 0 +[ "${enabled}" == "1" ] || { + mwan3_unlock "$ACTION" "$INTERFACE" + exit 0 +} if [ "$ACTION" == "ifup" ]; then config_get family $INTERFACE family ipv4 @@ -58,7 +63,6 @@ else running=1 fi -mwan3_lock "$ACTION" "$INTERFACE" $LOG notice "Execute "$ACTION" event on interface $INTERFACE (${DEVICE:-unknown})" case "$ACTION" in diff --git a/net/mwan3/files/etc/hotplug.d/iface/16-mwan3 b/net/mwan3/files/etc/hotplug.d/iface/16-mwan3 index c243d55ff..d68401fed 100644 --- a/net/mwan3/files/etc/hotplug.d/iface/16-mwan3 +++ b/net/mwan3/files/etc/hotplug.d/iface/16-mwan3 @@ -4,14 +4,17 @@ . /lib/functions/network.sh . /lib/mwan3/mwan3.sh +mwan3_lock "$ACTION" "mwan3rtmon" + config_load mwan3 config_get_bool enabled globals 'enabled' '0' -[ ${enabled} -gt 0 ] || exit 0 +[ ${enabled} -gt 0 ] || { + mwan3_unlock "$ACTION" "mwan3rtmon" + exit 0 +} if [ "$ACTION" == "ifup" ]; then - mwan3_lock "$ACTION" "mwan3rtmon" mwan3_rtmon - mwan3_unlock "$ACTION" "mwan3rtmon" fi config_get enabled $INTERFACE enabled 0 @@ -19,4 +22,6 @@ config_get enabled $INTERFACE enabled 0 mwan3_flush_conntrack "$INTERFACE" "$ACTION" } +mwan3_unlock "$ACTION" "mwan3rtmon" + exit 0 diff --git a/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user b/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user index 9372c736e..ef490e183 100644 --- a/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user +++ b/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user @@ -2,13 +2,25 @@ [ -f "/etc/mwan3.user" ] && { . /lib/functions.sh + . /lib/mwan3/mwan3.sh + + mwan3_lock "$ACTION" "user" config_load mwan3 config_get_bool enabled globals 'enabled' '0' - [ ${enabled} -gt 0 ] || exit 0 + [ ${enabled} -gt 0 ] || { + mwan3_unlock "$ACTION" "user" + exit 0 + } config_get enabled "$INTERFACE" enabled 0 - [ "${enabled}" = "1" ] || exit 0 + [ "${enabled}" = "1" ] || { + mwan3_unlock "$ACTION" "user" + exit 0 + } + + mwan3_unlock "$ACTION" "user" + env -i ACTION="$ACTION" INTERFACE="$INTERFACE" DEVICE="$DEVICE" \ /bin/sh /etc/mwan3.user } diff --git a/net/mwan3/files/usr/sbin/mwan3 b/net/mwan3/files/usr/sbin/mwan3 index b9a5afb9e..dc65279be 100755 --- a/net/mwan3/files/usr/sbin/mwan3 +++ b/net/mwan3/files/usr/sbin/mwan3 @@ -45,30 +45,39 @@ ifup() { local device enabled up l3_device status + mwan3_lock "command" "mwan3" + config_load mwan3 config_get_bool enabled globals 'enabled' 0 + [ ${enabled} -gt 0 ] || { echo "The service mwan3 is global disabled." echo "Please execute \"/etc/init.d/mwan3 start\" first." + mwan3_unlock "command" "mwan3" exit 1 } if [ -z "$1" ]; then - echo "Expecting interface. Usage: mwan3 ifup " && exit 0 + echo "Expecting interface. Usage: mwan3 ifup " + mwan3_unlock "command" "mwan3" + exit 0 fi if [ -n "$2" ]; then - echo "Too many arguments. Usage: mwan3 ifup " && exit 0 + echo "Too many arguments. Usage: mwan3 ifup " + mwan3_unlock "command" "mwan3" + exit 0 fi + config_get enabled "$1" enabled 0 + mwan3_unlock "command" "mwan3" + status=$(ubus -S call network.interface.$1 status) [ -n "$status" ] && { json_load "$status" json_get_vars up l3_device } - config_get enabled "$1" enabled 0 - if [ "$up" = "1" ] \ && [ -n "$l3_device" ] \ && [ "$enabled" = "1" ]; then @@ -130,7 +139,9 @@ start() { local enabled + mwan3_lock "command" "mwan3" uci_toggle_state mwan3 globals enabled "1" + mwan3_unlock "command" "mwan3" config_load mwan3 config_foreach ifup interface @@ -141,6 +152,7 @@ stop() local ipset route rule table IP IPT pid mwan3_lock "command" "mwan3" + uci_toggle_state mwan3 globals enabled "0" for pid in $(pgrep -f "mwan3rtmon"); do kill -TERM "$pid" > /dev/null 2>&1 @@ -200,8 +212,6 @@ stop() mwan3_lock_clean rm -rf $MWAN3_STATUS_DIR $MWAN3TRACK_STATUS_DIR - - uci_toggle_state mwan3 globals enabled "0" } restart() { From b5bd6d757b8ac9b3eb7fd1e19880924057cce7a7 Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Fri, 24 Jul 2020 08:02:49 +0200 Subject: [PATCH 2/5] mwan3: revert: also cleanup lock on mwan3 stop This reverts commit cde2a77ed3b3b0df9e693e121dccdc97ef163156. Applying this change has shown that it is even quicker to provoke the race condtition on simultan mwan3 commands execution. By reversing the change we have the same behaviour as before. But the race condition on mwan3 execute at the same time still exists. Signed-off-by: Florian Eckert --- net/mwan3/files/lib/mwan3/mwan3.sh | 7 ------- 1 file changed, 7 deletions(-) diff --git a/net/mwan3/files/lib/mwan3/mwan3.sh b/net/mwan3/files/lib/mwan3/mwan3.sh index 2ec760f9a..ab21ad3b2 100644 --- a/net/mwan3/files/lib/mwan3/mwan3.sh +++ b/net/mwan3/files/lib/mwan3/mwan3.sh @@ -204,13 +204,6 @@ mwan3_unlock() { } mwan3_lock_clean() { - for pid in $(pgrep -f "lock /var/run/mwan3.lock"); do - kill -TERM "$pid" > /dev/null 2>&1 - done - sleep 1 - for pid in $(pgrep -f "lock /var/run/mwan3.lock"); do - kill -KILL "$pid" > /dev/null 2>&1 - done rm -rf /var/run/mwan3.lock } From ab747fe0fb3b60c0bd42ea2c4fc045273eb46f6f Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Mon, 27 Jul 2020 09:02:40 +0200 Subject: [PATCH 3/5] mwan3: remove lock file entirely Removing the lock file ist not necessary Signed-off-by: Florian Eckert --- net/mwan3/files/lib/mwan3/mwan3.sh | 4 ---- net/mwan3/files/usr/sbin/mwan3 | 1 - 2 files changed, 5 deletions(-) diff --git a/net/mwan3/files/lib/mwan3/mwan3.sh b/net/mwan3/files/lib/mwan3/mwan3.sh index ab21ad3b2..3c7422dc0 100644 --- a/net/mwan3/files/lib/mwan3/mwan3.sh +++ b/net/mwan3/files/lib/mwan3/mwan3.sh @@ -203,10 +203,6 @@ mwan3_unlock() { lock -u /var/run/mwan3.lock } -mwan3_lock_clean() { - rm -rf /var/run/mwan3.lock -} - mwan3_get_iface_id() { local _tmp _iface _iface_count diff --git a/net/mwan3/files/usr/sbin/mwan3 b/net/mwan3/files/usr/sbin/mwan3 index dc65279be..79a0eba25 100755 --- a/net/mwan3/files/usr/sbin/mwan3 +++ b/net/mwan3/files/usr/sbin/mwan3 @@ -210,7 +210,6 @@ stop() mwan3_unlock "command" "mwan3" - mwan3_lock_clean rm -rf $MWAN3_STATUS_DIR $MWAN3TRACK_STATUS_DIR } From 981d1eb83a3b0676c71559ac8b514c4f8ff56e1b Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Mon, 27 Jul 2020 10:24:59 +0200 Subject: [PATCH 4/5] mwan3: unify variable check Signed-off-by: Florian Eckert --- net/mwan3/files/etc/hotplug.d/iface/15-mwan3 | 12 ++++++------ net/mwan3/files/etc/hotplug.d/iface/16-mwan3 | 8 ++++---- net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/net/mwan3/files/etc/hotplug.d/iface/15-mwan3 b/net/mwan3/files/etc/hotplug.d/iface/15-mwan3 index db2dc237a..645cdd3e4 100644 --- a/net/mwan3/files/etc/hotplug.d/iface/15-mwan3 +++ b/net/mwan3/files/etc/hotplug.d/iface/15-mwan3 @@ -9,13 +9,13 @@ [ -n "$INTERFACE" ] || exit 2 if [ "$ACTION" == "ifup" ]; then - [ -n "$DEVICE" ] || exit 3 + [ -n "$DEVICE" ] || exit 3 fi mwan3_lock "$ACTION" "$INTERFACE" config_load mwan3 config_get_bool enabled globals 'enabled' '0' -[ ${enabled} -gt 0 ] || { +[ "${enabled}" -gt 0 ] || { mwan3_unlock "$ACTION" "$INTERFACE" exit 0 } @@ -24,14 +24,14 @@ mwan3_init mwan3_set_connected_iptables mwan3_set_custom_ipset -config_get enabled $INTERFACE enabled 0 config_get initial_state $INTERFACE initial_state "online" -[ "${enabled}" == "1" ] || { +config_get_bool enabled $INTERFACE 'enabled' '0' +[ "${enabled}" -eq 1 ] || { mwan3_unlock "$ACTION" "$INTERFACE" exit 0 } -if [ "$ACTION" == "ifup" ]; then +if [ "$ACTION" = "ifup" ]; then config_get family $INTERFACE family ipv4 if [ "$family" = "ipv4" ]; then ubus call network.interface.${INTERFACE}_4 status &>/dev/null @@ -72,7 +72,7 @@ case "$ACTION" in mwan3_create_iface_iptables $INTERFACE $DEVICE mwan3_create_iface_rules $INTERFACE $DEVICE mwan3_create_iface_route $INTERFACE $DEVICE - if [ ${running} -eq 1 -a "${status}" = "online" ]; then + if [ "${running}" -eq 1 ] && [ "${status}" = "online" ]; then $LOG notice "Starting tracker on interface $INTERFACE (${DEVICE:-unknown})" mwan3_set_iface_hotplug_state $INTERFACE "online" mwan3_track $INTERFACE $DEVICE "online" "$src_ip" diff --git a/net/mwan3/files/etc/hotplug.d/iface/16-mwan3 b/net/mwan3/files/etc/hotplug.d/iface/16-mwan3 index d68401fed..dd09358eb 100644 --- a/net/mwan3/files/etc/hotplug.d/iface/16-mwan3 +++ b/net/mwan3/files/etc/hotplug.d/iface/16-mwan3 @@ -8,17 +8,17 @@ mwan3_lock "$ACTION" "mwan3rtmon" config_load mwan3 config_get_bool enabled globals 'enabled' '0' -[ ${enabled} -gt 0 ] || { +[ "${enabled}" -gt 0 ] || { mwan3_unlock "$ACTION" "mwan3rtmon" exit 0 } -if [ "$ACTION" == "ifup" ]; then +if [ "$ACTION" = "ifup" ]; then mwan3_rtmon fi -config_get enabled $INTERFACE enabled 0 -[ "${enabled}" = "0" ] || { +config_get_bool enabled "$INTERFACE" 'enabled' '0' +[ "${enabled}" -eq 0 ] || { mwan3_flush_conntrack "$INTERFACE" "$ACTION" } diff --git a/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user b/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user index ef490e183..af28b1f4f 100644 --- a/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user +++ b/net/mwan3/files/etc/hotplug.d/iface/16-mwan3-user @@ -8,13 +8,13 @@ config_load mwan3 config_get_bool enabled globals 'enabled' '0' - [ ${enabled} -gt 0 ] || { + [ "${enabled}" -gt 0 ] || { mwan3_unlock "$ACTION" "user" exit 0 } - config_get enabled "$INTERFACE" enabled 0 - [ "${enabled}" = "1" ] || { + config_get_bool enabled "$INTERFACE" enabled 0 + [ "${enabled}" -eq 1 ] || { mwan3_unlock "$ACTION" "user" exit 0 } From 85e91377cf2f1b74e469ad2a921fbcfc0846db9c Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Fri, 24 Jul 2020 08:09:05 +0200 Subject: [PATCH 5/5] mwan3: update version to 2.8.12 Signed-off-by: Florian Eckert --- net/mwan3/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/mwan3/Makefile b/net/mwan3/Makefile index ce4bfcec2..958c21061 100644 --- a/net/mwan3/Makefile +++ b/net/mwan3/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=mwan3 -PKG_VERSION:=2.8.11 +PKG_VERSION:=2.8.12 PKG_RELEASE:=1 PKG_MAINTAINER:=Florian Eckert PKG_LICENSE:=GPL-2.0