Hello,

If you want to make this script better, why not give the zone name as a parameter?

First of all it should be renamed to something like “local” (as in: not RED) and then you don’t have to use any symlinks and read back the script name like this:

  ZONE="$(basename $0)”


Otherwise I agree with introducing unified functions that check whether we have GREEN/BLUE/ORANGE and have them in one function. We should then go through the entire code base and replace those calls first before we are introducing any new changes.

I would prefer to give those function a “network_” prefix so that it becomes clear where this function is from and ideally shorten “is used” to just “have”. Like so: network_have_BLUE() and network_have_ORANGE().

Can we do that instead?

-Michael

> On 23 May 2023, at 18:23, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote:
> 
> We check in /etc/init.d/network if the current Configuration (RED+GREEN
> or RED+GREEN+BLUE) contains the zone we want to start or stop.
> We do this not in /etc/init.d/networking/green,blue,orange
> 
> As this checks make sense also there and as these scripts are called
> form /etc/init.d/network I moved the check to these scripts.
> 
> As CONFIG_TYPE == 2 is unreadable I wrote functions to make things at least a
> litte bit prettier.
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/initscripts/networking/any               | 27 +++++++++++++++++---
> src/initscripts/networking/functions.network | 12 +++++++++
> src/initscripts/system/network               | 16 ++++--------
> 3 files changed, 41 insertions(+), 14 deletions(-)
> 
> diff --git a/src/initscripts/networking/any b/src/initscripts/networking/any
> index dc4796e91..6dba5bef9 100644
> --- a/src/initscripts/networking/any
> +++ b/src/initscripts/networking/any
> @@ -21,23 +21,44 @@
> 
> . /etc/sysconfig/rc
> . ${rc_functions}
> +. /etc/init.d/networking/functions.network
> +
> eval $(/usr/local/bin/readhash /var/ipfire/ethernet/settings)
> 
> -if [ "$(basename $0)" == "green" ]; then
> +ZONE="$(basename $0)"
> +
> +
> +if [ "${ZONE}" == "green" ]; then
> + if ! is_green_used; then
> + boot_mesg "Green zone is not configured. No action can be taken on this zone." ${FAILURE}
> + echo_failure
> + exit 1
> + fi
> +
> DEVICE="${GREEN_DEV}"
> ADDRESS="${GREEN_ADDRESS}"
> NETADDRESS="${GREEN_NETADDRESS}"
> NETMASK="${GREEN_NETMASK}"
> DEVICE="${GREEN_DEV}"
> MTU="${GREEN_MTU}"
> -elif [ "$(basename $0)" == "blue" ]; then
> +elif [ "${ZONE}" == "blue" ]; then
> + if ! is_blue_used; then
> + boot_mesg "Blue zone is not configured. No action can be taken on this zone." ${FAILURE}
> + echo_failure
> + exit 1
> + fi
> DEVICE="${BLUE_DEV}"
> ADDRESS="${BLUE_ADDRESS}"
> NETADDRESS="${BLUE_NETADDRESS}"
> NETMASK="${BLUE_NETMASK}"
> DEVICE="${BLUE_DEV}"
> MTU="${BLUE_MTU}"
> -elif [ "$(basename $0)" == "orange" ]; then
> +elif [ "${ZONE}" == "orange" ]; then
> + if ! is_orange_used; then
> + boot_mesg "Orange zone is not configured. No action can be taken on this zone." ${FAILURE}
> + echo_failure
> + exit 1
> + fi
> DEVICE="${ORANGE_DEV}"
> ADDRESS="${ORANGE_ADDRESS}"
> NETADDRESS="${ORANGE_NETADDRESS}"
> diff --git a/src/initscripts/networking/functions.network b/src/initscripts/networking/functions.network
> index 4c7ad51d4..9cc4da24b 100644
> --- a/src/initscripts/networking/functions.network
> +++ b/src/initscripts/networking/functions.network
> @@ -285,3 +285,15 @@ qmi_assign_address() {
> # Change the MAC address
> ip link set "${intf}" address "${address}"
> }
> +
> +is_blue_used() {
> + [ "${CONFIG_TYPE}" = "3" ] || [ "${CONFIG_TYPE}" = "4" ]
> +}
> +
> +is_green_used() {
> + [ -n "${GREEN_DEV}" ] && [ -v "GREEN_DEV" ]
> +}
> +
> +is_orange_used() {
> + [ "${CONFIG_TYPE}" = "2" ] || [ "${CONFIG_TYPE}" = "4" ]
> +}
> diff --git a/src/initscripts/system/network b/src/initscripts/system/network
> index 0d63b4e8b..fda16919d 100644
> --- a/src/initscripts/system/network
> +++ b/src/initscripts/system/network
> @@ -54,12 +54,10 @@ case "${DO}" in
> [ "$green" == "1" ] && /etc/rc.d/init.d/networking/green start
> 
> # BLUE
> - [ "$blue" == "1" ] && [ "$CONFIG_TYPE" = "3" -o "$CONFIG_TYPE" = "4" ] && \
> - /etc/rc.d/init.d/networking/blue start
> + [ "$blue" == "1" ] && /etc/rc.d/init.d/networking/blue start
> 
> # ORANGE
> - [ "$orange" == "1" ] && [ "$CONFIG_TYPE" = "2" -o "$CONFIG_TYPE" = "4" ] && \
> - /etc/rc.d/init.d/networking/orange start
> + [ "$orange" == "1" ] && /etc/rc.d/init.d/networking/orange start
> 
> # RED
> if [ "$red" == "1" ]; then
> @@ -87,18 +85,14 @@ case "${DO}" in
> [ "$green" == "1" ] && /etc/rc.d/init.d/networking/green stop
> 
> # BLUE
> - [ "$blue" == "1" ] && [ "$CONFIG_TYPE" = "3" -o "$CONFIG_TYPE" = "4" ] && \
> - /etc/rc.d/init.d/networking/blue stop
> + [ "$blue" == "1" ] && /etc/rc.d/init.d/networking/blue stop
> 
> # ORANGE
> - [ "$orange" == "1" ] && [ "$CONFIG_TYPE" = "2" -o "$CONFIG_TYPE" = "4" ] && \
> - /etc/rc.d/init.d/networking/orange stop
> + [ "$orange" == "1" ] && /etc/rc.d/init.d/networking/orange stop
> 
> # RED
> if [ "$red" == "1" ]; then
> - if [ "$CONFIG_TYPE" = "1" -o "$CONFIG_TYPE" = "2" -o "$CONFIG_TYPE" = "3" -o "$CONFIG_TYPE" = "4" ]; then
> - /etc/rc.d/init.d/networking/red stop
> - fi
> + /etc/rc.d/init.d/networking/red stop
> fi
> 
> exit 0
> -- 
> 2.30.2
>