Re: [PATCH v2 14/14] net: ethernet: mtk_eth_soc: support creating mac address based offload entries

[Felix Fietkau]

On 12.04.22 16:21, Andrew Lunn wrote:
> On Tue, Apr 12, 2022 at 03:49:51PM +0200, Felix Fietkau wrote:
> > On 12.04.22 15:07, Andrew Lunn wrote:
> > > > > > > > I'm trying to understand the architecture here.
> > > > > > > > We have an Ethernet interface and a Wireless interface. The slow
> > > > > > path
> > > > > > > is that frames ingress from one of these interfaces, Linux decides
> > > > > > > what to do with them, either L2 or L3, and they then egress probably
> > > > > > > out the other interface.
> > > > > > > > The hardware will look at the frames and try to spot flows? It
> > > > > > will
> > > > > > > then report any it finds. You can then add an offload, telling it for
> > > > > > > a flow it needs to perform L2 or L3 processing, and egress out a
> > > > > > > specific port? Linux then no longer sees the frame, the hardware
> > > > > > > handles it, until the flow times out?
> > > > > > Yes, the hw handles it until either the flow times out, or the corresponding
> > > > > > offload entry is removed.
> > > > > > > > For OpenWrt I also wrote a daemon that uses tc classifier
> > > > BPF to accelerate
> > > > > > the software bridge and create hardware offload entries as well via hardware
> > > > > > TC flower rules: https://github.com/nbd168/bridger
> > > > > > It works in combination with these changes.
> > > > > > What about the bridge? In Linux, it is the software bridge which
> > > > > controls all this at L2, and it should be offloading the flows, via
> > > > > switchdev. The egress port you derive here is from the software bridge
> > > > > FDB?
> > > 
> > > > My code uses netlink to fetch and monitor the bridge configuration,
> > > > including fdb, port state, vlans, etc. and it uses that for the offload path
> > > > - no extra configuration needed.
> > > 
> > > So this is where we get into architecture issues. Do we really want
> > > Linux to have two ways for setting up L2 networking? It was decided
> > > that users should not need to know about how to use an accelerator,
> > > they should not use additional tools, it should just look like
> > > linux. The user should just add the WiFi netdev to the bridge and
> > > switchdev will do the rest to offload L2 switching to the hardware.
> > > 
> > > You appear to be saying you need a daemon in userspace. That is not
> > > how every other accelerate works in Linux networking.
> > > 
> > > We the Linux network community need to decided if we want this?
>
> > The problem here is that it can't be fully transparent. Enabling hardware
> > offload for LAN -> WiFi comes at a cost of bypassing airtime fairness and
> > mac80211's bufferbloat mitigation.
> > Some people want this anyway (often but not always for benchmark/marketing
> > purposes), but it's not something that I would want to have enabled by
> > default simply by a wifi netdev to a bridge.
>
> So this sounds like a generic issue. How does IPA handle this? Looping
> in Alex Elder.
>
> There is already something partially in this direction in the
> bridge. You can add a static entry with our without self. This
> controls if a specific static entry in the FDB is offloaded to the
> accelerate or not. Maybe you can add an attribute to a port which
> determines if dynamic entries are self or not, so you can decide if
> frames egressing out a specific interface are accelerated or not,
> depending on user choice. Since such a change should not touch the
> fast path, it has a better chance of being merged.
Sounds interesting. If there is some overlap and if we can get some 
common code in place, it might be worth looking into for the MTK offload 
as well. I do think this will take more time though.

> > Initially, I wanted to put more of the state tracking code in the kernel. I
> > made the first implementation of my acceleration code as a patch to the
> > network bridge - speeding up bridge unicast forwarding significantly for any
> > device regardless of hardware support. I wanted to build on that to avoid
> > putting a lot of FDB/VLAN related tracking directly into the driver.
>
> But the driver is the correct place for this. How generic is the state
> tracking? Do you expect any other hardware to need the same state
> tracking? IPA? Some other accelerate your know of?
It basically has to keep track of all possible destination ports, their 
STP state, all their fdb entries, member VLANs of all ports. It has to 
quickly react to changes in any of these.
In order to implement this properly, I would also need to make more 
changes to mac80211. Right now, mac80211 drivers do not have access to 
the net_device pointer of virtual interfaces. So mac80211 itself would 
likely need to implement the switchdev ops and handle some of this.

There are also some other issues where I don't know how this is supposed 
to be solved properly:
On MT7622 most of the bridge ports are connected to a MT7531 switch 
using DSA. Offloading (lan->wlan bridging or L3/L4 NAT/routing) is not 
handled by the switch itself, it is handled by a packet processing 
engine in the SoC, which knows how to handle the DSA tags of the MT7531 
switch.

So if I were to handle this through switchdev implemented on the wlan 
and ethernet devices, it would technically not be part of the same 
switch, since it's a behind a different component with a different driver.

Also, is switchdev able to handle the situation where only parts of the 
traffic is offloaded and the rest (e.g. multicast) is handled through 
the regular software path?

In my opinion, handling it through the TC offload has a number of 
advantages:
- It's a lot simpler
- It uses the same kind of offloading rules that my software fastpath 
already uses
- It allows more fine grained control over which traffic should be 
offloaded (src mac -> destination MAC tuple)

I also plan on extending my software fast path code to support emulating 
bridging of WiFi client mode interfaces. This involves doing some MAC 
address translation with some IP address tracking. I want that to 
support hardware offload as well.

I really don't think that desire for supporting switchdev based offload 
should be a blocker for accepting this code now, especially since my 
implementation relies on existing Linux network APIs without inventing 
any new ones, and there are valid use cases for using it, even with 
switchdev support in place.

- Felix