[v5,1/2] fff-network: Introduce function to set MAC on device

Submitted by Adrian Schmutzler on Aug. 7, 2018, 10:25 a.m.

Details

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

Commit Message

Adrian Schmutzler Aug. 7, 2018, 10:25 a.m.
Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

---

Changes in v2:
- Suppress output

Changes in v3:
- Rename iface and dev in explanation. The variables were not
  renamed, as shorter is better here.

Changes in v4:
- Rebased

Changes in v5:
- Rebased
---
 .../fff-network/files/lib/functions/fff/network    | 31 +++++++++++++++++++++
 .../fff-network/files/usr/sbin/configurenetwork    | 32 ++--------------------
 2 files changed, 33 insertions(+), 30 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/packages/fff/fff-network/files/lib/functions/fff/network b/src/packages/fff/fff-network/files/lib/functions/fff/network
index af9b3434..0e9b466b 100644
--- a/src/packages/fff/fff-network/files/lib/functions/fff/network
+++ b/src/packages/fff/fff-network/files/lib/functions/fff/network
@@ -96,3 +96,34 @@  macFlipLocalBit() {
 	echo "$mac" | awk -F: '{ printf("%02x:%s:%s:%s:%s:%s\n", xor(("0x"$1),2), $2, $3, $4, $5, $6) }'
 	return 0
 }
+
+fixMac() {
+	# Update MAC address on device/interface
+	#
+	# fixMac <newmac> <physdev> <interface> <overwrite (optional)>
+	#
+	# newmac: MAC address to be set
+	# physdev: Device name to be updated as in ifconfig (e.g. br-mesh)
+	# interface: Interface to be updated as in uci (e.g. mesh)
+	# overwrite: If non-zero, the MAC is always replaced; if not set, the MAC is only written if none is present
+
+	local newmac=$1
+	local dev=$2
+	local iface=$3
+	local overwrite=$4
+
+	if uci -q get "network.${iface}.macaddr" > /dev/null && [ ! -n "$overwrite" ] ; then
+		echo "MAC for ${iface} is already set"
+	else
+		echo "Fixing MAC on ${dev} (${iface})"
+		sleep 10
+
+		uci set "network.${iface}.macaddr=$newmac"
+		uci -q commit network
+
+		ifconfig "$dev" down
+		ifconfig "$dev" hw ether "$newmac"
+		ifconfig "$dev" up
+		/etc/init.d/network restart
+	fi
+}
diff --git a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
index 0e038a49..9dc29da1 100755
--- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
+++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
@@ -176,39 +176,11 @@  fi
 /etc/init.d/network restart
 
 if [ -n "$ETHMESHMAC" ]; then
-    if uci get network.ethmesh.macaddr
-    then
-        echo "MAC for ethmesh is set already"
-    else
-        echo "Fixing MAC on $SWITCHDEV.3 (ethmesh)"
-        sleep 10
-
-        uci set network.ethmesh.macaddr=$ETHMESHMAC
-        uci commit network
-
-        ifconfig $SWITCHDEV.3 down
-        ifconfig $SWITCHDEV.3 hw ether $ETHMESHMAC
-        ifconfig $SWITCHDEV.3 up
-        /etc/init.d/network restart
-    fi
+    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
 fi
 
 if [ -n "$ROUTERMAC" ]; then
-    if uci get network.mesh.macaddr
-    then
-        echo "MAC for mesh is set already"
-    else
-        echo "Fixing MAC on br-mesh (mesh)"
-        sleep 10
-
-        uci set network.mesh.macaddr=$ROUTERMAC
-        uci commit network
-
-        ifconfig br-mesh down
-        ifconfig br-mesh hw ether $ROUTERMAC
-        ifconfig br-mesh up
-        /etc/init.d/network restart
-    fi
+    fixMac "$ROUTERMAC" "br-mesh" "mesh"
 fi
 
 if [ -n "$ETH0MAC" ]; then

Comments

Robert Langhammer Nov. 6, 2018, 10:14 a.m.
Hallo Adrian,

nur ein paar Kleinigkeiten.

Am 07.08.18 um 12:25 schrieb Adrian Schmutzler:
> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>
> ---
>
> Changes in v2:
> - Suppress output
>
> Changes in v3:
> - Rename iface and dev in explanation. The variables were not
>   renamed, as shorter is better here.
>
> Changes in v4:
> - Rebased
>
> Changes in v5:
> - Rebased
> ---
>  .../fff-network/files/lib/functions/fff/network    | 31 +++++++++++++++++++++
>  .../fff-network/files/usr/sbin/configurenetwork    | 32 ++--------------------
>  2 files changed, 33 insertions(+), 30 deletions(-)
>
> diff --git a/src/packages/fff/fff-network/files/lib/functions/fff/network b/src/packages/fff/fff-network/files/lib/functions/fff/network
> index af9b3434..0e9b466b 100644
> --- a/src/packages/fff/fff-network/files/lib/functions/fff/network
> +++ b/src/packages/fff/fff-network/files/lib/functions/fff/network
> @@ -96,3 +96,34 @@ macFlipLocalBit() {
>  	echo "$mac" | awk -F: '{ printf("%02x:%s:%s:%s:%s:%s\n", xor(("0x"$1),2), $2, $3, $4, $5, $6) }'
>  	return 0
>  }
> +
> +fixMac() {
> +	# Update MAC address on device/interface
> +	#
> +	# fixMac <newmac> <physdev> <interface> <overwrite (optional)>
> +	#
> +	# newmac: MAC address to be set
> +	# physdev: Device name to be updated as in ifconfig (e.g. br-mesh)
> +	# interface: Interface to be updated as in uci (e.g. mesh)
> +	# overwrite: If non-zero, the MAC is always replaced; if not set, the MAC is only written if none is present
> +
> +	local newmac=$1
> +	local dev=$2
> +	local iface=$3
> +	local overwrite=$4
> +
> +	if uci -q get "network.${iface}.macaddr" > /dev/null && [ ! -n "$overwrite" ] ; then
das ist doppelt nein. [ $overwrite ] reicht oder mit -z
> +		echo "MAC for ${iface} is already set"
> +	else
> +		echo "Fixing MAC on ${dev} (${iface})"
> +		sleep 10
> +
> +		uci set "network.${iface}.macaddr=$newmac"
> +		uci -q commit network
> +
> +		ifconfig "$dev" down
> +		ifconfig "$dev" hw ether "$newmac"
> +		ifconfig "$dev" up

# ifconfig is deprecated.

ip link set "$dev" down
ip l s "$dev" address "$newmac"
ip l s "$dev" up

Und die Verwendung der {} ist etwas uneinheitlich. Man braucht das nur,
wenn hinten dran kein white character ist oder man Expansion machen
will. Oder man sagt, es schadet nie und macht die immer hin. Wie ist
denn da der allgemeine Tenor?

Robert

> +		/etc/init.d/network restart
> +	fi
> +}
> diff --git a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> index 0e038a49..9dc29da1 100755
> --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> @@ -176,39 +176,11 @@ fi
>  /etc/init.d/network restart
>  
>  if [ -n "$ETHMESHMAC" ]; then
> -    if uci get network.ethmesh.macaddr
> -    then
> -        echo "MAC for ethmesh is set already"
> -    else
> -        echo "Fixing MAC on $SWITCHDEV.3 (ethmesh)"
> -        sleep 10
> -
> -        uci set network.ethmesh.macaddr=$ETHMESHMAC
> -        uci commit network
> -
> -        ifconfig $SWITCHDEV.3 down
> -        ifconfig $SWITCHDEV.3 hw ether $ETHMESHMAC
> -        ifconfig $SWITCHDEV.3 up
> -        /etc/init.d/network restart
> -    fi
> +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
>  fi
>  
>  if [ -n "$ROUTERMAC" ]; then
> -    if uci get network.mesh.macaddr
> -    then
> -        echo "MAC for mesh is set already"
> -    else
> -        echo "Fixing MAC on br-mesh (mesh)"
> -        sleep 10
> -
> -        uci set network.mesh.macaddr=$ROUTERMAC
> -        uci commit network
> -
> -        ifconfig br-mesh down
> -        ifconfig br-mesh hw ether $ROUTERMAC
> -        ifconfig br-mesh up
> -        /etc/init.d/network restart
> -    fi
> +    fixMac "$ROUTERMAC" "br-mesh" "mesh"
>  fi
>  
>  if [ -n "$ETH0MAC" ]; then
Adrian Schmutzler Nov. 6, 2018, 10:55 a.m.
Hallo Robert,

