mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* net: dsa: Realtek switch drivers
@ 2024-06-14  1:49 Chris Packham
  2024-06-14  6:48 ` Linus Walleij
  2024-07-08 14:23 ` Vladimir Oltean
  0 siblings, 2 replies; 13+ messages in thread
From: Chris Packham @ 2024-06-14  1:49 UTC (permalink / raw)
  To: linus.walleij, alsi, Andrew Lunn, Florian Fainelli, olteanv,
	Marek Behún, ericwouds
  Cc: David Miller, Eric Dumazet, kuba, Paolo Abeni, luizluca,
	justinstitt, rmk+kernel, netdev, linux-kernel

Hi All,

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? If I were to start 
grabbing drivers from openwrt and trying to get them landed would that 
be a problem?

Thanks,
Chris

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-06-14  1:49 net: dsa: Realtek switch drivers Chris Packham
@ 2024-06-14  6:48 ` Linus Walleij
  2024-06-14 21:36   ` Luiz Angelo Daros de Luca
  2024-07-08 14:23 ` Vladimir Oltean
  1 sibling, 1 reply; 13+ messages in thread
From: Linus Walleij @ 2024-06-14  6:48 UTC (permalink / raw)
  To: Chris Packham
  Cc: alsi, Andrew Lunn, Florian Fainelli, olteanv, Marek Behún,
	ericwouds, David Miller, Eric Dumazet, kuba, Paolo Abeni,
	luizluca, justinstitt, rmk+kernel, netdev, linux-kernel

On Fri, Jun 14, 2024 at 3:49 AM Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:

> 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.

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.

But maybe I missed something?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-06-14  6:48 ` Linus Walleij
@ 2024-06-14 21:36   ` Luiz Angelo Daros de Luca
  2024-06-16 21:14     ` Chris Packham
  2024-07-02  2:09     ` Chris Packham
  0 siblings, 2 replies; 13+ messages in thread
From: Luiz Angelo Daros de Luca @ 2024-06-14 21:36 UTC (permalink / raw)
  To: Linus Walleij, Chris Packham
  Cc: alsi, Andrew Lunn, Florian Fainelli, olteanv, Marek Behún,
	ericwouds, David Miller, Eric Dumazet, kuba, Paolo Abeni,
	justinstitt, rmk+kernel, netdev, linux-kernel

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. 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. 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.

> Yours,
> Linus Walleij


Regards,

Luiz

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-06-14 21:36   ` Luiz Angelo Daros de Luca
@ 2024-06-16 21:14     ` Chris Packham
  2024-07-02  2:09     ` Chris Packham
  1 sibling, 0 replies; 13+ messages in thread
From: Chris Packham @ 2024-06-16 21:14 UTC (permalink / raw)
  To: Luiz Angelo Daros de Luca, Linus Walleij
  Cc: alsi, Andrew Lunn, Florian Fainelli, olteanv, Marek Behún,
	ericwouds, David Miller, Eric Dumazet, kuba, Paolo Abeni,
	justinstitt, rmk+kernel, netdev, linux-kernel


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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-06-14 21:36   ` Luiz Angelo Daros de Luca
  2024-06-16 21:14     ` Chris Packham
