From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonatan Schlag To: development@lists.ipfire.org Subject: Re: [PATCH] Core 152: the script "network-hotplug-bridges" now reads the variable ${ZONE}_STP from /var/ipfire/ethernet/settings so that STP can be turned on and off for each bridge Date: Fri, 27 Nov 2020 09:34:46 +0100 Message-ID: <8AFEC96C-5D6F-498D-A445-193CBDB623E8@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3285847883734662243==" List-Id: --===============3285847883734662243== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi, Sorry for jumping late into this conversation, time is rare in these days .... I will try to bring in my thoughts, but maybe they are not well structured. > Am 26.11.2020 um 15:47 schrieb Daniel Weism=C3=BCller : >=20 > =EF=BB=BFHi, >=20 > there is one thing that we didn't talked about... >=20 > STP and priority must only be activatable if the zone is in bridge mode oth= erwise it must be grayed out. >=20 Shouldn=E2=80=99t we just not display these fields, if they do not matter? >=20 > - >=20 > Daniel >=20 >=20 >=20 >> Am 25.11.20 um 21:57 schrieb Michael Tremer: >> Hello, >>=20 >>>> On 25 Nov 2020, at 17:00, Leo Hofmann wrote: >>>=20 >>> Hi Daniel, >>>=20 >>> thank you very much for the draft & the explanation! >>>=20 >>> Do you happen to know if there are any other zone-related options that mi= ght be added in the future? >>> If this is a possibility, I think we should add a second table. So we don= 't clutter the NIC assignment with unrelated options. Did I mention that it is very nice, when you wrote what I think so I don=E2= =80=99t have to write it again =F0=9F=98=89. +1 For a second structure >> Good question. On one hand it is good to have things that go together in o= ne place. On the other hand, this whole page is becoming longer and longer an= d that simply makes it complicated. Definitely to complicated. Already right now. >>=20 >> The only thing I can think of is MTU. We currently have no UI to set that,= but it has never been asked for. We set it automatically on some of the clou= d providers, but that is it. >>=20 >>> I took up your and Michael's suggestions and created a quick HTML demo. >> Looks good :) >>=20 Here I don=E2=80=99t think so, because more zones (4) will make it impossible= to display this on very small displays. Why not creating a table for every z= one and putting them among each other.=20 >>> This new table could be placed below the NIC assignment table. What do yo= u think? >> Call the checkboxes =E2=80=9CEnable=E2=80=9D, because that is what they do. >>=20 >> I would also suggest to have the labels (e.g. =E2=80=9CPriority=E2=80=9D) = on the left so that it is only wasting space once. With plenty of zones the t= able just becomes unnecessarily wide then. >>=20 >>> @Michael: I would like to base this new feature on my recently patched zo= neconf.cgi. Is this somehow a bad idea? >> Well, good question. I have no idea why I didn=E2=80=99t merge it yet. I d= idn=E2=80=99t realise it was ready. I will check if there is enough testing f= eedback already. >>=20 >> Best, >> -Michael >>=20 >>> Regards >>> Leo >>>=20 >>> Am 23.11.2020 um 16:13 schrieb Daniel Weism=C3=BCller: >>>> Hi Leo, >>>>=20 >>>> that pleases me to hear and I gladly accept your offer. ;-) >>>>=20 >>>> I quickly made a draft and attached it. As I said it is only a draft so = there is still plenty of room for improvement. >>>>=20 >>>> The checkbox switches the variable named ${ZONE}_STP to 0 or 1. >>>> The input field fills the variable named ${ZONE}_STP_PRIORITY. >>>> Here must a number between 1 and 65535 inserted. >>>>=20 >>>> - >>>>=20 >>>> Daniel >>>>=20 >>>> Am 21.11.20 um 17:39 schrieb Leo Hofmann: >>>>> Hi Daniel, >>>>>=20 >>>>> a few days ago I finally submitted my patches for zoneconf.cgi and I >>>>> would now have time to work on this as well. Thank you for submitting these patches. It is enjoyable to read good code. >>>>>=20 >>>>> (By the way, I almost forgot, thanks @Michael for reviewing my patches!) >>>>>=20 >>>>> If you want me to take this on, it would be very helpful if you could >>>>> summarize how this should work. For example, which config parameters >>>>> need to be modified. Perhaps you could even paint a simple GUI mock-up >>>>> like you did last time? >>>>>=20 >>>>> Regards, >>>>> Leo >>>>>=20 >>>>> Am 20.11.2020 um 19:31 schrieb Daniel Weism=C3=BCller: >>>>>> OK. ;-) >>>>>>=20 >>>>>> The first step will be the introduction of the possibility to enable S= TP. >>>>>>=20 >>>>>> The next step will be the implementation in the webif. >>>>>>=20 >>>>>> I hope I find someone who can do that. >>>>>>=20 >>>>>>=20 >>>>>> - >>>>>> Daniel >>>>>>=20 >>>>>> Am 20.11.2020 um 16:18 schrieb Kienker, Fred: >>>>>>> I'm with Michael on this one. If it deserves to be in IPFire, it >>>>>>> deserves to be on the web interface. Don't created exceptions which a= re >>>>>>> only available from a command line. >>>>>>>=20 >>>>>>> Best regards, >>>>>>> Fred >>>>>>>=20 >>>>>>> -----Original Message----- >>>>>>> From: Michael Tremer >>>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>>> To: Daniel Weism=C3=BCller >>>>>>> Cc: development(a)lists.ipfire.org >>>>>>> Subject: Re: [PATCH] Core 152: the script "network-hotplug-bridges" n= ow >>>>>>> reads the variable ${ZONE}_STP from /var/ipfire/ethernet/settings so >>>>>>> that STP can be turned on and off for each bridge >>>>>>>=20 >>>>>>> Hi, >>>>>>>=20 >>>>>>>> On 20 Nov 2020, at 06:58, Daniel Weism=C3=BCller >>>>>>> wrote: >>>>>>>> Hello, >>>>>>>>=20 >>>>>>>> In my opinion it is sufficient to be able to set these parameters via >>>>>>> command line. >>>>>>>=20 >>>>>>> Why is that? IPFire is a distribution that is supposed to be managed >>>>>>> entirely by the web user interface. And here I was wondering a lot. A lot of options are only available via comma= nd line. The setup command is entirely based on the command line. So where do= we draw a border what should be available from the webinterface? These and = some other questions I have belonging to the webinterface, are some how funda= mental, so that I like to discuss these in the telco ... Best Jonatan >>>>>>>=20 >>>>>>>> It should only be made sure that the settings are persitend and not >>>>>>> overwritten by a reboot or the webif. >>>>>>>=20 >>>>>>> They wont be as they are in /var/ipfire/ethernet/settings. >>>>>>>=20 >>>>>>> Best, >>>>>>> -Michael >>>>>>>=20 >>>>>>>> - >>>>>>>> Daniel >>>>>>>>=20 >>>>>>>> Am 19.11.2020 um 15:56 schrieb Michael Tremer: >>>>>>>>> Hello Daniel, >>>>>>>>>=20 >>>>>>>>> This patch looks good to me. >>>>>>>>>=20 >>>>>>>>> I had assumed that we automatically enabled STP on all bridges, but >>>>>>> apparently we do not. >>>>>>>>> How do we process with this? >>>>>>>>>=20 >>>>>>>>> I suppose it is not the most user-friendly way to ask the user to >>>>>>> edit the configuration file. This either must be documented somewhere= or >>>>>>> the zoneconfig.cgi script needs to be extended to allow enabling STP. >>>>>>>>> Does anyone want to be able to change any STP parameters like >>>>>>> priority or cost of the ports? >>>>>>>>> Best, >>>>>>>>> -Michael >>>>>>>>>=20 >>>>>>>>>> On 19 Nov 2020, at 13:18, Daniel Weism=C3=BCller >>>>>>> wrote: >>>>>>>>>> Signed-off-by: Daniel Weism=C3=BCller >>>>>>>>>> --- >>>>>>>>>> config/udev/network-hotplug-bridges | 4 +++- >>>>>>>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>>>>>>>=20 >>>>>>>>>> diff --git a/config/udev/network-hotplug-bridges >>>>>>> b/config/udev/network-hotplug-bridges >>>>>>>>>> index 33d6d65ba..7431377bb 100644 >>>>>>>>>> --- a/config/udev/network-hotplug-bridges >>>>>>>>>> +++ b/config/udev/network-hotplug-bridges >>>>>>>>>> @@ -81,6 +81,7 @@ MODE=3D"$(get_value "${ZONE}_MODE")" >>>>>>>>>>=20 >>>>>>>>>> # The name of the virtual bridge >>>>>>>>>> BRIDGE=3D"$(get_value "${ZONE}_DEV")" >>>>>>>>>> +STP=3D"$(get_value "${ZONE}_STP")" >>>>>>>>>>=20 >>>>>>>>>> case "${MODE}" in >>>>>>>>>> bridge) >>>>>>>>>> @@ -89,7 +90,8 @@ case "${MODE}" in >>>>>>>>>>=20 >>>>>>>>>> # We need to create the bridge if it doesn't exist, yet >>>>>>>>>> if [ ! -d "/sys/class/net/${BRIDGE}" ]; then >>>>>>>>>> - ip link add "${BRIDGE}" address "${ADDRESS}" type >>>>>>> bridge >>>>>>>>>> + ip link add "${BRIDGE}" address "${ADDRESS}" type >>>>>>> bridge \ >>>>>>>>>> + $([ "${STP}" =3D "on" ] && echo "stp_state 1") >>>>>>>>>> #ip link set "${BRIDGE}" up >>>>>>>>>> fi >>>>>>>>>>=20 >>>>>>>>>> --=20 >>>>>>>>>> 2.28.0 >>>>>>>>>>=20 >>>>>>>=20 >>> --===============3285847883734662243==--