netdev - Re: [PATCH v4 net-next 07/11] net: switchdev: remove lag_mod_cb from switchdev_handle_fdb_event_to

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]

Message-ID: <f3cf1c96-f1ec-7403-ae5c-c52667a232f0@gmail.com>
Date:   Mon, 21 Feb 2022 19:50:09 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Ansuel Smith <ansuelsmth@...il.com>,
        Tobias Waldekranz <tobias@...dekranz.com>,
        DENG Qingfang <dqfext@...il.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        UNGLinuxDriver@...rochip.com, Jiri Pirko <jiri@...nulli.us>,
        Ivan Vecera <ivecera@...hat.com>,
        Horatiu Vultur <horatiu.vultur@...rochip.com>
Subject: Re: [PATCH v4 net-next 07/11] net: switchdev: remove lag_mod_cb from
 switchdev_handle_fdb_event_to_device



On 2/21/2022 1:23 PM, Vladimir Oltean wrote:
> When the switchdev_handle_fdb_event_to_device() event replication helper
> was created, my original thought was that FDB events on LAG interfaces
> should most likely be special-cased, not just replicated towards all
> switchdev ports beneath that LAG. So this replication helper currently
> does not recurse through switchdev lower interfaces of LAG bridge ports,
> but rather calls the lag_mod_cb() if that was provided.
> 
> No switchdev driver uses this helper for FDB events on LAG interfaces
> yet, so that was an assumption which was yet to be tested. It is
> certainly usable for that purpose, as my RFC series shows:
> 
> https://patchwork.kernel.org/project/netdevbpf/cover/20220210125201.2859463-1-vladimir.oltean@nxp.com/
> 
> however this approach is slightly convoluted because:
> 
> - the switchdev driver gets a "dev" that isn't its own net device, but
>    rather the LAG net device. It must call switchdev_lower_dev_find(dev)
>    in order to get a handle of any of its own net devices (the ones that
>    pass check_cb).
> 
> - in order for FDB entries on LAG ports to be correctly refcounted per
>    the number of switchdev ports beneath that LAG, we haven't escaped the
>    need to iterate through the LAG's lower interfaces. Except that is now
>    the responsibility of the switchdev driver, because the replication
>    helper just stopped half-way.
> 
> So, even though yes, FDB events on LAG bridge ports must be
> special-cased, in the end it's simpler to let switchdev_handle_fdb_*
> just iterate through the LAG port's switchdev lowers, and let the
> switchdev driver figure out that those physical ports are under a LAG.
> 
> The switchdev_handle_fdb_event_to_device() helper takes a
> "foreign_dev_check" callback so it can figure out whether @dev can
> autonomously forward to @foreign_dev. DSA fills this method properly:
> if the LAG is offloaded by another port in the same tree as @dev, then
> it isn't foreign. If it is a software LAG, it is foreign - forwarding
> happens in software.
> 
> Whether an interface is foreign or not decides whether the replication
> helper will go through the LAG's switchdev lowers or not. Since the
> lan966x doesn't properly fill this out, FDB events on software LAG
> uppers will get called. By changing lan966x_foreign_dev_check(), we can
> suppress them.
> 
> Whereas DSA will now start receiving FDB events for its offloaded LAG
> uppers, so we need to return -EOPNOTSUPP, since we currently don't do
> the right thing for them.
> 
> Cc: Horatiu Vultur <horatiu.vultur@...rochip.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
-- 
Florian