@ 2024-07-02  2:09     ` Chris Packham
  2024-07-02  2:50       ` Andrew Lunn
  2024-07-03 20:31       ` Luiz Angelo Daros de Luca
  1 sibling, 2 replies; 13+ messages in thread
From: Chris Packham @ 2024-07-02  2:09 UTC (permalink / raw)
  To: Luiz Angelo Daros de Luca, Linus Walleij
  Cc: alsi, Andrew Lunn, Florian Fainelli, olteanv, Marek Behún,
	ericwouds, David Miller, Eric Dumazet, kuba, Paolo Abeni,
	justinstitt, rmk+kernel, netdev, linux-kernel, sander


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. 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. 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.

So now got access to the Realtek docs and I've been pouring over them 
and the openwrt code (I'm avoiding looking at the Realtek SDK for now, 
just to make sure I don't submit something I don't have the rights to).

If someone were to look at the block diagram in the brief datasheet 
they'd probably come away with the impression that it very much fits the 
DSA model. There's a SoC portion with the CPU, peripherals and a "NIC". 
That NIC is connected to the CPU MAC in the switch block. All seems like 
a pretty standard DSA type design and that's what the openwrt code 
implements a ethernet/rtl838x_eth.c driver for the Ethernet NIC and a 
dsa/rtl83xx driver for the DSA switch.

But when you start digging into the detail you find that the registers 
for the NIC are scattered through the address space for the switch. Same 
for the MDIO related registers. There is a more detailed block diagram 
in the CPU and Peripherals datasheet that shows the NIC and switch as 
part of the same IP block. The openwrt implementation does things that I 
think would be frowned upon upstream like calling from the Ethernet 
driver into the switch driver to access registers.

This leads me to conclude that what Realtek call the "NIC" is actually 
just the DMA interface for packets sent from or trapped to the CPU. 
Rather than trying to make this fit the DSA model I should be looking at 
using switchdev directly (I can still probably leverage a lot of code 
from the openwrt drivers because the switch tables are the same either way).

What would I be loosing if I don't use the DSA infrastructure? I got 
kind of hung up at the point where it really wanted a CPU port and I 
just couldn't provide a nice discrete NIC.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-07-02  2:09     ` Chris Packham
@ 2024-07-02  2:50       ` Andrew Lunn
  2024-07-03  2:39         ` Chris Packham
  2024-07-03 20:31       ` Luiz Angelo Daros de Luca
  1 sibling, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2024-07-02  2:50 UTC (permalink / raw)
  To: Chris Packham
  Cc: Luiz Angelo Daros de Luca, Linus Walleij, alsi, Florian Fainelli,
	olteanv, Marek Behún, ericwouds, David Miller, Eric Dumazet,
	kuba, Paolo Abeni, justinstitt, rmk+kernel, netdev, linux-kernel,
	sander

> What would I be loosing if I don't use the DSA infrastructure? I got kind of
> hung up at the point where it really wanted a CPU port and I just couldn't
> provide a nice discrete NIC.
 
DSA gives you a wrapper which handles some common stuff, which you
will end up implementing if you do a pure switchdev driver. Mostly
translating netdev to port index. The tagging part of DSA does not
apply if you have DMA per user port, so you don't loose anything
there.  I guess you cannot cascade multiple switches, so the D in DSA
also does not apply. You do lose out on tcpdump support, since you
don't have a conduit interface which all traffic goes through.

But you should not try to impose DSA if it does not fit. There are
'simple' pure switchdev drivers, you don't need to try to understand
the mellanox code. Look at the microchip drivers for something to
copy.

	Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-07-02  2:50       ` Andrew Lunn
