Message-ID: <20210624163542.5b6d87ee@ktm>
Date:   Thu, 24 Jun 2021 16:35:42 +0200
From:   Lukasz Majewski <lukma@...x.de>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Madalin Bucur <madalin.bucur@....nxp.com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Joakim Zhang <qiangqing.zhang@....com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>, netdev@...r.kernel.org,
        Arnd Bergmann <arnd@...db.de>,
        Mark Einon <mark.einon@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC 2/3] net: Provide switchdev driver for NXP's More Than IP
 L2 switch

Hi Andrew,

> > I'm not sure if the imx28 switch is similar to one from TI (cpsw-3g)
> > - it looks to me that the bypass mode for both seems to be very
> > different. For example, on NXP when switch is disabled we need to
> > handle two DMA[01]. When it is enabled, only one is used. The
> > approach with two DMAs is best handled with FEC driver
> > instantiation.  
> 
> I don't know if it applies to the FEC, but switches often have
> registers which control which egress port an ingress port can send
> packets to. So by default, you allow CPU to port0, CPU to port1, but
> block between port0 to port1. This would give you two independent
> interface, the switch enabled, and using one DMA. When the bridge is
> configured, you simply allow port0 and send/receive packets to/from
> port1. No change to the DMA setup, etc.

Please correct me if I misunderstood this concept - but it seems like
you refer to the use case where the switch is enabled, and by changing
it's "allowed internal port's" mapping it decides if frames are passed
between engress ports (port1 and port2).

	----------
DMA0 ->	|P0    P1| -> ENET-MAC (PHY control) -> eth0 (lan1)
	|L2 SW	 |
	|      P2| -> ENET-MAC (PHY control) -> eth1 (lan2)
	----------

DMA1 (not used)

We can use this approach when we keep always enabled L2 switch.

However now in FEC we use the "bypass" mode, where:
DMA0 -> ENET-MAC (FEC instance driver 1) -> eth0
DMA1 -> ENET-MAC (FEC instance driver 2) -> eth1

And the "bypass" mode is the default one.


I'm just concerned how we are going to gracefully "switch" between L2
switch and bypass configuration? In this patch series - I used the
"hook" corresponding to 'ip link set eth[01] master br0' command.

In other words - how we want to manage DMA0 and DMA1 when switch is
enabled and disabled (in "bypass mode").

> 
> > The code from [2] needs some vendor ioctl based tool (or hardcode)
> > to configure the switch.   
> 
> This would not be allowed. You configure switches in Linux using the
> existing user space tools. No vendor tools are used.

Exactly - that was the rationale to bring support for L2 switch to
mainline kernel.

> 
> > > and how well future features can be added. Do you have
> > > support for VLANS? Adding and removing entries to the lookup
> > > tables? How will IGMP snooping work? How will STP work?  
> > 
> > This can be easily added with serving netstack hooks (as it is
> > already done with cpsw_new) in the new switchdev based version [3]
> > (based on v5.12).  
> 
> Here i'm less convinced. I expect a fully functioning switch driver is
> going to need switch specific versions of some of the netdev ops
> functions, maybe the ethtool ops as well. 

Definately, the current L2 switch driver would need more work.

> It is going to want to add
> devlink ops. By hacking around with the FEC driver 

I believe that I will not touch fec_main.[hc] files more than I did in
the 
"[RFC 3/3] net: imx: Adjust fec_main.c to provide support for L2
switch"

as the switch management (and hooks) are going to be added solely to 
drivers/net/ethernet/freescale/mtipsw/fec_mtip.[hc]. [*]

This would separate L2 switch driver from the current FEC driver.

> in the way you are,
> you might get very basic switch operation working. 

Yes, this is the current status - only simple L2 switching works.

> But as we have seen
> with cpsw, going from very basic to a fully functioning switchdev
> driver required a new driver, cpsw_new.

The new driver for L2 switch has been introduced in [*]. The legacy FEC
driver will also work without it.

> It was getting more and more
> difficult to add features because its structure was just wrong. We
> don't want to add code to the kernel which is probably a dead end.
> 

I cannot say for sure, but all the switch/bridge related hooks can be
added to [*], so fec_main will not bloat.

>       Andrew




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@...x.de

Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists