[PATCH] Fix radvd startup

[PATCH] Fix radvd startup

[Jonatan Schlag]

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

We now only start radvd when we write a config fr a zone into the config
file and when no errors happen.

Fixes: #11450

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/functions/functions.radvd | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/functions/functions.radvd b/src/functions/functions.radvd
index 4e41160..bd016a0 100644
--- a/src/functions/functions.radvd
+++ b/src/functions/functions.radvd
@@ -23,16 +23,16 @@ RADVD_CONFIGFILE="/etc/radvd.conf"
 
 radvd_update() {
 	# (Re-)write the configuration file
-	radvd_write_config
-
-	# Reload the radvd service if it is already running
-	if service_is_active radvd; then
-		service_reload radvd
-		return ${EXIT_OK}
+	if radvd_write_config; then
+		# Reload the radvd service if it is already running
+		if service_is_active radvd; then
+			service_reload radvd
+			return ${EXIT_OK}
+		fi
+
+		# Start the radvd service
+		service_start radvd
 	fi
-
-	# Start the radvd service
-	service_start radvd
 }
 
 radvd_clear_config() {
@@ -48,12 +48,19 @@ radvd_write_config() {
 
 	# Write the configuration for all zones.
 	local zone
-	for zone in $(zones_get_local); do
-		__radvd_config_interface ${zone}
+	# The return value determine if radvd is started or not
+	local return_value=${EXIT_FALSE}
 
+	for zone in $(zones_get_local); do
+		if __radvd_config_interface ${zone} && [ ${return_value} = ${EXIT_TRUE} ]; then
+			# We return TRUE when __radvd_config_interface succeed and no errors happend till now
+			return_value=${EXIT_TRUE}
+		else
+			return_value=${EXIT_FALSE}
+		fi
 	done >> ${RADVD_CONFIGFILE}
 
-	return ${EXIT_OK}
+	return ${return_value}
 }
 
 __radvd_config_interface() {
-- 
2.6.3

Re: [PATCH] Fix radvd startup

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 2517 bytes --]

Hi,

On Sat, 2018-01-27 at 17:06 +0000, Jonatan Schlag wrote:
> We now only start radvd when we write a config fr a zone into the config
> file and when no errors happen.
> 
> Fixes: #11450
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  src/functions/functions.radvd | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/src/functions/functions.radvd b/src/functions/functions.radvd
> index 4e41160..bd016a0 100644
> --- a/src/functions/functions.radvd
> +++ b/src/functions/functions.radvd
> @@ -23,16 +23,16 @@ RADVD_CONFIGFILE="/etc/radvd.conf"
>  
>  radvd_update() {
>  	# (Re-)write the configuration file
> -	radvd_write_config
> -
> -	# Reload the radvd service if it is already running
> -	if service_is_active radvd; then
> -		service_reload radvd
> -		return ${EXIT_OK}
> +	if radvd_write_config; then
> +		# Reload the radvd service if it is already running
> +		if service_is_active radvd; then
> +			service_reload radvd
> +			return ${EXIT_OK}
> +		fi
> +
> +		# Start the radvd service
> +		service_start radvd
>  	fi
> -
> -	# Start the radvd service
> -	service_start radvd
>  }

This part is okay assuming that radvd_write_config is returning an error when
the configuration file is empty.

>  
>  radvd_clear_config() {
> @@ -48,12 +48,19 @@ radvd_write_config() {
>  
>  	# Write the configuration for all zones.
>  	local zone
> -	for zone in $(zones_get_local); do
> -		__radvd_config_interface ${zone}
> +	# The return value determine if radvd is started or not
> +	local return_value=${EXIT_FALSE}
>  
> +	for zone in $(zones_get_local); do
> +		if __radvd_config_interface ${zone} && [ ${return_value} =
> ${EXIT_TRUE} ]; then
> +			# We return TRUE when __radvd_config_interface
> succeed and no errors happend till now
> +			return_value=${EXIT_TRUE}
> +		else
> +			return_value=${EXIT_FALSE}
> +		fi
>  	done >> ${RADVD_CONFIGFILE}
>  
> -	return ${EXIT_OK}
> +	return ${return_value}
>  }

This is a bit spaghetti and can be written better. Like:

  r = TRUE

  for zone in ...; do
    if ! __radvd_config_interface "${zone}"; then
       r = FALSE
    fi
  done

  return r

Of course that is just pseudo-code, but it spares the else clause and the
comparison which isn't needed.

You will always run radvd even when you have interfaces that don't have IPv6. I
don't see where this is now being skipped.

-Michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]