Skip to content

realtek: Prepare for multiple SoC types #12541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

oliv3r
Copy link
Contributor

@oliv3r oliv3r commented May 5, 2023

This MR prepares the Makefile for a future where non-switch chips would also be supported.

In doing so, we introduce the 'switch' device type.

@github-actions github-actions bot added the target/realtek pull request/issue for realtek target label May 5, 2023
@oliv3r oliv3r force-pushed the dev/makefile branch 3 times, most recently from 094fcbb to 3ff1abe Compare May 5, 2023 14:37
@oliv3r oliv3r marked this pull request as draft May 5, 2023 16:10
@Borromini
Copy link

Borromini commented May 6, 2023

At least the present SoC families (RTL83xx and RTL93xx) are wholly unfit for routing (weak MIPS SoC and whatnot). In addition, there's little flash (even more so if we like to stick with the dual-partition layout multiple OEMs offer). I only see drawbacks in switching to the router DEVICE_TYPE. It will pull in dnsmasq, but also ppp support which eats flash space. The case for including dnsmasq is a flimsy one. Who has their switches acting as a DHCP server? I'm sure the few people who do can use the image builder or compile their own image.

The realtek target was switched away from the default router target earlier on, and with good reasons if you ask me. I think this is a bad idea.

@oliv3r
Copy link
Contributor Author

oliv3r commented May 7, 2023

At least the present SoC families (RTL83xx and RTL93xx) are wholly unfit for routing (weak MIPS SoC and whatnot). In addition, there's little flash (even more so if we like to stick with the dual-partition layout multiple OEMs offer). I only see drawbacks in switching to the router DEVICE_TYPE. It will pull in dnsmasq, but also ppp support which eats flash space. The case for including dnsmasq is a flimsy one. Who has their switches acting as a DHCP server? I'm sure the few people who do can use the image builder or compile their own image.

The realtek target was switched away from the default router target earlier on, and with good reasons if you ask me. I think this is a bad idea.

Currently, openwrt does not offer the 'switch' type. So a good argument can be made for one.

Secondly, it's all about choice, yeah, and 'they can use the builder' is a very poor form of choice. One can argue of having 2 images, one that's stripped down and in favor of limited flash space (6MiB flashes, and dual firmware support) where a more feature filled device would have 14MiB-ish of space. Btw, my branch has offered both types of images for 6 months already :) stuff that hasn't been merged, sure.

So dnsmasq isn't really that big, is it? And 'who uses their switch as DHCP? Many IT staffers. They get Cisco and HP switches, which all offer DHCP as an option, and some happilly configure it as such. So who is our end-user here?

So then the only point becomes PPP. That is a nasty one I admit, probably because of PPP-OE on the wan port (less likely for actual modems) but even here, 'home-users' may have this switch behind a ISP modem, which does basic firewalling and basic routing, but PPP-OE is handled by the switch :)

Anyway, the 'router' target is far more accurate then the 'basic' target. And that does not mean you have to use it as a router. There is no WAN port configured, it is not 'natural' to set it up as a router, but it is certainly possible. Users choice, freedom and all that.

Also, just because it is poor choice, doesn't mean its impossible. With L3 offloading, performance won't be that horrible, you don't have to route it all with the CPU...

@Borromini
Copy link

Currently, openwrt does not offer the 'switch' type. So a good argument can be made for one.

Honestly? Until this PR I actually thought we had one, but it turns out our hardware just extends the 'basic' device type. I do not know the rationale behind it. It does work, though.

Secondly, it's all about choice, yeah, and 'they can use the builder' is a very poor form of choice.

Well - on one hand, you'te talking about IT staffers. You can safely assume they're skilled at what they do. If they decide to use OpenWrt on their hardware in a production environment, they hopefully know their way around the buildroot or the image builder. On the other hand, you're talking about choice, yet would like to be your preference the default (which is your prerogative, of course). Including extra stuff/settings just for a tiny minority that might use it does not constitute 'choice'. Again: we're talking edge cases here. 'Use the image builder or buildroot to strip X or Y' or 'disable X or Y after installation' is not what I would consider choice.

One can argue of having 2 images, one that's stripped down and in favor of limited flash space (6MiB flashes, and dual firmware support) where a more feature filled device would have 14MiB-ish of space.

I am not immediately aware of other embedded stuff OpenWrt supports that offers multiple images for the exact same device. I think there's a strong point to be made in favour of sticking with the OEM dual partition layout, especially for the less mature realtek subtargets like rtl93xx. Switching back and forth between both firmware slots is very handy, still.

So dnsmasq isn't really that big, is it? And 'who uses their switch as DHCP? Many IT staffers. They get Cisco and HP switches, which all offer DHCP as an option, and some happilly configure it as such. So who is our end-user here?