danke für die Rückmeldung.

Das mit dem ip statt ifconfig teile ich, allerdings würde ich das gerne in einen separaten Patch bauen und nach dem Release machen, falls es unvorhergesehenes Verhalten durch den Austausch gibt. (Die jetzige Variante läuft schon sehr lange bei mir, daher habe ich da mehr Vertrauen.

Die Verneinung von -n hatte den Hintergrund, dass dann eine nicht gesetzte (!) Variable wie eine leere behandelt wird. Ist das dann auch bei -z so?

Zwecks der {} habe ich das auch eher gemischt gemacht, und immer wenn ich mir nicht sicher war habe ich sicherheitshalber welche hingemacht.

In irgendeiner Sprache hatte ich da mal ein Problem, dass dann die Variablen nicht aufgelöst werden konnten, wenn das falsche dahinter stand.

Könnte man aber gerne irgendwie vereinheitlichen.

Grüße

Adrian

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of robert
> Sent: Dienstag, 6. November 2018 11:15
> To: franken-dev@freifunk.net
> Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set MAC on
> device
> 
> Hallo Adrian,
> 
> nur ein paar Kleinigkeiten.
> 
> Am 07.08.18 um 12:25 schrieb Adrian Schmutzler:
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> > ---
> >
> > Changes in v2:
> > - Suppress output
> >
> > Changes in v3:
> > - Rename iface and dev in explanation. The variables were not
> >   renamed, as shorter is better here.
> >
> > Changes in v4:
> > - Rebased
> >
> > Changes in v5:
> > - Rebased
> > ---
> >  .../fff-network/files/lib/functions/fff/network    | 31
> +++++++++++++++++++++
> >  .../fff-network/files/usr/sbin/configurenetwork    | 32 ++--------------------
> >  2 files changed, 33 insertions(+), 30 deletions(-)
> >
> > diff --git
> > a/src/packages/fff/fff-network/files/lib/functions/fff/network
> > b/src/packages/fff/fff-network/files/lib/functions/fff/network
> > index af9b3434..0e9b466b 100644
> > --- a/src/packages/fff/fff-network/files/lib/functions/fff/network
> > +++ b/src/packages/fff/fff-network/files/lib/functions/fff/network
> > @@ -96,3 +96,34 @@ macFlipLocalBit() {
> >  	echo "$mac" | awk -F: '{ printf("%02x:%s:%s:%s:%s:%s\n",
> xor(("0x"$1),2), $2, $3, $4, $5, $6) }'
> >  	return 0
> >  }
> > +
> > +fixMac() {
> > +	# Update MAC address on device/interface
> > +	#
> > +	# fixMac <newmac> <physdev> <interface> <overwrite (optional)>
> > +	#
> > +	# newmac: MAC address to be set
> > +	# physdev: Device name to be updated as in ifconfig (e.g. br-mesh)
> > +	# interface: Interface to be updated as in uci (e.g. mesh)
> > +	# overwrite: If non-zero, the MAC is always replaced; if not set,
> > +the MAC is only written if none is present
> > +
> > +	local newmac=$1
> > +	local dev=$2
> > +	local iface=$3
> > +	local overwrite=$4
> > +
> > +	if uci -q get "network.${iface}.macaddr" > /dev/null && [ ! -n
> > +"$overwrite" ] ; then
> das ist doppelt nein. [ $overwrite ] reicht oder mit -z
> > +		echo "MAC for ${iface} is already set"
> > +	else
> > +		echo "Fixing MAC on ${dev} (${iface})"
> > +		sleep 10
> > +
> > +		uci set "network.${iface}.macaddr=$newmac"
> > +		uci -q commit network
> > +
> > +		ifconfig "$dev" down
> > +		ifconfig "$dev" hw ether "$newmac"
> > +		ifconfig "$dev" up
> 
> # ifconfig is deprecated.
> 
> ip link set "$dev" down
> ip l s "$dev" address "$newmac"
> ip l s "$dev" up
> 
> Und die Verwendung der {} ist etwas uneinheitlich. Man braucht das nur,
> wenn hinten dran kein white character ist oder man Expansion machen will.
> Oder man sagt, es schadet nie und macht die immer hin. Wie ist denn da der
> allgemeine Tenor?
> 
> Robert
> 
> > +		/etc/init.d/network restart
> > +	fi
> > +}
> > diff --git
> > a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > index 0e038a49..9dc29da1 100755
> > --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > @@ -176,39 +176,11 @@ fi
> >  /etc/init.d/network restart
> >
> >  if [ -n "$ETHMESHMAC" ]; then
> > -    if uci get network.ethmesh.macaddr
> > -    then
> > -        echo "MAC for ethmesh is set already"
> > -    else
> > -        echo "Fixing MAC on $SWITCHDEV.3 (ethmesh)"
> > -        sleep 10
> > -
> > -        uci set network.ethmesh.macaddr=$ETHMESHMAC
> > -        uci commit network
> > -
> > -        ifconfig $SWITCHDEV.3 down
> > -        ifconfig $SWITCHDEV.3 hw ether $ETHMESHMAC
> > -        ifconfig $SWITCHDEV.3 up
> > -        /etc/init.d/network restart
> > -    fi
> > +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
> >  fi
> >
> >  if [ -n "$ROUTERMAC" ]; then
> > -    if uci get network.mesh.macaddr
> > -    then
> > -        echo "MAC for mesh is set already"
> > -    else
> > -        echo "Fixing MAC on br-mesh (mesh)"
> > -        sleep 10
> > -
> > -        uci set network.mesh.macaddr=$ROUTERMAC
> > -        uci commit network
> > -
> > -        ifconfig br-mesh down
> > -        ifconfig br-mesh hw ether $ROUTERMAC
> > -        ifconfig br-mesh up
> > -        /etc/init.d/network restart
> > -    fi
> > +    fixMac "$ROUTERMAC" "br-mesh" "mesh"
> >  fi
> >
> >  if [ -n "$ETH0MAC" ]; then
Robert Langhammer Nov. 6, 2018, 11:49 a.m.
Dann schreiben wir uns doch eine Aufräumaktion auf die todo Liste und
machen erst mal die Patches fertig. Den 2/2 schau ich mir spaeter in
Ruhe an, was der macht.

Hier ein

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

Am 06.11.18 um 11:55 schrieb mail@adrianschmutzler.de:
> Hallo Robert,
>
> danke für die Rückmeldung.
>
> Das mit dem ip statt ifconfig teile ich, allerdings würde ich das gerne in einen separaten Patch bauen und nach dem Release machen, falls es unvorhergesehenes Verhalten durch den Austausch gibt. (Die jetzige Variante läuft schon sehr lange bei mir, daher habe ich da mehr Vertrauen.
>
> Die Verneinung von -n hatte den Hintergrund, dass dann eine nicht gesetzte (!) Variable wie eine leere behandelt wird. Ist das dann auch bei -z so?

Das mit der leeren Variable musste ich noch mal testen. Es ist so, dass
es keinen Unterschied macht:

Eine leere aber vorhandene

root@c2600:~# var=
root@c2600:~# set | tail -2
_='FILE'
var=''
root@c2600:~# [ $var ] && echo ja
root@c2600:~# [ -z $var ] && echo ja
ja
root@c2600:~# [ ! -n $var ] && echo ja
root@c2600:~# [ !  $var ] && echo ja
ja

und eine nicht vorhandene:

root@c2600:~# [ $ldfghs ] && echo ja
root@c2600:~#
root@c2600:~# [ -z $ldfghs ] && echo ja
ja
root@c2600:~#

-z stimmte nicht, war mein Dekfehler.

>
> Zwecks der {} habe ich das auch eher gemischt gemacht, und immer wenn ich mir nicht sicher war habe ich sicherheitshalber welche hingemacht.
>
> In irgendeiner Sprache hatte ich da mal ein Problem, dass dann die Variablen nicht aufgelöst werden konnten, wenn das falsche dahinter stand.
>
> Könnte man aber gerne irgendwie vereinheitlichen.
>
> Grüße
>
> Adrian
>
>> -----Original Message-----
>> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
>> Of robert
>> Sent: Dienstag, 6. November 2018 11:15
>> To: franken-dev@freifunk.net
>> Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set MAC on
>> device
>>
>> Hallo Adrian,
>>
>> nur ein paar Kleinigkeiten.
>>
>> Am 07.08.18 um 12:25 schrieb Adrian Schmutzler:
>>> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>>>
>>> ---
>>>
>>> Changes in v2:
>>> - Suppress output
>>>
>>> Changes in v3:
>>> - Rename iface and dev in explanation. The variables were not
>>>   renamed, as shorter is better here.
>>>
>>> Changes in v4:
>>> - Rebased
>>>
>>> Changes in v5:
>>> - Rebased
>>> ---
>>>  .../fff-network/files/lib/functions/fff/network    | 31
>> +++++++++++++++++++++
>>>  .../fff-network/files/usr/sbin/configurenetwork    | 32 ++--------------------
>>>  2 files changed, 33 insertions(+), 30 deletions(-)
>>>
>>> diff --git
>>> a/src/packages/fff/fff-network/files/lib/functions/fff/network
>>> b/src/packages/fff/fff-network/files/lib/functions/fff/network
>>> index af9b3434..0e9b466b 100644
>>> --- a/src/packages/fff/fff-network/files/lib/functions/fff/network
>>> +++ b/src/packages/fff/fff-network/files/lib/functions/fff/network
>>> @@ -96,3 +96,34 @@ macFlipLocalBit() {
>>>  	echo "$mac" | awk -F: '{ printf("%02x:%s:%s:%s:%s:%s\n",
>> xor(("0x"$1),2), $2, $3, $4, $5, $6) }'
>>>  	return 0
>>>  }
>>> +
>>> +fixMac() {
>>> +	# Update MAC address on device/interface
>>> +	#
>>> +	# fixMac <newmac> <physdev> <interface> <overwrite (optional)>
>>> +	#
>>> +	# newmac: MAC address to be set
>>> +	# physdev: Device name to be updated as in ifconfig (e.g. br-mesh)
>>> +	# interface: Interface to be updated as in uci (e.g. mesh)
>>> +	# overwrite: If non-zero, the MAC is always replaced; if not set,
>>> +the MAC is only written if none is present
>>> +
>>> +	local newmac=$1
>>> +	local dev=$2
>>> +	local iface=$3
>>> +	local overwrite=$4
>>> +
>>> +	if uci -q get "network.${iface}.macaddr" > /dev/null && [ ! -n
>>> +"$overwrite" ] ; then
>> das ist doppelt nein. [ $overwrite ] reicht oder mit -z
>>> +		echo "MAC for ${iface} is already set"
>>> +	else
>>> +		echo "Fixing MAC on ${dev} (${iface})"
>>> +		sleep 10
>>> +
>>> +		uci set "network.${iface}.macaddr=$newmac"
>>> +		uci -q commit network
>>> +
>>> +		ifconfig "$dev" down
>>> +		ifconfig "$dev" hw ether "$newmac"
>>> +		ifconfig "$dev" up
>> # ifconfig is deprecated.
>>
>> ip link set "$dev" down
>> ip l s "$dev" address "$newmac"
>> ip l s "$dev" up
>>
>> Und die Verwendung der {} ist etwas uneinheitlich. Man braucht das nur,
>> wenn hinten dran kein white character ist oder man Expansion machen will.
>> Oder man sagt, es schadet nie und macht die immer hin. Wie ist denn da der
>> allgemeine Tenor?
>>
>> Robert
>>
>>> +		/etc/init.d/network restart
>>> +	fi
>>> +}
>>> diff --git
>>> a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
>>> b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
>>> index 0e038a49..9dc29da1 100755
>>> --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
>>> +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
>>> @@ -176,39 +176,11 @@ fi
>>>  /etc/init.d/network restart
>>>
>>>  if [ -n "$ETHMESHMAC" ]; then
>>> -    if uci get network.ethmesh.macaddr
>>> -    then
>>> -        echo "MAC for ethmesh is set already"
>>> -    else
>>> -        echo "Fixing MAC on $SWITCHDEV.3 (ethmesh)"
>>> -        sleep 10
>>> -
>>> -        uci set network.ethmesh.macaddr=$ETHMESHMAC
>>> -        uci commit network
>>> -
>>> -        ifconfig $SWITCHDEV.3 down
>>> -        ifconfig $SWITCHDEV.3 hw ether $ETHMESHMAC
>>> -        ifconfig $SWITCHDEV.3 up
>>> -        /etc/init.d/network restart
>>> -    fi
>>> +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
>>>  fi
>>>
>>>  if [ -n "$ROUTERMAC" ]; then
>>> -    if uci get network.mesh.macaddr
>>> -    then
>>> -        echo "MAC for mesh is set already"
>>> -    else
>>> -        echo "Fixing MAC on br-mesh (mesh)"
>>> -        sleep 10
>>> -
>>> -        uci set network.mesh.macaddr=$ROUTERMAC
>>> -        uci commit network
>>> -
>>> -        ifconfig br-mesh down
>>> -        ifconfig br-mesh hw ether $ROUTERMAC
>>> -        ifconfig br-mesh up
>>> -        /etc/init.d/network restart
>>> -    fi
>>> +    fixMac "$ROUTERMAC" "br-mesh" "mesh"
>>>  fi
>>>
>>>  if [ -n "$ETH0MAC" ]; then
Adrian Schmutzler Nov. 6, 2018, 12:28 p.m.
Hallo robert,

 

