From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH 2/5] functions.network: Add network_zone_exists Date: Mon, 18 Mar 2024 16:32:22 +0000 Message-ID: <56759C3B-A892-4B33-85E0-1E9A0393BC2A@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2436573196984032881==" List-Id: --===============2436573196984032881== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable > On 16 Mar 2024, at 09:43, Jonatan Schlag wrot= e: >=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 singl= e way how to check whether a zone exists or not. >> In your proposed patch, you have different ways to check whether a zone ex= ists or not. One is to check if there is a device (e.g. GREEN_DEV), and somet= imes you are checking CONFIG_TYPE. >> I suppose you got your inspiration from here: >> 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 netw= ork=E2=80=9D to make it consistent? >=20 > According to here, there always exists a green network: https://git.ipfire.= org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dsrc/setup/networking.c;h=3D9dd5205e5a1cb= 60c4a0bb003ec2a8348848822f9;hb=3Drefs/heads/next#l22 > So we could just return True. CONFIG_TYPE is a bitmap. Almost at least: GREEN sadly didn=E2=80=99t get a bit for itself. If RED is present (at least when it is an Ethernet connection) it has the 0x0= 4 bit set. If ORANGE exists, 0x02 is set. If BLUE exists 0x01 is set. So checking the bits is a bare minimum. >=20 >> I would also suggest to keep the function name short and rename it to =E2= =80=9Czone_exists=E2=80=9D. Since the function is in =E2=80=9Cnetworks=E2=80= =9D, I think it is pretty clear what this function would do. There is litte c= hance this 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 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 network_have_ORANGE(). > " - "Re: [Patch RFC 04/15] network initscripts: check if the zone in the cu= rrent config exists" Well that depends on how you are looking at it. =E2=80=9Chave_BLUE" I would find a little bit too generic. There is no connec= tion from BLUE to networking. =E2=80=9CZone=E2=80=9D hints heavily at network= ing in our context. >=20 > An ever moving target is hard to hit :-) >=20 > Jonatan >>> On 2 Mar 2024, at 11:09, Jonatan Schlag wro= te: >>> As our Network is quite static a case does the trick >>> Signed-off-by: Jonatan Schlag >>> --- >>> src/initscripts/networking/functions.network | 22 ++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> diff --git a/src/initscripts/networking/functions.network b/src/initscrip= ts/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 --===============2436573196984032881==--