vpn-select: Demand hood file to be provided as argument

Submitted by Adrian Schmutzler on July 28, 2018, 9:15 p.m.

Details

Message ID 20180728211523.10210-1-freifunk@adrianschmutzler.de
State Superseded
Headers show

Commit Message

Adrian Schmutzler July 28, 2018, 9:15 p.m.
By removing the reference to the hood file from vpn-select, we
remove the entire dependency from fff-hoodutils.
vpn-select will now work with any file provided, as long as
it has the correct syntax. At the moment, the only provider
is the configurehood script. Since the various hood file variants
are handled there, it seems logical that configurehood also
chooses and provides the correct hood file for vpn-select, instead
of vpn-select which had no other contact with hood file choice.

This is simple, tidy and effective.

Fixes #106

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

---

I raised the version for fff-vpn-select by 2, since it still was
on 1. The never existant version 2 would have been the initial
KeyXchangeV2 version.
---
 src/packages/fff/fff-hoods/Makefile                       | 2 +-
 src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
 src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
 src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
 4 files changed, 7 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/packages/fff/fff-hoods/Makefile b/src/packages/fff/fff-hoods/Makefile
index b565ac7..93fd430 100644
--- a/src/packages/fff/fff-hoods/Makefile
+++ b/src/packages/fff/fff-hoods/Makefile
@@ -1,7 +1,7 @@ 
 include $(TOPDIR)/rules.mk
 
 PKG_NAME:=fff-hoods
-PKG_VERSION:=2
+PKG_VERSION:=3
 PKG_RELEASE:=1
 
 PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
index 57c6f9f..6565c80 100755
--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
@@ -195,7 +195,7 @@  if [ -s "$hoodfiletmp" ]; then
 	# and now we get to vpn-select script and load VPNs directly from /tmp/keyxchangev2data
 	
 	if hasInternet ; then
-		sh /usr/sbin/vpn-select
+		sh /usr/sbin/vpn-select "$hoodfiletmp"
 	else
 		sh /usr/sbin/vpn-stop
 	fi
diff --git a/src/packages/fff/fff-vpn-select/Makefile b/src/packages/fff/fff-vpn-select/Makefile
index 4e2d89b..27cff09 100644
--- a/src/packages/fff/fff-vpn-select/Makefile
+++ b/src/packages/fff/fff-vpn-select/Makefile
@@ -1,7 +1,7 @@ 
 include $(TOPDIR)/rules.mk
 
 PKG_NAME:=fff-vpn-select
-PKG_VERSION:=1
+PKG_VERSION:=3
 PKG_RELEASE:=1
 
 PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
index 7c9bced..86236e8 100755
--- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
+++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
@@ -1,15 +1,16 @@ 
 #!/bin/sh
 
-. /lib/functions/fff/keyxchange
 . /usr/share/libubox/jshn.sh
 