alles klar, ich lasse mich sowohl für eine Aufräumaktion zwecks ifconfig als auch für eine zwecks Variablennamen begeistert.

 

Zwecks –z:

 

Ich gehe davon aus, das die Negation auch mit einer nicht gesetzten Variable klappt, wenn man diese in Anführungszeichen schreibt:

 

[ -z „$a“ ] ergibt wahrscheinlich genau das Gleiche wie [ ! -n „$a“ ] .

 

Ich bin mir nur nicht zu 100 % sicher. Soll ich das beim Applien noch auf -z ändern?

 

Grüße

 

Adrian

 

 

From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf Of robert
Sent: Dienstag, 6. November 2018 12:49
To: franken-dev@freifunk.net
Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set MAC on device

 

Dann schreiben wir uns doch eine Aufräumaktion auf die todo Liste und 
machen erst mal die Patches fertig. Den 2/2 schau ich mir spaeter in 
Ruhe an, was der macht. 

Hier ein 

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

Am 06.11.18 um 11:55 schrieb mail@adrianschmutzler.de <mailto:mail@adrianschmutzler.de> : 
> Hallo Robert, 
> 
> danke für die Rückmeldung. 
> 
> Das mit dem ip statt ifconfig teile ich, allerdings würde ich das gerne in einen separaten Patch bauen und nach dem Release machen, falls es unvorhergesehenes Verhalten durch den Austausch gibt. (Die jetzige Variante läuft schon sehr lange bei mir, daher habe ich da mehr Vertrauen.

> 
> Die Verneinung von -n hatte den Hintergrund, dass dann eine nicht gesetzte (!) Variable wie eine leere behandelt wird. Ist das dann auch bei -z so?

Das mit der leeren Variable musste ich noch mal testen. Es ist so, dass 
es keinen Unterschied macht: 

Eine leere aber vorhandene 

root@c2600:~# var= 
root@c2600:~# set | tail -2 
_='FILE' 
var='' 
root@c2600:~# [ $var ] && echo ja 
root@c2600:~# [ -z $var ] && echo ja 
ja 
root@c2600:~# [ ! -n $var ] && echo ja 
root@c2600:~# [ !  $var ] && echo ja 
ja 

und eine nicht vorhandene: 

root@c2600:~# [ $ldfghs ] && echo ja 
root@c2600:~# 
root@c2600:~# [ -z $ldfghs ] && echo ja 
ja 
root@c2600:~# 

-z stimmte nicht, war mein Dekfehler. 

> 
> Zwecks der {} habe ich das auch eher gemischt gemacht, und immer wenn ich mir nicht sicher war habe ich sicherheitshalber welche hingemacht.

> 
> In irgendeiner Sprache hatte ich da mal ein Problem, dass dann die Variablen nicht aufgelöst werden konnten, wenn das falsche dahinter stand.

> 
> Könnte man aber gerne irgendwie vereinheitlichen. 
> 
> Grüße 
> 
> Adrian 
> 
>> -----Original Message----- 
>> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf 
>> Of robert 
>> Sent: Dienstag, 6. November 2018 11:15 
>> To: franken-dev@freifunk.net <mailto:franken-dev@freifunk.net>  
>> Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set MAC on 
>> device 
>> 
>> Hallo Adrian, 
>> 
>> nur ein paar Kleinigkeiten. 
>> 
>> Am 07.08.18 um 12:25 schrieb Adrian Schmutzler: 
>>> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de <mailto:freifunk@adrianschmutzler.de> > 
>>> 
>>> --- 
>>> 
>>> Changes in v2: 
>>> - Suppress output 
>>> 
>>> Changes in v3: 
>>> - Rename iface and dev in explanation. The variables were not 
>>>   renamed, as shorter is better here. 
>>> 
>>> Changes in v4: 
>>> - Rebased 
>>> 
>>> Changes in v5: 
>>> - Rebased 
>>> --- 
>>>  .../fff-network/files/lib/functions/fff/network    | 31 
>> +++++++++++++++++++++ 
>>>  .../fff-network/files/usr/sbin/configurenetwork    | 32 ++-------------------- 
>>>  2 files changed, 33 insertions(+), 30 deletions(-) 
>>> 
>>> diff --git 
>>> a/src/packages/fff/fff-network/files/lib/functions/fff/network 
>>> b/src/packages/fff/fff-network/files/lib/functions/fff/network 
>>> index af9b3434..0e9b466b 100644 
>>> --- a/src/packages/fff/fff-network/files/lib/functions/fff/network 
>>> +++ b/src/packages/fff/fff-network/files/lib/functions/fff/network 
>>> @@ -96,3 +96,34 @@ macFlipLocalBit() { 
>>>     echo "$mac" | awk -F: '{ printf("%02x:%s:%s:%s:%s:%s\n", 
>> xor(("0x"$1),2), $2, $3, $4, $5, $6) }' 
>>>     return 0 
>>>  } 
>>> + 
>>> +fixMac() { 
>>> +   # Update MAC address on device/interface 
>>> +   # 
>>> +   # fixMac <newmac> <physdev> <interface> <overwrite (optional)> 
>>> +   # 
>>> +   # newmac: MAC address to be set 
>>> +   # physdev: Device name to be updated as in ifconfig (e.g. br-mesh) 
>>> +   # interface: Interface to be updated as in uci (e.g. mesh) 
>>> +   # overwrite: If non-zero, the MAC is always replaced; if not set, 
>>> +the MAC is only written if none is present 
>>> + 
>>> +   local newmac=$1 
>>> +   local dev=$2 
>>> +   local iface=$3 
>>> +   local overwrite=$4 
>>> + 
>>> +   if uci -q get "network.${iface}.macaddr" > /dev/null && [ ! -n 
>>> +"$overwrite" ] ; then 
>> das ist doppelt nein. [ $overwrite ] reicht oder mit -z 
>>> +           echo "MAC for ${iface} is already set" 
>>> +   else 
>>> +           echo "Fixing MAC on ${dev} (${iface})" 
>>> +           sleep 10 
>>> + 
>>> +           uci set "network.${iface}.macaddr=$newmac" 
>>> +           uci -q commit network 
>>> + 
>>> +           ifconfig "$dev" down 
>>> +           ifconfig "$dev" hw ether "$newmac" 
>>> +           ifconfig "$dev" up 
>> # ifconfig is deprecated. 
>> 
>> ip link set "$dev" down 
>> ip l s "$dev" address "$newmac" 
>> ip l s "$dev" up 
>> 
>> Und die Verwendung der {} ist etwas uneinheitlich. Man braucht das nur, 
>> wenn hinten dran kein white character ist oder man Expansion machen will. 
>> Oder man sagt, es schadet nie und macht die immer hin. Wie ist denn da der 
>> allgemeine Tenor? 
>> 
>> Robert 
>> 
>>> +           /etc/init.d/network restart 
>>> +   fi 
>>> +} 
>>> diff --git 
>>> a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork 
>>> b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork 
>>> index 0e038a49..9dc29da1 100755 
>>> --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork 
>>> +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork 
>>> @@ -176,39 +176,11 @@ fi 
>>>  /etc/init.d/network restart 
>>> 
>>>  if [ -n "$ETHMESHMAC" ]; then 
>>> -    if uci get network.ethmesh.macaddr 
>>> -    then 
>>> -        echo "MAC for ethmesh is set already" 
>>> -    else 
>>> -        echo "Fixing MAC on $SWITCHDEV.3 (ethmesh)" 
>>> -        sleep 10 
>>> - 
>>> -        uci set network.ethmesh.macaddr=$ETHMESHMAC 
>>> -        uci commit network 
>>> - 
>>> -        ifconfig $SWITCHDEV.3 down 
>>> -        ifconfig $SWITCHDEV.3 hw ether $ETHMESHMAC 
>>> -        ifconfig $SWITCHDEV.3 up 
>>> -        /etc/init.d/network restart 
>>> -    fi 
>>> +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh" 
>>>  fi 
>>> 
>>>  if [ -n "$ROUTERMAC" ]; then 
>>> -    if uci get network.mesh.macaddr 
>>> -    then 
>>> -        echo "MAC for mesh is set already" 
>>> -    else 
>>> -        echo "Fixing MAC on br-mesh (mesh)" 
>>> -        sleep 10 
>>> - 
>>> -        uci set network.mesh.macaddr=$ROUTERMAC 
>>> -        uci commit network 
>>> - 
>>> -        ifconfig br-mesh down 
>>> -        ifconfig br-mesh hw ether $ROUTERMAC 
>>> -        ifconfig br-mesh up 
>>> -        /etc/init.d/network restart 
>>> -    fi 
>>> +    fixMac "$ROUTERMAC" "br-mesh" "mesh" 
>>>  fi 
>>> 
>>>  if [ -n "$ETH0MAC" ]; then
Robert Langhammer Nov. 6, 2018, 1:30 p.m.
Am 06.11.18 um 13:28 schrieb Adrian Schmutzler:
>
> Hallo robert,
>
>  
>
> alles klar, ich lasse mich sowohl für eine Aufräumaktion zwecks
> ifconfig als auch für eine zwecks Variablennamen begeistert.
>
>  
>
> Zwecks –z:
>
>  
>
> Ich gehe davon aus, das die Negation auch mit einer nicht gesetzten
> Variable klappt, wenn man diese in Anführungszeichen schreibt:
>
>  
>
> [ -z „$a“ ] ergibt wahrscheinlich genau das Gleiche wie [ ! -n „$a“ ] .
>
ja, das ist egal. Mir ist nur ein seltsamer Effekt aufgefallen:
root@c2600:~# set | tail -1
var=''
root@c2600:~# [ -n $var ] && echo true
true
root@c2600:~# [ -n "$var" ] && echo true
root@c2600:~#

