Re: net: dsa: Realtek switch drivers

[Chris Packham]

On 15/06/24 09:36, Luiz Angelo Daros de Luca wrote:
> Hello Chris and Linus,
>
>>> I'm starting to look at some L2/L3 switches with Realtek silicon. I see
>>> in the upstream kernel there are dsa drivers for a couple of simple L2
>>> switches. While openwrt has support for a lot of the more advanced
>>> silicon. I'm just wondering if there is a particular reason no-one has
>>> attempted to upstream support for these switches?
>> It began with the RTL8366RB ("RTL8366 revision B") which I think is
>> equivalent to RTL8366S as well, but have not been able to test.
>>
>> Then Luiz and Alvin jumped in and fixed up the RTL8365MB family.
>>
>> So the support is pretty much what is stated in the DT bindings
>> in Documentation/devicetree/bindings/net/dsa/realtek.yaml:
>>
>> properties:
>>    compatible:
>>      enum:
>>        - realtek,rtl8365mb
>>        - realtek,rtl8366rb
>>      description: |
>>        realtek,rtl8365mb:
>>          Use with models RTL8363NB, RTL8363NB-VB, RTL8363SC, RTL8363SC-VB,
>>          RTL8364NB, RTL8364NB-VB, RTL8365MB, RTL8366SC, RTL8367RB-VB, RTL8367S,
>>          RTL8367SB, RTL8370MB, RTL8310SR
>>        realtek,rtl8366rb:
>>          Use with models RTL8366RB, RTL8366S
>>
>> It may look like just RTL8365 and RTL8366 on the surface but the sub-version
>> is detected at runtime.
>>
>>> If I were to start
>>> grabbing drivers from openwrt and trying to get them landed would that
>>> be a problem?
>> I think the base is there, when I started with RTL8366RB it was pretty
>> uphill but the kernel DSA experts (Vladimir & Andrew especially) are super
>> helpful so eventually we have arrived at something that works reasonably.
>>
>> The RTL8356MB-family driver is more advanced and has a lot more features,
>> notably it supports all known RTL8367 variants.
> I played with RTL8367R. It mostly works with rtl8365mb driver but I
> wasn't able to enable the CPU tagging. Although
>
>> The upstream OpenWrt in target/linux/generic/files/drivers/net/phy
>> has the following drivers for the old switchdev:
>> -rw-r--r--. 1 linus linus 25382 Jun  7 21:44 rtl8306.c
>> -rw-r--r--. 1 linus linus 40268 Jun  7 21:44 rtl8366rb.c
>> -rw-r--r--. 1 linus linus 33681 Jun  7 21:44 rtl8366s.c
>> -rw-r--r--. 1 linus linus 36324 Jun  7 21:44 rtl8366_smi.c
>> -rw-r--r--. 1 linus linus  4838 Jun  7 21:44 rtl8366_smi.h
>> -rw-r--r--. 1 linus linus 58021 Jun 12 18:50 rtl8367b.c
>> -rw-r--r--. 1 linus linus 59612 Jun 12 18:50 rtl8367.c
>>
>> As far as I can tell we cover all but RTL8306 with the current in-tree
>> drivers, the only reason these are still in OpenWrt would be that some
>> boards are not migrated to DSA.
> These drivers you listed are mostly found in old or low spec devices.
> There is little incentive to invest too much time to migrate them. For
> rtl8365mb, it still lacks support for vlan and forwarding offload. So,
> the swconfig driver still makes sense.
> There is also a performance problem with checksum offloading. These
> switches are used with non-realtek SoC, which might lead to:
>
> "Checksum offload should work with category 1 and 2 taggers when the
> DSA conduit driver declares NETIF_F_HW_CSUM in vlan_features and looks
> at csum_start and csum_offset. For those cases, DSA will shift the
> checksum start and offset by the tag size. If the DSA conduit driver
> still uses the legacy NETIF_F_IP_CSUM or NETIF_F_IPV6_CSUM in
> vlan_features, the offload might only work if the offload hardware
> already expects that specific tag (perhaps due to matching vendors).
> DSA user ports inherit those flags from the conduit, and it is up to
> the driver to correctly fall back to software checksum when the IP
> header is not where the hardware expects. If that check is
> ineffective, the packets might go to the network without a proper
> checksum (the checksum field will have the pseudo IP header sum). For
> category 3, when the offload hardware does not already expect the
> switch tag in use, the checksum must be calculated before any tag is
> inserted (i.e. inside the tagger). Otherwise, the DSA conduit would
> include the tail tag in the (software or hardware) checksum
> calculation. Then, when the tag gets stripped by the switch during
> transmission, it will leave an incorrect IP checksum in place."
> See: https://docs.kernel.org/networking/dsa/dsa.html
>
>> But maybe I missed something?
> I guess Chris is talking about the realtek target that uses Realtek
> SoC (target/linux/realtek/files-5.15/). That is a completely different
> beast.

Correct. My interest right now is around the integrated RTL930x and 
possibly the RTL838x. These have integrated mip32 CPUs. There's a 
collection of peripheral drivers in openwrt that would need upstreaming 
to just support these as general CPUs. I think that's generally doable 
but there would be a fair bit of back and forth to get the drivers and 
dtbindings in shape.

> Although it might share some (or a lot) logic with current
> upstream drivers, it is way more complex. It might require a
> multi-function device driver.

For the dsa part I'd probably want to start by adding a realtek-mmio.c 
next to realtek-smi.c and realtek-mdio.c although maybe regmap would 
make that redundant. As Luiz mentions the newer silicon is much more 
capable than the RTL830x/RTL836x. So there would be a lot of work 
bringing in new silicon families with more capabilities.

I don't think we'd need a MFD at least not with the integrated 
peripherals that are accessible with MMIO.

> Anyway, the current realtek SoC/target
> drivers need some love, like using regmap, implement functions using
> an abstraction layer (and not if model a inside the code), get rid of
> all magic numbers and replace them with meaningful macros, create a
> proper tagger (and not translate a generic one just before forwarding
> it). In OpenWrt, a code that gets things done might be acceptable but
> the upstream kernel requires something more maintainable. So, if you
> want to upstream those drivers, you can start by improving them in the
> openwrt.

I'll see what I can do. I'm still getting up to speed with developing on 
openwrt. There do seem to be a lot of patches that are just back-ports 
so maybe I can just point it at a bleeding edge kernel to apply what 
they have on-top.

I personally prefer going upstream first but as you say that raises the 
bar a bit higher.