linux-kernel - Re: [RFC net-next 3/3] net: dsa: tag_qca: set offload_fwd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon,  9 Aug 2021 00:12:24 +0800
From:   DENG Qingfang <dqfext@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Hauke Mehrtens <hauke@...ke-m.de>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        "open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Ansuel Smith <ansuelsmth@...il.com>,
        Jonathan McDowell <noodles@...th.li>,
        Michal Vokáč <vokac.m@...il.com>,
        Christian Lamparter <chunkeey@...il.com>,
        Nishka Dasgupta <nishkadg.linux@...il.com>,
        John Crispin <john@...ozen.org>,
        Stefan Lippers-Hollmann <s.l-h@....de>,
        Hannu Nyman <hannu.nyman@....fi>,
        Imran Khan <gururug@...il.com>,
        Frank Wunderlich <frank-w@...lic-files.de>,
        Nick Lowe <nick.lowe@...il.com>,
        André Valentin <avalentin@...cant.net>
Subject: Re: [RFC net-next 3/3] net: dsa: tag_qca: set offload_fwd_mark

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.