diff mbox

[v5,1/1] set simple network setup via the system configuration submenu

Message ID 1418114924-18826-1-git-send-email-jeremy.rosen@openwide.fr
State Rejected
Headers show

Commit Message

Jeremy Rosen Dec. 9, 2014, 8:48 a.m. UTC 
This patch allows the setup of simple /etc/network/interfaces (or
/etc/systemd/network/80-buildroot.network if networkd is enabled) via the
configuration menus instead of using an overlay

* supports disabling the interface
* supports manual ipv4 configuration
* supports dhcp configuration


Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
---

This patch is here to avoid having to do an overlay for the most common
cases (ipv4 with fixed IP or DHCP)

This is reduced to the bare minimum to cover the most common case. Anything
more complex can be done by using overlays instead.

I have not added the option to (de)activate persistant naming when networkd
is enabled because it logically belongs to the networkd package and not to
the simple network configuration
---
v5:
* remove use of "let" which is not supported by all shells
v4:
* correctly create /etc/systemd/networkd
* rework logic to simplify a bit
v3 :
* rename the script to a more generic name
* reorganized script to separate param verification from generation
* quoted params properly to protect against faulty params
* removed /etc/network/interfaces from skeleton
* reorganized options in configuration menu
* added networkd configuration support

v2 :
* moved to TARGET_FINALIZE
* removed support for lo. It should always be on.
* reworked default parameters
---
 support/scripts/generate-network-config.sh | 132 +++++++++++++++++++++++++++++
 system/Config.in                           |  60 +++++++++++++
 system/skeleton/etc/network/interfaces     |   4 -
 system/system.mk                           |   5 ++
 4 files changed, 197 insertions(+), 4 deletions(-)
 create mode 100755 support/scripts/generate-network-config.sh
 delete mode 100644 system/skeleton/etc/network/interfaces

Comments

Thomas Petazzoni Dec. 9, 2014, 9:02 a.m. UTC | #1 
Dear Jérémy Rosen,

On Tue,  9 Dec 2014 09:48:44 +0100, Jérémy Rosen wrote:

> +check_configuration ()
> +{
> +	if [ -z "$BR2_SIMPLE_NETWORK_NONE" ] ; then

Can we try to avoid unnecessary indentation, and so things the other
way around, i.e if BR2_SIMPLE_NETWORK_NONE is not empty, bail out from
the function?

	if [ -n "$BR2_SIMPLE_NETWORK_NONE" ] ; then
		return
	fi

> +		if [ -z "$BR2_SIMPLE_NETWORK_NAME" ] ; then
> +			echo ERROR no name specified for first network interface
> +			exit 1
> +		fi
> +		if [ "$BR2_SIMPLE_NETWORK_IPV4_MANUAL" ] ; then

No condition?

> +	if [ -z "$BR2_SIMPLE_NETWORK_NONE" ] ; then

Same comment here.

> +		echo "auto $BR2_SIMPLE_NETWORK_NAME"
> +		if [ "$BR2_SIMPLE_NETWORK_IPV4_DHCP" ] ; then

And here.

That being said, generally, I find this quite complicated, and my
preference would be to continue with what we have today, and simply let
the user override things with a root filesystem overlay or a post-build
script.

Best regards,

Thomas
Jeremy Rosen Dec. 9, 2014, 9:23 a.m. UTC | #2 
----- Mail original -----
> Dear Jérémy Rosen,
> 
> On Tue,  9 Dec 2014 09:48:44 +0100, Jérémy Rosen wrote:
> 
> > +check_configuration ()
> > +{
> > +	if [ -z "$BR2_SIMPLE_NETWORK_NONE" ] ; then
> 
> Can we try to avoid unnecessary indentation, and so things the other
> way around, i.e if BR2_SIMPLE_NETWORK_NONE is not empty, bail out
> from
> the function?

can do...

> 
> 	if [ -n "$BR2_SIMPLE_NETWORK_NONE" ] ; then
> 		return
> 	fi
> 
> > +		if [ -z "$BR2_SIMPLE_NETWORK_NAME" ] ; then
> > +			echo ERROR no name specified for first network interface
> > +			exit 1
> > +		fi
> > +		if [ "$BR2_SIMPLE_NETWORK_IPV4_MANUAL" ] ; then
> 
> No condition?
> 


That was discussed in a previous iteration and was considered ok at
the time, but I can add an explicit condition


> > +	if [ -z "$BR2_SIMPLE_NETWORK_NONE" ] ; then
> 
> Same comment here.
> 

ok

> > +		echo "auto $BR2_SIMPLE_NETWORK_NAME"
> > +		if [ "$BR2_SIMPLE_NETWORK_IPV4_DHCP" ] ; then
> 
> And here.
> 

ok

> That being said, generally, I find this quite complicated, and my
> preference would be to continue with what we have today, and simply
> let
> the user override things with a root filesystem overlay or a
> post-build
> script.
> 

Well, the point is to avoid having an overlay just to enable DHCP, which
I thought was the direction BR wanted to go... simple stuff can be 
configured directly from menuconfig...


I can simplify the thing by dropping neworkd support if that makes you 
feel better, that would avoid the whole mess of parsing both mask formatq

but previous reviews had me add it to have it supported.

is that a formal rejection, or should I send a v6 ?


Best regards

Jérémy


> Best regards,
> 
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>
Thomas Petazzoni Dec. 9, 2014, 11:04 p.m. UTC | #3 
Dear Jérémy Rosen,

On Tue,  9 Dec 2014 09:48:44 +0100, Jérémy Rosen wrote:
> This patch allows the setup of simple /etc/network/interfaces (or
> /etc/systemd/network/80-buildroot.network if networkd is enabled) via the
> configuration menus instead of using an overlay

So, we discussed this tonight during the patchwork review with Yann,
Peter and Samuel, and the general opinion is that this is too
complicated, and we don't want to handle all those cases through
configuration options, and we instead want to leave that to
project-specific customization through a rootfs overlay or a post-build
script.

However, we would be willing to accept a patch that adds a single
string option that allows to specify one network interface on which
DHCP should be used. It would basically add "inet <foo> dhcp"
to /etc/network/interfaces if non-empty, and otherwise do nothing. The
default should be nothing, so that we keep the existing behavior.

Best regards,

Thomas
diff mbox

Patch

diff --git a/support/scripts/generate-network-config.sh b/support/scripts/generate-network-config.sh
new file mode 100755
index 0000000..31156c4
--- /dev/null
+++ b/support/scripts/generate-network-config.sh
@@ -0,0 +1,132 @@ 
+#!/bin/sh
+
+#extract our parameters from the config file
+# see comment in support/scripts/mkusers at to why we can't simply source
+for PARAM in \
+	BR2_PACKAGE_SYSTEMD_NETWORKD \
+	BR2_SIMPLE_NETWORK_NONE \
+	BR2_SIMPLE_NETWORK_IPV4_DHCP \
+	BR2_SIMPLE_NETWORK_IPV4_MANUAL \
+	; do
+TMP=$(sed -r -e "/^$PARAM=(.*)$/!d;" -e 's//\1/;' $BR2_CONFIG)
+export $PARAM=$TMP
+done
+
+for PARAM in \
+	BR2_SIMPLE_NETWORK_NAME \
+	BR2_SIMPLE_NETWORK_IPV4_ADDRESS \
+	BR2_SIMPLE_NETWORK_IPV4_NETMASK \
+	BR2_SIMPLE_NETWORK_IPV4_BROADCAST \
+	BR2_SIMPLE_NETWORK_IPV4_GATEWAY \
+	; do
+TMP=$(sed -r -e "/^$PARAM=\"(.*)\"$/!d;" -e 's//\1/;' $BR2_CONFIG)
+export $PARAM="$TMP"
+done
+
+check_configuration ()
+{
+	if [ -z "$BR2_SIMPLE_NETWORK_NONE" ] ; then
+		if [ -z "$BR2_SIMPLE_NETWORK_NAME" ] ; then
+			echo ERROR no name specified for first network interface
+			exit 1
+		fi
+		if [ "$BR2_SIMPLE_NETWORK_IPV4_MANUAL" ] ; then
+			if [ -z "$BR2_SIMPLE_NETWORK_IPV4_ADDRESS" ] ; then
+				echo ERROR BR2_SIMPLE_NETWORK_IPV4_ADDRESS not set 1>&2
+				exit 1
+			fi
+			if [ -z "$BR2_SIMPLE_NETWORK_IPV4_NETMASK" ] ; then
+				echo ERROR BR2_SIMPLE_NETWORK_IPV4_NETMASK not set 1>&2
+				exit 1
+			fi
+			OLDIFS="$IFS"; IFS=.
+			for dec in $BR2_SIMPLE_NETWORK_IPV4_NETMASK ; do
+				case $dec in
+					255 | 254 | 252 |248 | 240 |224 | 192 | 128 | 0 );;
+					*) echo "Error: $BR2_SIMPLE_NETWORK_IPV4_NETMASK is not a correct NETMASK" 1>&2; exit 1 ;;
+				esac
+			done
+			IFS="$OLDIFS"
+		fi
+	fi
+}
+
+do_generate_interfaces ()
+{
+	echo "# interface file auto-generated by buildroot"
+	echo
+	echo "auto lo"
+	echo "iface lo inet loopback"
+	echo
+
+	if [ -z "$BR2_SIMPLE_NETWORK_NONE" ] ; then
+		echo "auto $BR2_SIMPLE_NETWORK_NAME"
+		if [ "$BR2_SIMPLE_NETWORK_IPV4_DHCP" ] ; then
+			echo "iface $BR2_SIMPLE_NETWORK_NAME inet dhcp"
+		elif [ "$BR2_SIMPLE_NETWORK_IPV4_MANUAL" ] ; then
+			echo "iface $BR2_SIMPLE_NETWORK_NAME inet static"
+			echo "	address $BR2_SIMPLE_NETWORK_IPV4_ADDRESS"
+			echo "	netmask $BR2_SIMPLE_NETWORK_IPV4_NETMASK"
+
+			if [ "$BR2_SIMPLE_NETWORK_IPV4_BROADCAST" ] ; then
+				echo "	broadcast $BR2_SIMPLE_NETWORK_IPV4_BROADCAST"
+			fi
+			if [ "$BR2_SIMPLE_NETWORK_IPV4_GATEWAY" ] ; then
+				echo "	gateway $BR2_SIMPLE_NETWORK_IPV4_GATEWAY"
+			fi
+		fi
+	fi
+}
+
+do_generate_network_service ()
+{
+	if [ "$BR2_SIMPLE_NETWORK_NONE" ] ; then
+		return
+	fi
+	echo "[Match]"
+	echo "Name=$BR2_SIMPLE_NETWORK_NAME"
+	echo
+	echo "[Network]"
+	echo "Description=buildroot-configured interface"
+	if [ $BR2_SIMPLE_NETWORK_IPV4_DHCP ] ; then
+		echo "DHCP=v4"
+	elif [ $BR2_SIMPLE_NETWORK_IPV4_MANUAL ] && \
+		[ "$BR2_SIMPLE_NETWORK_IPV4_ADDRESS" != "0.0.0.0" ] ; then
+		nbits=0
+		OLDIFS="$IFS"; IFS=.
+		for dec in $BR2_SIMPLE_NETWORK_IPV4_NETMASK ; do
+			case $dec in
+				255) nbits = $((nbits+=8));;
+				254) nbits = $((nbits+=7));;
+				252) nbits = $((nbits+=6));;
+				248) nbits = $((nbits+=5));;
+				240) nbits = $((nbits+=4));;
+				224) nbits = $((nbits+=3));;
+				192) nbits = $((nbits+=2));;
+				128) nbits = $((nbits+=1));;
+			esac
+		done
+		IFS="$OLDIFS"
+
+		if [ "$BR2_SIMPLE_NETWORK_IPV4_GATEWAY" ] ; then
+			echo "Gateway=$BR2_SIMPLE_NETWORK_IPV4_GATEWAY"
+			echo
+		fi
+		echo "[Address]"
+		echo "Address=$BR2_SIMPLE_NETWORK_IPV4_ADDRESS/$nbits"
+		if [ "$BR2_SIMPLE_NETWORK_IPV4_BROADCAST" ] ; then
+			echo "Broadcast=$BR2_SIMPLE_NETWORK_IPV4_BROADCAST"
+		fi
+	fi
+}
+
+check_configuration
+mkdir -p $TARGET_DIR/etc/network/
+rm -f -- $TARGET_DIR/etc/systemd/network/80-buildroot.network
+if [ "$BR2_PACKAGE_SYSTEMD_NETWORKD" ] ; then
+	echo > $TARGET_DIR/etc/network/interfaces
+	mkdir -p $TARGET_DIR/etc/systemd/network/
+	do_generate_network_service > $TARGET_DIR/etc/systemd/network/80-buildroot.network
+else
+	do_generate_interfaces > $TARGET_DIR/etc/network/interfaces
+fi
diff --git a/system/Config.in b/system/Config.in
index 39f27c7..46f2014 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -324,6 +324,66 @@  config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
 
 endif # BR2_ROOTFS_SKELETON_DEFAULT
 
+choice
+	prompt 'Simple network configuration'
+	default BR2_SIMPLE_NETWORK_NONE
+	help
+	  buildroot will create a simple network configuration for a network
+	  interface. This will generate the corresponding lines in
+	  /etc/network/interfaces which is used by ifupdown.
+
+	  The simple network configuration only supports a single network
+	  interface using a static or DHCP-allocated IPv4 address. If you
+	  need something more complicate, create your own configuration file
+	  in the BR2_ROOTFS_OVERLAY.
+
+config BR2_SIMPLE_NETWORK_NONE
+	bool "No configured network interface"
+	help
+	  Do not configure the network interface, only configure the
+	  loopback device
+
+config BR2_SIMPLE_NETWORK_IPV4_DHCP
+	bool "IPv4 with DHCP"
+	help
+	  Use DHCP to configure this interface using the IPv4 protocol
+
+config BR2_SIMPLE_NETWORK_IPV4_MANUAL
+	bool "IPv4 with static IP address"
+	help
+	  Configure IPv4 by specifying each parameter separately
+
+endchoice
+
+if !BR2_SIMPLE_NETWORK_NONE
+
+config BR2_SIMPLE_NETWORK_NAME
+	string "name of the physical network interface"
+	default "eth0"
+	help
+	  The name used to recognise the network interface as reported
+	  by the kernel
+
+if BR2_SIMPLE_NETWORK_IPV4_MANUAL
+config BR2_SIMPLE_NETWORK_IPV4_ADDRESS
+	string "IP Address of the network interface"
+	default "0.0.0.0"
+
+config BR2_SIMPLE_NETWORK_IPV4_NETMASK
+	string "Netmask of the network interface"
+	default "255.255.255.255"
+
+config BR2_SIMPLE_NETWORK_IPV4_BROADCAST
+	string "Broadcast Address of the network interface"
+
+config BR2_SIMPLE_NETWORK_IPV4_GATEWAY
+	string "Address of the gateway for the network interface"
+
+endif # BR2_SIMPLE_NETWORK_IPV4_MANUAL
+
+endif # !BR2_SIMPLE_NETWORK_NONE
+
+
 config BR2_TARGET_TZ_INFO
 	bool "Install timezone info"
 	# No timezone for musl; only for uClibc or (e)glibc.
diff --git a/system/skeleton/etc/network/interfaces b/system/skeleton/etc/network/interfaces
deleted file mode 100644
index 218b82c..0000000
--- a/system/skeleton/etc/network/interfaces
+++ /dev/null
@@ -1,4 +0,0 @@ 
-# Configure Loopback
-auto lo
-iface lo inet loopback
-
diff --git a/system/system.mk b/system/system.mk
index e4a3160..bb933f6 100644
--- a/system/system.mk
+++ b/system/system.mk
@@ -38,6 +38,11 @@  ifneq ($(TARGET_GENERIC_ROOT_PASSWD),)
 TARGETS += host-mkpasswd
 endif
 
+define SIMPLE_NETWORK
+	$(TOPDIR)/support/scripts/generate-network-config.sh $(TARGET_DIR)
+endef
+TARGET_FINALIZE_HOOKS += SIMPLE_NETWORK
+
 ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
 
 define SYSTEM_ROOT_PASSWD