From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel =?utf-8?q?Weism=C3=BCller?= 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: Thu, 26 Nov 2020 15:47:11 +0100 Message-ID: In-Reply-To: <2ADAAE4E-3FF0-4CF9-83AB-D4D69D88917B@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3840830175382350185==" List-Id: --===============3840830175382350185== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi, there is one thing that we didn't talked about... STP and priority must only be activatable if the zone is in bridge mode=20 otherwise it must be grayed out. - Daniel Am 25.11.20 um 21:57 schrieb Michael Tremer: > Hello, > >> On 25 Nov 2020, at 17:00, Leo Hofmann wrote: >> >> Hi Daniel, >> >> thank you very much for the draft & the explanation! >> >> Do you happen to know if there are any other zone-related options that mig= ht 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 on= e place. On the other hand, this whole page is becoming longer and longer and= that simply makes it complicated. > > 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 cloud= providers, 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) o= n the left so that it is only wasting space once. With plenty of zones the ta= ble just becomes unnecessarily wide then. > >> @Michael: I would like to base this new feature on my recently patched zon= econf.cgi. Is this somehow a bad idea? > Well, good question. I have no idea why I didn=E2=80=99t merge it yet. I di= dn=E2=80=99t realise it was ready. I will check if there is enough testing fe= edback already. > > Best, > -Michael > >> Regards >> Leo >> >> Am 23.11.2020 um 16:13 schrieb Daniel Weism=C3=BCller: >>> Hi Leo, >>> >>> that pleases me to hear and I gladly accept your offer. ;-) >>> >>> I quickly made a draft and attached it. As I said it is only a draft so t= here is still plenty of room for improvement. >>> >>> 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. >>> >>> - >>> >>> Daniel >>> >>> Am 21.11.20 um 17:39 schrieb Leo Hofmann: >>>> Hi Daniel, >>>> >>>> a few days ago I finally submitted my patches for zoneconf.cgi and I >>>> would now have time to work on this as well. >>>> >>>> (By the way, I almost forgot, thanks @Michael for reviewing my patches!) >>>> >>>> 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? >>>> >>>> Regards, >>>> Leo >>>> >>>> Am 20.11.2020 um 19:31 schrieb Daniel Weism=C3=BCller: >>>>> OK. ;-) >>>>> >>>>> The first step will be the introduction of the possibility to enable ST= P. >>>>> >>>>> The next step will be the implementation in the webif. >>>>> >>>>> I hope I find someone who can do that. >>>>> >>>>> >>>>> - >>>>> Daniel >>>>> >>>>> 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. >>>>>> >>>>>> Best regards, >>>>>> Fred >>>>>> >>>>>> -----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 >>>>>> >>>>>> Hi, >>>>>> >>>>>>> On 20 Nov 2020, at 06:58, Daniel Weism=C3=BCller >>>>>> wrote: >>>>>>> Hello, >>>>>>> >>>>>>> In my opinion it is sufficient to be able to set these parameters via >>>>>> command line. >>>>>> >>>>>> Why is that? IPFire is a distribution that is supposed to be managed >>>>>> entirely by the web user interface. >>>>>> >>>>>>> It should only be made sure that the settings are persitend and not >>>>>> overwritten by a reboot or the webif. >>>>>> >>>>>> They wont be as they are in /var/ipfire/ethernet/settings. >>>>>> >>>>>> Best, >>>>>> -Michael >>>>>> >>>>>>> - >>>>>>> Daniel >>>>>>> >>>>>>> Am 19.11.2020 um 15:56 schrieb Michael Tremer: >>>>>>>> Hello Daniel, >>>>>>>> >>>>>>>> This patch looks good to me. >>>>>>>> >>>>>>>> I had assumed that we automatically enabled STP on all bridges, but >>>>>> apparently we do not. >>>>>>>> How do we process with this? >>>>>>>> >>>>>>>> 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 >>>>>>>> >>>>>>>>> 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(-) >>>>>>>>> >>>>>>>>> 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")" >>>>>>>>> >>>>>>>>> # The name of the virtual bridge >>>>>>>>> BRIDGE=3D"$(get_value "${ZONE}_DEV")" >>>>>>>>> +STP=3D"$(get_value "${ZONE}_STP")" >>>>>>>>> >>>>>>>>> case "${MODE}" in >>>>>>>>> bridge) >>>>>>>>> @@ -89,7 +90,8 @@ case "${MODE}" in >>>>>>>>> >>>>>>>>> # 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 >>>>>>>>> 2.28.0 >>>>>>>>> >>>>>> >> --===============3840830175382350185==--