Hello, > On 18 Aug 2023, at 13:55, Jonatan Schlag wrote: > > Hi list, > > Am Mittwoch, dem 24.05.2023 um 10:00 +0100 schrieb Michael Tremer: >> 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)” >> > > I did not want to make such a big change, as people might rely on these > files. But I am happy to create a script and delete the other > scripts/links. I suggest as naming: > > /etc/init.d/networking/local > > Calling the script could look like: > > For one Zone > > /etc/init.d/networking/local start ${ZONE} > /etc/init.d/networking/local stop ${ZONE} > > For all Zones > > /etc/init.d/networking/local start > /etc/init.d/networking/local stop > > Look this fine to you? This looks good to me. >> 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. > > Ok, fine by me. >> >> 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? > > > Yes, we can do that. I have only comments on the naming of the > functions. I would like to reuse the naming style of the old ipfire-3.x > networking code. So I would propose: > > network_zone_exists(ZONE) > > network_zones_get_all() > > network_zones_get_local() > > network_zones_get_nonlocal() > > > As we all agreed on this naming some time ago, I think it faster to use > these names instead of inventing new ones. Also okay for me. Best, -Michael > > Greetings Jonatan >> >> -Michael >> >>> On 23 May 2023, at 18:23, Jonatan Schlag >>> 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 >>> --- >>> 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 >>> >> > >