Re: [Patch RFC 04/15] network initscripts: check if the zone in the current config exists

[Jonatan Schlag <jonatan.schlag@ipfire.org>]

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

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?


> 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.

Greetings Jonatan
> 
> -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
> > 
>