@ 2024-07-03  2:39         ` Chris Packham
  2024-07-03  3:39           ` Andrew Lunn
  2024-07-03  7:59           ` Linus Walleij
  0 siblings, 2 replies; 13+ messages in thread
From: Chris Packham @ 2024-07-03  2:39 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Luiz Angelo Daros de Luca, Linus Walleij, alsi, Florian Fainelli,
	olteanv, Marek Behún, ericwouds, David Miller, Eric Dumazet,
	kuba, Paolo Abeni, justinstitt, rmk+kernel, netdev, linux-kernel,
	sander


On 2/07/24 14:50, Andrew Lunn wrote:
>> What would I be loosing if I don't use the DSA infrastructure? I got kind of
>> hung up at the point where it really wanted a CPU port and I just couldn't
>> provide a nice discrete NIC.
>   
> DSA gives you a wrapper which handles some common stuff, which you
> will end up implementing if you do a pure switchdev driver. Mostly
> translating netdev to port index. The tagging part of DSA does not
> apply if you have DMA per user port, so you don't loose anything
> there.  I guess you cannot cascade multiple switches, so the D in DSA
> also does not apply. You do lose out on tcpdump support, since you
> don't have a conduit interface which all traffic goes through.
>
> But you should not try to impose DSA if it does not fit. There are
> 'simple' pure switchdev drivers, you don't need to try to understand
> the mellanox code. Look at the microchip drivers for something to
> copy.
One reason for using DSAI've just found is that in theory the RTL930x 
supports a CPU disabled mode where you do connect it to an external CPU 
and the data travels over SGMII like you'd get with a traditional DSA 
design. It's not a mode I'm planning on supporting anytime soon but it 
might come up when I get round to submitting some patches.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-07-03  2:39         ` Chris Packham
@ 2024-07-03  3:39           ` Andrew Lunn
  2024-07-08 14:14             ` Vladimir Oltean
  2024-07-03  7:59           ` Linus Walleij
  1 sibling, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2024-07-03  3:39 UTC (permalink / raw)
  To: Chris Packham
  Cc: Luiz Angelo Daros de Luca, Linus Walleij, alsi, Florian Fainelli,
	olteanv, Marek Behún, ericwouds, David Miller, Eric Dumazet,
	kuba, Paolo Abeni, justinstitt, rmk+kernel, netdev, linux-kernel,
	sander

> One reason for using DSAI've just found is that in theory the RTL930x
> supports a CPU disabled mode where you do connect it to an external CPU and
> the data travels over SGMII like you'd get with a traditional DSA design.
> It's not a mode I'm planning on supporting anytime soon but it might come up
> when I get round to submitting some patches.

You might want to look at ocelot, which is both a pure switchdev and a
DSA driver. Its design is not great because it became dual later in
its life. I suspect it would be designed differently if this has been
considered at the beginning.

	   Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-07-03  2:39         ` Chris Packham
  2024-07-03  3:39           ` Andrew Lunn
@ 2024-07-03  7:59           ` Linus Walleij
  1 sibling, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2024-07-03  7:59 UTC (permalink / raw)
  To: Chris Packham
  Cc: Andrew Lunn, Luiz Angelo Daros de Luca, alsi, Florian Fainelli,
	olteanv, Marek Behún, ericwouds, David Miller, Eric Dumazet,
	kuba, Paolo Abeni, justinstitt, rmk+kernel, netdev, linux-kernel,
	sander

On Wed, Jul 3, 2024 at 4:39 AM Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:

> I've just found is that in theory the RTL930x
> supports a CPU disabled mode where you do connect it to an external CPU
> and the data travels over SGMII like you'd get with a traditional DSA
> design.

The Vitesse VSC73xx has this as well, it has an internal Intel 8051 (!)
CPU so the switch can be used stand-alone with a smallish PROM for
a random switchbox, or it can be used over SPI or memory-mapped from
another CPU as a DSA switch.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-07-02  2:09     ` Chris Packham
  2024-07-02  2:50       ` Andrew Lunn
