[PATCH v4 01/11] simple-tc: Fix simple-tc not being active if set

Di Aug 1 00:12:08 CEST 2017

Hallo Adrian,

ich hab mir gerade mal das mantis-issue und den Patch angeschaut und das war 
jetzt gar nicht so leicht sich hier eine Meinung zu bilden. 

Wenn ich mich nicht irre liegen dem nicht-Funktionieren von simple-tc doch 2 
Probleme zugrunde:

1. Wir konfigurieren simple-tc nicht ordentlich und lassen den Standardwert 
für das interface mit "example" stehen.
- ich denke hier sollten wir das "ordentlich" konfigurieren und example    
  löschen und das jeweilige "wan" konfigurieren. Dafür bietet sich vermutlich
  ein entsprechendes uci-default script an

2. hotplug.d/net wird nicht mit unserem WAN aufgerufen. 
Nachdem ich auf die schnelle keine ordentliche Doku zu hotplug.de/net gefunden 
habe, gehe ich mal davon aus, dass sich das an der falschen Stelle befindet 
und eher in hotplug.d/iface landen hätte sollen. So oder so würde ich das 
ganze eher über ein init-Skript lösen, das durch ein procd_add_reload_trigger 
"simple-tc" immer mit dem "reload"-Parameter aufgerufen wird, wenn sich was im 
/etc/config/simplce-tc ändert. Dann müssen wir uns auch keine Gedanken machen, 
wann wir es manuell aufrufen müssen. Für ein solches Skript, könnten 
vermutlich die meisten Teile von dem Skript /etc/hotplug.d/net/50-simple-tc 
wieder verwendet werden. Die vorgeschlagene Lösung ist mir hier etwas zu 
"ungenerisch", da sie fest den wert für das interface "example" auf das wan-
interface anwenden.

Zusätzlich zu diesen zwei Überlegungen, frage ich mich noch _wo_ man solche 
Änderungen (vor allem die 2.) unterbringt. Eigentlich wäre hier ein Patch 
gegen den gluon-Feed, der durch unser buildskript aufgespielt wird, fast das 
sauberste. Diesen könnte man dann auch versuchen upstream zu kriegen. 
Allerdings können wir das auch wie vorgeschlagen in fff-network lassen(das für 
uns kaputte hotplug Skript tut ja nicht weh) oder aber (das finde ich wiederum 
etwas sauberer) ein fff-simpletc Paket einführen.

Wenn ich mir das so ansehe weiß ich wieder, warum ich simple-tc immer 
ignoriert habe und gehofft habe, dass es einfach tut.... :-D

Ich hab inline noch ein paar Anmerkungen zum Code.

Viele Grüße
Tobias

Am Freitag, 21. Juli 2017, 15:08:39 CEST schrieb Adrian Schmutzler:
> Simple-tc is active immediately after the call and needs no
> restart of anything
> 
> Signed-off-by: Adrian Schmutzler <freifunk at adrianschmutzler.de>
> 
> Tested-by: Adrian Schmutzler <freifunk at adrianschmutzler.de>
> ---
>  src/packages/fff/fff-network/Makefile                      |  2 +-
>  src/packages/fff/fff-network/files/usr/sbin/applysimpletc  | 14
> ++++++++++++++ .../fff/fff-network/files/usr/sbin/configurenetwork        |
> 12 ++++++++++++ .../fff/fff-web/files/www/ssl/cgi-bin/settings.html       
> |  3 +++ 4 files changed, 30 insertions(+), 1 deletion(-)
>  create mode 100755
> src/packages/fff/fff-network/files/usr/sbin/applysimpletc
> 
> diff --git a/src/packages/fff/fff-network/Makefile
> b/src/packages/fff/fff-network/Makefile index fee3f98..f7dc63c 100644
> --- a/src/packages/fff/fff-network/Makefile
> +++ b/src/packages/fff/fff-network/Makefile
> @@ -1,7 +1,7 @@
>  include $(TOPDIR)/rules.mk
> 
>  PKG_NAME:=fff-network
> -PKG_VERSION:=0.0.6
> +PKG_VERSION:=7
Das ist ein gehöriger Versionssprung. ;-)

>  PKG_RELEASE:=1
> 
>  PKG_BUILD_DIR:=$(BUILD_DIR)/fff-network
> diff --git a/src/packages/fff/fff-network/files/usr/sbin/applysimpletc
> b/src/packages/fff/fff-network/files/usr/sbin/applysimpletc new file mode
> 100755
> index 0000000..a9744b0
> --- /dev/null
> +++ b/src/packages/fff/fff-network/files/usr/sbin/applysimpletc
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +tc_device=$(uci -q get "network.wan.ifname")
> +tc_enabled=$(uci -q get "simple-tc.example.enabled")
> +tc_in=$(uci -q get "simple-tc.example.limit_ingress")
> +tc_out=$(uci -q get "simple-tc.example.limit_egress")
> +
> +if [ "$tc_enabled" -eq 1 ] ; then
> +	test -n "$tc_in" || tc_in=-
> +	test -n "$tc_out" || tc_out=-
tc_out=${tc_out:-"-"} ist etwas eleganter und weißt tc_out den Wert von 
$tc_out zu oder, falls nicht vorhanden, "-".

> +	simple-tc "$tc_device" "$tc_in" "$tc_out"
Wenn man diese Zeile ans Ende des if/else zieht kann man sich den else-Zweig 
sparen und muss in Zukunft nur einen simple-tc-Aufruf "pflegen".
> +else
> +	simple-tc "$tc_device" - -
> +fi
> diff --git a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork index
> 7ecfa3b..63b216a 100755
> --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> @@ -189,3 +189,15 @@ else
> 
>      /etc/init.d/fff-uradvd restart
>  fi
> +
> +# Apply traffic control
Wenn wir das skript "applysimpletc" doch drin lassen, sollte es hier direkt 
aufgerufen werden. Auch so sparen wir uns duplizierten und vor allem 
dupliziert zu pflegenden Code.

> +tc_device=$(uci -q get "network.wan.ifname")
> +tc_enabled=$(uci -q get "simple-tc.example.enabled")
> +tc_in=$(uci -q get "simple-tc.example.limit_ingress")
> +tc_out=$(uci -q get "simple-tc.example.limit_egress")
> +
> +if [ "$tc_enabled" -eq 1 ] ; then
> +	test -n "$tc_in" || tc_in=-
> +	test -n "$tc_out" || tc_out=-
> +	simple-tc "$tc_device" "$tc_in" "$tc_out"
> +fi
> diff --git a/src/packages/fff/fff-web/files/www/ssl/cgi-bin/settings.html
> b/src/packages/fff/fff-web/files/www/ssl/cgi-bin/settings.html index
> a7417dc..bdbd69b 100755
> --- a/src/packages/fff/fff-web/files/www/ssl/cgi-bin/settings.html
> +++ b/src/packages/fff/fff-web/files/www/ssl/cgi-bin/settings.html
> @@ -32,6 +32,9 @@ if [ "$REQUEST_METHOD" == "POST" ] ; then
>  		uci -q set "simple-tc.example.limit_egress=${POST_limit_egress}"
> 
>  		uci commit
> +
> +		/usr/sbin/applysimpletc
> +
>  		MSG='<span class="green">Daten gespeichert! - Bitte Router
> neustarten.</span>' fi
>  fi