All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
@ 2015-10-16  7:16 Peter Korsgaard
  2015-10-26 20:07 ` Ryan Barnett
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Korsgaard @ 2015-10-16  7:16 UTC (permalink / raw)
  To: buildroot

commit: http://git.buildroot.net/buildroot/commit/?id=49964858f45d2243c513e6d362e992ad89ec7a45
branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/master

On some machines, the network interface is slow to appear. For example,
on the Raspberry Pi, the network interface eth0 is an ethernet-over-USB,
and our standard boot process is too fast, so our network startup script
is called before the USB bus is compeltely enumerated, thus it can't
configure eth0.

Closes #8116.

[Peter: move to S40network, handle multiple interfaces]
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
 package/initscripts/init.d/S40network |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/package/initscripts/init.d/S40network b/package/initscripts/init.d/S40network
index 7b11d8b..a8d7c5d 100755
--- a/package/initscripts/init.d/S40network
+++ b/package/initscripts/init.d/S40network
@@ -6,8 +6,37 @@
 # Debian ifupdown needs the /run/network lock directory
 mkdir -p /run/network
 
+# In case we have a slow-to-appear interface (e.g. eth-over-USB),
+# and we need to configure it, wait until it appears, but not too
+# long either. WAIT_DELAY is in seconds.
+WAIT_DELAY=15
+
+wait_for_interfaces() {
+	IFACES=$(awk '/^auto/ { print $2 }' /etc/network/interfaces)
+	[ -n "$IFACES" ] || return
+
+	printf "Waiting for network interfaces to appear"
+
+	for i in $(seq $WAIT_DELAY); do
+	    for IFACE in $IFACES; do
+		if [ ! -e "/sys/class/net/$IFACE" ]; then
+		    printf "."
+		    sleep 1
+		    continue 2
+		fi
+	    done
+
+	    printf " ok\n"; return
+	done
+
+	printf " timeout\n"
+	exit 1
+}
+
 case "$1" in
   start)
+	wait_for_interfaces
+
  	echo "Starting network..."
 	/sbin/ifup -a
 	;;

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
  2015-10-16  7:16 [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear Peter Korsgaard
@ 2015-10-26 20:07 ` Ryan Barnett
  2015-10-26 20:23   ` Peter Korsgaard
  2015-10-27  9:22   ` Nicolas Cavallari
  0 siblings, 2 replies; 9+ messages in thread
From: Ryan Barnett @ 2015-10-26 20:07 UTC (permalink / raw)
  To: buildroot

Peter, Yann, All,

On Fri, Oct 16, 2015 at 2:16 AM, Peter Korsgaard <peter@korsgaard.com> wrote:

[...]

>
> diff --git a/package/initscripts/init.d/S40network b/package/initscripts/init.d/S40network
> index 7b11d8b..a8d7c5d 100755
> --- a/package/initscripts/init.d/S40network
> +++ b/package/initscripts/init.d/S40network
> @@ -6,8 +6,37 @@
>  # Debian ifupdown needs the /run/network lock directory
>  mkdir -p /run/network
>
> +# In case we have a slow-to-appear interface (e.g. eth-over-USB),
> +# and we need to configure it, wait until it appears, but not too
> +# long either. WAIT_DELAY is in seconds.
> +WAIT_DELAY=15
> +
> +wait_for_interfaces() {
> +       IFACES=$(awk '/^auto/ { print $2 }' /etc/network/interfaces)

This new way to handle bringing up interfaces doesn't work well if you
have defined virtual interfaces in your /etc/network/interfaces.
Having virtual interfaces in your /etc/network/interfaces file I
believe is a valid use case that I think buildroot's default
S40network should handle. The specific use case that will fail is
outlined below:

In the actual use case demonstrated below, the network interfaces file
contains 2 virtual interfaces on eth3.  Virtual interfaces do not get
a unique entry in /sys/class/net.  The function "wait_for_interfaces"
added to /package/initscripts/init.d/S40network makes an assumption
that all interfaces that may be "auto" will have a /sys/class/net
entry.  In the case of a virtual interface, the function will always
timeout, and "ifup -a" is never called.

# awk '/^auto/ { print $2 }' /etc/network/interfaces
lo
eth3
eth3:1
eth3:2

# ls /sys/class/net/
eth0   eth1   eth2   eth3 lo

I'm not an awk expert so I'm not exactly sure how to fix this issue
but I think this is a valid use case that should be addressed.

Thanks,
-Ryan

> +       [ -n "$IFACES" ] || return
> +
> +       printf "Waiting for network interfaces to appear"
> +
> +       for i in $(seq $WAIT_DELAY); do
> +           for IFACE in $IFACES; do
> +               if [ ! -e "/sys/class/net/$IFACE" ]; then
> +                   printf "."
> +                   sleep 1
> +                   continue 2
> +               fi
> +           done
> +
> +           printf " ok\n"; return
> +       done
> +
> +       printf " timeout\n"
> +       exit 1
> +}
> +
>  case "$1" in
>    start)
> +       wait_for_interfaces
> +
>         echo "Starting network..."
>         /sbin/ifup -a
>         ;;


-- 
Ryan Barnett / Sr Software Engineer
Airborne Information Systems / Security Systems and Software
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
ryan.barnett at rockwellcollins.com
www.rockwellcollins.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
  2015-10-26 20:07 ` Ryan Barnett
@ 2015-10-26 20:23   ` Peter Korsgaard
       [not found]     ` <CADZ9A7pN8Tq8NzDbnNDjjSP0Y7Edvo7TbngcFNhttO+v+ybMXg@mail.gmail.com>
  2015-10-27  9:22   ` Nicolas Cavallari
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Korsgaard @ 2015-10-26 20:23 UTC (permalink / raw)
  To: buildroot

>>>>> "Ryan" == Ryan Barnett <ryan.barnett@rockwellcollins.com> writes:

 > Peter, Yann, All,
 > On Fri, Oct 16, 2015 at 2:16 AM, Peter Korsgaard <peter@korsgaard.com> wrote:

 > [...]

 >> 
 >> diff --git a/package/initscripts/init.d/S40network b/package/initscripts/init.d/S40network
 >> index 7b11d8b..a8d7c5d 100755
 >> --- a/package/initscripts/init.d/S40network
 >> +++ b/package/initscripts/init.d/S40network
 >> @@ -6,8 +6,37 @@
 >> # Debian ifupdown needs the /run/network lock directory
 >> mkdir -p /run/network
 >> 
 >> +# In case we have a slow-to-appear interface (e.g. eth-over-USB),
 >> +# and we need to configure it, wait until it appears, but not too
 >> +# long either. WAIT_DELAY is in seconds.
 >> +WAIT_DELAY=15
 >> +
 >> +wait_for_interfaces() {
 >> +       IFACES=$(awk '/^auto/ { print $2 }' /etc/network/interfaces)

 > This new way to handle bringing up interfaces doesn't work well if you
 > have defined virtual interfaces in your /etc/network/interfaces.
 > Having virtual interfaces in your /etc/network/interfaces file I
 > believe is a valid use case that I think buildroot's default
 > S40network should handle. The specific use case that will fail is
 > outlined below:

 > In the actual use case demonstrated below, the network interfaces file
 > contains 2 virtual interfaces on eth3.  Virtual interfaces do not get
 > a unique entry in /sys/class/net.  The function "wait_for_interfaces"
 > added to /package/initscripts/init.d/S40network makes an assumption
 > that all interfaces that may be "auto" will have a /sys/class/net
 > entry.  In the case of a virtual interface, the function will always
 > timeout, and "ifup -a" is never called.

Ok, I suggest we do two things:

We change to awk statement to only consider interface names up to ':':

awk '/^auto/ { split($2, iface, ":"); print iface[1] }' /etc/network/interfaces

And we still continue with ifup -a even if we time out by changing the
exit 1 to a return 1 in wait_for_interfaces().

Would that work for you?

-- 
Venlig hilsen,
Peter Korsgaard 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
       [not found]     ` <CADZ9A7pN8Tq8NzDbnNDjjSP0Y7Edvo7TbngcFNhttO+v+ybMXg@mail.gmail.com>
@ 2015-10-26 20:56       ` Peter Korsgaard
  2015-10-26 21:04       ` Yann E. MORIN
  1 sibling, 0 replies; 9+ messages in thread
From: Peter Korsgaard @ 2015-10-26 20:56 UTC (permalink / raw)
  To: buildroot

>>>>> "Maury" == Maury Anderson <maury.anderson@rockwellcollins.com> writes:

 > I believe both suggestions should be added.

Indeed.

> Thank you for the awk magic.  The modified awk magic creates duplicates,
 > for instance if you had "lo eth3 eth3:1 eth3:2" you end up with "lo eth3
 > eth3 eth3".  The duplicates don't hurt anything, just waste time in the
 > loop.  Can we pipe it through uniq?  (Provided uniq is in the default
 > busybox.)

We could, but as we are already waiting, the trivial overhead of a few
more stat("/sys/class/net/<iface>") calls imho isn't worth requiring
uniq for.

-- 
Venlig hilsen,
Peter Korsgaard 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
       [not found]     ` <CADZ9A7pN8Tq8NzDbnNDjjSP0Y7Edvo7TbngcFNhttO+v+ybMXg@mail.gmail.com>
  2015-10-26 20:56       ` Peter Korsgaard
@ 2015-10-26 21:04       ` Yann E. MORIN
  2015-10-26 21:24         ` Ryan Barnett
  1 sibling, 1 reply; 9+ messages in thread
From: Yann E. MORIN @ 2015-10-26 21:04 UTC (permalink / raw)
  To: buildroot

Maury, Ryan, Peter, All,

On 2015-10-26 15:53 -0500, Maury Anderson spake thusly:
> I believe both suggestions should be added.
> 
> Thank you for the awk magic.  The modified awk magic creates duplicates,
> for instance if you had "lo eth3 eth3:1 eth3:2" you end up with "lo eth3
> eth3 eth3".  The duplicates don't hurt anything, just waste time in the
> loop.  Can we pipe it through uniq?  (Provided uniq is in the default
> busybox.)

Or still in awk:

    ifaces=$(awk '/^auto/ { split($2, iface, ":"); ifaces[iface[1]]=1; }
                  END { for(i in ifaces) { printf( "%s\n", i); } }
                 ' \
                 /etc/network/interfaces
            )

Regards,
Yann E. MORIN.

> Thank you,
> Maury Anderson
> 
> On Mon, Oct 26, 2015 at 3:23 PM, Peter Korsgaard <peter@korsgaard.com>
> wrote:
> 
> > >>>>> "Ryan" == Ryan Barnett <ryan.barnett@rockwellcollins.com> writes:
> >
> >  > Peter, Yann, All,
> >  > On Fri, Oct 16, 2015 at 2:16 AM, Peter Korsgaard <peter@korsgaard.com>
> > wrote:
> >
> >  > [...]
> >
> >  >>
> >  >> diff --git a/package/initscripts/init.d/S40network
> > b/package/initscripts/init.d/S40network
> >  >> index 7b11d8b..a8d7c5d 100755
> >  >> --- a/package/initscripts/init.d/S40network
> >  >> +++ b/package/initscripts/init.d/S40network
> >  >> @@ -6,8 +6,37 @@
> >  >> # Debian ifupdown needs the /run/network lock directory
> >  >> mkdir -p /run/network
> >  >>
> >  >> +# In case we have a slow-to-appear interface (e.g. eth-over-USB),
> >  >> +# and we need to configure it, wait until it appears, but not too
> >  >> +# long either. WAIT_DELAY is in seconds.
> >  >> +WAIT_DELAY=15
> >  >> +
> >  >> +wait_for_interfaces() {
> >  >> +       IFACES=$(awk '/^auto/ { print $2 }' /etc/network/interfaces)
> >
> >  > This new way to handle bringing up interfaces doesn't work well if you
> >  > have defined virtual interfaces in your /etc/network/interfaces.
> >  > Having virtual interfaces in your /etc/network/interfaces file I
> >  > believe is a valid use case that I think buildroot's default
> >  > S40network should handle. The specific use case that will fail is
> >  > outlined below:
> >
> >  > In the actual use case demonstrated below, the network interfaces file
> >  > contains 2 virtual interfaces on eth3.  Virtual interfaces do not get
> >  > a unique entry in /sys/class/net.  The function "wait_for_interfaces"
> >  > added to /package/initscripts/init.d/S40network makes an assumption
> >  > that all interfaces that may be "auto" will have a /sys/class/net
> >  > entry.  In the case of a virtual interface, the function will always
> >  > timeout, and "ifup -a" is never called.
> >
> > Ok, I suggest we do two things:
> >
> > We change to awk statement to only consider interface names up to ':':
> >
> > awk '/^auto/ { split($2, iface, ":"); print iface[1] }'
> > /etc/network/interfaces
> >
> > And we still continue with ifup -a even if we time out by changing the
> > exit 1 to a return 1 in wait_for_interfaces().
> >
> > Would that work for you?
> >
> > --
> > Venlig hilsen,
> > Peter Korsgaard
> >
> 
> 
> 
> -- 
> Maury Anderson
> Sr Software Engineer
> AIS Platform Security - Rockwell Collins

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
  2015-10-26 21:04       ` Yann E. MORIN
@ 2015-10-26 21:24         ` Ryan Barnett
  0 siblings, 0 replies; 9+ messages in thread
From: Ryan Barnett @ 2015-10-26 21:24 UTC (permalink / raw)
  To: buildroot

Yann, Peter,

On Mon, Oct 26, 2015 at 4:04 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Maury, Ryan, Peter, All,
>
> On 2015-10-26 15:53 -0500, Maury Anderson spake thusly:
>> I believe both suggestions should be added.
>>
>> Thank you for the awk magic.  The modified awk magic creates duplicates,
>> for instance if you had "lo eth3 eth3:1 eth3:2" you end up with "lo eth3
>> eth3 eth3".  The duplicates don't hurt anything, just waste time in the
>> loop.  Can we pipe it through uniq?  (Provided uniq is in the default
>> busybox.)
>
> Or still in awk:
>
>     ifaces=$(awk '/^auto/ { split($2, iface, ":"); ifaces[iface[1]]=1; }
>                   END { for(i in ifaces) { printf( "%s\n", i); } }
>                  ' \
>                  /etc/network/interfaces
>             )

For what my two USD cents is worth:  ;)

I prefer just keeping Peter's awk suggestion of:

awk '/^auto/ { split($2, iface, ":"); print iface[1] }' /etc/network/interfaces

The reason I prefer this is because of the simplicity of it and it is
much easier to read and understand for people who are causal users of
AWK syntax (such as myself). As Peter said, I think overhead of the
simplified command is worth it over the complication of this awk
command above.

A patch should be inbound to the mailing list shortly addressing this issue.

Thanks,
-Ryan

-- 
Ryan Barnett / Sr Software Engineer
Airborne Information Systems / Security Systems and Software
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
ryan.barnett at rockwellcollins.com
www.rockwellcollins.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
  2015-10-26 20:07 ` Ryan Barnett
  2015-10-26 20:23   ` Peter Korsgaard
@ 2015-10-27  9:22   ` Nicolas Cavallari
  2015-10-27 22:39     ` Arnout Vandecappelle
  1 sibling, 1 reply; 9+ messages in thread
From: Nicolas Cavallari @ 2015-10-27  9:22 UTC (permalink / raw)
  To: buildroot

On 26/10/2015 21:07, Ryan Barnett wrote:
> Peter, Yann, All,
> 
> On Fri, Oct 16, 2015 at 2:16 AM, Peter Korsgaard <peter@korsgaard.com> wrote:
> 
> [...]
> 
>>
>> diff --git a/package/initscripts/init.d/S40network b/package/initscripts/init.d/S40network
>> index 7b11d8b..a8d7c5d 100755
>> --- a/package/initscripts/init.d/S40network
>> +++ b/package/initscripts/init.d/S40network
>> @@ -6,8 +6,37 @@
>>  # Debian ifupdown needs the /run/network lock directory
>>  mkdir -p /run/network
>>
>> +# In case we have a slow-to-appear interface (e.g. eth-over-USB),
>> +# and we need to configure it, wait until it appears, but not too
>> +# long either. WAIT_DELAY is in seconds.
>> +WAIT_DELAY=15
>> +
>> +wait_for_interfaces() {
>> +       IFACES=$(awk '/^auto/ { print $2 }' /etc/network/interfaces)
> 
> This new way to handle bringing up interfaces doesn't work well if you
> have defined virtual interfaces in your /etc/network/interfaces.
> Having virtual interfaces in your /etc/network/interfaces file I
> believe is a valid use case that I think buildroot's default
> S40network should handle. The specific use case that will fail is
> outlined below:
> 
> In the actual use case demonstrated below, the network interfaces file
> contains 2 virtual interfaces on eth3.  Virtual interfaces do not get
> a unique entry in /sys/class/net.  The function "wait_for_interfaces"
> added to /package/initscripts/init.d/S40network makes an assumption
> that all interfaces that may be "auto" will have a /sys/class/net
> entry.  In the case of a virtual interface, the function will always
> timeout, and "ifup -a" is never called.
> 
> # awk '/^auto/ { print $2 }' /etc/network/interfaces
> lo
> eth3
> eth3:1
> eth3:2

These are not virtual interfaces in any way.  These aren't even
interfaces.  Actually, they are address labels.

Debian's ifupdown still support them, but never supported it correctly
in the first place. Nowadays, it has more or less the same effect as
just adding multiple eth3 definitions, something that busybox
unfortunately does not support (i just looked at the code, and
supporting it could be possibly done only by removing code, i should
submit a patch).

Anyway, one simple fix would be to just test
/sys/class/net/${IFACE%:*} instead.

However, true virtual interfaces may also be defined in
/etc/network/interfaces and created via a pre-up stanza:

auto br0
iface br0 inet static
    pre-up ip link add br0 type bridge
    post-down ip link del br0
    address 203.0.113.42
    netmask 255.255.255.0

In this case, S40network will wait forever.  However, if the feature
was implemented via ifupdown pre-up hooks, then this use-case could
actually work, since hooks are executed after pre-up stanzas. But that
is maybe too complex for buildroot.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
  2015-10-27  9:22   ` Nicolas Cavallari
@ 2015-10-27 22:39     ` Arnout Vandecappelle
  2015-10-27 22:46       ` Peter Korsgaard
  0 siblings, 1 reply; 9+ messages in thread
From: Arnout Vandecappelle @ 2015-10-27 22:39 UTC (permalink / raw)
  To: buildroot

On 27-10-15 10:22, Nicolas Cavallari wrote:
> However, true virtual interfaces may also be defined in
> /etc/network/interfaces and created via a pre-up stanza:
> 
> auto br0
> iface br0 inet static
>     pre-up ip link add br0 type bridge
>     post-down ip link del br0
>     address 203.0.113.42
>     netmask 255.255.255.0
> 
> In this case, S40network will wait forever.  However, if the feature
> was implemented via ifupdown pre-up hooks, then this use-case could
> actually work, since hooks are executed after pre-up stanzas. But that
> is maybe too complex for buildroot.

 Actually, this sounds less complicated than the script we currently have.

 At least, if both busybox and ifupdown behave the same way.

 Does systemd use /etc/network/interfaces and/or the hooks?

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear
  2015-10-27 22:39     ` Arnout Vandecappelle
@ 2015-10-27 22:46       ` Peter Korsgaard
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Korsgaard @ 2015-10-27 22:46 UTC (permalink / raw)
  To: buildroot

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> In this case, S40network will wait forever.  However, if the feature
 >> was implemented via ifupdown pre-up hooks, then this use-case could
 >> actually work, since hooks are executed after pre-up stanzas. But that
 >> is maybe too complex for buildroot.

 >  Actually, this sounds less complicated than the script we currently have.

 >  At least, if both busybox and ifupdown behave the same way.

Yes, this was already discussed:

http://lists.busybox.net/pipermail/buildroot/2015-October/142329.html

But Jerome didn't send a patch yet. The only disadvantage would be that
the timeout is per-interface instead of globally, but that is imho not
that bad.

 >  Does systemd use /etc/network/interfaces and/or the hooks?

I don't think so.

-- 
Venlig hilsen,
Peter Korsgaard 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-10-27 22:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-16  7:16 [Buildroot] [git commit] package/initscripts: S40network: wait for network interfaces to appear Peter Korsgaard
2015-10-26 20:07 ` Ryan Barnett
2015-10-26 20:23   ` Peter Korsgaard
     [not found]     ` <CADZ9A7pN8Tq8NzDbnNDjjSP0Y7Edvo7TbngcFNhttO+v+ybMXg@mail.gmail.com>
2015-10-26 20:56       ` Peter Korsgaard
2015-10-26 21:04       ` Yann E. MORIN
2015-10-26 21:24         ` Ryan Barnett
2015-10-27  9:22   ` Nicolas Cavallari
2015-10-27 22:39     ` Arnout Vandecappelle
2015-10-27 22:46       ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.