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

Mi Aug 2 00:06:41 CEST 2017

Nachtrag:

Warum steht der Code im configurenetwork?

=> Um Race-Conditions auszuschließen, wollte ich simple-tc erst direkt
aufrufen, wenn das Netzwerk "konfiguriert" ist. Da configurenetwork in der
rc.local aufgerufen wird, also meines Wissens nach allem anderen was es so
als boot-init-Zeugs gibt, muss ich simple-tc wieder danach callen (sonst
gibt es eth0 bzw. eth1 evtl. gar nicht). Also entweder (wie im Patch) am
Ende von configurenetwork einfügen oder als eigenes Skript in rc.local
aufrufen (was mir nicht so gut gefallen hat).

Grüße

Adrian

-----Original Message-----
From: franken-dev [mailto:franken-dev-bounces at freifunk.net] On Behalf Of
Tobias Klaus
Sent: Dienstag, 1. August 2017 00:12
To: franken-dev at freifunk.net; Adrian Schmutzler
<freifunk at adrianschmutzler.de>
Subject: Re: [PATCH v4 01/11] simple-tc: Fix simple-tc not being active if
set

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

--
franken-dev mailing list
franken-dev at freifunk.net
http://lists.freifunk.net/mailman/listinfo/franken-dev-freifunk.net