+hoodfile="$1"
+
 make_config() {
 # remove old config
 >/etc/config/tunneldigger
 rm /tmp/fastd_fff_peers/*
 count=0
 Index=1
-json_load "$(cat "$hoodfiletmp")"
+json_load "$(cat "$hoodfile")"
 json_select vpn
 # get fastd peers
 while json_select "$Index" > /dev/null
@@ -54,7 +55,7 @@  json_select ".." # back to root
 # main
 
 # Only do something when file is here and greater 0 byte
-if [ -s "$hoodfiletmp" ]; then
+if [ -s "$hoodfile" ]; then
 	# set some vars
 	hostname=$(cat /proc/sys/kernel/hostname)
 	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }' /sys/class/net/br-mesh/address 2>/dev/null)

Comments

Robert Langhammer July 28, 2018, 10:16 p.m.
Hi Adrian,

die mehrheitliche Meinung scheint ja zu sein, hoodlocal zu behalten.
Hierfür ist diese Lösung gut, da wir auch noch eine Abhängigkeit los werden.

Reviewed-by: Robert Langhammer <rlanghammer@web.de>

Am 28.07.2018 um 23:15 schrieb Adrian Schmutzler:
> By removing the reference to the hood file from vpn-select, we
> remove the entire dependency from fff-hoodutils.
> vpn-select will now work with any file provided, as long as
> it has the correct syntax. At the moment, the only provider
> is the configurehood script. Since the various hood file variants
> are handled there, it seems logical that configurehood also
> chooses and provides the correct hood file for vpn-select, instead
> of vpn-select which had no other contact with hood file choice.
>
> This is simple, tidy and effective.
>
> Fixes #106
>
> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>
> ---
>
> I raised the version for fff-vpn-select by 2, since it still was
> on 1. The never existant version 2 would have been the initial
> KeyXchangeV2 version.
> ---
>  src/packages/fff/fff-hoods/Makefile                       | 2 +-
>  src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
>  src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
>  src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
>  4 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/packages/fff/fff-hoods/Makefile b/src/packages/fff/fff-hoods/Makefile
> index b565ac7..93fd430 100644
> --- a/src/packages/fff/fff-hoods/Makefile
> +++ b/src/packages/fff/fff-hoods/Makefile
> @@ -1,7 +1,7 @@
>  include $(TOPDIR)/rules.mk
>  
>  PKG_NAME:=fff-hoods
> -PKG_VERSION:=2
> +PKG_VERSION:=3
>  PKG_RELEASE:=1
>  
>  PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> index 57c6f9f..6565c80 100755
> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> @@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
>  	# and now we get to vpn-select script and load VPNs directly from /tmp/keyxchangev2data
>  	
>  	if hasInternet ; then
> -		sh /usr/sbin/vpn-select
> +		sh /usr/sbin/vpn-select "$hoodfiletmp"
>  	else
>  		sh /usr/sbin/vpn-stop
>  	fi
> diff --git a/src/packages/fff/fff-vpn-select/Makefile b/src/packages/fff/fff-vpn-select/Makefile
> index 4e2d89b..27cff09 100644
> --- a/src/packages/fff/fff-vpn-select/Makefile
> +++ b/src/packages/fff/fff-vpn-select/Makefile
> @@ -1,7 +1,7 @@
>  include $(TOPDIR)/rules.mk
>  
>  PKG_NAME:=fff-vpn-select
> -PKG_VERSION:=1
> +PKG_VERSION:=3
>  PKG_RELEASE:=1
>  
>  PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> index 7c9bced..86236e8 100755
> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> @@ -1,15 +1,16 @@
>  #!/bin/sh
>  
> -. /lib/functions/fff/keyxchange
>  . /usr/share/libubox/jshn.sh
>  
> +hoodfile="$1"
> +
>  make_config() {
>  # remove old config
>  >/etc/config/tunneldigger
>  rm /tmp/fastd_fff_peers/*
>  count=0
>  Index=1
> -json_load "$(cat "$hoodfiletmp")"
> +json_load "$(cat "$hoodfile")"
>  json_select vpn
>  # get fastd peers
>  while json_select "$Index" > /dev/null
> @@ -54,7 +55,7 @@ json_select ".." # back to root
>  # main
>  
>  # Only do something when file is here and greater 0 byte
> -if [ -s "$hoodfiletmp" ]; then
> +if [ -s "$hoodfile" ]; then
>  	# set some vars
>  	hostname=$(cat /proc/sys/kernel/hostname)
>  	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }' /sys/class/net/br-mesh/address 2>/dev/null)
Adrian Schmutzler July 28, 2018, 10:38 p.m.
Hallo,

ich bin schon dafür, hoodlocal zu behalten.

Aber selbst wenn wir es entfernen oder Fabians Lösung nehmen, würde ich den Patch unten trotzdem einspielen, weil er die Abhängigkeit entfernt.

Grüße

Adrian

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of robert
> Sent: Sonntag, 29. Juli 2018 00:16
> To: franken-dev@freifunk.net
> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
> argument
> 
> Hi Adrian,
> 
> die mehrheitliche Meinung scheint ja zu sein, hoodlocal zu behalten.
> Hierfür ist diese Lösung gut, da wir auch noch eine Abhängigkeit los werden.
> 
> Reviewed-by: Robert Langhammer <rlanghammer@web.de>
> 
> Am 28.07.2018 um 23:15 schrieb Adrian Schmutzler:
> > By removing the reference to the hood file from vpn-select, we remove
> > the entire dependency from fff-hoodutils.
> > vpn-select will now work with any file provided, as long as it has the
> > correct syntax. At the moment, the only provider is the configurehood
> > script. Since the various hood file variants are handled there, it
> > seems logical that configurehood also chooses and provides the correct
> > hood file for vpn-select, instead of vpn-select which had no other
> > contact with hood file choice.
> >
> > This is simple, tidy and effective.
> >
> > Fixes #106
> >
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> > ---
> >
> > I raised the version for fff-vpn-select by 2, since it still was on 1.
> > The never existant version 2 would have been the initial
> > KeyXchangeV2 version.
> > ---
> >  src/packages/fff/fff-hoods/Makefile                       | 2 +-
> >  src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
> >  src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
> >  src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
> >  4 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/packages/fff/fff-hoods/Makefile
> > b/src/packages/fff/fff-hoods/Makefile
> > index b565ac7..93fd430 100644
> > --- a/src/packages/fff/fff-hoods/Makefile
> > +++ b/src/packages/fff/fff-hoods/Makefile
> > @@ -1,7 +1,7 @@
> >  include $(TOPDIR)/rules.mk
> >
> >  PKG_NAME:=fff-hoods
> > -PKG_VERSION:=2
> > +PKG_VERSION:=3
> >  PKG_RELEASE:=1
> >
> >  PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > index 57c6f9f..6565c80 100755
> > --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > @@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
> >  	# and now we get to vpn-select script and load VPNs directly from
> > /tmp/keyxchangev2data
> >
> >  	if hasInternet ; then
> > -		sh /usr/sbin/vpn-select
> > +		sh /usr/sbin/vpn-select "$hoodfiletmp"
> >  	else
> >  		sh /usr/sbin/vpn-stop
> >  	fi
> > diff --git a/src/packages/fff/fff-vpn-select/Makefile
> > b/src/packages/fff/fff-vpn-select/Makefile
> > index 4e2d89b..27cff09 100644
> > --- a/src/packages/fff/fff-vpn-select/Makefile
> > +++ b/src/packages/fff/fff-vpn-select/Makefile
> > @@ -1,7 +1,7 @@
> >  include $(TOPDIR)/rules.mk
> >
> >  PKG_NAME:=fff-vpn-select
> > -PKG_VERSION:=1
> > +PKG_VERSION:=3
> >  PKG_RELEASE:=1
> >
> >  PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > index 7c9bced..86236e8 100755
> > --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > @@ -1,15 +1,16 @@
> >  #!/bin/sh
> >
> > -. /lib/functions/fff/keyxchange
> >  . /usr/share/libubox/jshn.sh
> >
> > +hoodfile="$1"
> > +
> >  make_config() {
> >  # remove old config
> >  >/etc/config/tunneldigger
> >  rm /tmp/fastd_fff_peers/*
> >  count=0
> >  Index=1
> > -json_load "$(cat "$hoodfiletmp")"
> > +json_load "$(cat "$hoodfile")"
> >  json_select vpn
> >  # get fastd peers
> >  while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@ json_select
> > ".." # back to root  # main
> >
> >  # Only do something when file is here and greater 0 byte -if [ -s
> > "$hoodfiletmp" ]; then
> > +if [ -s "$hoodfile" ]; then
> >  	# set some vars
> >  	hostname=$(cat /proc/sys/kernel/hostname)
> >  	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
> > /sys/class/net/br-mesh/address 2>/dev/null)
>
Tim Niemeyer July 29, 2018, 9:06 a.m.
Hi

Sieht sehr gut aus. Unten ne Kleinigkeit.


Am 28. Juli 2018 23:15:23 MESZ schrieb Adrian Schmutzler <freifunk@adrianschmutzler.de>:
>By removing the reference to the hood file from vpn-select, we
>remove the entire dependency from fff-hoodutils.
>vpn-select will now work with any file provided, as long as
>it has the correct syntax. At the moment, the only provider
>is the configurehood script. Since the various hood file variants
>are handled there, it seems logical that configurehood also
>chooses and provides the correct hood file for vpn-select, instead
>of vpn-select which had no other contact with hood file choice.
>
>This is simple, tidy and effective.
>
>Fixes #106
>
>Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>
>---
>
>I raised the version for fff-vpn-select by 2, since it still was
>on 1. The never existant version 2 would have been the initial
>KeyXchangeV2 version.
>---
> src/packages/fff/fff-hoods/Makefile                       | 2 +-
> src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
> src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
> src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
> 4 files changed, 7 insertions(+), 6 deletions(-)
>
>diff --git a/src/packages/fff/fff-hoods/Makefile
>b/src/packages/fff/fff-hoods/Makefile
>index b565ac7..93fd430 100644
>--- a/src/packages/fff/fff-hoods/Makefile
>+++ b/src/packages/fff/fff-hoods/Makefile
>@@ -1,7 +1,7 @@
> include $(TOPDIR)/rules.mk
> 
> PKG_NAME:=fff-hoods
>-PKG_VERSION:=2
>+PKG_VERSION:=3
> PKG_RELEASE:=1
> 
> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>index 57c6f9f..6565c80 100755
>--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>@@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
>	# and now we get to vpn-select script and load VPNs directly from
>/tmp/keyxchangev2data
> 	
> 	if hasInternet ; then
>-		sh /usr/sbin/vpn-select
>+		sh /usr/sbin/vpn-select "$hoodfiletmp"
> 	else
> 		sh /usr/sbin/vpn-stop
> 	fi
>diff --git a/src/packages/fff/fff-vpn-select/Makefile
>b/src/packages/fff/fff-vpn-select/Makefile
>index 4e2d89b..27cff09 100644
>--- a/src/packages/fff/fff-vpn-select/Makefile
>+++ b/src/packages/fff/fff-vpn-select/Makefile
>@@ -1,7 +1,7 @@
> include $(TOPDIR)/rules.mk
> 
> PKG_NAME:=fff-vpn-select
>-PKG_VERSION:=1
>+PKG_VERSION:=3
> PKG_RELEASE:=1
> 
> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>index 7c9bced..86236e8 100755
>--- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>+++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>@@ -1,15 +1,16 @@
> #!/bin/sh
> 
>-. /lib/functions/fff/keyxchange
> . /usr/share/libubox/jshn.sh
> 
>+hoodfile="$1"
>+

Bitte noch kurz prüfen ob der Parameter übergeben wurde und am besten auch, ob es eine lesbare Datei ist. Ansonsten mit Fehlermeldung ausbrechen.

Tim


> make_config() {
> # remove old config
> >/etc/config/tunneldigger
> rm /tmp/fastd_fff_peers/*
> count=0
> Index=1
>-json_load "$(cat "$hoodfiletmp")"
>+json_load "$(cat "$hoodfile")"
> json_select vpn
> # get fastd peers
> while json_select "$Index" > /dev/null
>@@ -54,7 +55,7 @@ json_select ".." # back to root
> # main
> 
> # Only do something when file is here and greater 0 byte
>-if [ -s "$hoodfiletmp" ]; then
>+if [ -s "$hoodfile" ]; then
> 	# set some vars
> 	hostname=$(cat /proc/sys/kernel/hostname)
>	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
>/sys/class/net/br-mesh/address 2>/dev/null)
Adrian Schmutzler July 29, 2018, 9:57 a.m.
Hallo Tim,

siehe unten.

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of Tim Niemeyer
> Sent: Sonntag, 29. Juli 2018 11:07
> To: franken-dev@freifunk.net
> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
> argument
> 
> Hi
> 
> Sieht sehr gut aus. Unten ne Kleinigkeit.
> 
> 
> Am 28. Juli 2018 23:15:23 MESZ schrieb Adrian Schmutzler
> <freifunk@adrianschmutzler.de>:
> >By removing the reference to the hood file from vpn-select, we remove
> >the entire dependency from fff-hoodutils.
> >vpn-select will now work with any file provided, as long as it has the
> >correct syntax. At the moment, the only provider is the configurehood
> >script. Since the various hood file variants are handled there, it
> >seems logical that configurehood also chooses and provides the correct
> >hood file for vpn-select, instead of vpn-select which had no other
> >contact with hood file choice.
> >
> >This is simple, tidy and effective.
> >
> >Fixes #106
> >
> >Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> >---
> >
> >I raised the version for fff-vpn-select by 2, since it still was on 1.
> >The never existant version 2 would have been the initial
> >KeyXchangeV2 version.
> >---
> > src/packages/fff/fff-hoods/Makefile                       | 2 +-
> > src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
> > src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
> > src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
> > 4 files changed, 7 insertions(+), 6 deletions(-)
> >
> >diff --git a/src/packages/fff/fff-hoods/Makefile
> >b/src/packages/fff/fff-hoods/Makefile
> >index b565ac7..93fd430 100644
> >--- a/src/packages/fff/fff-hoods/Makefile
> >+++ b/src/packages/fff/fff-hoods/Makefile
> >@@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> >
> > PKG_NAME:=fff-hoods
> >-PKG_VERSION:=2
> >+PKG_VERSION:=3
> > PKG_RELEASE:=1
> >
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> >diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >index 57c6f9f..6565c80 100755
> >--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >@@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
> >	# and now we get to vpn-select script and load VPNs directly from
> >/tmp/keyxchangev2data
> >
> > 	if hasInternet ; then
> >-		sh /usr/sbin/vpn-select
> >+		sh /usr/sbin/vpn-select "$hoodfiletmp"
> > 	else
> > 		sh /usr/sbin/vpn-stop
> > 	fi
> >diff --git a/src/packages/fff/fff-vpn-select/Makefile
> >b/src/packages/fff/fff-vpn-select/Makefile
> >index 4e2d89b..27cff09 100644
> >--- a/src/packages/fff/fff-vpn-select/Makefile
> >+++ b/src/packages/fff/fff-vpn-select/Makefile
> >@@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> >
> > PKG_NAME:=fff-vpn-select
> >-PKG_VERSION:=1
> >+PKG_VERSION:=3
> > PKG_RELEASE:=1
> >
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> >diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >index 7c9bced..86236e8 100755
> >--- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >+++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >@@ -1,15 +1,16 @@
> > #!/bin/sh
> >
> >-. /lib/functions/fff/keyxchange
> > . /usr/share/libubox/jshn.sh
> >
> >+hoodfile="$1"
> >+
> 
> Bitte noch kurz prüfen ob der Parameter übergeben wurde und am besten
> auch, ob es eine lesbare Datei ist. Ansonsten mit Fehlermeldung ausbrechen.
> 
> Tim

Wir prüfen das weiter unten, nämlich ...

> 
> 
> > make_config() {
> > # remove old config
> > >/etc/config/tunneldigger
> > rm /tmp/fastd_fff_peers/*
> > count=0
> > Index=1
> >-json_load "$(cat "$hoodfiletmp")"
> >+json_load "$(cat "$hoodfile")"
> > json_select vpn
> > # get fastd peers
> > while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@ json_select
> >".." # back to root  # main
> >
> > # Only do something when file is here and greater 0 byte -if [ -s
> >"$hoodfiletmp" ]; then
> >+if [ -s "$hoodfile" ]; then

... hier. Wenn $1 nicht gesetzt wurde, nicht zu einer lesbaren Datei führt oder zu einer leeren Datei führt, tut das Skript einfach nichts.

Es gibt allerdings nicht die von dir gewünschte Fehlermeldung, das Skript tut einfach nur etwas, wenn es die richtigen Daten bekommt. Für mich wäre das so okay.

Wenn du die Fehlermeldung möchtest, könnte man das hier als "else" mit einbauen, dann prüft man gleich den File, und nicht nur das Argument.

Bitte kommentieren.

Grüße

Adrian


> > 	# set some vars
> > 	hostname=$(cat /proc/sys/kernel/hostname)
> >	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
> >/sys/class/net/br-mesh/address 2>/dev/null)
Tim Niemeyer July 29, 2018, 10:03 a.m.
Hi Adrian

Am 29. Juli 2018 11:57:42 MESZ schrieb mail@adrianschmutzler.de:
>Hallo Tim,
>
>siehe unten.
>
>> -----Original Message-----
>> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
>> Of Tim Niemeyer
>> Sent: Sonntag, 29. Juli 2018 11:07
>> To: franken-dev@freifunk.net
>> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
>> argument
>> 
>> Hi
>> 
>> Sieht sehr gut aus. Unten ne Kleinigkeit.
>> 
>> 
>> Am 28. Juli 2018 23:15:23 MESZ schrieb Adrian Schmutzler
>> <freifunk@adrianschmutzler.de>:
>> >By removing the reference to the hood file from vpn-select, we
>remove
>> >the entire dependency from fff-hoodutils.
>> >vpn-select will now work with any file provided, as long as it has
>the
>> >correct syntax. At the moment, the only provider is the
>configurehood
>> >script. Since the various hood file variants are handled there, it
>> >seems logical that configurehood also chooses and provides the
>correct
>> >hood file for vpn-select, instead of vpn-select which had no other
>> >contact with hood file choice.
>> >
>> >This is simple, tidy and effective.
>> >
>> >Fixes #106
>> >
>> >Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>> >
>> >---
>> >
>> >I raised the version for fff-vpn-select by 2, since it still was on
>1.
>> >The never existant version 2 would have been the initial
>> >KeyXchangeV2 version.
>> >---
>> > src/packages/fff/fff-hoods/Makefile                       | 2 +-
>> > src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
>> > src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
>> > src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7
>++++---
>> > 4 files changed, 7 insertions(+), 6 deletions(-)
>> >
>> >diff --git a/src/packages/fff/fff-hoods/Makefile
>> >b/src/packages/fff/fff-hoods/Makefile
>> >index b565ac7..93fd430 100644
>> >--- a/src/packages/fff/fff-hoods/Makefile
>> >+++ b/src/packages/fff/fff-hoods/Makefile
>> >@@ -1,7 +1,7 @@
>> > include $(TOPDIR)/rules.mk
>> >
>> > PKG_NAME:=fff-hoods
>> >-PKG_VERSION:=2
>> >+PKG_VERSION:=3
>> > PKG_RELEASE:=1
>> >
>> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>> >diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> >b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> >index 57c6f9f..6565c80 100755
>> >--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> >+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> >@@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
>> >	# and now we get to vpn-select script and load VPNs directly from
>> >/tmp/keyxchangev2data
>> >
>> > 	if hasInternet ; then
>> >-		sh /usr/sbin/vpn-select
>> >+		sh /usr/sbin/vpn-select "$hoodfiletmp"
>> > 	else
>> > 		sh /usr/sbin/vpn-stop
>> > 	fi
>> >diff --git a/src/packages/fff/fff-vpn-select/Makefile
>> >b/src/packages/fff/fff-vpn-select/Makefile
>> >index 4e2d89b..27cff09 100644
>> >--- a/src/packages/fff/fff-vpn-select/Makefile
>> >+++ b/src/packages/fff/fff-vpn-select/Makefile
>> >@@ -1,7 +1,7 @@
>> > include $(TOPDIR)/rules.mk
>> >
>> > PKG_NAME:=fff-vpn-select
>> >-PKG_VERSION:=1
>> >+PKG_VERSION:=3
>> > PKG_RELEASE:=1
>> >
>> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>> >diff --git
>a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> >b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> >index 7c9bced..86236e8 100755
>> >--- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> >+++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> >@@ -1,15 +1,16 @@
>> > #!/bin/sh
>> >
>> >-. /lib/functions/fff/keyxchange
>> > . /usr/share/libubox/jshn.sh
>> >
>> >+hoodfile="$1"
>> >+
>> 
>> Bitte noch kurz prüfen ob der Parameter übergeben wurde und am besten
>> auch, ob es eine lesbare Datei ist. Ansonsten mit Fehlermeldung
>ausbrechen.
>> 
>> Tim
>
>Wir prüfen das weiter unten, nämlich ...
>
>> 
>> 
>> > make_config() {
>> > # remove old config
>> > >/etc/config/tunneldigger
>> > rm /tmp/fastd_fff_peers/*
>> > count=0
>> > Index=1
>> >-json_load "$(cat "$hoodfiletmp")"
>> >+json_load "$(cat "$hoodfile")"

Ich dachte weil hier dann ein leerer String geladen wird.

>> > json_select vpn

Und hier dann vermutlich ne verwirrende Meldung kommt.

>> > # get fastd peers
>> > while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@
>json_select
>> >".." # back to root  # main
>> >
>> > # Only do something when file is here and greater 0 byte -if [ -s
>> >"$hoodfiletmp" ]; then
>> >+if [ -s "$hoodfile" ]; then
>
>... hier. Wenn $1 nicht gesetzt wurde, nicht zu einer lesbaren Datei
>führt oder zu einer leeren Datei führt, tut das Skript einfach nichts.
>
>Es gibt allerdings nicht die von dir gewünschte Fehlermeldung, das
>Skript tut einfach nur etwas, wenn es die richtigen Daten bekommt. Für
>mich wäre das so okay.
>
>Wenn du die Fehlermeldung möchtest, könnte man das hier als "else" mit
>einbauen, dann prüft man gleich den File, und nicht nur das Argument.
>
>Bitte kommentieren.

Im Grunde finde ich die Eingabeprüfung hier nicht sooo wichtig. Der architekturuelle Umbau ist sehr gut und überwiegt.

Trotzdem gehört es vom Code-Style für mich einfach dazu die Eingaben möglichst zeitnah zu prüfen.

Ich denke das ist nur noch ne kleine Verbesserung und du kannst anschließend direkt mein reviewed by mit dran tun. Roberts wirst du auch übernehmen können.

Insofern hier mein
Reviewed-by: Tim Niemeyer <tim@tn-x.org>

Wäre halt wirklich cool, wenn du die zwei Minuten noch eben rein stecken könntest.

Tim


>Grüße
>
>Adrian
>
>
>> > 	# set some vars
>> > 	hostname=$(cat /proc/sys/kernel/hostname)
>> >	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
>> >/sys/class/net/br-mesh/address 2>/dev/null)
Fabian Blaese July 29, 2018, 10:09 a.m.
Moin,

das klingt für mich sehr vernünftig, die Abhängigkeit los zu werden.

Allerdings sollten wir entweder:
- Die hoodfiletmp aus hoodutils entfernen oder
- hoodfilelocal trotzdem nach hoodfiletmp kopieren
damit trotzdem alles weiterhin so ist, wie man erwartet.

Allerdings nicht in diesem Patch sondern generell.

Für diesen Patch:
Reviewed-by: Fabian Bläse <fabian@blaese.de>

Fabian

> On 28. Jul 2018, at 23:15, Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:
> 
> By removing the reference to the hood file from vpn-select, we
> remove the entire dependency from fff-hoodutils.
> vpn-select will now work with any file provided, as long as
> it has the correct syntax. At the moment, the only provider
> is the configurehood script. Since the various hood file variants
> are handled there, it seems logical that configurehood also
> chooses and provides the correct hood file for vpn-select, instead
> of vpn-select which had no other contact with hood file choice.
> 
> This is simple, tidy and effective.
> 
> Fixes #106
> 
> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> 
> ---
> 
> I raised the version for fff-vpn-select by 2, since it still was
> on 1. The never existant version 2 would have been the initial
> KeyXchangeV2 version.
> ---
> src/packages/fff/fff-hoods/Makefile                       | 2 +-
> src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
> src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
> src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
> 4 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/src/packages/fff/fff-hoods/Makefile b/src/packages/fff/fff-hoods/Makefile
> index b565ac7..93fd430 100644
> --- a/src/packages/fff/fff-hoods/Makefile
> +++ b/src/packages/fff/fff-hoods/Makefile
> @@ -1,7 +1,7 @@
> include $(TOPDIR)/rules.mk
> 
> PKG_NAME:=fff-hoods
> -PKG_VERSION:=2
> +PKG_VERSION:=3
> PKG_RELEASE:=1
> 
> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> index 57c6f9f..6565c80 100755
> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> @@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
> 	# and now we get to vpn-select script and load VPNs directly from /tmp/keyxchangev2data
> 
> 	if hasInternet ; then
> -		sh /usr/sbin/vpn-select
> +		sh /usr/sbin/vpn-select "$hoodfiletmp"
> 	else
> 		sh /usr/sbin/vpn-stop
> 	fi
> diff --git a/src/packages/fff/fff-vpn-select/Makefile b/src/packages/fff/fff-vpn-select/Makefile
> index 4e2d89b..27cff09 100644
> --- a/src/packages/fff/fff-vpn-select/Makefile
> +++ b/src/packages/fff/fff-vpn-select/Makefile
> @@ -1,7 +1,7 @@
> include $(TOPDIR)/rules.mk
> 
> PKG_NAME:=fff-vpn-select
> -PKG_VERSION:=1
> +PKG_VERSION:=3
> PKG_RELEASE:=1
> 
> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> index 7c9bced..86236e8 100755
> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> @@ -1,15 +1,16 @@
> #!/bin/sh
> 
> -. /lib/functions/fff/keyxchange
> . /usr/share/libubox/jshn.sh
> 
> +hoodfile="$1"
> +
> make_config() {
> # remove old config
>> /etc/config/tunneldigger
> rm /tmp/fastd_fff_peers/*
> count=0
> Index=1
> -json_load "$(cat "$hoodfiletmp")"
> +json_load "$(cat "$hoodfile")"
> json_select vpn
> # get fastd peers
> while json_select "$Index" > /dev/null
> @@ -54,7 +55,7 @@ json_select ".." # back to root
> # main
> 
> # Only do something when file is here and greater 0 byte
> -if [ -s "$hoodfiletmp" ]; then
> +if [ -s "$hoodfile" ]; then
> 	# set some vars
> 	hostname=$(cat /proc/sys/kernel/hostname)
> 	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }' /sys/class/net/br-mesh/address 2>/dev/null)
> --
> 2.11.0
>
Adrian Schmutzler July 29, 2018, 10:18 a.m.
Hallo nochmal,

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of Tim Niemeyer
> Sent: Sonntag, 29. Juli 2018 12:04
> To: franken-dev@freifunk.net
> Subject: RE: [PATCH] vpn-select: Demand hood file to be provided as
> argument
> 
> Hi Adrian
> 
> Am 29. Juli 2018 11:57:42 MESZ schrieb mail@adrianschmutzler.de:
> >Hallo Tim,
> >
> >siehe unten.
> >
> >> -----Original Message-----
> >> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On
> Behalf
> >> Of Tim Niemeyer
> >> Sent: Sonntag, 29. Juli 2018 11:07
> >> To: franken-dev@freifunk.net
> >> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
> >> argument
> >>
> >> Hi
> >>
> >> Sieht sehr gut aus. Unten ne Kleinigkeit.
> >>
> >>
> >> Am 28. Juli 2018 23:15:23 MESZ schrieb Adrian Schmutzler
> >> <freifunk@adrianschmutzler.de>:
> >> >By removing the reference to the hood file from vpn-select, we
> >remove
> >> >the entire dependency from fff-hoodutils.
> >> >vpn-select will now work with any file provided, as long as it has
> >the
> >> >correct syntax. At the moment, the only provider is the
> >configurehood
> >> >script. Since the various hood file variants are handled there, it
> >> >seems logical that configurehood also chooses and provides the
> >correct
> >> >hood file for vpn-select, instead of vpn-select which had no other
> >> >contact with hood file choice.
> >> >
> >> >This is simple, tidy and effective.
> >> >
> >> >Fixes #106
> >> >
> >> >Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >> >
> >> >---
> >> >
> >> >I raised the version for fff-vpn-select by 2, since it still was on
> >1.
> >> >The never existant version 2 would have been the initial
> >> >KeyXchangeV2 version.
> >> >---
> >> > src/packages/fff/fff-hoods/Makefile                       | 2 +-
> >> > src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
> >> > src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
> >> > src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7
> >++++---
> >> > 4 files changed, 7 insertions(+), 6 deletions(-)
> >> >
> >> >diff --git a/src/packages/fff/fff-hoods/Makefile
> >> >b/src/packages/fff/fff-hoods/Makefile
> >> >index b565ac7..93fd430 100644
> >> >--- a/src/packages/fff/fff-hoods/Makefile
> >> >+++ b/src/packages/fff/fff-hoods/Makefile
> >> >@@ -1,7 +1,7 @@
> >> > include $(TOPDIR)/rules.mk
> >> >
> >> > PKG_NAME:=fff-hoods
> >> >-PKG_VERSION:=2
> >> >+PKG_VERSION:=3
> >> > PKG_RELEASE:=1
> >> >
> >> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> >> >diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >> >b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >> >index 57c6f9f..6565c80 100755
> >> >--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >> >+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >> >@@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
> >> >	# and now we get to vpn-select script and load VPNs directly from
> >> >/tmp/keyxchangev2data
> >> >
> >> > 	if hasInternet ; then
> >> >-		sh /usr/sbin/vpn-select
> >> >+		sh /usr/sbin/vpn-select "$hoodfiletmp"
> >> > 	else
> >> > 		sh /usr/sbin/vpn-stop
> >> > 	fi
> >> >diff --git a/src/packages/fff/fff-vpn-select/Makefile
> >> >b/src/packages/fff/fff-vpn-select/Makefile
> >> >index 4e2d89b..27cff09 100644
> >> >--- a/src/packages/fff/fff-vpn-select/Makefile
> >> >+++ b/src/packages/fff/fff-vpn-select/Makefile
> >> >@@ -1,7 +1,7 @@
> >> > include $(TOPDIR)/rules.mk
> >> >
> >> > PKG_NAME:=fff-vpn-select
> >> >-PKG_VERSION:=1
> >> >+PKG_VERSION:=3
> >> > PKG_RELEASE:=1
> >> >
> >> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> >> >diff --git
> >a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >> >b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >> >index 7c9bced..86236e8 100755
> >> >--- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >> >+++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >> >@@ -1,15 +1,16 @@
> >> > #!/bin/sh
> >> >
> >> >-. /lib/functions/fff/keyxchange
> >> > . /usr/share/libubox/jshn.sh
> >> >
> >> >+hoodfile="$1"
> >> >+
> >>
> >> Bitte noch kurz prüfen ob der Parameter übergeben wurde und am
> besten
> >> auch, ob es eine lesbare Datei ist. Ansonsten mit Fehlermeldung
> >ausbrechen.
> >>
> >> Tim
> >
> >Wir prüfen das weiter unten, nämlich ...
> >
> >>
> >>
> >> > make_config() {
> >> > # remove old config
> >> > >/etc/config/tunneldigger
> >> > rm /tmp/fastd_fff_peers/*
> >> > count=0
> >> > Index=1
> >> >-json_load "$(cat "$hoodfiletmp")"
> >> >+json_load "$(cat "$hoodfile")"
> 
> Ich dachte weil hier dann ein leerer String geladen wird.

Das steht aber innerhalb einer Funktion.
Die Funktion make_config wird wiederum nur innerhalb der unten diskutierten if-Schleife aufgerufen, d.h. dass [ -s "$hoodfile" ] dann bereits geprüft wurde. Und dieses File wird so wohl auch nicht gesourced, so dass man sich über die Verwendung der Funktion an anderer Stelle Gedanken machen muss.

Weiteres weiter unten.


> 
> >> > json_select vpn
> 
> Und hier dann vermutlich ne verwirrende Meldung kommt.
> 
> >> > # get fastd peers
> >> > while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@
> >json_select
> >> >".." # back to root  # main
> >> >
> >> > # Only do something when file is here and greater 0 byte -if [ -s
> >> >"$hoodfiletmp" ]; then
> >> >+if [ -s "$hoodfile" ]; then
> >
> >... hier. Wenn $1 nicht gesetzt wurde, nicht zu einer lesbaren Datei
> >führt oder zu einer leeren Datei führt, tut das Skript einfach nichts.
> >
> >Es gibt allerdings nicht die von dir gewünschte Fehlermeldung, das
> >Skript tut einfach nur etwas, wenn es die richtigen Daten bekommt. Für
> >mich wäre das so okay.
> >
> >Wenn du die Fehlermeldung möchtest, könnte man das hier als "else" mit
> >einbauen, dann prüft man gleich den File, und nicht nur das Argument.
> >
> >Bitte kommentieren.
> 
> Im Grunde finde ich die Eingabeprüfung hier nicht sooo wichtig. Der
> architekturuelle Umbau ist sehr gut und überwiegt.
> 
> Trotzdem gehört es vom Code-Style für mich einfach dazu die Eingaben
> möglichst zeitnah zu prüfen.

Ist schon klar, aber wie willst du es nun konkret haben?

Ich werde jetzt mal die Variante mit else bauen, dass wird dann quasi direkt nach dem Start ausgeführt, es steht nur nicht ganz oben. Dafür muss man nicht das Gleiche zweimal machen.

Grüße

Adrian

> 
> Ich denke das ist nur noch ne kleine Verbesserung und du kannst
> anschließend direkt mein reviewed by mit dran tun. Roberts wirst du auch
> übernehmen können.
> 
> Insofern hier mein
> Reviewed-by: Tim Niemeyer <tim@tn-x.org>
> 
> Wäre halt wirklich cool, wenn du die zwei Minuten noch eben rein stecken
> könntest.
> 
> Tim
> 
> 
> >Grüße
> >
> >Adrian
> >
> >
> >> > 	# set some vars
> >> > 	hostname=$(cat /proc/sys/kernel/hostname)
> >> >	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
> >> >/sys/class/net/br-mesh/address 2>/dev/null)
Tim Niemeyer July 29, 2018, 10:21 a.m.
Hi

Am 29. Juli 2018 12:18:37 MESZ schrieb mail@adrianschmutzler.de:
>Hallo nochmal,
>
>> -----Original Message-----
>> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
>> Of Tim Niemeyer
>> Sent: Sonntag, 29. Juli 2018 12:04
>> To: franken-dev@freifunk.net
>> Subject: RE: [PATCH] vpn-select: Demand hood file to be provided as
>> argument
>> 
>> Hi Adrian
>> 
>> Am 29. Juli 2018 11:57:42 MESZ schrieb mail@adrianschmutzler.de:
>> >Hallo Tim,
>> >
>> >siehe unten.
>> >
>> >> -----Original Message-----
>> >> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On
>> Behalf
>> >> Of Tim Niemeyer
>> >> Sent: Sonntag, 29. Juli 2018 11:07
>> >> To: franken-dev@freifunk.net
>> >> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided
>as
>> >> argument
>> >>
>> >> Hi
>> >>
>> >> Sieht sehr gut aus. Unten ne Kleinigkeit.
>> >>
>> >>
>> >> Am 28. Juli 2018 23:15:23 MESZ schrieb Adrian Schmutzler
>> >> <freifunk@adrianschmutzler.de>:
>> >> >By removing the reference to the hood file from vpn-select, we
>> >remove
>> >> >the entire dependency from fff-hoodutils.
>> >> >vpn-select will now work with any file provided, as long as it
>has
>> >the
>> >> >correct syntax. At the moment, the only provider is the
>> >configurehood
>> >> >script. Since the various hood file variants are handled there,
>it
>> >> >seems logical that configurehood also chooses and provides the
>> >correct
>> >> >hood file for vpn-select, instead of vpn-select which had no
>other
>> >> >contact with hood file choice.
>> >> >
>> >> >This is simple, tidy and effective.
>> >> >
>> >> >Fixes #106
>> >> >
>> >> >Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>> >> >
>> >> >---
>> >> >
>> >> >I raised the version for fff-vpn-select by 2, since it still was
>on
>> >1.
>> >> >The never existant version 2 would have been the initial
>> >> >KeyXchangeV2 version.
>> >> >---
>> >> > src/packages/fff/fff-hoods/Makefile                       | 2 +-
>> >> > src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
>> >> > src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
>> >> > src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7
>> >++++---
>> >> > 4 files changed, 7 insertions(+), 6 deletions(-)
>> >> >
>> >> >diff --git a/src/packages/fff/fff-hoods/Makefile
>> >> >b/src/packages/fff/fff-hoods/Makefile
>> >> >index b565ac7..93fd430 100644
>> >> >--- a/src/packages/fff/fff-hoods/Makefile
>> >> >+++ b/src/packages/fff/fff-hoods/Makefile
>> >> >@@ -1,7 +1,7 @@
>> >> > include $(TOPDIR)/rules.mk
>> >> >
>> >> > PKG_NAME:=fff-hoods
>> >> >-PKG_VERSION:=2
>> >> >+PKG_VERSION:=3
>> >> > PKG_RELEASE:=1
>> >> >
>> >> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>> >> >diff --git
>a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> >> >b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> >> >index 57c6f9f..6565c80 100755
>> >> >--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> >> >+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> >> >@@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
>> >> >	# and now we get to vpn-select script and load VPNs directly
>from
>> >> >/tmp/keyxchangev2data
>> >> >
>> >> > 	if hasInternet ; then
>> >> >-		sh /usr/sbin/vpn-select
>> >> >+		sh /usr/sbin/vpn-select "$hoodfiletmp"
>> >> > 	else
>> >> > 		sh /usr/sbin/vpn-stop
>> >> > 	fi
>> >> >diff --git a/src/packages/fff/fff-vpn-select/Makefile
>> >> >b/src/packages/fff/fff-vpn-select/Makefile
>> >> >index 4e2d89b..27cff09 100644
>> >> >--- a/src/packages/fff/fff-vpn-select/Makefile
>> >> >+++ b/src/packages/fff/fff-vpn-select/Makefile
>> >> >@@ -1,7 +1,7 @@
>> >> > include $(TOPDIR)/rules.mk
>> >> >
>> >> > PKG_NAME:=fff-vpn-select
>> >> >-PKG_VERSION:=1
>> >> >+PKG_VERSION:=3
>> >> > PKG_RELEASE:=1
>> >> >
>> >> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>> >> >diff --git
>> >a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> >> >b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> >> >index 7c9bced..86236e8 100755
>> >> >--- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> >> >+++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> >> >@@ -1,15 +1,16 @@
>> >> > #!/bin/sh
>> >> >
>> >> >-. /lib/functions/fff/keyxchange
>> >> > . /usr/share/libubox/jshn.sh
>> >> >
>> >> >+hoodfile="$1"
>> >> >+
>> >>
>> >> Bitte noch kurz prüfen ob der Parameter übergeben wurde und am
>> besten
>> >> auch, ob es eine lesbare Datei ist. Ansonsten mit Fehlermeldung
>> >ausbrechen.
>> >>
>> >> Tim
>> >
>> >Wir prüfen das weiter unten, nämlich ...
>> >
>> >>
>> >>
>> >> > make_config() {
>> >> > # remove old config
>> >> > >/etc/config/tunneldigger
>> >> > rm /tmp/fastd_fff_peers/*
>> >> > count=0
>> >> > Index=1
>> >> >-json_load "$(cat "$hoodfiletmp")"
>> >> >+json_load "$(cat "$hoodfile")"
>> 
>> Ich dachte weil hier dann ein leerer String geladen wird.
>
>Das steht aber innerhalb einer Funktion.

Ha. Okay. Das hab ich übersehen.

Dann passt es eigentlich so auch.

Tim

>Die Funktion make_config wird wiederum nur innerhalb der unten
>diskutierten if-Schleife aufgerufen, d.h. dass [ -s "$hoodfile" ] dann
>bereits geprüft wurde. Und dieses File wird so wohl auch nicht
>gesourced, so dass man sich über die Verwendung der Funktion an anderer
>Stelle Gedanken machen muss.
>
>Weiteres weiter unten.
>
>
>> 
>> >> > json_select vpn
>> 
>> Und hier dann vermutlich ne verwirrende Meldung kommt.
>> 
>> >> > # get fastd peers
>> >> > while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@
>> >json_select
>> >> >".." # back to root  # main
>> >> >
>> >> > # Only do something when file is here and greater 0 byte -if [
>-s
>> >> >"$hoodfiletmp" ]; then
>> >> >+if [ -s "$hoodfile" ]; then
>> >
>> >... hier. Wenn $1 nicht gesetzt wurde, nicht zu einer lesbaren Datei
>> >führt oder zu einer leeren Datei führt, tut das Skript einfach
>nichts.
>> >
>> >Es gibt allerdings nicht die von dir gewünschte Fehlermeldung, das
>> >Skript tut einfach nur etwas, wenn es die richtigen Daten bekommt.
>Für
>> >mich wäre das so okay.
>> >
>> >Wenn du die Fehlermeldung möchtest, könnte man das hier als "else"
>mit
>> >einbauen, dann prüft man gleich den File, und nicht nur das
>Argument.
>> >
>> >Bitte kommentieren.
>> 
>> Im Grunde finde ich die Eingabeprüfung hier nicht sooo wichtig. Der
>> architekturuelle Umbau ist sehr gut und überwiegt.
>> 
>> Trotzdem gehört es vom Code-Style für mich einfach dazu die Eingaben
>> möglichst zeitnah zu prüfen.
>
>Ist schon klar, aber wie willst du es nun konkret haben?
>
>Ich werde jetzt mal die Variante mit else bauen, dass wird dann quasi
>direkt nach dem Start ausgeführt, es steht nur nicht ganz oben. Dafür
>muss man nicht das Gleiche zweimal machen.
>
>Grüße
>
>Adrian
>
>> 
>> Ich denke das ist nur noch ne kleine Verbesserung und du kannst
>> anschließend direkt mein reviewed by mit dran tun. Roberts wirst du
>auch
>> übernehmen können.
>> 
>> Insofern hier mein
>> Reviewed-by: Tim Niemeyer <tim@tn-x.org>
>> 
>> Wäre halt wirklich cool, wenn du die zwei Minuten noch eben rein
>stecken
>> könntest.
>> 
>> Tim
>> 
>> 
>> >Grüße
>> >
>> >Adrian
>> >
>> >
>> >> > 	# set some vars
>> >> > 	hostname=$(cat /proc/sys/kernel/hostname)
>> >> >	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
>> >> >/sys/class/net/br-mesh/address 2>/dev/null)
Robert Langhammer July 29, 2018, 10:27 a.m.
Hallo,

s. unten


Am 29.07.2018 um 12:03 schrieb Tim Niemeyer:
> Hi Adrian
>
> Am 29. Juli 2018 11:57:42 MESZ schrieb mail@adrianschmutzler.de:
>> Hallo Tim,
>>
>> siehe unten.
>>
>>> -----Original Message-----
>>> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
>>> Of Tim Niemeyer
>>> Sent: Sonntag, 29. Juli 2018 11:07
>>> To: franken-dev@freifunk.net
>>> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
>>> argument
>>>
>>> Hi
>>>
>>> Sieht sehr gut aus. Unten ne Kleinigkeit.
>>>
>>>
>>> Am 28. Juli 2018 23:15:23 MESZ schrieb Adrian Schmutzler
>>> <freifunk@adrianschmutzler.de>:
>>>> By removing the reference to the hood file from vpn-select, we
>> remove
>>>> the entire dependency from fff-hoodutils.
>>>> vpn-select will now work with any file provided, as long as it has
>> the
>>>> correct syntax. At the moment, the only provider is the
>> configurehood
>>>> script. Since the various hood file variants are handled there, it
>>>> seems logical that configurehood also chooses and provides the
>> correct
>>>> hood file for vpn-select, instead of vpn-select which had no other
>>>> contact with hood file choice.
>>>>
>>>> This is simple, tidy and effective.
>>>>
>>>> Fixes #106
>>>>
>>>> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>>>>
>>>> ---
>>>>
>>>> I raised the version for fff-vpn-select by 2, since it still was on
>> 1.
>>>> The never existant version 2 would have been the initial
>>>> KeyXchangeV2 version.
>>>> ---
>>>> src/packages/fff/fff-hoods/Makefile                       | 2 +-
>>>> src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
>>>> src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
>>>> src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7
>> ++++---
>>>> 4 files changed, 7 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/src/packages/fff/fff-hoods/Makefile
>>>> b/src/packages/fff/fff-hoods/Makefile
>>>> index b565ac7..93fd430 100644
>>>> --- a/src/packages/fff/fff-hoods/Makefile
>>>> +++ b/src/packages/fff/fff-hoods/Makefile
>>>> @@ -1,7 +1,7 @@
>>>> include $(TOPDIR)/rules.mk
>>>>
>>>> PKG_NAME:=fff-hoods
>>>> -PKG_VERSION:=2
>>>> +PKG_VERSION:=3
>>>> PKG_RELEASE:=1
>>>>
>>>> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>>>> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>>>> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>>>> index 57c6f9f..6565c80 100755
>>>> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>>>> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>>>> @@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
>>>> 	# and now we get to vpn-select script and load VPNs directly from
>>>> /tmp/keyxchangev2data
>>>>
>>>> 	if hasInternet ; then
>>>> -		sh /usr/sbin/vpn-select
>>>> +		sh /usr/sbin/vpn-select "$hoodfiletmp"
>>>> 	else
>>>> 		sh /usr/sbin/vpn-stop
>>>> 	fi
>>>> diff --git a/src/packages/fff/fff-vpn-select/Makefile
>>>> b/src/packages/fff/fff-vpn-select/Makefile
>>>> index 4e2d89b..27cff09 100644
>>>> --- a/src/packages/fff/fff-vpn-select/Makefile
>>>> +++ b/src/packages/fff/fff-vpn-select/Makefile
>>>> @@ -1,7 +1,7 @@
>>>> include $(TOPDIR)/rules.mk
>>>>
>>>> PKG_NAME:=fff-vpn-select
>>>> -PKG_VERSION:=1
>>>> +PKG_VERSION:=3
>>>> PKG_RELEASE:=1
>>>>
>>>> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>>>> diff --git
>> a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>>>> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>>>> index 7c9bced..86236e8 100755
>>>> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>>>> +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>>>> @@ -1,15 +1,16 @@
>>>> #!/bin/sh
>>>>
>>>> -. /lib/functions/fff/keyxchange
>>>> . /usr/share/libubox/jshn.sh
>>>>
>>>> +hoodfile="$1"
>>>> +
>>> Bitte noch kurz prüfen ob der Parameter übergeben wurde und am besten
>>> auch, ob es eine lesbare Datei ist. Ansonsten mit Fehlermeldung
>> ausbrechen.
>>> Tim
>> Wir prüfen das weiter unten, nämlich ...
>>
>>>
>>>> make_config() {
>>>> # remove old config
>>>>> /etc/config/tunneldigger
>>>> rm /tmp/fastd_fff_peers/*
>>>> count=0
>>>> Index=1
>>>> -json_load "$(cat "$hoodfiletmp")"
>>>> +json_load "$(cat "$hoodfile")"
> Ich dachte weil hier dann ein leerer String geladen wird.
>
>>>> json_select vpn
> Und hier dann vermutlich ne verwirrende Meldung kommt.
>
>>>> # get fastd peers
>>>> while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@
>> json_select
>>>> ".." # back to root  # main
>>>>
>>>> # Only do something when file is here and greater 0 byte -if [ -s
>>>> "$hoodfiletmp" ]; then
>>>> +if [ -s "$hoodfile" ]; then
>> ... hier. Wenn $1 nicht gesetzt wurde, nicht zu einer lesbaren Datei
>> führt oder zu einer leeren Datei führt, tut das Skript einfach nichts.
>>
>> Es gibt allerdings nicht die von dir gewünschte Fehlermeldung, das
>> Skript tut einfach nur etwas, wenn es die richtigen Daten bekommt. Für
>> mich wäre das so okay.
>>
>> Wenn du die Fehlermeldung möchtest, könnte man das hier als "else" mit
>> einbauen, dann prüft man gleich den File, und nicht nur das Argument.
>>
>> Bitte kommentieren.
> Im Grunde finde ich die Eingabeprüfung hier nicht sooo wichtig. Der architekturuelle Umbau ist sehr gut und überwiegt.
>
> Trotzdem gehört es vom Code-Style für mich einfach dazu die Eingaben möglichst zeitnah zu prüfen.
>
> Ich denke das ist nur noch ne kleine Verbesserung und du kannst anschließend direkt mein reviewed by mit dran tun. Roberts wirst du auch übernehmen können.
>
> Insofern hier mein
> Reviewed-by: Tim Niemeyer <tim@tn-x.org>
>
> Wäre halt wirklich cool, wenn du die zwei Minuten noch eben rein stecken könntest.
>
> Tim
Ein direktes Prüfen der Parameter ist immer gut! Auch wenn es in unseren
speziellen Fall nicht so wichtig ist, da wir in configurehood schon auf
-s testen und im Fehlerfall gar nicht so weit kommen.

darum kannst du meine Review auf jeden Fall dran lassen!

Adrian, wenn du dich nochmal drüber machst, kannst du gleich das
hässliche sh vorne dran weg machen, wenn du magst. Ist aber alles nicht
so wichtig, da wir uns so und so nochmal damit beschäftigen müssen um
den fürchterlichen wget los zu werden und die l2tp-vpn Daten ohne
Abhängigkeit von fastd aus dem Hoodfile zu holen.
Robert
>
>
>> Grüße
>>
>> Adrian
>>
>>
>>>> 	# set some vars
>>>> 	hostname=$(cat /proc/sys/kernel/hostname)
>>>> 	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
>>>> /sys/class/net/br-mesh/address 2>/dev/null)
Adrian Schmutzler July 29, 2018, 10:43 a.m.
Hallo Fabian,

nur zwecks hoodfilelocal:

Mir würde es besser gefallen, nicht nochmal ein File zu kopieren.

Das Entfernen von hoodfiletmp aus hoodutils hätte einen gewissen Charme, da dann das Verhalten wieder konsistent wird. Zudem wird hoodfiletmp ja ohnehin jeweils zu Beginn der Laufzeit von configurehood gelöscht. Es wäre dann halt während der Konfigurationsphase in configurehood (also bei einem Wechsel solange, bis hoodfileref angelegt wird) kein hoodfile für die restlichen Komponenten sichtbar.
Hier müsste man mal genauer überlegen, was das für Einflüsse hat (ggf. ist es sogar positiv, wenn nachgeordnete Komponenten WÄHREND der Konfiguration davon ausgehen, dass es noch kein Hoodfile gibt).

Ich würde mit dieser Änderung aber bis nach einem stable Release warten, damit wir nichts kaputt machen.

Ansonsten ist die Lösung mit dem Kopieren des Files natürlich eine, die funktionieren sollte und jetzt auch nicht "schlecht" ist.

Grüße

Adrian


> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of Fabian Bläse
> Sent: Sonntag, 29. Juli 2018 12:09
> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; franken-dev
> <franken-dev@freifunk.net>
> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
> argument
> 
> Moin,
> 
> das klingt für mich sehr vernünftig, die Abhängigkeit los zu werden.
> 
> Allerdings sollten wir entweder:
> - Die hoodfiletmp aus hoodutils entfernen oder
> - hoodfilelocal trotzdem nach hoodfiletmp kopieren damit trotzdem alles
> weiterhin so ist, wie man erwartet.
> 
> Allerdings nicht in diesem Patch sondern generell.
> 
> Für diesen Patch:
> Reviewed-by: Fabian Bläse <fabian@blaese.de>
> 
> Fabian
> 
> > On 28. Jul 2018, at 23:15, Adrian Schmutzler
> <freifunk@adrianschmutzler.de> wrote:
> >
> > By removing the reference to the hood file from vpn-select, we remove
> > the entire dependency from fff-hoodutils.
> > vpn-select will now work with any file provided, as long as it has the
> > correct syntax. At the moment, the only provider is the configurehood
> > script. Since the various hood file variants are handled there, it
> > seems logical that configurehood also chooses and provides the correct
> > hood file for vpn-select, instead of vpn-select which had no other
> > contact with hood file choice.
> >
> > This is simple, tidy and effective.
> >
> > Fixes #106
> >
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> > ---
> >
> > I raised the version for fff-vpn-select by 2, since it still was on 1.
> > The never existant version 2 would have been the initial
> > KeyXchangeV2 version.
> > ---
> > src/packages/fff/fff-hoods/Makefile                       | 2 +-
> > src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
> > src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
> > src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
> > 4 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/packages/fff/fff-hoods/Makefile
> > b/src/packages/fff/fff-hoods/Makefile
> > index b565ac7..93fd430 100644
> > --- a/src/packages/fff/fff-hoods/Makefile
> > +++ b/src/packages/fff/fff-hoods/Makefile
> > @@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> >
> > PKG_NAME:=fff-hoods
> > -PKG_VERSION:=2
> > +PKG_VERSION:=3
> > PKG_RELEASE:=1
> >
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > index 57c6f9f..6565c80 100755
> > --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > @@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
> > 	# and now we get to vpn-select script and load VPNs directly from
> > /tmp/keyxchangev2data
> >
> > 	if hasInternet ; then
> > -		sh /usr/sbin/vpn-select
> > +		sh /usr/sbin/vpn-select "$hoodfiletmp"
> > 	else
> > 		sh /usr/sbin/vpn-stop
> > 	fi
> > diff --git a/src/packages/fff/fff-vpn-select/Makefile
> > b/src/packages/fff/fff-vpn-select/Makefile
> > index 4e2d89b..27cff09 100644
> > --- a/src/packages/fff/fff-vpn-select/Makefile
> > +++ b/src/packages/fff/fff-vpn-select/Makefile
> > @@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> >
> > PKG_NAME:=fff-vpn-select
> > -PKG_VERSION:=1
> > +PKG_VERSION:=3
> > PKG_RELEASE:=1
> >
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > index 7c9bced..86236e8 100755
> > --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > @@ -1,15 +1,16 @@
> > #!/bin/sh
> >
> > -. /lib/functions/fff/keyxchange
> > . /usr/share/libubox/jshn.sh
> >
> > +hoodfile="$1"
> > +
> > make_config() {
> > # remove old config
> >> /etc/config/tunneldigger
> > rm /tmp/fastd_fff_peers/*
> > count=0
> > Index=1
> > -json_load "$(cat "$hoodfiletmp")"
> > +json_load "$(cat "$hoodfile")"
> > json_select vpn
> > # get fastd peers
> > while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@ json_select
> > ".." # back to root # main
> >
> > # Only do something when file is here and greater 0 byte -if [ -s
> > "$hoodfiletmp" ]; then
> > +if [ -s "$hoodfile" ]; then
> > 	# set some vars
> > 	hostname=$(cat /proc/sys/kernel/hostname)
> > 	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
> > /sys/class/net/br-mesh/address 2>/dev/null)
> > --
> > 2.11.0
> >
Adrian Schmutzler July 29, 2018, 12:10 p.m.
Hallo nochmal,

ich habe nochmal gründlich nachgedacht und schließe mich nun Fabians Vorschlag an (die Datei zu kopieren).

Dies ist die logischere Variante in dem Sinne, wie wir configurehood konzipiert haben. /etc/hoodfile ist damit einfach eine Quelle wie alle anderen auch, von der aus hoodfiletmp kopiert wird.

Damit sparen wir uns auch, stundenlang die Auswirkungen einer Änderungen von hoodutils zu überlegen und dann doch etwas zu übersehen.

Die Frage ist nur, ob wir das vor oder nach einem Release ändern wollen.

Grüße

Adrian

PS: Mir ist in dem Zuge aber eine andere hilfreiche Änderung in hoodutils eingefallen, dazu kommt gleich noch ein Patch.

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of Fabian Bläse
> Sent: Sonntag, 29. Juli 2018 12:09
> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; franken-dev
> <franken-dev@freifunk.net>
> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
> argument
> 
> Moin,
> 
> das klingt für mich sehr vernünftig, die Abhängigkeit los zu werden.
> 
> Allerdings sollten wir entweder:
> - Die hoodfiletmp aus hoodutils entfernen oder
> - hoodfilelocal trotzdem nach hoodfiletmp kopieren damit trotzdem alles
> weiterhin so ist, wie man erwartet.
> 
> Allerdings nicht in diesem Patch sondern generell.
> 
> Für diesen Patch:
> Reviewed-by: Fabian Bläse <fabian@blaese.de>
> 
> Fabian
> 
> > On 28. Jul 2018, at 23:15, Adrian Schmutzler
> <freifunk@adrianschmutzler.de> wrote:
> >
> > By removing the reference to the hood file from vpn-select, we remove
> > the entire dependency from fff-hoodutils.
> > vpn-select will now work with any file provided, as long as it has the
> > correct syntax. At the moment, the only provider is the configurehood
> > script. Since the various hood file variants are handled there, it
> > seems logical that configurehood also chooses and provides the correct
> > hood file for vpn-select, instead of vpn-select which had no other
> > contact with hood file choice.
> >
> > This is simple, tidy and effective.
> >
> > Fixes #106
> >
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> > ---
> >
> > I raised the version for fff-vpn-select by 2, since it still was on 1.
> > The never existant version 2 would have been the initial
> > KeyXchangeV2 version.
> > ---
> > src/packages/fff/fff-hoods/Makefile                       | 2 +-
> > src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
> > src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
> > src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
> > 4 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/packages/fff/fff-hoods/Makefile
> > b/src/packages/fff/fff-hoods/Makefile
> > index b565ac7..93fd430 100644
> > --- a/src/packages/fff/fff-hoods/Makefile
> > +++ b/src/packages/fff/fff-hoods/Makefile
> > @@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> >
> > PKG_NAME:=fff-hoods
> > -PKG_VERSION:=2
> > +PKG_VERSION:=3
> > PKG_RELEASE:=1
> >
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > index 57c6f9f..6565c80 100755
> > --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > @@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
> > 	# and now we get to vpn-select script and load VPNs directly from
> > /tmp/keyxchangev2data
> >
> > 	if hasInternet ; then
> > -		sh /usr/sbin/vpn-select
> > +		sh /usr/sbin/vpn-select "$hoodfiletmp"
> > 	else
> > 		sh /usr/sbin/vpn-stop
> > 	fi
> > diff --git a/src/packages/fff/fff-vpn-select/Makefile
> > b/src/packages/fff/fff-vpn-select/Makefile
> > index 4e2d89b..27cff09 100644
> > --- a/src/packages/fff/fff-vpn-select/Makefile
> > +++ b/src/packages/fff/fff-vpn-select/Makefile
> > @@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> >
> > PKG_NAME:=fff-vpn-select
> > -PKG_VERSION:=1
> > +PKG_VERSION:=3
> > PKG_RELEASE:=1
> >
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > index 7c9bced..86236e8 100755
> > --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> > @@ -1,15 +1,16 @@
> > #!/bin/sh
> >
> > -. /lib/functions/fff/keyxchange
> > . /usr/share/libubox/jshn.sh
> >
> > +hoodfile="$1"
> > +
> > make_config() {
> > # remove old config
> >> /etc/config/tunneldigger
> > rm /tmp/fastd_fff_peers/*
> > count=0
> > Index=1
> > -json_load "$(cat "$hoodfiletmp")"
> > +json_load "$(cat "$hoodfile")"
> > json_select vpn
> > # get fastd peers
> > while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@ json_select
> > ".." # back to root # main
> >
> > # Only do something when file is here and greater 0 byte -if [ -s
> > "$hoodfiletmp" ]; then
> > +if [ -s "$hoodfile" ]; then
> > 	# set some vars
> > 	hostname=$(cat /proc/sys/kernel/hostname)
> > 	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
> > /sys/class/net/br-mesh/address 2>/dev/null)
> > --
> > 2.11.0
> >
Fabian Blaese July 29, 2018, 12:43 p.m.
Moin,

da hast du mich falsch verstanden. Beide Möglichkeiten waren zusätzlich zu diesem Patch gedacht, nicht um ihn zu ersetzen;
Sehr gut wäre, wenn man das hoodfiletmp außerhalb des configurehood komplett los werden würde und dann auch direkt nach dessen Durchlauf entfernen kann.

Das sollten wir aber wohl erst nach Release angehen.

Das /etc/hoodfile irgendwie zu reparieren würde ich noch vor einem Release angehen.

Fabian

> On 29. Jul 2018, at 14:10, <mail@adrianschmutzler.de> <mail@adrianschmutzler.de> wrote:
> 
> Hallo nochmal,
> 
> ich habe nochmal gründlich nachgedacht und schließe mich nun Fabians Vorschlag an (die Datei zu kopieren).
> 
> Dies ist die logischere Variante in dem Sinne, wie wir configurehood konzipiert haben. /etc/hoodfile ist damit einfach eine Quelle wie alle anderen auch, von der aus hoodfiletmp kopiert wird.
> 
> Damit sparen wir uns auch, stundenlang die Auswirkungen einer Änderungen von hoodutils zu überlegen und dann doch etwas zu übersehen.
> 
> Die Frage ist nur, ob wir das vor oder nach einem Release ändern wollen.
> 
> Grüße
> 
> Adrian
> 
> PS: Mir ist in dem Zuge aber eine andere hilfreiche Änderung in hoodutils eingefallen, dazu kommt gleich noch ein Patch.
> 
>> -----Original Message-----
>> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
>> Of Fabian Bläse
>> Sent: Sonntag, 29. Juli 2018 12:09
>> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; franken-dev
>> <franken-dev@freifunk.net>
>> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
>> argument
>> 
>> Moin,
>> 
>> das klingt für mich sehr vernünftig, die Abhängigkeit los zu werden.
>> 
>> Allerdings sollten wir entweder:
>> - Die hoodfiletmp aus hoodutils entfernen oder
>> - hoodfilelocal trotzdem nach hoodfiletmp kopieren damit trotzdem alles
>> weiterhin so ist, wie man erwartet.
>> 
>> Allerdings nicht in diesem Patch sondern generell.
>> 
>> Für diesen Patch:
>> Reviewed-by: Fabian Bläse <fabian@blaese.de>
>> 
>> Fabian
>> 
>>> On 28. Jul 2018, at 23:15, Adrian Schmutzler
>> <freifunk@adrianschmutzler.de> wrote:
>>> 
>>> By removing the reference to the hood file from vpn-select, we remove
>>> the entire dependency from fff-hoodutils.
>>> vpn-select will now work with any file provided, as long as it has the
>>> correct syntax. At the moment, the only provider is the configurehood
>>> script. Since the various hood file variants are handled there, it
>>> seems logical that configurehood also chooses and provides the correct
>>> hood file for vpn-select, instead of vpn-select which had no other
>>> contact with hood file choice.
>>> 
>>> This is simple, tidy and effective.
>>> 
>>> Fixes #106
>>> 
>>> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>>> 
>>> ---
>>> 
>>> I raised the version for fff-vpn-select by 2, since it still was on 1.
>>> The never existant version 2 would have been the initial
>>> KeyXchangeV2 version.
>>> ---
>>> src/packages/fff/fff-hoods/Makefile                       | 2 +-
>>> src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
>>> src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
>>> src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7 ++++---
>>> 4 files changed, 7 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/src/packages/fff/fff-hoods/Makefile
>>> b/src/packages/fff/fff-hoods/Makefile
>>> index b565ac7..93fd430 100644
>>> --- a/src/packages/fff/fff-hoods/Makefile
>>> +++ b/src/packages/fff/fff-hoods/Makefile
>>> @@ -1,7 +1,7 @@
>>> include $(TOPDIR)/rules.mk
>>> 
>>> PKG_NAME:=fff-hoods
>>> -PKG_VERSION:=2
>>> +PKG_VERSION:=3
>>> PKG_RELEASE:=1
>>> 
>>> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>>> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>>> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>>> index 57c6f9f..6565c80 100755
>>> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>>> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>>> @@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
>>> 	# and now we get to vpn-select script and load VPNs directly from
>>> /tmp/keyxchangev2data
>>> 
>>> 	if hasInternet ; then
>>> -		sh /usr/sbin/vpn-select
>>> +		sh /usr/sbin/vpn-select "$hoodfiletmp"
>>> 	else
>>> 		sh /usr/sbin/vpn-stop
>>> 	fi
>>> diff --git a/src/packages/fff/fff-vpn-select/Makefile
>>> b/src/packages/fff/fff-vpn-select/Makefile
>>> index 4e2d89b..27cff09 100644
>>> --- a/src/packages/fff/fff-vpn-select/Makefile
>>> +++ b/src/packages/fff/fff-vpn-select/Makefile
>>> @@ -1,7 +1,7 @@
>>> include $(TOPDIR)/rules.mk
>>> 
>>> PKG_NAME:=fff-vpn-select
>>> -PKG_VERSION:=1
>>> +PKG_VERSION:=3
>>> PKG_RELEASE:=1
>>> 
>>> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
>>> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>>> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>>> index 7c9bced..86236e8 100755
>>> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>>> +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>>> @@ -1,15 +1,16 @@
>>> #!/bin/sh
>>> 
>>> -. /lib/functions/fff/keyxchange
>>> . /usr/share/libubox/jshn.sh
>>> 
>>> +hoodfile="$1"
>>> +
>>> make_config() {
>>> # remove old config
>>>> /etc/config/tunneldigger
>>> rm /tmp/fastd_fff_peers/*
>>> count=0
>>> Index=1
>>> -json_load "$(cat "$hoodfiletmp")"
>>> +json_load "$(cat "$hoodfile")"
>>> json_select vpn
>>> # get fastd peers
>>> while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@ json_select
>>> ".." # back to root # main
>>> 
>>> # Only do something when file is here and greater 0 byte -if [ -s
>>> "$hoodfiletmp" ]; then
>>> +if [ -s "$hoodfile" ]; then
>>> 	# set some vars
>>> 	hostname=$(cat /proc/sys/kernel/hostname)
>>> 	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
>>> /sys/class/net/br-mesh/address 2>/dev/null)
>>> --
>>> 2.11.0
>>> 
> 
>
Adrian Schmutzler July 29, 2018, 12:48 p.m.
Hallo,

das habe ich schon richtig verstanden.

Meine Darstellungen unten waren auch immer ZUSÄTZLICH zu dem vpn-select Patch gedacht.

Den Rest von dir interpretiere ich so, dass wir VOR dem Release jetzt erstmal nur den vpn-select einspielen. Das behebt den Bug. Den Rest dann danach.

Grüße

Adrian

> -----Original Message-----
> From: Fabian Bläse [mailto:fabian@blaese.de]
> Sent: Sonntag, 29. Juli 2018 14:44
> To: mail@adrianschmutzler.de
> Cc: franken-dev@freifunk.net
> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
> argument
> 
> Moin,
> 
> da hast du mich falsch verstanden. Beide Möglichkeiten waren zusätzlich zu
> diesem Patch gedacht, nicht um ihn zu ersetzen; Sehr gut wäre, wenn man
> das hoodfiletmp außerhalb des configurehood komplett los werden würde
> und dann auch direkt nach dessen Durchlauf entfernen kann.
> 
> Das sollten wir aber wohl erst nach Release angehen.
> 
> Das /etc/hoodfile irgendwie zu reparieren würde ich noch vor einem Release
> angehen.
> 
> Fabian
> 
> > On 29. Jul 2018, at 14:10, <mail@adrianschmutzler.de>
> <mail@adrianschmutzler.de> wrote:
> >
> > Hallo nochmal,
> >
> > ich habe nochmal gründlich nachgedacht und schließe mich nun Fabians
> Vorschlag an (die Datei zu kopieren).
> >
> > Dies ist die logischere Variante in dem Sinne, wie wir configurehood
> konzipiert haben. /etc/hoodfile ist damit einfach eine Quelle wie alle
> anderen auch, von der aus hoodfiletmp kopiert wird.
> >
> > Damit sparen wir uns auch, stundenlang die Auswirkungen einer
> Änderungen von hoodutils zu überlegen und dann doch etwas zu
> übersehen.
> >
> > Die Frage ist nur, ob wir das vor oder nach einem Release ändern wollen.
> >
> > Grüße
> >
> > Adrian
> >
> > PS: Mir ist in dem Zuge aber eine andere hilfreiche Änderung in hoodutils
> eingefallen, dazu kommt gleich noch ein Patch.
> >
> >> -----Original Message-----
> >> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On
> Behalf
> >> Of Fabian Bläse
> >> Sent: Sonntag, 29. Juli 2018 12:09
> >> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; franken-dev
> >> <franken-dev@freifunk.net>
> >> Subject: Re: [PATCH] vpn-select: Demand hood file to be provided as
> >> argument
> >>
> >> Moin,
> >>
> >> das klingt für mich sehr vernünftig, die Abhängigkeit los zu werden.
> >>
> >> Allerdings sollten wir entweder:
> >> - Die hoodfiletmp aus hoodutils entfernen oder
> >> - hoodfilelocal trotzdem nach hoodfiletmp kopieren damit trotzdem
> >> alles weiterhin so ist, wie man erwartet.
> >>
> >> Allerdings nicht in diesem Patch sondern generell.
> >>
> >> Für diesen Patch:
> >> Reviewed-by: Fabian Bläse <fabian@blaese.de>
> >>
> >> Fabian
> >>
> >>> On 28. Jul 2018, at 23:15, Adrian Schmutzler
> >> <freifunk@adrianschmutzler.de> wrote:
> >>>
> >>> By removing the reference to the hood file from vpn-select, we
> >>> remove the entire dependency from fff-hoodutils.
> >>> vpn-select will now work with any file provided, as long as it has
> >>> the correct syntax. At the moment, the only provider is the
> >>> configurehood script. Since the various hood file variants are
> >>> handled there, it seems logical that configurehood also chooses and
> >>> provides the correct hood file for vpn-select, instead of vpn-select
> >>> which had no other contact with hood file choice.
> >>>
> >>> This is simple, tidy and effective.
> >>>
> >>> Fixes #106
> >>>
> >>> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >>>
> >>> ---
> >>>
> >>> I raised the version for fff-vpn-select by 2, since it still was on 1.
> >>> The never existant version 2 would have been the initial
> >>> KeyXchangeV2 version.
> >>> ---
> >>> src/packages/fff/fff-hoods/Makefile                       | 2 +-
> >>> src/packages/fff/fff-hoods/files/usr/sbin/configurehood   | 2 +-
> >>> src/packages/fff/fff-vpn-select/Makefile                  | 2 +-
> >>> src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select | 7
> >>> ++++---
> >>> 4 files changed, 7 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/src/packages/fff/fff-hoods/Makefile
> >>> b/src/packages/fff/fff-hoods/Makefile
> >>> index b565ac7..93fd430 100644
> >>> --- a/src/packages/fff/fff-hoods/Makefile
> >>> +++ b/src/packages/fff/fff-hoods/Makefile
> >>> @@ -1,7 +1,7 @@
> >>> include $(TOPDIR)/rules.mk
> >>>
> >>> PKG_NAME:=fff-hoods
> >>> -PKG_VERSION:=2
> >>> +PKG_VERSION:=3
> >>> PKG_RELEASE:=1
> >>>
> >>> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> >>> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >>> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >>> index 57c6f9f..6565c80 100755
> >>> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >>> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> >>> @@ -195,7 +195,7 @@ if [ -s "$hoodfiletmp" ]; then
> >>> 	# and now we get to vpn-select script and load VPNs directly from
> >>> /tmp/keyxchangev2data
> >>>
> >>> 	if hasInternet ; then
> >>> -		sh /usr/sbin/vpn-select
> >>> +		sh /usr/sbin/vpn-select "$hoodfiletmp"
> >>> 	else
> >>> 		sh /usr/sbin/vpn-stop
> >>> 	fi
> >>> diff --git a/src/packages/fff/fff-vpn-select/Makefile
> >>> b/src/packages/fff/fff-vpn-select/Makefile
> >>> index 4e2d89b..27cff09 100644
> >>> --- a/src/packages/fff/fff-vpn-select/Makefile
> >>> +++ b/src/packages/fff/fff-vpn-select/Makefile
> >>> @@ -1,7 +1,7 @@
> >>> include $(TOPDIR)/rules.mk
> >>>
> >>> PKG_NAME:=fff-vpn-select
> >>> -PKG_VERSION:=1
> >>> +PKG_VERSION:=3
> >>> PKG_RELEASE:=1
> >>>
> >>> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> >>> diff --git
> >>> a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >>> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >>> index 7c9bced..86236e8 100755
> >>> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >>> +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> >>> @@ -1,15 +1,16 @@
> >>> #!/bin/sh
> >>>
> >>> -. /lib/functions/fff/keyxchange
> >>> . /usr/share/libubox/jshn.sh
> >>>
> >>> +hoodfile="$1"
> >>> +
> >>> make_config() {
> >>> # remove old config
> >>>> /etc/config/tunneldigger
> >>> rm /tmp/fastd_fff_peers/*
> >>> count=0
> >>> Index=1
> >>> -json_load "$(cat "$hoodfiletmp")"
> >>> +json_load "$(cat "$hoodfile")"
> >>> json_select vpn
> >>> # get fastd peers
> >>> while json_select "$Index" > /dev/null @@ -54,7 +55,7 @@ json_select
> >>> ".." # back to root # main
> >>>
> >>> # Only do something when file is here and greater 0 byte -if [ -s
> >>> "$hoodfiletmp" ]; then
> >>> +if [ -s "$hoodfile" ]; then
> >>> 	# set some vars
> >>> 	hostname=$(cat /proc/sys/kernel/hostname)
> >>> 	mac=$(awk '{ mac=toupper($1); gsub(":", "", mac); print mac }'
> >>> /sys/class/net/br-mesh/address 2>/dev/null)
> >>> --
> >>> 2.11.0
> >>>
> >
> >