linux-kernel - Re: [PATCH net-next v2 06/16] net: bridge: Stop calling switchdev_port_attr

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] [day] [month] [year] [list]

Message-ID: <20190212142022.GA18413@splinter>
Date:   Tue, 12 Feb 2019 14:20:26 +0000
From:   Ido Schimmel <idosch@...lanox.com>
To:     Florian Fainelli <f.fainelli@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "bridge@...ts.linux-foundation.org" 
        <bridge@...ts.linux-foundation.org>,
        Jiri Pirko <jiri@...lanox.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>
Subject: Re: [PATCH net-next v2 06/16] net: bridge: Stop calling
 switchdev_port_attr_get()

On Sun, Feb 10, 2019 at 11:34:14AM -0800, Florian Fainelli wrote:
> Le 2/10/19 à 11:05 AM, Ido Schimmel a écrit :
> > On Sun, Feb 10, 2019 at 09:50:55AM -0800, Florian Fainelli wrote:
> >> Now that all switchdev drivers have been converted to checking the
> >> bridge port flags during the prepare phase of the
> >> switchdev_port_attr_set(), we can move straight to trying to set the
> >> desired flag through SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS.
> >>
> >> Acked-by: Jiri Pirko <jiri@...lanox.com>
> >> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> >> ---
> >>  net/bridge/br_switchdev.c | 20 +++-----------------
> >>  1 file changed, 3 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> >> index db9e8ab96d48..939f300522c5 100644
> >> --- a/net/bridge/br_switchdev.c
> >> +++ b/net/bridge/br_switchdev.c
> >> @@ -64,29 +64,15 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
> >>  {
> >>  	struct switchdev_attr attr = {
> >>  		.orig_dev = p->dev,
> >> -		.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
> >> +		.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
> >> +		.flags = SWITCHDEV_F_DEFER,
> > 
> > How does this work? You defer the operation, so the driver cannot veto
> > it. This is why we have SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
> > which is not deferred.
> 
> I missed that indeed, how would you feel about splitting the attribute
> setting into different phases:
> 
> - checking that the attribute setting is supported (caller context, so
> possibly atomic)
> - allocating and committing resources (deferred context)

Yes, this is what I suggested in the other thread. Lets continue
discussion there.

We are doing that when processing route notifications (for example), but
we are missing a check in caller context that resources can actually be
allocated. That's part of a bigger task to track resources in mlxsw.

> 
> For pretty much any DSA driver, we will have to be in sleepable context
> anyway because of MDIO, SPI, I2C, whatever transport layer.
> 
> Not sure how to best approach this now...
> -- 
> Florian