netdev - Re: [PATCH v2 net-next 0/5] Make SWITCHDEV_FDB_{ADD,DEL}_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: <20210821002212.ebi7cdugxdk5os7m@skbuf>
Date:   Sat, 21 Aug 2021 03:22:12 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Nikolay Aleksandrov <nikolay@...dia.com>
Cc:     Ido Schimmel <idosch@...sch.org>,
        Vladimir Oltean <vladimir.oltean@....com>,
        netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Roopa Prabhu <roopa@...dia.com>, Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vadym Kochan <vkochan@...vell.com>,
        Taras Chornyi <tchornyi@...vell.com>,
        Jiri Pirko <jiri@...dia.com>, Ido Schimmel <idosch@...dia.com>,
        UNGLinuxDriver@...rochip.com,
        Grygorii Strashko <grygorii.strashko@...com>,
        Marek Behun <kabel@...ckhole.sk>,
        DENG Qingfang <dqfext@...il.com>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        George McCollister <george.mccollister@...il.com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Leon Romanovsky <leon@...nel.org>,
        Lars Povlsen <lars.povlsen@...rochip.com>,
        Steen Hegelund <Steen.Hegelund@...rochip.com>,
        Julian Wiedmann <jwi@...ux.ibm.com>,
        Karsten Graul <kgraul@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Ivan Vecera <ivecera@...hat.com>,
        Vlad Buslov <vladbu@...dia.com>,
        Jianbo Liu <jianbol@...dia.com>,
        Mark Bloch <mbloch@...dia.com>, Roi Dayan <roid@...dia.com>,
        Tobias Waldekranz <tobias@...dekranz.com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>
Subject: Re: [PATCH v2 net-next 0/5] Make SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE
 blocking

On Sat, Aug 21, 2021 at 02:36:26AM +0300, Nikolay Aleksandrov wrote:
> Hi,
> I don't like the double-queuing for each fdb for everyone either, it's forcing them
> to rework it asap due to inefficiency even though that shouldn't be necessary.

Let's be honest, with the vast majority of drivers having absurdities such as the
"if (!fdb_info->added_by_user || fdb_info->is_local) => nothing to do here, bye"
check placed _inside_ the actual work item (and therefore scheduling for nothing
for entries dynamically learned by the bridge), it's hard to believe that driver
authors cared too much about inefficiency when mindlessly copy-pasting that snippet
from mlxsw

[ which for the record does call mlxsw_sp_span_respin for dynamically learned FDB
  entries, so that driver doesn't schedule for nothing like the rest - although
  maybe even mlxsw could call mlxsw_sp_port_dev_lower_find_rcu instead of
  mlxsw_sp_port_dev_lower_find, and could save a queue_work for FDB entries on
  foreign && non-VXLAN ports. Who knows?! ]

Now I get to care for them.

But I can see how a partial conversion could leave things in an even more absurd position.
I don't want to contribute to the absurdity.

> In the
> long run I hope everyone would migrate to such scheme, but perhaps we can do it gradually.
> For most drivers this is introducing more work (as in processing) rather than helping
> them right now, give them the option to convert to it on their own accord or bite
> the bullet and convert everyone so the change won't affect them, it holds rtnl, it is blocking
> I don't see why not convert everyone to just execute their otherwise queued work.
> I'm sure driver maintainers would appreciate such help and would test and review it. You're
> halfway there already..

Agree, this needs more work. Thanks for looking.