From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer 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: Wed, 25 Nov 2020 20:57:23 +0000 Message-ID: <2ADAAE4E-3FF0-4CF9-83AB-D4D69D88917B@ipfire.org> In-Reply-To: <4bd57771-dfe2-032f-5230-09f96e5ad81d@leo-andres.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2721731080127545747==" List-Id: --===============2721731080127545747== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, > 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 migh= t 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. Good question. On one hand it is good to have things that go together in one = place. On the other hand, this whole page is becoming longer and longer and t= hat simply makes it complicated. The only thing I can think of is MTU. We currently have no UI to set that, bu= t it has never been asked for. We set it automatically on some of the cloud p= roviders, but that is it. > I took up your and Michael's suggestions and created a quick HTML demo. Looks good :) > This new table could be placed below the NIC assignment table. What do you = think? Call the checkboxes =E2=80=9CEnable=E2=80=9D, because that is what they do. 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 tabl= e just becomes unnecessarily wide then. > @Michael: I would like to base this new feature on my recently patched zone= conf.cgi. Is this somehow a bad idea? Well, good question. I have no idea why I didn=E2=80=99t merge it yet. I didn= =E2=80=99t realise it was ready. I will check if there is enough testing feed= back already. Best, -Michael > 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 th= ere 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. >>>=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 STP. >>>>=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 are >>>>> 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" now >>>>> 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. >>>>>=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 >>>>>=20 >>>>=20 > --===============2721731080127545747==--