Message-ID: <Z2CfVnhNGYMRv4cN@lore-desk>
Date: Mon, 16 Dec 2024 22:44:54 +0100
From: Lorenzo Bianconi <lorenzo@...nel.org>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Vladimir Oltean <olteanv@...il.com>,
	Oleksij Rempel <linux@...pel-privat.de>, netdev@...r.kernel.org,
	andrew@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, nbd@....name,
	sean.wang@...iatek.com, Mark-MC.Lee@...iatek.com,
	lorenzo.bianconi83@...il.com
Subject: Re: [RFC net-next 0/5] Add ETS and TBF Qdisc offload for Airoha
 EN7581 SoC

> On Mon, Dec 16, 2024 at 08:01:33PM +0100, Lorenzo Bianconi wrote:
> > > On Mon, Dec 16, 2024 at 01:09:01PM +0100, Lorenzo Bianconi wrote:
> > > > I guess what I did not make clear here is that we are discussing about
> > > > 'routed' traffic (sorry for that). The traffic is received from the WAN
> > > > interface and routed to a DSA port (or the other way around).
> > > > In this scenario the 3-way handshake will be received by the CPU via the
> > > > WAN port (or the conduit port) while the subsequent packets will be hw
> > > > forwarded from WAN to LAN (or LAN to WAN). For EN7581 [0], the traffic
> > > > will be received by the system from GDM2 (WAN) and the PSE/PPE blocks
> > > > will forward it to the GDM1 port that is connected to the DSA cpu port.
> > > > 
> > > > The proposed series is about adding the control path to apply a given Qdisc
> > > > (ETS or TBF for EN7581) to the traffic that is following the described path
> > > > without creating it directly on the DSA switch port (for the reasons described
> > > > before). E.g. the user would want to apply an ETS Qdisc just for traffic
> > > > egressing via lan0.
> > > > 
> > > > This series is not strictly related to the airoha_eth flowtable offload
> > > > implementation but the latter is required to have a full pictures of the
> > > > possible use case (this is why I was saying it is better to post it first).
> > > 
> > > It's good to know this does not depend on flowtable.
> > > 
> > > When you add an offloaded Qdisc to the egress of a net device, you don't
> > > affect just the traffic L3 routed to that device, but all traffic (also
> > > includes the packets sent to it using L2 forwarding). As such, I simply
> > > don't believe that the way in which the UAPI is interpreted here (root
> > > egress qdisc matches only routed traffic) is proper.
> > > 
> > > Ack?
> > 
> > Considering patch [0], we are still offloading the Qdisc on the provided
> > DSA switch port (e.g. LANx) via the port_setup_tc() callback available in
> > dsa_user_setup_qdisc(), but we are introducing even the ndo_setup_tc_conduit()
> > callback in order to use the hw Qdisc capabilities available on the mac chip
> > (e.g. EN7581) for the routed traffic from WAN to LANx. We will still apply
> > the Qdisc defined on LANx for L2 traffic from LANy to LANx. Agree?
> > 
> > > 
> > > > > I'm trying to look at the big picture and abstract away the flowtable a
> > > > > bit. I don't think the tc rule should be on the user port. Can the
> > > > > redirection of packets destined towards a particular switch port be
> > > > > accomplished with a tc u32 filter on the conduit interface instead?
> > > > > If the tc primitives for either the filter or the action don't exist,
> > > > > maybe those could be added instead? Like DSA keys in "flower" which gain
> > > > > introspection into the encapsulated packet headers?
> > > > 
> > > > The issue with the current DSA infrastructure is there is no way to use
> > > > the conduit port to offload a Qdisc policy to a given lan port since we
> > > > are missing in the APIs the information about what user port we are
> > > > interested in (this is why I added the new netdev callback).
> > > 
> > > How does the introduction of ndo_setup_tc_conduit() help, since the problem
> > > is elsewhere? You are not making "tc qdisc add lanN root ets" work correctly.
> > > It is simply not comparable to the way in which it is offloaded by
> > > drivers/net/dsa/microchip/ksz_common.c, even though the user space
> > > syntax is the same. Unless you're suggesting that for ksz it is not
> > > offloaded correctly?
> > 
> > nope, I am not saying the current Qdisc DSA infrastructure is wrong, it just
> > does not allow to exploit all hw capabilities available on EN7581 when the
> > traffic is routed from the WAN port to a given DSA switch port. If we do:
> > 
> > $tc qdisc add dev lan0 root handle 1: ets strict 8 priomap ...
> > 
> > in the current upstream implementation we do:
> >   dsa_user_setup_tc():
> >      ...
> >        -> ds->ops->port_setup_tc(ds, dp->index, type, type_data)
> >           (it applies the Qdisc on lan0 configuring the hw switch)
> > 
> > adding the ndo_setup_tc_conduit() callback we have:
> > 
> >   dsa_user_setup_qdisc()
> >     ...
> >       -> conduit->netdev_ops->ndo_setup_tc_conduit(conduit, dp->index, type, type_data)
> >          (it applies the Qdisc on EN7581 mac chip for the routed traffic destinated to lan0)
> > 
> >       -> ds->ops->port_setup_tc(ds, dp->index, type, type_data)
> >          (it applies the Qdisc on lan0 configuring the hw switch)
> > 
> > > 
> > > Oleksij, am I missing something?
> > > 
> > > > Please consider here we are discussing about Qdisc policies and not flower
> > > > rules to mangle the traffic.
> > > 
> > > What's a Qdisc policy?
> > 
> > I mean a queue scheduler algorithm (e.g. TBF, ETS, HTB, ...)
> > 
> > > 
> > > Also, flower is a classifier, not an action. It doesn't mangle packets
> > > by the very definition of what a classifier is.
> > 
> > yes, but goal of the series is the Queue scheduler offloading, not
> > classifier/action. Agree?
> > 
> > > 
> > > > The hw needs to be configured in advance to apply the requested policy
> > > > (e.g TBF for traffic shaping).
> > > 
> > > What are you missing exactly to make DSA packets go to a particular
> > > channel on the conduit?
> > > 
> > > For Qdisc offloading you want to configure the NIC in advance, of course.
> > > 
> > > Can't you do something like this to guide packets to the correct channels?
> > > 
> > > tc qdisc add dev eth0 clsact
> > > tc qdisc add dev eth0 root handle 1: ets strict 8 priomap ...
> > > tc filter add dev eth0 egress ${u32 or flower filter to match on DSA tagged packets} \
> > > 	flowid 1:1
> > 
> > If we apply the Qdisc directly on the conduit port (eth0) we can just apply the
> > queue scheduler on all the traffic egressing via the DSA switch while I would
> > like to apply it on per DSA port basis (but using the mac chip hw capabilities),
> > got my point?
> 
> Hm, I guess I have similar use case in one of my projects. In my case, the CPU
> interface is 1Gbit the switch ports are 100Mbit each. It is still
> possible to keep the CPU interface busy by sending 1Gbit UDP stream, so
> 900Mbit is dropped by the switch. I would like to have traffic limiter
> per virtual DSA port on the SoC site to reduce the load on DSA conduit.
> Currently it was not possible.

Does the mac chip in your setup support TX shaping (e.g. via HTB or TBF)?
If so, I guess you could do it via the ndo_setup_tc_conduit() callback.

Regards,
Lorenzo

> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists