[v3] radvd: improve startup script

- Add start, stop and restart/reload options.

- Do nothing if /etc/radvd.conf does not exist instead of printing an
  error message. It is valid to install radvd without a configuration
  file. The daemon may be started later by another service with a
  configuration created at run-time.

- Print an error message if the kernel does not support IPv6 forwarding,
  which is required by radvd.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v2->v3
- Don't the test if the binary is executable. It's unlikely to happen
  because Buildroot installs both radvd and its init script as part of
  the same package. But if it ever happens for some reason, the error
  message from start-stop-daemon should be pretty clear (Thomas
  Petazzoni).
- Move start and stop to functions and rewrite the error handling code
  to improve its readability.
- Add a one second sleep between stop and start, in restart, as made in
  several other scripts.

Changes v1->v2
- Print error message is /usr/sbin/radvd is missing
- Print error message if /proc/sys/net/ipv6/conf/all/forwarding is
  missing (kernel does not support IPv6 forwarding)
- Echo "1" to /proc/sys/net/ipv6/conf/all/forwarding upon start
---
 package/radvd/S50radvd | 54 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 13 deletions(-)

Hello,

On Sun, 15 Apr 2018 23:10:37 -0300, Carlos Santos wrote:

> -RADVD=/usr/sbin/radvd
> +test -f /etc/radvd.conf || exit 0

I'm still not impressed by silent exit cases. Shouldn't we let radvd
fail to start and complain about the lack of radvd.conf ?

> +start() {
> +	printf "Starting radvd: "
> +	echo "1" > /proc/sys/net/ipv6/conf/all/forwarding
> +	start-stop-daemon -S -x /usr/sbin/radvd || {
> +		echo "FAIL"
> +		exit 1
> +	}

Can we use the

	[ $? = 0 ] && echo "OK" || echo "FAIL"

syntax that we use in almost all other init scripts ?

> +	echo "OK"
> +}
> +
> +stop() {
> +	printf "Stopping radvd: "
> +	start-stop-daemon -K -q -x /usr/sbin/radvd || {
> +		echo "FAIL"
> +		exit 1
> +	}

Ditto here.

Also, can we use a pid file managed by start-stop-daemon, like
S50dropbear is doing (and many other init scripts) ?

Thanks!

Thomas

> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "buildroot" <buildroot@buildroot.org>
> Sent: Wednesday, April 25, 2018 6:25:12 PM
> Subject: Re: [Buildroot] [PATCH v3] radvd: improve startup script

> Hello,
> 
> On Sun, 15 Apr 2018 23:10:37 -0300, Carlos Santos wrote:
> 
>> -RADVD=/usr/sbin/radvd
>> +test -f /etc/radvd.conf || exit 0
> 
> I'm still not impressed by silent exit cases. Shouldn't we let radvd
> fail to start and complain about the lack of radvd.conf ?

Hum, yes, but as already discussed in a different thread there is no
standard regarding these cases.

So far I'm just following the example existing in other start-up scripts:

    $ grep 'test -f .* || exit' package/*/S[0-9]*
    package/dhcp/S80dhcp-relay:test -f /usr/sbin/dhcrelay || exit 0
    package/dhcp/S80dhcp-server:test -f /usr/sbin/dhcpd || exit 0
    package/dhcp/S80dhcp-server:test -f /etc/dhcp/dhcpd.conf || exit 0
    package/mpd/S95mpd:test -f /etc/mpd.conf || exit 0

>> +start() {
>> +	printf "Starting radvd: "
>> +	echo "1" > /proc/sys/net/ipv6/conf/all/forwarding
>> +	start-stop-daemon -S -x /usr/sbin/radvd || {
>> +		echo "FAIL"
>> +		exit 1
>> +	}
> 
> Can we use the
> 
>	[ $? = 0 ] && echo "OK" || echo "FAIL"
> 
> syntax that we use in almost all other init scripts ?

No, because the echo "FAIL" command succeeds, masking the error result:

    $ (false; [ $? = 0 ] && echo "OK" || echo "FAIL";); echo $?
    FAIL
    0

instead of

    $ (false || { echo "FAIL"; exit 1; }; echo "OK";); echo $?
    FAIL
    1

Moreover shellcheck complains that it is bad syntax:

    $ shellcheck package/radvd/S50radvd 

    In package/radvd/S50radvd line 14:
            [ $? = 0 ] && echo "OK" || echo "FAIL"
              ^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.

In fact many Buildroot start-up scripts make shellcheck have heart
attacks. :-)

> 
>> +	echo "OK"
>> +}
>> +
>> +stop() {
>> +	printf "Stopping radvd: "
>> +	start-stop-daemon -K -q -x /usr/sbin/radvd || {
>> +		echo "FAIL"
>> +		exit 1
>> +	}
> 
> Ditto here.
> 
> Also, can we use a pid file managed by start-stop-daemon, like
> S50dropbear is doing (and many other init scripts) ?

Yes, but I'd prefer make this change in a different patch if you don't
mind, since it requires additional testing.