Das ist hinterhaeltig. Muss man aufpassen. War mir auch nicht klar. Bin
bis jetzt davon ausgegangen, das sollte gleich sein.

-z macht das nicht

root@c2600:~# [ -z "$var" ] && echo true
true
root@c2600:~# [ -z $var ] && echo true
true
root@c2600:~#

Robert

>
> Ich bin mir nur nicht zu 100 % sicher. Soll ich das beim Applien noch
> auf -z ändern?
>
>  
>
> Grüße
>
>  
>
> Adrian
>
>  
>
>  
>
> *From:*franken-dev [mailto:franken-dev-bounces@freifunk.net] *On
> Behalf Of *robert
> *Sent:* Dienstag, 6. November 2018 12:49
> *To:* franken-dev@freifunk.net
> *Subject:* Re: [PATCH v5 1/2] fff-network: Introduce function to set
> MAC on device
>
>  
>
> Dann schreiben wir uns doch eine Aufräumaktion auf die todo Liste und
> machen erst mal die Patches fertig. Den 2/2 schau ich mir spaeter in
> Ruhe an, was der macht.
>
> Hier ein
>
> Reviewed-by: Robert Langhammer <rlanghammer@web.de
> <mailto:rlanghammer@web.de>>
>
> Am 06.11.18 um 11:55 schrieb mail@adrianschmutzler.de
> <mailto:mail@adrianschmutzler.de>:
> > Hallo Robert,
> >
> > danke für die Rückmeldung.
> >
> > Das mit dem ip statt ifconfig teile ich, allerdings würde ich das
> gerne in einen separaten Patch bauen und nach dem Release machen,
> falls es unvorhergesehenes Verhalten durch den Austausch gibt. (Die
> jetzige Variante läuft schon sehr lange bei mir, daher habe ich da
> mehr Vertrauen.
>
> >
> > Die Verneinung von -n hatte den Hintergrund, dass dann eine nicht
> gesetzte (!) Variable wie eine leere behandelt wird. Ist das dann auch
> bei -z so?
>
> Das mit der leeren Variable musste ich noch mal testen. Es ist so, dass
> es keinen Unterschied macht:
>
> Eine leere aber vorhandene
>
> root@c2600:~# var=
> root@c2600:~# set | tail -2
> _='FILE'
> var=''
> root@c2600:~# [ $var ] && echo ja
> root@c2600:~# [ -z $var ] && echo ja
> ja
> root@c2600:~# [ ! -n $var ] && echo ja
> root@c2600:~# [ !  $var ] && echo ja
> ja
>
> und eine nicht vorhandene:
>
> root@c2600:~# [ $ldfghs ] && echo ja
> root@c2600:~#
> root@c2600:~# [ -z $ldfghs ] && echo ja
> ja
> root@c2600:~#
>
> -z stimmte nicht, war mein Dekfehler.
>
> >
> > Zwecks der {} habe ich das auch eher gemischt gemacht, und immer wenn
> ich mir nicht sicher war habe ich sicherheitshalber welche hingemacht.
>
> >
> > In irgendeiner Sprache hatte ich da mal ein Problem, dass dann die
> Variablen nicht aufgelöst werden konnten, wenn das falsche dahinter stand.
>
> >
> > Könnte man aber gerne irgendwie vereinheitlichen.
> >
> > Grüße
> >
> > Adrian
> >
> >> -----Original Message-----
> >> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> >> Of robert
> >> Sent: Dienstag, 6. November 2018 11:15
> >> To: franken-dev@freifunk.net <mailto:franken-dev@freifunk.net>
> >> Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set
> MAC on
> >> device
> >>
> >> Hallo Adrian,
> >>
> >> nur ein paar Kleinigkeiten.
> >>
> >> Am 07.08.18 um 12:25 schrieb Adrian Schmutzler:
> >>> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de
> <mailto:freifunk@adrianschmutzler.de>>
> >>>
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - Suppress output
> >>>
> >>> Changes in v3:
> >>> - Rename iface and dev in explanation. The variables were not
> >>>   renamed, as shorter is better here.
> >>>
> >>> Changes in v4:
> >>> - Rebased
> >>>
> >>> Changes in v5:
> >>> - Rebased
> >>> ---
> >>>  .../fff-network/files/lib/functions/fff/network    | 31
> >> +++++++++++++++++++++
> >>>  .../fff-network/files/usr/sbin/configurenetwork    | 32
> ++--------------------
> >>>  2 files changed, 33 insertions(+), 30 deletions(-)
> >>>
> >>> diff --git
> >>> a/src/packages/fff/fff-network/files/lib/functions/fff/network
> >>> b/src/packages/fff/fff-network/files/lib/functions/fff/network
> >>> index af9b3434..0e9b466b 100644
> >>> --- a/src/packages/fff/fff-network/files/lib/functions/fff/network
> >>> +++ b/src/packages/fff/fff-network/files/lib/functions/fff/network
> >>> @@ -96,3 +96,34 @@ macFlipLocalBit() {
> >>>     echo "$mac" | awk -F: '{ printf("%02x:%s:%s:%s:%s:%s\n",
> >> xor(("0x"$1),2), $2, $3, $4, $5, $6) }'
> >>>     return 0
> >>>  }
> >>> +
> >>> +fixMac() {
> >>> +   # Update MAC address on device/interface
> >>> +   #
> >>> +   # fixMac <newmac> <physdev> <interface> <overwrite (optional)>
> >>> +   #
> >>> +   # newmac: MAC address to be set
> >>> +   # physdev: Device name to be updated as in ifconfig (e.g. br-mesh)
> >>> +   # interface: Interface to be updated as in uci (e.g. mesh)
> >>> +   # overwrite: If non-zero, the MAC is always replaced; if not set,
> >>> +the MAC is only written if none is present
> >>> +
> >>> +   local newmac=$1
> >>> +   local dev=$2
> >>> +   local iface=$3
> >>> +   local overwrite=$4
> >>> +
> >>> +   if uci -q get "network.${iface}.macaddr" > /dev/null && [ ! -n
> >>> +"$overwrite" ] ; then
> >> das ist doppelt nein. [ $overwrite ] reicht oder mit -z
> >>> +           echo "MAC for ${iface} is already set"
> >>> +   else
> >>> +           echo "Fixing MAC on ${dev} (${iface})"
> >>> +           sleep 10
> >>> +
> >>> +           uci set "network.${iface}.macaddr=$newmac"
> >>> +           uci -q commit network
> >>> +
> >>> +           ifconfig "$dev" down
> >>> +           ifconfig "$dev" hw ether "$newmac"
> >>> +           ifconfig "$dev" up
> >> # ifconfig is deprecated.
> >>
> >> ip link set "$dev" down
> >> ip l s "$dev" address "$newmac"
> >> ip l s "$dev" up
> >>
> >> Und die Verwendung der {} ist etwas uneinheitlich. Man braucht das nur,
> >> wenn hinten dran kein white character ist oder man Expansion machen
> will.
> >> Oder man sagt, es schadet nie und macht die immer hin. Wie ist denn
> da der
> >> allgemeine Tenor?
> >>
> >> Robert
> >>
> >>> +           /etc/init.d/network restart
> >>> +   fi
> >>> +}
> >>> diff --git
> >>> a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> >>> b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> >>> index 0e038a49..9dc29da1 100755
> >>> --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> >>> +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> >>> @@ -176,39 +176,11 @@ fi
> >>>  /etc/init.d/network restart
> >>>
> >>>  if [ -n "$ETHMESHMAC" ]; then
> >>> -    if uci get network.ethmesh.macaddr
> >>> -    then
> >>> -        echo "MAC for ethmesh is set already"
> >>> -    else
> >>> -        echo "Fixing MAC on $SWITCHDEV.3 (ethmesh)"
> >>> -        sleep 10
> >>> -
> >>> -        uci set network.ethmesh.macaddr=$ETHMESHMAC
> >>> -        uci commit network
> >>> -
> >>> -        ifconfig $SWITCHDEV.3 down
> >>> -        ifconfig $SWITCHDEV.3 hw ether $ETHMESHMAC
> >>> -        ifconfig $SWITCHDEV.3 up
> >>> -        /etc/init.d/network restart
> >>> -    fi
> >>> +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
> >>>  fi
> >>>
> >>>  if [ -n "$ROUTERMAC" ]; then
> >>> -    if uci get network.mesh.macaddr
> >>> -    then
> >>> -        echo "MAC for mesh is set already"
> >>> -    else
> >>> -        echo "Fixing MAC on br-mesh (mesh)"
> >>> -        sleep 10
> >>> -
> >>> -        uci set network.mesh.macaddr=$ROUTERMAC
> >>> -        uci commit network
> >>> -
> >>> -        ifconfig br-mesh down
> >>> -        ifconfig br-mesh hw ether $ROUTERMAC
> >>> -        ifconfig br-mesh up
> >>> -        /etc/init.d/network restart
> >>> -    fi
> >>> +    fixMac "$ROUTERMAC" "br-mesh" "mesh"
> >>>  fi
> >>>
> >>>  if [ -n "$ETH0MAC" ]; then
>
>  
>
Adrian Schmutzler Nov. 6, 2018, 1:34 p.m.
Oh, das ist mies.

 

