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

Submitted by Adrian Schmutzler on Jan. 2, 2018, 11:34 p.m.

Details

Message ID 1514936089-49073-1-git-send-email-freifunk@adrianschmutzler.de
State Superseded
Headers show

Commit Message

Adrian Schmutzler Jan. 2, 2018, 11:34 p.m. 
Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

---

Changes in v2:
- Suppress output
---
 .../fff-network/files/lib/functions/fff/network    | 31 +++++++++++++++++++
 .../fff-network/files/usr/sbin/configurenetwork    | 36 +++-------------------
 2 files changed, 35 insertions(+), 32 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 dc26938..a8d53b3 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> <dev> <iface> <overwrite (optional)>
+	#
+	# newmac: MAC address to be set
+	# dev: Device to be updated (e.g. br-mesh)
+	# iface: Interface to be updated (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 df50540..281f4d4 100755
--- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
+++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
@@ -159,40 +159,12 @@  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
+if [ -n "$ETHMESHMAC" ]; then
+    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
+if [ -n "$ROUTERMAC" ]; then
+    fixMac "$ROUTERMAC" "br-mesh" "mesh"
 fi
 
 if [[ -n "$ETH0MAC" ]]; then

Comments

Tim Niemeyer Jan. 20, 2018, 12:45 p.m. 
Moin Adrian

Von meinen bisherigen Review-Anmerkungen wurde offenbar nichts
umgesetzt, vielleicht vergessen/übersehen. Daher hier nochmal.

Am Mittwoch, den 03.01.2018, 00:34 +0100 schrieb Adrian Schmutzler:
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> 
> ---
> 
> Changes in v2:
> - Suppress output
> ---
>  .../fff-network/files/lib/functions/fff/network    | 31 +++++++++++++++++++
>  .../fff-network/files/usr/sbin/configurenetwork    | 36 +++-------------------
>  2 files changed, 35 insertions(+), 32 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 dc26938..a8d53b3 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> <dev> <iface> <overwrite (optional)>
<iface> würde ich in <interface> (entsprechend UCI) umbenennen
<dev> hier ganz raus

> +	#
> > +	# newmac: MAC address to be set
> > +	# dev: Device to be updated (e.g. br-mesh)
> > +	# iface: Interface to be updated (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
dev raus.

> +	local iface=$3
> +	local overwrite=$4

Hier dann:
network_get_physdev dev $interface

Und natürlich im Header noch ergänzen:
. /lib/functions/network.sh

> +
> > +	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 df50540..281f4d4 100755
> --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> @@ -159,40 +159,12 @@ 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
> +if [ -n "$ETHMESHMAC" ]; then
> +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
Da durch, dass wir hier das harte Interface dann nicht mehr brauchen,
könnten wir bei den ONE_PORT's auch einfach die ETHMESHMAC auf
"$(macFlipLocalBit "$ROUTERMAC")"
setzen.

Tim

>  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
> +if [ -n "$ROUTERMAC" ]; then
> +    fixMac "$ROUTERMAC" "br-mesh" "mesh"
>  fi
>  
>  if [[ -n "$ETH0MAC" ]]; then
> -- 
> 2.7.4
>
Adrian Schmutzler Jan. 20, 2018, 12:52 p.m. 
Hallo Tim,

ich habe die Änderung bewusst nicht eingebaut, weil es mir so lieber ist.

Meine Lösung stellt eine Verbesserung der alten Lösung dar, aber eine weniger weitreichende als dir vorschwebt. Ich möchte aber bewusst die Devices (zunächst) separat festlegen. Deine automatische Bestimmung stellt dann eine weitere Verallgemeinerung dar, die ich in einem eigenen Patch sehen (probieren, testen, ...) möchte. Dies ist auf Basis dieses Patches dann relativ einfach möglich.

Da wir also hier zunächst ein effektive Verbesserung haben, und das von dir beschriebene Problem etwas ist, was vorher schon da war, möchte ich diesen Patch gerne so lassen. Danach können wir gerne separat über die allgemeinere Lösung diskutieren. Entsprechend bitte ich, auf dieser Basis zu reviewen.

Ob ich iface oder interface schreibe ist mir hingegen egal, das kann ich gerne noch ändern.

Beste Grüße

Adrian


> -----Original Message-----
> From: Tim Niemeyer [mailto:tim@tn-x.org]
> Sent: Samstag, 20. Januar 2018 13:45
> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; franken-
> dev@freifunk.net
> Subject: Re: [PATCH v2 1/2] fff-network: Introduce function to set MAC on
> device
> 
> Moin Adrian
> 
> Von meinen bisherigen Review-Anmerkungen wurde offenbar nichts
> umgesetzt, vielleicht vergessen/übersehen. Daher hier nochmal.
> 
> Am Mittwoch, den 03.01.2018, 00:34 +0100 schrieb Adrian Schmutzler:
> > > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> > ---
> >
> > Changes in v2:
> > - Suppress output
> > ---
> >  .../fff-network/files/lib/functions/fff/network    | 31
> > +++++++++++++++++++
> >  .../fff-network/files/usr/sbin/configurenetwork    | 36
> > +++-------------------
> >  2 files changed, 35 insertions(+), 32 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 dc26938..a8d53b3 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> <dev> <iface> <overwrite (optional)>
> <iface> würde ich in <interface> (entsprechend UCI) umbenennen <dev>
> hier ganz raus
> 
> > +	#
> > > +	# newmac: MAC address to be set
> > > +	# dev: Device to be updated (e.g. br-mesh)
> > > +	# iface: Interface to be updated (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
> dev raus.
> 
> > +	local iface=$3
> > +	local overwrite=$4
> 
> Hier dann:
> network_get_physdev dev $interface
> 
> Und natürlich im Header noch ergänzen:
> . /lib/functions/network.sh
> 
> > +
> > > +	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 df50540..281f4d4 100755
> > --- a/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > +++ b/src/packages/fff/fff-network/files/usr/sbin/configurenetwork
> > @@ -159,40 +159,12 @@ 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
> > +if [ -n "$ETHMESHMAC" ]; then
> > +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
> Da durch, dass wir hier das harte Interface dann nicht mehr brauchen,
> könnten wir bei den ONE_PORT's auch einfach die ETHMESHMAC auf
> "$(macFlipLocalBit "$ROUTERMAC")"
> setzen.
> 
> Tim
> 
> >  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
> > +if [ -n "$ROUTERMAC" ]; then
> > +    fixMac "$ROUTERMAC" "br-mesh" "mesh"
> >  fi
> >
> >  if [[ -n "$ETH0MAC" ]]; then
> > --
> > 2.7.4
> >
Tim Niemeyer Jan. 20, 2018, 1:39 p.m. 
Am Samstag, den 20.01.2018, 13:52 +0100 schrieb
mail@adrianschmutzler.de:
> Hallo Tim,
> 
> ich habe die Änderung bewusst nicht eingebaut, weil es mir so lieber
> ist.
Es wäre gut gewesen, dass zu kommunizieren. Ich habe mir extra Zeit
genommen den Patch anzuschauen und halt eine Anmerkung gefunden. Beim
zweiten anschauen das selbe wieder gefunden. Das ist dann halt echt
ärgerlich.

> Meine Lösung stellt eine Verbesserung der alten Lösung dar, aber eine
> weniger weitreichende als dir vorschwebt. Ich möchte aber bewusst die
> Devices (zunächst) separat festlegen. Deine automatische Bestimmung
> stellt dann eine weitere Verallgemeinerung dar, die ich in einem
> eigenen Patch sehen (probieren, testen, ...) möchte. Dies ist auf
> Basis dieses Patches dann relativ einfach möglich.
Ich sehe das als kleinere Verbesserung an. Das Ziel der Funktion sollte
sich ja damit decken.

Wie dem auch sei, durch den Patch 2/2 wird das dann eh obsolete.

> Da wir also hier zunächst ein effektive Verbesserung haben, und das
> von dir beschriebene Problem etwas ist, was vorher schon da war,
> möchte ich diesen Patch gerne so lassen. Danach können wir gerne
> separat über die allgemeinere Lösung diskutieren. Entsprechend bitte
> ich, auf dieser Basis zu reviewen.
> 
> Ob ich iface oder interface schreibe ist mir hingegen egal, das kann
> ich gerne noch ändern.
Dann müsstest du aber dev glaube ich noch umbenennen. Ich glaube das
wird dann ifname werden müssen.

Tim

> Beste Grüße
> 
> Adrian
> 
> 
> > -----Original Message-----
> > From: Tim Niemeyer [mailto:tim@tn-x.org]
> > Sent: Samstag, 20. Januar 2018 13:45
> > To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; franken-
> > dev@freifunk.net
> > Subject: Re: [PATCH v2 1/2] fff-network: Introduce function to set
> > MAC on
> > device
> > 
> > Moin Adrian
> > 
> > Von meinen bisherigen Review-Anmerkungen wurde offenbar nichts
> > umgesetzt, vielleicht vergessen/übersehen. Daher hier nochmal.
> > 
> > Am Mittwoch, den 03.01.2018, 00:34 +0100 schrieb Adrian Schmutzler:
> > > > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> > > 
> > > ---
> > > 
> > > Changes in v2:
> > > - Suppress output
> > > ---
> > >  .../fff-network/files/lib/functions/fff/network    | 31
> > > +++++++++++++++++++
> > >  .../fff-network/files/usr/sbin/configurenetwork    | 36
> > > +++-------------------
> > >  2 files changed, 35 insertions(+), 32 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 dc26938..a8d53b3 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> <dev> <iface> <overwrite (optional)>
> > 
> > <iface> würde ich in <interface> (entsprechend UCI) umbenennen
> > <dev>
> > hier ganz raus
> > 
> > > +	#
> > > > +	# newmac: MAC address to be set
> > > > +	# dev: Device to be updated (e.g. br-mesh)
> > > > +	# iface: Interface to be updated (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
> > 
> > dev raus.
> > 
> > > +	local iface=$3
> > > +	local overwrite=$4
> > 
> > Hier dann:
> > network_get_physdev dev $interface
> > 
> > Und natürlich im Header noch ergänzen:
> > . /lib/functions/network.sh
> > 
> > > +
> > > > +	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 df50540..281f4d4 100755
> > > --- a/src/packages/fff/fff-
> > > network/files/usr/sbin/configurenetwork
> > > +++ b/src/packages/fff/fff-
> > > network/files/usr/sbin/configurenetwork
> > > @@ -159,40 +159,12 @@ 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
> > > +if [ -n "$ETHMESHMAC" ]; then
> > > +    fixMac "$ETHMESHMAC" "${SWITCHDEV}.3" "ethmesh"
> > 
> > Da durch, dass wir hier das harte Interface dann nicht mehr
> > brauchen,
> > könnten wir bei den ONE_PORT's auch einfach die ETHMESHMAC auf
> > "$(macFlipLocalBit "$ROUTERMAC")"
> > setzen.
> > 
> > Tim
> > 
> > >  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
> > > +if [ -n "$ROUTERMAC" ]; then
> > > +    fixMac "$ROUTERMAC" "br-mesh" "mesh"
> > >  fi
> > > 
> > >  if [[ -n "$ETH0MAC" ]]; then
> > > --
> > > 2.7.4
> > > 
> 
>