Difuse/packages
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

sqm-scripts: change default for qdisc target parameter

Browse source 
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 <moeller0@gmx.de>
This commit is contained in:
Sebastian Moeller 2015-03-20 22:13:37 +01:00 committed by Toke Høiland-Jørgensen
parent 99d4c95ad3
commit 1561003a70
2 changed files with 37 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
net/luci-app-sqm/files/sqm-cbi.lua
View file

@ -89,7 +89,7 @@ sc.default = "simple.qos"
sc.rmempty = false sc.rmempty = false
sc.description = qos_desc 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.default = false
ad.rmempty = true ad.rmempty = true
@ -121,7 +121,7 @@ eecn.default = "NOECN"
eecn.rmempty = true eecn.rmempty = true
eecn:depends("qdisc_advanced", "1") 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.default = false
ad2.rmempty = true ad2.rmempty = true
ad2:depends("qdisc_advanced", "1") ad2:depends("qdisc_advanced", "1")
@ -140,12 +140,12 @@ elim.rmempty = true
elim:depends("qdisc_really_really_advanced", "1") 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.datatype = "string"
itarg.rmempty = true itarg.rmempty = true
itarg:depends("qdisc_really_really_advanced", "1") 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.datatype = "string"
etarg.rmempty = true etarg.rmempty = true
etarg:depends("qdisc_really_really_advanced", "1") etarg:depends("qdisc_really_really_advanced", "1")

38
net/sqm-scripts/files/usr/lib/sqm/functions.sh
View file

@ -253,8 +253,8 @@ get_target() {
# either e.g. 100ms or auto # either e.g. 100ms or auto
CUR_TARGET_VALUE=$( echo ${CUR_TARGET} | grep -o -e \^'[[:digit:]]\+' ) CUR_TARGET_VALUE=$( echo ${CUR_TARGET} | grep -o -e \^'[[:digit:]]\+' )
CUR_TARGET_UNIT=$( echo ${CUR_TARGET} | grep -o -e '[[:alpha:]]\+'\$ ) 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_VALUE: $CUR_TARGET_VALUE"
# [ ! -z "$CUR_TARGET" ] && sqm_logger "CUR_TARGET_UNIT: $CUR_TARGET_UNIT" #[ ! -z "$CUR_TARGET" ] && sqm_logger "CUR_TARGET_UNIT: $CUR_TARGET_UNIT"
AUTO_TARGET= AUTO_TARGET=
UNIT_VALID= UNIT_VALID=
@ -271,14 +271,42 @@ get_target() {
;; ;;
esac esac
fi fi
case ${CUR_TARGET_UNIT} in # empty field in GUI or undefined GUI variable now defaults to auto
auto|Auto|AUTO) if [ -z "${CUR_TARGET_VALUE}" -a -z "${CUR_TARGET_UNIT}" ];
then
if [ ! -z "${CUR_LINK_KBPS}" ]; if [ ! -z "${CUR_LINK_KBPS}" ];
then then
TMP_TARGET_US=$( adapt_target_to_slow_link $CUR_LINK_KBPS ) TMP_TARGET_US=$( adapt_target_to_slow_link $CUR_LINK_KBPS )
TMP_INTERVAL_STRING=$( adapt_interval_to_slow_link $TMP_TARGET_US ) TMP_INTERVAL_STRING=$( adapt_interval_to_slow_link $TMP_TARGET_US )
CUR_TARGET_STRING="target ${TMP_TARGET_US}us ${TMP_INTERVAL_STRING}" CUR_TARGET_STRING="target ${TMP_TARGET_US}us ${TMP_INTERVAL_STRING}"
AUTO_TARGET="1" 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 else
sqm_logger "required link bandwidth in kbps not passed to get_target()." sqm_logger "required link bandwidth in kbps not passed to get_target()."
fi fi
@ -288,7 +316,7 @@ get_target() {
then then
if [ -z "${CUR_TARGET_VALUE}" -o -z "${UNIT_VALID}" ]; if [ -z "${CUR_TARGET_VALUE}" -o -z "${UNIT_VALID}" ];
then 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
fi fi
;; ;;