Re: [RFC net-next 3/3] net: dsa: tag_qca: set offload_fwd_mark

[DENG Qingfang]

On Sun, Aug 08, 2021 at 01:57:21AM +0300, Vladimir Oltean wrote:
> In this day and age, I consider this commit to be a bug fix, since the
> software bridge, seeing an skb with offload_fwd_mark = false on an
> offloaded port, will think it hasn't been forwarded and do that job
> itself. So all broadcast and multicast traffic flooded to the CPU will
> end up being transmitted with duplicates on the other bridge ports.
> 
> When the qca8k tagger was added in 2016 in commit cafdc45c949b
> ("net-next: dsa: add Qualcomm tag RX/TX handler"), the offload_fwd_mark
> framework was already there, but no DSA driver was using it - the first
> commit I can find that uses offload_fwd_mark in DSA is f849772915e5
> ("net: dsa: lan9303: lan9303_rcv set skb->offload_fwd_mark") in 2017,
> and then quite a few more followed suit. But you could still blame
> commit cafdc45c949b.

The driver currently only enables flooding to the CPU port (like MT7530
back then), so offload_fwd_mark should NOT be set until bridge flags
offload is supported.

> 
> Curious, I also see that the gswip driver is in the same situation: it
> implements .port_bridge_join but does not set skb->offload_fwd_mark.
> I've copied Hauke Mehrtens to make him aware. I would rather not send
> the patch myself because I would do a rather lousy job and set it
> unconditionally to 'true', but the hardware can probably do better in
> informing the tagger about whether a frame was received only by the host
> or not, since it has an 8 byte header on RX.
> 
> For the record, I've checked the other tagging drivers too, to see who
> else does not set skb->offload_fwd_mark, and they all correspond to
> switch drivers which don't implement .port_bridge_join, which in that
> case would be the correct thing to do.