[Buildroot] [RFC: PATCH 0/3] DHCP configuration via networkd

[Buildroot] [RFC: PATCH 0/3] DHCP configuration via networkd

[Eric Le Bihan]

This small series removes the current limitation of not allowing the user to
have a network interface configured automatically via DHCP when
systemd-networkd is used.

To do so, the systemd package is updated to generate a .network file for the
configured interface.

Also add an option to enable or disable predictable network interface names
when udev is used. It is disabled by default, so the end user gets the same
interface names across busybox/systemd configurations.

Eric Le Bihan (3):
  systemd: configure DHCP via networkd
  system: allow auto net configuration with networkd
  system: enable predictable net interface names

 package/skeleton/skeleton.mk | 11 +++++++++++
 package/systemd/systemd.mk   | 13 +++++++++++++
 system/Config.in             | 20 ++++++++++++++------
 3 files changed, 38 insertions(+), 6 deletions(-)

-- 
2.4.11

[Buildroot] [RFC: PATCH 1/3] systemd: configure DHCP via networkd

[Eric Le Bihan]

If systemd-networkd is enabled and $BR2_SYSTEM_DHCP is set, then create
a .network file to configure the selected network interface via DHCP.

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/systemd/systemd.mk | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
index fb07819..5062750 100644
--- a/package/systemd/systemd.mk
+++ b/package/systemd/systemd.mk
@@ -275,6 +275,18 @@ define SYSTEMD_INSTALL_RESOLVCONF_HOOK
 	ln -sf ../run/systemd/resolve/resolv.conf \
 		$(TARGET_DIR)/etc/resolv.conf
 endef
+SYSTEMD_NETWORK_DHCP_IFACE = $(call qstrip,$(BR2_SYSTEM_DHCP))
+ifneq ($(SYSTEMD_NETWORK_DHCP_IFACE),)
+define SYSTEMD_INSTALL_NETWORK_CONFS
+	( \
+		echo "[Match]"; \
+		echo "Name=$(SYSTEMD_NETWORK_DHCP_IFACE)"; \
+		echo; \
+		echo "[Network]"; \
+		echo "DHCP=ipv4"; \
+	) > $(TARGET_DIR)/etc/systemd/network/dhcp.network
+endef
+endif
 else
 SYSTEMD_CONF_OPTS += --disable-networkd
 define SYSTEMD_INSTALL_SERVICE_NETWORK
@@ -377,6 +389,7 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD
 	$(SYSTEMD_INSTALL_SERVICE_TTY)
 	$(SYSTEMD_INSTALL_SERVICE_NETWORK)
 	$(SYSTEMD_INSTALL_SERVICE_TIMESYNC)
+	$(SYSTEMD_INSTALL_NETWORK_CONFS)
 endef
 
 $(eval $(autotools-package))
-- 
2.4.11

[Buildroot] [RFC: PATCH 1/3] systemd: configure DHCP via networkd

[Thomas Petazzoni]

Hello,

(Adding in Cc more people interested in systemd)

On Sat, 17 Sep 2016 13:58:37 +0200, Eric Le Bihan wrote:
> If systemd-networkd is enabled and $BR2_SYSTEM_DHCP is set, then create
> a .network file to configure the selected network interface via DHCP.
> 
> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>

The idea looks good to me, so unless the people interested in systemd
raise their voice, I'm going to apply something like this. However, I
have one suggestion below.

> ---
>  package/systemd/systemd.mk | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
> index fb07819..5062750 100644
> --- a/package/systemd/systemd.mk
> +++ b/package/systemd/systemd.mk
> @@ -275,6 +275,18 @@ define SYSTEMD_INSTALL_RESOLVCONF_HOOK
>  	ln -sf ../run/systemd/resolve/resolv.conf \
>  		$(TARGET_DIR)/etc/resolv.conf
>  endef
> +SYSTEMD_NETWORK_DHCP_IFACE = $(call qstrip,$(BR2_SYSTEM_DHCP))
> +ifneq ($(SYSTEMD_NETWORK_DHCP_IFACE),)
> +define SYSTEMD_INSTALL_NETWORK_CONFS
> +	( \
> +		echo "[Match]"; \
> +		echo "Name=$(SYSTEMD_NETWORK_DHCP_IFACE)"; \
> +		echo; \
> +		echo "[Network]"; \
> +		echo "DHCP=ipv4"; \
> +	) > $(TARGET_DIR)/etc/systemd/network/dhcp.network
> +endef

I find having all those echo lines in the middle of the .mk file not
very pretty. What about instead:

package/systemd/dhcp.network containing:

[Match]
Name=SYSTEMD_NETWORK_DHCP_IFACE
[Network]
DHCP=ipv4

package/systemd/systemd.mk doing:

define SYSTEMD_INSTALL_NETWORK_CONF
	sed s/SYSTEMD_NETWORK_DHCP_IFACE/$(SYSTEMD_NETWORK_DHCP_IFACE)/ \
		package/systemd/dhcp.network > \
		$(TARGET_DIR)/etc/systemd/network/dhcp.network
endef

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [RFC: PATCH 1/3] systemd: configure DHCP via networkd

[Thomas Petazzoni]

Hello,

On Sat, 17 Sep 2016 13:58:37 +0200, Eric Le Bihan wrote:
> If systemd-networkd is enabled and $BR2_SYSTEM_DHCP is set, then create
> a .network file to configure the selected network interface via DHCP.
> 
> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
> ---
>  package/systemd/systemd.mk | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Applied to master, with the change I suggested a while ago (i.e
generate the file from a template).

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [RFC: PATCH 2/3] system: allow auto net configuration with networkd

[Eric Le Bihan]

Allow automatic network configuration via systemd-networkd if selected.

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 system/Config.in | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/system/Config.in b/system/Config.in
index 77c665b..4f27d4f 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -365,7 +365,7 @@ config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
 config BR2_SYSTEM_DHCP
 	string "Network interface to configure through DHCP"
 	default ""
-	depends on !BR2_PACKAGE_SYSTEMD_NETWORKD && (BR2_PACKAGE_BUSYBOX || BR2_PACKAGE_IFUPDOWN)
+	depends on (BR2_PACKAGE_BUSYBOX || BR2_PACKAGE_IFUPDOWN || BR2_PACKAGE_SYSTEMD_NETWORKD)
 	help
 	  Enter here the name of the network interface (E.G. eth0) to
 	  automatically configure through DHCP at bootup.
@@ -375,11 +375,8 @@ config BR2_SYSTEM_DHCP
 	  For more complicated network setups use an overlay to overwrite
 	  /etc/network/interfaces or add a networkd configuration file.
 
-comment "automatic network configuration via DHCP is not compatible with networkd"
-	depends on BR2_PACKAGE_SYSTEMD_NETWORKD
-
-comment "automatic network configuration via DHCP needs ifupdown or busybox"
-	depends on !(BR2_PACKAGE_BUSYBOX || BR2_PACKAGE_IFUPDOWN)
+comment "automatic network configuration via DHCP needs ifupdown or busybox or networkd"
+	depends on !(BR2_PACKAGE_BUSYBOX || BR2_PACKAGE_IFUPDOWN || BR2_PACKAGE_SYSTEMD_NETWORKD)
 
 endif # BR2_ROOTFS_SKELETON_DEFAULT
 
-- 
2.4.11

[Buildroot] [RFC: PATCH 2/3] system: allow auto net configuration with networkd

[Thomas Petazzoni]

Hello,

On Sat, 17 Sep 2016 13:58:38 +0200, Eric Le Bihan wrote:
> Allow automatic network configuration via systemd-networkd if selected.
> 
> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>

Looks good to me, with one possible suggestion.

> -comment "automatic network configuration via DHCP needs ifupdown or busybox"
> -	depends on !(BR2_PACKAGE_BUSYBOX || BR2_PACKAGE_IFUPDOWN)
> +comment "automatic network configuration via DHCP needs ifupdown or busybox or networkd"
> +	depends on !(BR2_PACKAGE_BUSYBOX || BR2_PACKAGE_IFUPDOWN || BR2_PACKAGE_SYSTEMD_NETWORKD)

I think I'd remove this comment entirely, I'm not sure it's very useful.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [RFC: PATCH 2/3] system: allow auto net configuration with networkd

[Thomas Petazzoni]

Hello,

On Sat, 17 Sep 2016 13:58:38 +0200, Eric Le Bihan wrote:
> Allow automatic network configuration via systemd-networkd if selected.
> 
> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
> ---
>  system/Config.in | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

I've applied, but in fact squashed into the previous patch as it really
is the same change.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [RFC: PATCH 3/3] system: enable predictable net interface names

[Eric Le Bihan]

Since v197, udev and eudev automatically assign predictable, stable
network interface names for all local Ethernet, WLAN and WWAN
interfaces.

This is a departure from the traditional interface naming scheme
(eth0, eth1, etc).

Add a configuration option to enable or disable this feature, which is
disabled by default to be coherent with the behavior set when udev is
not used.

See https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
for details.

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/skeleton/skeleton.mk | 11 +++++++++++
 system/Config.in             | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
index 1000161..74b0636 100644
--- a/package/skeleton/skeleton.mk
+++ b/package/skeleton/skeleton.mk
@@ -242,6 +242,17 @@ endif
 TARGET_FINALIZE_HOOKS += SKELETON_REMOUNT_RW
 endif # BR2_INIT_BUSYBOX || BR2_INIT_SYSV
 
+ifeq ($(BR2_SYSTEM_PREDICTABLE_NETIF_NAMES),y)
+define SKELETON_MANAGE_PREDITABLE_NETIF_NAMES
+	rm -f $(TARGET_DIR)/etc/udev/rules.d/80-net-setup-links.rules
+endef
+else
+define SKELETON_MANAGE_PREDITABLE_NETIF_NAMES
+	touch $(TARGET_DIR)/etc/udev/rules.d/80-net-setup-links.rules
+endef
+endif
+TARGET_FINALIZE_HOOKS += SKELETON_MANAGE_PREDITABLE_NETIF_NAMES
+
 endif # BR2_ROOTFS_SKELETON_DEFAULT
 
 $(eval $(generic-package))
diff --git a/system/Config.in b/system/Config.in
index 4f27d4f..59af77e 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -172,6 +172,17 @@ endchoice
 comment "/dev management using udev (from systemd)"
 	depends on BR2_INIT_SYSTEMD
 
+config BR2_SYSTEM_PREDICTABLE_NETIF_NAMES
+	bool "Predictable network interface names"
+	depends on BR2_PACKAGE_HAS_UDEV
+	default n
+	help
+	  udev can assign predictable, stable network interface names for all
+	  local Ethernet, WLAN and WWAN interfaces. This is a departure from
+	  the traditional interface naming scheme (eth0, eth1, etc).
+
+	  Say yes to active this naming scheme.
+
 config BR2_ROOTFS_DEVICE_TABLE
 	string "Path to the permission tables"
 	default "system/device_table.txt"
-- 
2.4.11

[Buildroot] [RFC: PATCH 3/3] system: enable predictable net interface names

[Thomas Petazzoni]

Hello,

On Sat, 17 Sep 2016 13:58:39 +0200, Eric Le Bihan wrote:
> Since v197, udev and eudev automatically assign predictable, stable
> network interface names for all local Ethernet, WLAN and WWAN
> interfaces.
> 
> This is a departure from the traditional interface naming scheme
> (eth0, eth1, etc).
> 
> Add a configuration option to enable or disable this feature, which is
> disabled by default to be coherent with the behavior set when udev is
> not used.
> 
> See https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
> for details.
> 
> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>

While I am happy with the principles of patches 1/3 and 2/3, I'm a lot
less convinced by this one. Indeed, the "System configuration" menu
already has a good number of options, and I'm not a fan of adding yet
another option for just a relatively minor system configuration detail.
I'd prefer to leave this to per-project customization through
post-build scripts, and just use the upstream default (i.e predictable
interface names).

I'll keep this patch in patchwork for some time in order to allow
others to raise their voice. I've also Cc'ed a bunch of people so they
can give their opinion.

> +ifeq ($(BR2_SYSTEM_PREDICTABLE_NETIF_NAMES),y)
> +define SKELETON_MANAGE_PREDITABLE_NETIF_NAMES
> +	rm -f $(TARGET_DIR)/etc/udev/rules.d/80-net-setup-links.rules
> +endef
> +else
> +define SKELETON_MANAGE_PREDITABLE_NETIF_NAMES
> +	touch $(TARGET_DIR)/etc/udev/rules.d/80-net-setup-links.rules

Why are you touching the file here?

> +config BR2_SYSTEM_PREDICTABLE_NETIF_NAMES
> +	bool "Predictable network interface names"
> +	depends on BR2_PACKAGE_HAS_UDEV
> +	default n

"default n" is never needed: the default for an option is to be
disabled.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [RFC: PATCH 3/3] system: enable predictable net interface names

[Peter Korsgaard]

On Sat, Sep 17, 2016 at 1:58 PM, Eric Le Bihan
<eric.le.bihan.dev@free.fr> wrote:

Hi and sorry for the slow response.

> Since v197, udev and eudev automatically assign predictable, stable
> network interface names for all local Ethernet, WLAN and WWAN
> interfaces.

Are you sure about eudev? I don't seem to find any references to that
rules file (or anything related) in eudev-3.2.1.

> This is a departure from the traditional interface naming scheme
> (eth0, eth1, etc).
>
> Add a configuration option to enable or disable this feature, which is
> disabled by default to be coherent with the behavior set when udev is
> not used.
>
> See https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
> for details.

This document talks about adding a symlink to /dev/null from
/etc/systemd/network/99-default.link, but that is not what you are
doing. Why?

> diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
> index 1000161..74b0636 100644
> --- a/package/skeleton/skeleton.mk
> +++ b/package/skeleton/skeleton.mk
> @@ -242,6 +242,17 @@ endif
>  TARGET_FINALIZE_HOOKS += SKELETON_REMOUNT_RW
>  endif # BR2_INIT_BUSYBOX || BR2_INIT_SYSV
>
> +ifeq ($(BR2_SYSTEM_PREDICTABLE_NETIF_NAMES),y)
> +define SKELETON_MANAGE_PREDITABLE_NETIF_NAMES
> +       rm -f $(TARGET_DIR)/etc/udev/rules.d/80-net-setup-links.rules
> +endef
> +else
> +define SKELETON_MANAGE_PREDITABLE_NETIF_NAMES
> +       touch $(TARGET_DIR)/etc/udev/rules.d/80-net-setup-links.rules
> +endef
> +endif

I take it that the idea is for this file to "shadow" a file of the
same name in /lib/udev/rules? Looking in systemd-232 there is only a
80-net-setup-link.rules file (link not links)?

If it all boils down to it being a systemd specific option that can be
handled by a single symlink in an overlay/post-build script, then I
don't think it is worth to have it.

-- 
Bye, Peter Korsgaard

[Buildroot] [RFC: PATCH 3/3] system: enable predictable net interface names

[Thomas Petazzoni]

Hello,

On Mon, 6 Feb 2017 21:07:00 +0100, Peter Korsgaard wrote:

> If it all boils down to it being a systemd specific option that can be
> handled by a single symlink in an overlay/post-build script, then I
> don't think it is worth to have it.

Considering the lack of feedback from Eric on the topic, I've marked
his patch as Rejected in patchwork.

Please do not hesitate to resubmit with a better explanation that
answers the questions asked during the review. Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com