Deshalb habe zumindest ich diese Statements immer mit Anführungszeichen um die Variable verwendet.

 

Damit ist es in meinen Augen am nachvollziehbarsten, da offensichtlicher ist, wann ein „leerer“ und wann ein „nicht-leerer“ String vorliegt.

 

Grüße

 

Adrian

 

 

From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf Of robert
Sent: Dienstag, 6. November 2018 14:31
To: franken-dev@freifunk.net
Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set MAC on device

 

 

Am 06.11.18 um 13:28 schrieb Adrian Schmutzler:

	Hallo robert,

	 

	alles klar, ich lasse mich sowohl für eine Aufräumaktion zwecks ifconfig als auch für eine zwecks Variablennamen begeistert.

	 

	Zwecks –z:

	 

	Ich gehe davon aus, das die Negation auch mit einer nicht gesetzten Variable klappt, wenn man diese in Anführungszeichen schreibt:

	 

	[ -z „$a“ ] ergibt wahrscheinlich genau das Gleiche wie [ ! -n „$a“ ] .

ja, das ist egal. Mir ist nur ein seltsamer Effekt aufgefallen:
root@c2600:~# set | tail -1
var=''
root@c2600:~# [ -n $var ] && echo true
true
root@c2600:~# [ -n "$var" ] && echo true
root@c2600:~#

Das ist hinterhaeltig. Muss man aufpassen. War mir auch nicht klar. Bin bis jetzt davon ausgegangen, das sollte gleich sein.

-z macht das nicht

root@c2600:~# [ -z "$var" ] && echo true
true
root@c2600:~# [ -z $var ] && echo true
true
root@c2600:~# 

Robert

	
	
	

	Ich bin mir nur nicht zu 100 % sicher. Soll ich das beim Applien noch auf -z ändern?

	 

	Grüße

	 

	Adrian

	 

	 

	From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf Of robert
	Sent: Dienstag, 6. November 2018 12:49
	To: franken-dev@freifunk.net <mailto:franken-dev@freifunk.net> 
	Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set MAC on device

	 

	Dann schreiben wir uns doch eine Aufräumaktion auf die todo Liste und 
	machen erst mal die Patches fertig. Den 2/2 schau ich mir spaeter in 
	Ruhe an, was der macht. 

	Hier ein 

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

	Am 06.11.18 um 11:55 schrieb mail@adrianschmutzler.de <mailto:mail@adrianschmutzler.de> : 
	> Hallo Robert, 
	> 
	> danke für die Rückmeldung. 
	> 
	> Das mit dem ip statt ifconfig teile ich, allerdings würde ich das gerne in einen separaten Patch bauen und nach dem Release machen, falls es unvorhergesehenes Verhalten durch den Austausch gibt. (Die jetzige Variante läuft schon sehr lange bei mir, daher habe ich da mehr Vertrauen.

	> 
	> Die Verneinung von -n hatte den Hintergrund, dass dann eine nicht gesetzte (!) Variable wie eine leere behandelt wird. Ist das dann auch bei -z so?

	Das mit der leeren Variable musste ich noch mal testen. Es ist so, dass 
	es keinen Unterschied macht: 

	Eine leere aber vorhandene 

	root@c2600:~# var= 
	root@c2600:~# set | tail -2 
	_='FILE' 
	var='' 
	root@c2600:~# [ $var ] && echo ja 
	root@c2600:~# [ -z $var ] && echo ja 
	ja 
	root@c2600:~# [ ! -n $var ] && echo ja 
	root@c2600:~# [ !  $var ] && echo ja 
	ja 

	und eine nicht vorhandene: 

	root@c2600:~# [ $ldfghs ] && echo ja 
	root@c2600:~# 
	root@c2600:~# [ -z $ldfghs ] && echo ja 
	ja 
	root@c2600:~# 

	-z stimmte nicht, war mein Dekfehler. 

	> 
	> Zwecks der {} habe ich das auch eher gemischt gemacht, und immer wenn ich mir nicht sicher war habe ich sicherheitshalber welche hingemacht.

	> 
	> In irgendeiner Sprache hatte ich da mal ein Problem, dass dann die Variablen nicht aufgelöst werden konnten, wenn das falsche dahinter stand.

	> 
	> Könnte man aber gerne irgendwie vereinheitlichen. 
	> 
	> Grüße 
	> 
	> Adrian 
	> 
	>> -----Original Message----- 
	>> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf 
	>> Of robert 
	>> Sent: Dienstag, 6. November 2018 11:15 
	>> To: franken-dev@freifunk.net <mailto:franken-dev@freifunk.net>  
	>> Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set MAC on 
	>> device 
	>> 
	>> Hallo Adrian, 
	>> 
	>> nur ein paar Kleinigkeiten. 
	>> 
	>> Am 07.08.18 um 12:25 schrieb Adrian Schmutzler: 
	>>> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de <mailto:freifunk@adrianschmutzler.de> > 
	>>> 
	>>> --- 
	>>> 
	>>> Changes in v2: 
	>>> - Suppress output 
	>>> 
	>>> Changes in v3: 
	>>> - Rename iface and dev in explanation. The variables were not 
	>>>   renamed, as shorter is better here. 
	>>> 
	>>> Changes in v4: 
	>>> - Rebased 
	>>> 
	>>> Changes in v5: 
	>>> - Rebased 
	>>> --- 
	>>>  .../fff-network/files/lib/functions/fff/network    | 31 
	>> +++++++++++++++++++++ 
	>>>  .../fff-network/files/usr/sbin/configurenetwork    | 32 ++-------------------- 
	>>>  2 files changed, 33 insertions(+), 30 deletions(-) 
	>>> 
	>>> diff --git 
	>>> a/src/packages/fff/fff-network/files/lib/functions/fff/network 
	>>> b/src/packages/fff/fff-network/files/lib/functions/fff/network 
	>>> index af9b3434..0e9b466b 100644 
	>>> --- a/src/packages/fff/fff-network/files/lib/functions/fff/network 
	>>> +++ b/src/packages/fff/fff-network/files/lib/functions/fff/network 
	>>> @@ -96,3 +96,34 @@ macFlipLocalBit() { 
	>>>     echo "$mac" | awk -F: '{ printf("%02x:%s:%s:%s:%s:%s\n", 
	>> xor(("0x"$1),2), $2, $3, $4, $5, $6) }' 
	>>>     return 0 
	>>>  } 
	>>> + 
	>>> +fixMac() { 
	>>> +   # Update MAC address on device/interface 
	>>> +   # 
	>>> +   # fixMac <newmac> <physdev> <interface> <overwrite (optional)> 
	>>> +   # 
	>>> +   # newmac: MAC address to be set 
	>>> +   # physdev: Device name to be updated as in ifconfig (e.g. br-mesh) 
	>>> +   # interface: Interface to be updated as in uci (e.g. mesh) 
	>>> +   # overwrite: If non-zero, the MAC is always replaced; if not set, 
	>>> +the MAC is only written if none is present 
	>>> + 
	>>> +   local newmac=$1 
	>>> +   local dev=$2 
	>>> +   local iface=$3 
	>>> +   local overwrite=$4 
	>>> + 
	>>> +   if uci -q get "network.${iface}.macaddr" > /dev/null && [ ! -n 
	>>> +"$overwrite" ] ; then 
	>> das ist doppelt nein. [ $overwrite ] reicht oder mit -z 
	>>> +           echo "MAC for ${iface} is already set" 
	>>> +   else 
	>>> +           echo "Fixing MAC on ${dev} (${iface})" 
	>>> +           sleep 10 
	>>> + 
	>>> +           uci set "network.${iface}.macaddr=$newmac" 
	>>> +           uci -q commit network 
	>>> + 
	>>> +           ifconfig "$dev" down 
	>>> +           ifconfig "$dev" hw ether "$newmac" 
	>>> +           ifconfig "$dev" up 
	>> # ifconfig is deprecated. 
	>> 
	>> ip link set "$dev" down 
	>> ip l s "$dev" address "$newmac" 
	>> ip l s "$dev" up 
	>> 
	>> Und die Verwendung der {} ist etwas uneinheitlich. Man braucht das nur, 
	>> wenn hinten dran kein white character ist oder man Expansion machen will. 
	>> Oder man sagt, es schadet nie und macht die immer hin. Wie ist denn da der 
	>> allgemeine Tenor? 
	>> 
	>> Robert 
	>> 
	>>> +           /etc/init.d/network restart 
	>>> +   fi 
	>>> +} 
	>>> diff --git 
	>>> a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork 
	>>> b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork 
	>>> index 0e038a49..9dc29da1 100755 
	>>> --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork 
	>>> +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork 
	>>> @@ -176,39 +176,11 @@ fi 
	>>>  /etc/init.d/network restart 
	>>> 
	>>>  if [ -n "$ETHMESHMAC" ]; then 
	>>> -    if uci get network.ethmesh.macaddr 
	>>> -    then 
	>>> -        echo "MAC for ethmesh is set already" 
	>>> -    else 
	>>> -        echo "Fixing MAC on $SWITCHDEV.3 (ethmesh)" 
	>>> -        sleep 10 
	>>> - 
	>>> -        uci set network.ethmesh.macaddr=$ETHMESHMAC 
	>>> -        uci commit network 
	>>> - 
	>>> -        ifconfig $SWITCHDEV.3 down 
	>>> -        ifconfig $SWITCHDEV.3 hw ether $ETHMESHMAC 
	>>> -        ifconfig $SWITCHDEV.3 up 
	>>> -        /etc/init.d/network restart 
	>>> -    fi 
	>>> +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh" 
	>>>  fi 
	>>> 
	>>>  if [ -n "$ROUTERMAC" ]; then 
	>>> -    if uci get network.mesh.macaddr 
	>>> -    then 
	>>> -        echo "MAC for mesh is set already" 
	>>> -    else 
	>>> -        echo "Fixing MAC on br-mesh (mesh)" 
	>>> -        sleep 10 
	>>> - 
	>>> -        uci set network.mesh.macaddr=$ROUTERMAC 
	>>> -        uci commit network 
	>>> - 
	>>> -        ifconfig br-mesh down 
	>>> -        ifconfig br-mesh hw ether $ROUTERMAC 
	>>> -        ifconfig br-mesh up 
	>>> -        /etc/init.d/network restart 
	>>> -    fi 
	>>> +    fixMac "$ROUTERMAC" "br-mesh" "mesh" 
	>>>  fi 
	>>> 
	>>>  if [ -n "$ETH0MAC" ]; then
Adrian Schmutzler Jan. 27, 2019, 5:23 p.m.
Hallo,

kann ich zumindest den ersten Teil dieses Sets schon mal einkippen?

Hier sind die Änderungen ja noch recht einfach nachzuvollziehen und 1x reviewed ist er ja schon.

Grüße

Adrian

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of robert
> Sent: Dienstag, 6. November 2018 11:15
> To: franken-dev@freifunk.net
> Subject: Re: [PATCH v5 1/2] fff-network: Introduce function to set MAC on
> device
> 
> Hallo Adrian,
> 
> nur ein paar Kleinigkeiten.
> 
> Am 07.08.18 um 12:25 schrieb Adrian Schmutzler:
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> > ---
> >
> > Changes in v2:
> > - Suppress output
> >
> > Changes in v3:
> > - Rename iface and dev in explanation. The variables were not
> >   renamed, as shorter is better here.
> >
> > Changes in v4:
> > - Rebased
> >
> > Changes in v5:
> > - Rebased
> > ---
> >  .../fff-network/files/lib/functions/fff/network    | 31
> +++++++++++++++++++++
> >  .../fff-network/files/usr/sbin/configurenetwork    | 32 ++--------------------
> >  2 files changed, 33 insertions(+), 30 deletions(-)
> >
> > diff --git
> > a/src/packages/fff/fff-network/files/lib/functions/fff/network
> > b/src/packages/fff/fff-network/files/lib/functions/fff/network
> > index af9b3434..0e9b466b 100644
> > --- a/src/packages/fff/fff-network/files/lib/functions/fff/network
> > +++ b/src/packages/fff/fff-network/files/lib/functions/fff/network
> > @@ -96,3 +96,34 @@ macFlipLocalBit() {
> >  	echo "$mac" | awk -F: '{ printf("%02x:%s:%s:%s:%s:%s\n",
> xor(("0x"$1),2), $2, $3, $4, $5, $6) }'
> >  	return 0
> >  }
> > +
> > +fixMac() {
> > +	# Update MAC address on device/interface
> > +	#
> > +	# fixMac <newmac> <physdev> <interface> <overwrite (optional)>
> > +	#
> > +	# newmac: MAC address to be set
> > +	# physdev: Device name to be updated as in ifconfig (e.g. br-mesh)
> > +	# interface: Interface to be updated as in uci (e.g. mesh)
> > +	# overwrite: If non-zero, the MAC is always replaced; if not set,
> > +the MAC is only written if none is present
> > +
> > +	local newmac=$1
> > +	local dev=$2
> > +	local iface=$3
> > +	local overwrite=$4
> > +
> > +	if uci -q get "network.${iface}.macaddr" > /dev/null && [ ! -n
> > +"$overwrite" ] ; then
> das ist doppelt nein. [ $overwrite ] reicht oder mit -z
> > +		echo "MAC for ${iface} is already set"
> > +	else
> > +		echo "Fixing MAC on ${dev} (${iface})"
> > +		sleep 10
> > +
> > +		uci set "network.${iface}.macaddr=$newmac"
> > +		uci -q commit network
> > +
> > +		ifconfig "$dev" down
> > +		ifconfig "$dev" hw ether "$newmac"
> > +		ifconfig "$dev" up
> 
> # ifconfig is deprecated.
> 
> ip link set "$dev" down
> ip l s "$dev" address "$newmac"
> ip l s "$dev" up
> 
> Und die Verwendung der {} ist etwas uneinheitlich. Man braucht das nur,
> wenn hinten dran kein white character ist oder man Expansion machen will.
> Oder man sagt, es schadet nie und macht die immer hin. Wie ist denn da der
> allgemeine Tenor?
> 
> Robert
> 
> > +		/etc/init.d/network restart
> > +	fi
> > +}
> > diff --git
> > a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > index 0e038a49..9dc29da1 100755
> > --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > @@ -176,39 +176,11 @@ fi
> >  /etc/init.d/network restart
> >
> >  if [ -n "$ETHMESHMAC" ]; then
> > -    if uci get network.ethmesh.macaddr
> > -    then
> > -        echo "MAC for ethmesh is set already"
> > -    else
> > -        echo "Fixing MAC on $SWITCHDEV.3 (ethmesh)"
> > -        sleep 10
> > -
> > -        uci set network.ethmesh.macaddr=$ETHMESHMAC
> > -        uci commit network
> > -
> > -        ifconfig $SWITCHDEV.3 down
> > -        ifconfig $SWITCHDEV.3 hw ether $ETHMESHMAC
> > -        ifconfig $SWITCHDEV.3 up
> > -        /etc/init.d/network restart
> > -    fi
> > +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
> >  fi
> >
> >  if [ -n "$ROUTERMAC" ]; then
> > -    if uci get network.mesh.macaddr
> > -    then
> > -        echo "MAC for mesh is set already"
> > -    else
> > -        echo "Fixing MAC on br-mesh (mesh)"
> > -        sleep 10
> > -
> > -        uci set network.mesh.macaddr=$ROUTERMAC
> > -        uci commit network
> > -
> > -        ifconfig br-mesh down
> > -        ifconfig br-mesh hw ether $ROUTERMAC
> > -        ifconfig br-mesh up
> > -        /etc/init.d/network restart
> > -    fi
> > +    fixMac "$ROUTERMAC" "br-mesh" "mesh"
> >  fi
> >
> >  if [ -n "$ETH0MAC" ]; then