The FOSS and networking enthusiasts are. Since even rtl83xx is not feature complete, I do not see any IT professionals deploy them with OpenWrt in a production environment, unless you do not need the stuff like LACP and the likes. All my rtl83xx switches do run OpenWrt at home, but for another deployment where I did need LACP, they're still on ZyXEL's OEM firmware, as much as it pains me.

Ask yourself how many people will be weirded out when their network starts behaving badly upon installing OpenWrt with a DHCP server on a switch - switches which default to exactly the opposite behaviour on OEM firmware - and how long it will take them to figure out why they have multiple DHCP servers running. That's bad publicity. Again: people who do need it will know full well how to pull in a DHCP server. Easy peasy. Done in a jiffy.

Anyway, the 'router' target is far more accurate then the 'basic' target. And that does not mean you have to use it as a router. There is no WAN port configured, it is not 'natural' to set it up as a router, but it is certainly possible. Users choice, freedom and all that.

The present defaults are sane if you ask me. Users are free enough to hack together whatever they'd like.

Also, just because it is poor choice, doesn't mean its impossible. With L3 offloading, performance won't be that horrible, you don't have to route it all with the CPU...

Let's not encourage poor choices just because they are possible.

@oliv3r oliv3r force-pushed the dev/makefile branch 2 times, most recently from 2c39a71 to 84b7186 Compare May 7, 2023 13:03
@oliv3r
Copy link
Contributor Author

oliv3r commented May 7, 2023

Currently, openwrt does not offer the 'switch' type. So a good argument can be made for one.

Honestly? Until this PR I actually thought we had one, but it turns out our hardware just extends the 'basic' device type. I do not know the rationale behind it. It does work, though.

I initially had that in my branch for 6 months, not realizing it didn't even work lol.

After writing my earlier comment, I realized, we should have. So I added it as part of this MR :)

Secondly, it's all about choice, yeah, and 'they can use the builder' is a very poor form of choice.

Well - on one hand, you'te talking about IT staffers. You can safely assume they're skilled at what they do. If they decide to use OpenWrt on their hardware in a production environment, they hopefully know their way around the buildroot or the image builder. On the other hand, you're talking about choice, yet would like to be your preference the default (which is your prerogative, of course). Including extra stuff/settings just for a tiny minority that might use it does not constitute 'choice'. Again: we're talking edge cases here. 'Use the image builder or buildroot to strip X or Y' or 'disable X or Y after installation' is not what I would consider choice.

Well we are talking about 'a sensible default for most users' though. E.g. if joe-sixpack downloads openwrt, installs it on his hardware, and what he expects to be there. I think comparing it to 'off-the-shelf' solutions, isn't far-fetched tbh. My 15 year old SoHo 8 port switch offers these things too.

One can argue of having 2 images, one that's stripped down and in favor of limited flash space (6MiB flashes, and dual firmware support) where a more feature filled device would have 14MiB-ish of space.

I am not immediately aware of other embedded stuff OpenWrt supports that offers multiple images for the exact same device. I think there's a strong point to be made in favour of sticking with the OEM dual partition layout, especially for the less mature realtek subtargets like rtl93xx. Switching back and forth between both firmware slots is very handy, still.

afaik, 'off-the-shelf' openwrt doesn't support dual partitioning, does it? Anyway, I'm already offering 2 images, one that fits in the OEM partition layout, one that fits into the whole flash space.

Having said that, the 'oem/vendor' layout will likely move towards the 'tiny' subset, that some routers have (think 4/8MiB devices). I think these switches (with dual-firmware layout) will be restricted herein and offer far less functionality. But that's fine, it's a choice in the end. I think we're lucky in that we don't need a lot of the router/wifi packages which cost a lot of space as well.

So dnsmasq isn't really that big, is it? And 'who uses their switch as DHCP? Many IT staffers. They get Cisco and HP switches, which all offer DHCP as an option, and some happilly configure it as such. So who is our end-user here?

The FOSS and networking enthusiasts are. Since even rtl83xx is not feature complete, I do not see any IT professionals deploy them with OpenWrt in a production environment, unless you do not need the stuff like LACP and the likes. All my rtl83xx switches do run OpenWrt at home, but for another deployment where I did need LACP, they're still on ZyXEL's OEM firmware, as much as it pains me.

Ask yourself how many people will be weirded out when their network starts behaving badly upon installing OpenWrt with a DHCP server on a switch - switches which default to exactly the opposite behaviour on OEM firmware - and how long it will take them to figure out why they have multiple DHCP servers running. That's bad publicity. Again: people who do need it will know full well how to pull in a DHCP server. Easy peasy. Done in a jiffy.