@ 2024-07-03 20:31       ` Luiz Angelo Daros de Luca
  1 sibling, 0 replies; 13+ messages in thread
From: Luiz Angelo Daros de Luca @ 2024-07-03 20:31 UTC (permalink / raw)
  To: Chris Packham
  Cc: Linus Walleij, alsi, Andrew Lunn, Florian Fainelli, olteanv,
	Marek Behún, ericwouds, David Miller, Eric Dumazet, kuba,
	Paolo Abeni, justinstitt, rmk+kernel, netdev, linux-kernel,
	sander

Em seg., 1 de jul. de 2024, 23:09, Chris Packham
<chris.packham@alliedtelesis.co.nz> escreveu:
>
>
> 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. 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. 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.
>
> So now got access to the Realtek docs and I've been pouring over them
> and the openwrt code (I'm avoiding looking at the Realtek SDK for now,
> just to make sure I don't submit something I don't have the rights to).
>
> If someone were to look at the block diagram in the brief datasheet
> they'd probably come away with the impression that it very much fits the
> DSA model. There's a SoC portion with the CPU, peripherals and a "NIC".
> That NIC is connected to the CPU MAC in the switch block. All seems like
> a pretty standard DSA type design and that's what the openwrt code
> implements a ethernet/rtl838x_eth.c driver for the Ethernet NIC and a
> dsa/rtl83xx driver for the DSA switch.
>
> But when you start digging into the detail you find that the registers
> for the NIC are scattered through the address space for the switch. Same
> for the MDIO related registers. There is a more detailed block diagram
> in the CPU and Peripherals datasheet that shows the NIC and switch as
> part of the same IP block. The openwrt implementation does things that I
> think would be frowned upon upstream like calling from the Ethernet
> driver into the switch driver to access registers.

Wouldn't that be a case for Multi-Function Device driver? From docs,
"A typical MFD can be:
- A mixed signal ASIC on an external bus, sometimes a PMIC (Power
Management Integrated Circuit) that is manufactured in a lower
technology node (rough silicon) that handles analog drivers for things
like audio amplifiers, LED drivers, level shifters, PHY (physical
interfaces to things like USB or ethernet), regulators etc.
- A range of memory registers containing "miscellaneous system
registers" also known as a system controller "syscon" or any other
memory range containing a mix of unrelated hardware devices."

Vladimir Oltean was the first to suggest to me the use of MFD for DSA
switches as they commonly need two drivers: DSA and the user mii bus
driver. The Realtek SoC just seems to make it more needed as we have
other functions registers scattered.
https://lore.kernel.org/netdev/20231211143513.n6ms3dlp6rrcqya6@skbuf/

The OpenWrt driver does work and I think it can be incrementally
improved up to upstream status. It is a bit messy but you could start
untangling it there, trying to decouple (the non-existing) tagger,
ethernet, phy and DSA drivers (and whatever else shares the same
register space). The upstream realtek drivers use a common
code+subdrivers model while the Realtek SoC DSA driver in OpenWrt uses
a single driver with lots of ifs inside each function. I don't know if
they can be merged into a single driver tree, sharing as much as
possible, or Realtek SoC will require a completely duplicated tree.

> This leads me to conclude that what Realtek call the "NIC" is actually
> just the DMA interface for packets sent from or trapped to the CPU.
> Rather than trying to make this fit the DSA model I should be looking at
> using switchdev directly (I can still probably leverage a lot of code
> from the openwrt drivers because the switch tables are the same either way).
>
> What would I be loosing if I don't use the DSA infrastructure? I got
> kind of hung up at the point where it really wanted a CPU port and I
> just couldn't provide a nice discrete NIC.

---
     Luiz Angelo Daros de Luca
            luizluca@gmail.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-07-03  3:39           ` Andrew Lunn
@ 2024-07-08 14:14             ` Vladimir Oltean
  0 siblings, 0 replies; 13+ messages in thread
From: Vladimir Oltean @ 2024-07-08 14:14 UTC (permalink / raw)
  To: Chris Packham
  Cc: Andrew Lunn, Luiz Angelo Daros de Luca, Linus Walleij, alsi,
	Florian Fainelli, Marek Behún, ericwouds, David Miller,
	Eric Dumazet, kuba, Paolo Abeni, justinstitt, rmk+kernel, netdev,
	linux-kernel, sander

On Wed, Jul 03, 2024 at 05:39:49AM +0200, Andrew Lunn wrote:
> > One reason for using DSAI've just found is that in theory the RTL930x
> > supports a CPU disabled mode where you do connect it to an external CPU and
> > the data travels over SGMII like you'd get with a traditional DSA design.
> > It's not a mode I'm planning on supporting anytime soon but it might come up
> > when I get round to submitting some patches.
> 
> You might want to look at ocelot, which is both a pure switchdev and a
> DSA driver. Its design is not great because it became dual later in
> its life. I suspect it would be designed differently if this has been
> considered at the beginning.
> 
> 	   Andrew

Another thing to consider is that you might want to join efforts with
Bootlin who had a similar (and still unresolved) issue with ipqess/qca8k.
See some of the feedback that they received.
https://lore.kernel.org/netdev/20231116215645.3ex4yp36hrrm4uti@skbuf/

In short, I am in principle in favor of extracting core stuff out of
DSA into a more generic 'eth switch' library which is frontend-agnostic
(this is also what the TODO in Documentation/networking/dsa/dsa.rst is
really about, despite not being very well described). That new framework
would essentially be the things that DSA does well, except for tagging
packets, handling the conduit interface, etc. I just don't have the time
(or incentive, sadly) to do it.

The most advanced switchdev drivers like mlxsw will probably not benefit
from it, because they have their own highly specific use cases, although
most of the rest should, since there is a lot of boilerplate which could
go to common code, and which nobody really likes to re-re-re-write or
re-re-re-review.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-06-14  1:49 net: dsa: Realtek switch drivers Chris Packham
  2024-06-14  6:48 ` Linus Walleij
@ 2024-07-08 14:23 ` Vladimir Oltean
  2024-07-08 20:42   ` Chris Packham
  1 sibling, 1 reply; 13+ messages in thread
From: Vladimir Oltean @ 2024-07-08 14:23 UTC (permalink / raw)
  To: Chris Packham
  Cc: linus.walleij, alsi, Andrew Lunn, Florian Fainelli,
	Marek Behún, ericwouds, David Miller, Eric Dumazet, kuba,
	Paolo Abeni, luizluca, justinstitt, rmk+kernel, netdev,
	linux-kernel

On Fri, Jun 14, 2024 at 01:49:35AM +0000, Chris Packham wrote:
> Hi All,
> 
> 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? If I were to start 
> grabbing drivers from openwrt and trying to get them landed would that 
> be a problem?
> 
> Thanks,
> Chris

What do you mean by the L3 switching functionality, exactly? Offloading
of inter-VLAN routing? Or routing offload in general?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: net: dsa: Realtek switch drivers
  2024-07-08 14:23 ` Vladimir Oltean
@ 2024-07-08 20:42   ` Chris Packham
  0 siblings, 0 replies; 13+ messages in thread
From: Chris Packham @ 2024-07-08 20:42 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linus.walleij, alsi, Andrew Lunn, Florian Fainelli,
	Marek Behún, ericwouds, David Miller, Eric Dumazet, kuba,
	Paolo Abeni, luizluca, justinstitt, rmk+kernel, netdev,
	linux-kernel

Hi Vladmir,

On 9/07/24 02:23, Vladimir Oltean wrote:
> On Fri, Jun 14, 2024 at 01:49:35AM +0000, Chris Packham wrote:
>> Hi All,
>>
>> 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? If I were to start
>> grabbing drivers from openwrt and trying to get them landed would that
>> be a problem?
>>
>> Thanks,
>> Chris
> What do you mean by the L3 switching functionality, exactly? Offloading
> of inter-VLAN routing? Or routing offload in general?

Offloading of inter-VLAN routing. There's also some multicast and qos 
capability too.

I gather the general idea is to provide mechanisms for the hardware to 
accelerate parts of the forwarding plane. I haven't got into the details 
of that yet, my first set of requirements is just around basic L2 
switching which I think is pretty well supported at this point.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-07-08 20:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-14  1:49 net: dsa: Realtek switch drivers Chris Packham
2024-06-14  6:48 ` Linus Walleij
2024-06-14 21:36   ` Luiz Angelo Daros de Luca
2024-06-16 21:14     ` Chris Packham
2024-07-02  2:09     ` Chris Packham
2024-07-02  2:50       ` Andrew Lunn
2024-07-03  2:39         ` Chris Packham
2024-07-03  3:39           ` Andrew Lunn
2024-07-08 14:14             ` Vladimir Oltean
2024-07-03  7:59           ` Linus Walleij
2024-07-03 20:31       ` Luiz Angelo Daros de Luca
2024-07-08 14:23 ` Vladimir Oltean
2024-07-08 20:42   ` Chris Packham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®