diff mbox

[v2] skeleton: /etc/network/interface.d/* support

Message ID 1471361908-51757-1-git-send-email-matthew.weber@rockwellcollins.com
State Rejected
Headers show

Commit Message

Matt Weber Aug. 16, 2016, 3:38 p.m. UTC
Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
---

v1 -> v2
 - interfaces.d is only supported with full ifupdown package
---
 package/skeleton/skeleton.mk                    | 10 ++++++++++
 system/device_table.txt                         |  1 +
 system/skeleton/etc/network/interfaces.d/.empty |  0
 3 files changed, 11 insertions(+)
 create mode 100644 system/skeleton/etc/network/interfaces.d/.empty

diff --git a/system/skeleton/etc/network/interfaces.d/.empty b/system/skeleton/etc/network/interfaces.d/.empty
new file mode 100644
index 0000000..e69de29

Comments

Thomas Petazzoni Oct. 22, 2016, 9:28 p.m. UTC | #1
Hello,

On Tue, 16 Aug 2016 10:38:28 -0500, Matt Weber wrote:

> diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
> index 1000161..39e2cbb 100644
> --- a/package/skeleton/skeleton.mk
> +++ b/package/skeleton/skeleton.mk
> @@ -170,9 +170,19 @@ define SKELETON_SET_NETWORK_DHCP
>  endef
>  endif
>  
> +ifeq ($(BR2_PACKAGE_IFUPDOWN),y)
> +define SKELETON_SET_NETWORK_SUPPORT_INTERFACES_D
> +	( \
> +		echo ;                                               \
> +		echo "source interfaces.d/*";                        \
> +	) >> $(TARGET_DIR)/etc/network/interfaces
> +endef
> +endif
> +
>  define SKELETON_SET_NETWORK
>  	mkdir -p $(TARGET_DIR)/etc/network/
>  	$(SKELETON_SET_NETWORK_LOCALHOST)
> +	$(SKELETON_SET_NETWORK_SUPPORT_INTERFACES_D)
>  	$(SKELETON_SET_NETWORK_DHCP)
>  endef
>  
> diff --git a/system/device_table.txt b/system/device_table.txt
> index dc1af51..69b1d46 100644
> --- a/system/device_table.txt
> +++ b/system/device_table.txt
> @@ -17,5 +17,6 @@
>  /etc/network/if-pre-up.d		d	755	0	0	-	-	-	-	-
>  /etc/network/if-down.d			d	755	0	0	-	-	-	-	-
>  /etc/network/if-post-down.d		d	755	0	0	-	-	-	-	-
> +/etc/network/interfaces.d		d	755	0	0	-	-	-	-	-
>  # uncomment this to allow starting x as non-root
>  #/usr/X11R6/bin/Xfbdev		     	f	4755	0	0	-	-	-	-	-
> diff --git a/system/skeleton/etc/network/interfaces.d/.empty b/system/skeleton/etc/network/interfaces.d/.empty
> new file mode 100644
> index 0000000..e69de29

Sorry for the late answer. I believe none of this should go in the main
skeleton, as it is really specific to ifupdown. What about instead
adding the following in ifupdown.mk:

ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
define IFUPDOWN_TUNE_ETC_NETWORK_INTERFACES
	mkdir -p $(TARGET_DIR)/etc/network/interfaces.d
	echo "source interfaces.d/*" >> $(TARGET_DIR)/etc/network/interfaces
endef
IFUPDOWN_TARGET_FINALIZE_HOOKS += IFUPDOWN_TUNE_ETC_NETWORK_INTERFACES
endif

define IFUPDOWN_PERMISSIONS
/etc/network/interfaces.d		d	755	0	0	-	-	-	-	-
endef

This should do exactly the same thing, but is isolated in the ifupdown
package.

Could you look into this?

Thanks!

Thomas
Peter Korsgaard Oct. 23, 2016, 12:06 p.m. UTC | #2
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 > Sorry for the late answer. I believe none of this should go in the main
 > skeleton, as it is really specific to ifupdown.

Indeed. From what I can see, the busybox applet doesn't support
wildcards in the source lines (and doesn't support source-directory).

What is the reason to use source with a wildcard instead of
source-directory? From the man page:

       source interfaces.d/machine-dependent

       source-directory interfaces.d

It does say though:

Currently, "source-directory" isn't supported by network-manager and guessnet.

By default, on a freshly installed Debian system, the interfaces file
includes a line to source files in the /etc/network/interfaces.d
directory.

From the postinst script:

  if [ ! -f /etc/network/interfaces ] ; then
    if [ -z "$2" ]; then
      echo "Creating /etc/network/interfaces."
      echo "# interfaces(5) file used by ifup(8) and ifdown(8)" > /etc/network/interfaces
      echo "# Include files from /etc/network/interfaces.d:" >> /etc/network/interfaces
      echo "source-directory /etc/network/interfaces.d" >> /etc/network/interfaces

 > adding the following in ifupdown.mk:

 > ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
 > define IFUPDOWN_TUNE_ETC_NETWORK_INTERFACES
 > 	mkdir -p $(TARGET_DIR)/etc/network/interfaces.d
 > 	echo "source interfaces.d/*" >> $(TARGET_DIR)/etc/network/interfaces
 > endef
 > IFUPDOWN_TARGET_FINALIZE_HOOKS += IFUPDOWN_TUNE_ETC_NETWORK_INTERFACES

We afaik provide no explicit guarantees on the ordering of the various
_TARGET_FINALIZE hooks, but it afaik will run in (package-)alphabetical
order, E.G. before the finalize hooks of skeleton so this doesn't work
:/
Thomas Petazzoni Oct. 23, 2016, 12:17 p.m. UTC | #3
Hello,

On Sun, 23 Oct 2016 14:06:12 +0200, Peter Korsgaard wrote:

> We afaik provide no explicit guarantees on the ordering of the various
> _TARGET_FINALIZE hooks

Correct.

>, but it afaik will run in (package-)alphabetical
> order, E.G. before the finalize hooks of skeleton so this doesn't work
> :/

Hum, my thinking was that it did not matter if the "source" line is
before or after the lo definition. However, the skeleton package
completely overwrites the file, so indeed, it doesn't work.

Best regards,

Thomas
Matt Weber Oct. 25, 2016, 2:22 a.m. UTC | #4
Thomas/ Peter

On Sun, Oct 23, 2016 at 7:17 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
>
> Hello,
>
> On Sun, 23 Oct 2016 14:06:12 +0200, Peter Korsgaard wrote:
>
> > We afaik provide no explicit guarantees on the ordering of the various
> > _TARGET_FINALIZE hooks
>
> Correct.
>
> >, but it afaik will run in (package-)alphabetical
> > order, E.G. before the finalize hooks of skeleton so this doesn't work
> > :/
>
> Hum, my thinking was that it did not matter if the "source" line is
> before or after the lo definition. However, the skeleton package
> completely overwrites the file, so indeed, it doesn't work.


So leave it as is in the skeleton.mk?
Peter Korsgaard Oct. 25, 2016, 6:25 a.m. UTC | #5
>>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes:

Hi,

 >> > We afaik provide no explicit guarantees on the ordering of the various
 >> > _TARGET_FINALIZE hooks
 >> 
 >> Correct.
 >> 
 >> >, but it afaik will run in (package-)alphabetical
 >> > order, E.G. before the finalize hooks of skeleton so this doesn't work
 >> > :/
 >> 
 >> Hum, my thinking was that it did not matter if the "source" line is
 >> before or after the lo definition. However, the skeleton package
 >> completely overwrites the file, so indeed, it doesn't work.

 > So leave it as is in the skeleton.mk?

I think that is the only place we can add it, yes.

With that said, there's afaik no packages using interface.d, so if this
is just for project customization then you might as well handle it there
(E.G. you might already have a custom interfaces or appending the line
to it in your post-build is easy).
Matt Weber Oct. 25, 2016, 1:14 p.m. UTC | #6
Peter,

On Tue, Oct 25, 2016 at 1:25 AM, Peter Korsgaard <peter@korsgaard.com> wrote:
>>>>>> "Matthew" == Matthew Weber <matthew.weber@rockwellcollins.com> writes:
>
> Hi,
>
>  >> > We afaik provide no explicit guarantees on the ordering of the various
>  >> > _TARGET_FINALIZE hooks
>  >>
>  >> Correct.
>  >>
>  >> >, but it afaik will run in (package-)alphabetical
>  >> > order, E.G. before the finalize hooks of skeleton so this doesn't work
>  >> > :/
>  >>
>  >> Hum, my thinking was that it did not matter if the "source" line is
>  >> before or after the lo definition. However, the skeleton package
>  >> completely overwrites the file, so indeed, it doesn't work.
>
>  > So leave it as is in the skeleton.mk?
>
> I think that is the only place we can add it, yes.
>
> With that said, there's afaik no packages using interface.d, so if this
> is just for project customization then you might as well handle it there
> (E.G. you might already have a custom interfaces or appending the line
> to it in your post-build is easy).

Agree, but in this case we thought it might make sense to include by
default, so that we encourage the practice of using that style of
interface configuration which follows the behavior/intent of larger
Linux distros.  Especially since the user intentionally enables
ifupdown for more advanced use cases.

I'm good either way.
diff mbox

Patch

diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
index 1000161..39e2cbb 100644
--- a/package/skeleton/skeleton.mk
+++ b/package/skeleton/skeleton.mk
@@ -170,9 +170,19 @@  define SKELETON_SET_NETWORK_DHCP
 endef
 endif
 
+ifeq ($(BR2_PACKAGE_IFUPDOWN),y)
+define SKELETON_SET_NETWORK_SUPPORT_INTERFACES_D
+	( \
+		echo ;                                               \
+		echo "source interfaces.d/*";                        \
+	) >> $(TARGET_DIR)/etc/network/interfaces
+endef
+endif
+
 define SKELETON_SET_NETWORK
 	mkdir -p $(TARGET_DIR)/etc/network/
 	$(SKELETON_SET_NETWORK_LOCALHOST)
+	$(SKELETON_SET_NETWORK_SUPPORT_INTERFACES_D)
 	$(SKELETON_SET_NETWORK_DHCP)
 endef
 
diff --git a/system/device_table.txt b/system/device_table.txt
index dc1af51..69b1d46 100644
--- a/system/device_table.txt
+++ b/system/device_table.txt
@@ -17,5 +17,6 @@ 
 /etc/network/if-pre-up.d		d	755	0	0	-	-	-	-	-
 /etc/network/if-down.d			d	755	0	0	-	-	-	-	-
 /etc/network/if-post-down.d		d	755	0	0	-	-	-	-	-
+/etc/network/interfaces.d		d	755	0	0	-	-	-	-	-
 # uncomment this to allow starting x as non-root
 #/usr/X11R6/bin/Xfbdev		     	f	4755	0	0	-	-	-	-	-