But why would we enable the dhcp server by default? We just want the option to be a click away to turn it on. I wouldn't expect that a dhcp server be running by default handing out IP addresses without user intervention!

Anyway, the 'router' target is far more accurate then the 'basic' target. And that does not mean you have to use it as a router. There is no WAN port configured, it is not 'natural' to set it up as a router, but it is certainly possible. Users choice, freedom and all that.

The present defaults are sane if you ask me. Users are free enough to hack together whatever they'd like.

You do know, that we already add firewalling packages as part of our specific basic + packages, yeah? :) so we're not even using the basic target :p

Also, just because it is poor choice, doesn't mean its impossible. With L3 offloading, performance won't be that horrible, you don't have to route it all with the CPU...

Let's not encourage poor choices just because they are possible.

But that's in the eye of the beholder. Again, if we look at cisco, HP or Jupiter, what they offer 'out of the box' is not an unreasonable default to have in terms of software availability :)

@oliv3r oliv3r marked this pull request as ready for review May 7, 2023 13:48
@github-actions github-actions bot added the build/scripts/tools pull request/issues for build, scripts and tools related changes label May 7, 2023
@@ -1,10 +1,11 @@
# SPDX-License-Identifier: GPL-2.0-only
ARCH:=mips
SUBTARGET:=rtl930x
CPU_TYPE:=24kc
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, dropping this change, it builds and runs; but spewing illegal instruction.

rebuilding with 24kc to confirm this was causing it first. weirdly, the CPU IS being identified as 34kc ...

Copy link
Member

@KanjiMonster KanjiMonster May 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference of 34kc vs 24kc is only relevant for the kernel. For userspace GCC generates identical code for both. So changing this from 24kc to 34kc has no advantage as the built binaries will still behave the same, but has the disadvantage of now requiring a mips_34kc stage 2 builder and its own package set.

So a NAK on this change anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already gone, but I don't think this is true any longer. I'm getting 'Illegal instruction' errors with 34kc, and 24kc works fine. So technically this chip isn't truly 34kc compliant anyway it seems.

Anyway, what triggered me was the following patch: 308-mips32r2_tune.patch, which does some explicit 34kc stuff.

Copy link
Member

@KanjiMonster KanjiMonster May 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is/was more likely that there is no CPU_CFLAGS_34kc defined anymore in include/target.mk, so GCC probably defaulted to mips32r1. It was removed in a11843b.

FEATURES:=ramdisk squashfs
SUBTARGETS:=rtl838x rtl839x rtl930x rtl931x

KERNEL_PATCHVER:=5.15

define Target/Description
Build firmware images for Realtek RTL83xx based boards.
Build firmware images for Realtek RTL83xx/RTL93XX based boards.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using a more generic name for all of the boards? If I add rtd129x, rtd1319 and rtd1619b, the list will be a little bit lengthy ? How about put these SoC specific in the subtarget makefile ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name would be extended to RTL83xx/RTL93xx/RTD1xxx so not too bad. We should put the names somewhere, so users can find it easily. Oth, the subtargets will already list it anyway, so removing it is not a bad idea.

Copy link

@phinexhung phinexhung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good for me to extend RTD1xxx sub targets.

@oliv3r
Copy link
Contributor Author

oliv3r commented May 9, 2023

@Ansuel this has a little global impact as well, so if you can share your 2 cents ...

oliv3r added 4 commits May 11, 2023 17:46
OpenWRT has been happily supporting WiFi routers. However, we are slowly
taking on switches as well, which function slightly different than
routers.

One could argue, a router is an advanced switch. As such, introduce the
more restrictive switch type, so we can have specific packages for
routers.

Also, setting up the 'device_type' to the proper value reads nicer to
boot.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
We can be a bit more descriptive with our naming that faces our users.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Our SoC is more that of a router, then a switch due to its L3 offloading
capabilities. Software such as dnsmasq are certainly not strange to
have, and can often be seen on 'regular' switches as well.

Using it as a main firewall is probably not a good idea, but that is up
to the user to decide, the switches should be able to handle 100Mbit
routing even.

While here, move the device-type to a 'per-target' basis, so we have
room to extend this with non-switch realtek SoC devices.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Having a shared file for switches, allows for easier addition of
non-switch realtek targets.

There's a little cheat here, as 'ethtool' is already in the common
Makefile, as it is known that the upcomming realtek media/NAS devices
also need this.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
@Neustradamus
Copy link

@oliv3r: Have you progressed on this PR?

@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 3, 2025

I think it was stuck a bit on the reivew part wasn't it @Neustradamus

@phinexhung
Copy link

Any suggestions that we can do to let this go further?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build/scripts/tools pull request/issues for build, scripts and tools related changes target/realtek pull request/issue for realtek target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants