From: Sebastian Moeller Date: Fri, 20 Mar 2015 21:13:37 +0000 (+0100) Subject: sqm-scripts: change default for qdisc target parameter X-Git-Url: http://git.lede-project.org./?a=commitdiff_plain;h=1561003a7017cf9bffae25b82c71bc26d749f411;p=feed%2Fpackages.git sqm-scripts: change default for qdisc target parameter Alan Jenkins noted a bug in the smq luci GUI that effectively erased several configuration paramters if two checkboxes were deselected. This behaviour seems consistent in luci but certainly has the potential to confuse users. While confusion can not really be avoided generally it seems wise to change the default interpretation for empty or non-existent itarget and etarget variables from the qdisc's default (5ms in the case of one of the codels) to automatic determination of tghis variable dependent on the configured bandwidth, as codels target variable should be large enough to contain at least one full packet. With this change sqm-scripts will do the right thing by default, but will yet allow the user to specify over-ridding values (as long as the user does not un-check the entry-field exposing check boxes). Survives light testing... This change set also changes the sqm-scripts luci gui to note the user of the change. For compatibility with existing setups sqm-scripts will still honor "auto" as an alternative explicit way of requesting automatic target selection. This might turn into a warning in the future and might be phased out... Signed-off-by: Sebastian Moeller --- diff --git a/net/luci-app-sqm/files/sqm-cbi.lua b/net/luci-app-sqm/files/sqm-cbi.lua index e7d79df00e..8379025979 100644 --- a/net/luci-app-sqm/files/sqm-cbi.lua +++ b/net/luci-app-sqm/files/sqm-cbi.lua @@ -89,7 +89,7 @@ sc.default = "simple.qos" sc.rmempty = false sc.description = qos_desc -ad = s:taboption("tab_qdisc", Flag, "qdisc_advanced", translate("Show Advanced Configuration")) +ad = s:taboption("tab_qdisc", Flag, "qdisc_advanced", translate("Show and Use Advanced Configuration")) ad.default = false ad.rmempty = true @@ -121,7 +121,7 @@ eecn.default = "NOECN" eecn.rmempty = true eecn:depends("qdisc_advanced", "1") -ad2 = s:taboption("tab_qdisc", Flag, "qdisc_really_really_advanced", translate("Show Dangerous Configuration")) +ad2 = s:taboption("tab_qdisc", Flag, "qdisc_really_really_advanced", translate("Show and Use Dangerous Configuration")) ad2.default = false ad2.rmempty = true ad2:depends("qdisc_advanced", "1") @@ -140,12 +140,12 @@ elim.rmempty = true elim:depends("qdisc_really_really_advanced", "1") -itarg = s:taboption("tab_qdisc", Value, "itarget", translate("Latency target for ingress, e.g 5ms [units: s, ms, or us]; leave empty for default, or auto for automatic selection.")) +itarg = s:taboption("tab_qdisc", Value, "itarget", translate("Latency target for ingress, e.g 5ms [units: s, ms, or us]; leave empty for automatic selection, put in the word default for the qdisc's default.")) itarg.datatype = "string" itarg.rmempty = true itarg:depends("qdisc_really_really_advanced", "1") -etarg = s:taboption("tab_qdisc", Value, "etarget", translate("Latency target for egress, e.g. 5ms [units: s, ms, or us]; leave empty for default, or auto for automatic selection.")) +etarg = s:taboption("tab_qdisc", Value, "etarget", translate("Latency target for egress, e.g. 5ms [units: s, ms, or us]; leave empty for automatic selection, put in the word default for the qdisc's default.")) etarg.datatype = "string" etarg.rmempty = true etarg:depends("qdisc_really_really_advanced", "1") diff --git a/net/sqm-scripts/files/usr/lib/sqm/functions.sh b/net/sqm-scripts/files/usr/lib/sqm/functions.sh index 3411b8f31e..a0b2c6ffde 100644 --- a/net/sqm-scripts/files/usr/lib/sqm/functions.sh +++ b/net/sqm-scripts/files/usr/lib/sqm/functions.sh @@ -253,8 +253,8 @@ get_target() { # either e.g. 100ms or auto CUR_TARGET_VALUE=$( echo ${CUR_TARGET} | grep -o -e \^'[[:digit:]]\+' ) CUR_TARGET_UNIT=$( echo ${CUR_TARGET} | grep -o -e '[[:alpha:]]\+'\$ ) -# [ ! -z "$CUR_TARGET" ] && sqm_logger "CUR_TARGET_VALUE: $CUR_TARGET_VALUE" -# [ ! -z "$CUR_TARGET" ] && sqm_logger "CUR_TARGET_UNIT: $CUR_TARGET_UNIT" + #[ ! -z "$CUR_TARGET" ] && sqm_logger "CUR_TARGET_VALUE: $CUR_TARGET_VALUE" + #[ ! -z "$CUR_TARGET" ] && sqm_logger "CUR_TARGET_UNIT: $CUR_TARGET_UNIT" AUTO_TARGET= UNIT_VALID= @@ -271,14 +271,42 @@ get_target() { ;; esac fi - case ${CUR_TARGET_UNIT} in - auto|Auto|AUTO) + # empty field in GUI or undefined GUI variable now defaults to auto + if [ -z "${CUR_TARGET_VALUE}" -a -z "${CUR_TARGET_UNIT}" ]; + then if [ ! -z "${CUR_LINK_KBPS}" ]; then TMP_TARGET_US=$( adapt_target_to_slow_link $CUR_LINK_KBPS ) TMP_INTERVAL_STRING=$( adapt_interval_to_slow_link $TMP_TARGET_US ) CUR_TARGET_STRING="target ${TMP_TARGET_US}us ${TMP_INTERVAL_STRING}" AUTO_TARGET="1" + sqm_logger "get_target defaulting to auto." + else + sqm_logger "required link bandwidth in kbps not passed to get_target()." + fi + fi + # but still allow explicit use of the keyword auto for backward compatibility + case ${CUR_TARGET_UNIT} in + auto|Auto|AUTO) + if [ ! -z "${CUR_LINK_KBPS}" ]; + then + TMP_TARGET_US=$( adapt_target_to_slow_link $CUR_LINK_KBPS ) + TMP_INTERVAL_STRING=$( adapt_interval_to_slow_link $TMP_TARGET_US ) + CUR_TARGET_STRING="target ${TMP_TARGET_US}us ${TMP_INTERVAL_STRING}" + AUTO_TARGET="1" + else + sqm_logger "required link bandwidth in kbps not passed to get_target()." + fi + ;; + esac + + case ${CUR_TARGET_UNIT} in + default|Default|DEFAULT) + if [ ! -z "${CUR_LINK_KBPS}" ]; + then + CUR_TARGET_STRING="" # return nothing so the default target is not over-ridden... + AUTO_TARGET="1" + #sqm_logger "get_target using qdisc default, no explicit target string passed." else sqm_logger "required link bandwidth in kbps not passed to get_target()." fi @@ -288,7 +316,7 @@ get_target() { then if [ -z "${CUR_TARGET_VALUE}" -o -z "${UNIT_VALID}" ]; then - [ -z "$AUTO_TARGET" ] && sqm_logger "${CUR_TARGET} is not a well formed tc target specifier; e.g.: 5ms (or s, us), or the string auto." + [ -z "$AUTO_TARGET" ] && sqm_logger "${CUR_TARGET} is not a well formed tc target specifier; e.g.: 5ms (or s, us), or one of the strings auto or default." fi fi ;;