From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jonatan Schlag <jonatan.schlag@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 2/5] functions.network: Add network_zone_exists
Date: Mon, 25 Mar 2024 08:55:31 +0100
Message-ID: <13416660cfb2c3e17c8078e0f13ff9d5@ipfire.org>
In-Reply-To: <56759C3B-A892-4B33-85E0-1E9A0393BC2A@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3554129626416935876=="
List-Id: <development.lists.ipfire.org>

--===============3554129626416935876==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi,

Am 2024-03-18 17:32, schrieb Michael Tremer:
>> On 16 Mar 2024, at 09:43, Jonatan Schlag <jonatan.schlag(a)ipfire.org>=20
>> wrote:
>>=20
>> Am 2024-03-12 10:59, schrieb Michael Tremer:
>>> Hello,
>>> We really need to go through the entire code base and come up with a=20
>>> single way how to check whether a zone exists or not.
>>> In your proposed patch, you have different ways to check whether a=20
>>> zone exists or not. One is to check if there is a device (e.g.=20
>>> GREEN_DEV), and sometimes you are checking CONFIG_TYPE.
>>> I suppose you got your inspiration from here:
>>> =20
>>> https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dconfig/cfgroot/he=
ader.pl;hb=3D6d501c05583a4efa513ff4b04a48ef41d5e8170e#l147
>>=20
>> No, that is precisely the code copied from the network script.
>>=20
>>> Should we not go and redefine CONFIG_TYPE=3D1 as =E2=80=9Chas a green net=
work=E2=80=9D=20
>>> to make it consistent?
>>=20
>> According to here, there always exists a green network:=20
>> https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dsrc/setup/networki=
ng.c;h=3D9dd5205e5a1cb60c4a0bb003ec2a8348848822f9;hb=3Drefs/heads/next#l22
>> So we could just return True.
>=20
> CONFIG_TYPE is a bitmap. Almost at least:
>=20
> GREEN sadly didn=E2=80=99t get a bit for itself.
>=20
> If RED is present (at least when it is an Ethernet connection) it has=20
> the 0x04 bit set.
>=20
> If ORANGE exists, 0x02 is set. If BLUE exists 0x01 is set.
>=20
> So checking the bits is a bare minimum.
>=20


So what I got here is:

00 1
01 2
10 3
11 4

as we do not use CONFIG_TYPE zero. So BLUE would have the 0x02 Bit set.=20
Orange the 0x01 bit. Green is always there, so does red if the=20
CONFIG_TYPE is not zero. I suggest, simply copying the
statements from here and translate them to bash=20
https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dsrc/setup/networking.=
c;h=3D9dd5205e5a1cb60c4a0bb003ec2a8348848822f9;hb=3Drefs/heads/next#l22

Are you okay with that?

>>=20
>>> I would also suggest to keep the function name short and rename it to=20
>>> =E2=80=9Czone_exists=E2=80=9D. Since the function is in =E2=80=9Cnetworks=
=E2=80=9D, I think it is=20
>>> pretty clear what this function would do. There is litte chance this=20
>>> name would clash with anything else.
>>=20
>> Just for the records
>>=20
>> "I would prefer to give those function a =E2=80=9Cnetwork_=E2=80=9D prefix=
 so that it=20
>> becomes clear where this function is from and ideally shorten =E2=80=9Cis =

>> used=E2=80=9D to just =E2=80=9Chave=E2=80=9D. Like so: network_have_BLUE()=
 and=20
>> network_have_ORANGE().
>> " - "Re: [Patch RFC 04/15] network initscripts: check if the zone in=20
>> the current config exists"
>=20
> Well that depends on how you are looking at it.
>=20
> =E2=80=9Chave_BLUE" I would find a little bit too generic. There is no=20
> connection from BLUE to networking. =E2=80=9CZone=E2=80=9D hints heavily at=
 networking=20
> in our context.
>=20

I will just change the name.

Jonatan

>>=20
>> An ever moving target is hard to hit :-)
>>=20
>> Jonatan
>>>> On 2 Mar 2024, at 11:09, Jonatan Schlag <jonatan.schlag(a)ipfire.org>=20
>>>> wrote:
>>>> As our Network is quite static a case does the trick
>>>> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
>>>> ---
>>>> src/initscripts/networking/functions.network | 22=20
>>>> ++++++++++++++++++++
>>>> 1 file changed, 22 insertions(+)
>>>> diff --git a/src/initscripts/networking/functions.network=20
>>>> b/src/initscripts/networking/functions.network
>>>> index dedbb6f7f..ad0d3f36a 100644
>>>> --- a/src/initscripts/networking/functions.network
>>>> +++ b/src/initscripts/networking/functions.network
>>>> @@ -289,3 +289,25 @@ qmi_assign_address() {
>>>> # Change the MAC address
>>>> ip link set "${intf}" address "${address}"
>>>> }
>>>> +
>>>> +network_zone_exists(){
>>>> + local zone=3D"${1}"
>>>> +
>>>> + case "${zone}" in
>>>> + "blue")
>>>> + [ "${CONFIG_TYPE}" =3D "3" ] || [ "${CONFIG_TYPE}" =3D "4" ]
>>>> + ;;
>>>> + "green")
>>>> + [ -n "${GREEN_DEV}" ] && [ -v "GREEN_DEV" ]
>>> What is test -v doing? I can=E2=80=99t even find it on the man page.
>>>> + ;;
>>>> + "orange")
>>>> + [ "${CONFIG_TYPE}" =3D "2" ] || [ "${CONFIG_TYPE}" =3D "4" ]
>>>> + ;;
>>>> + "red")
>>>> + return ${EXIT_TRUE}
>>>> + ;;
>>>> + *)
>>>> + return ${EXIT_FALSE}
>>>> + ;;
>>>> + esac
>>>> +}
>>>> --
>>>> 2.39.2
>>> -Michael

--===============3554129626416935876==--