From: Ido Schimmel <idosch@mellanox.com> To: Florian Fainelli <f.fainelli@gmail.com> Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>, "bridge@lists.linux-foundation.org" <bridge@lists.linux-foundation.org>, Jiri Pirko <jiri@mellanox.com>, "andrew@lunn.ch" <andrew@lunn.ch>, "vivien.didelot@gmail.com" <vivien.didelot@gmail.com> Subject: Re: [PATCH net-next v2 06/16] net: bridge: Stop calling switchdev_port_attr_get() Date: Tue, 12 Feb 2019 14:20:26 +0000 [thread overview] Message-ID: <20190212142022.GA18413@splinter> (raw) In-Reply-To: <73a10fbb-7689-74a7-3fbf-076f8dc9d76f@gmail.com> 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@mellanox.com> > >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.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
WARNING: multiple messages have this Message-ID (diff)
From: Ido Schimmel <idosch@mellanox.com> To: Florian Fainelli <f.fainelli@gmail.com> Cc: "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>, "andrew@lunn.ch" <andrew@lunn.ch>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "bridge@lists.linux-foundation.org" <bridge@lists.linux-foundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Jiri Pirko <jiri@mellanox.com>, "vivien.didelot@gmail.com" <vivien.didelot@gmail.com> Subject: Re: [Bridge] [PATCH net-next v2 06/16] net: bridge: Stop calling switchdev_port_attr_get() Date: Tue, 12 Feb 2019 14:20:26 +0000 [thread overview] Message-ID: <20190212142022.GA18413@splinter> (raw) In-Reply-To: <73a10fbb-7689-74a7-3fbf-076f8dc9d76f@gmail.com> 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@mellanox.com> > >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.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
next prev parent reply other threads:[~2019-02-12 14:20 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-10 17:50 [PATCH net-next v2 00/16] net: Remove switchdev_ops Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 01/16] Documentation: networking: switchdev: Update port parent ID section Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 02/16] mlxsw: spectrum: Check bridge flags during prepare phase Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 03/16] staging: fsl-dpaa2: ethsw: Check bridge port flags during set Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 04/16] net: dsa: Add setter for SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 05/16] rocker: Check bridge flags during prepare phase Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 06/16] net: bridge: Stop calling switchdev_port_attr_get() Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 19:05 ` Ido Schimmel 2019-02-10 19:05 ` [Bridge] " Ido Schimmel 2019-02-10 19:34 ` Florian Fainelli 2019-02-10 19:34 ` [Bridge] " Florian Fainelli 2019-02-12 14:20 ` Ido Schimmel [this message] 2019-02-12 14:20 ` Ido Schimmel 2019-02-10 17:50 ` [PATCH net-next v2 07/16] net: Remove SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 08/16] net: Get rid of switchdev_port_attr_get() Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 09/16] switchdev: Add SWITCHDEV_PORT_ATTR_SET Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:50 ` [PATCH net-next v2 10/16] rocker: Handle SWITCHDEV_PORT_ATTR_SET Florian Fainelli 2019-02-10 17:50 ` [Bridge] " Florian Fainelli 2019-02-10 17:51 ` [PATCH net-next v2 11/16] net: dsa: " Florian Fainelli 2019-02-10 17:51 ` [Bridge] " Florian Fainelli 2019-02-10 17:51 ` [PATCH net-next v2 12/16] mlxsw: spectrum_switchdev: " Florian Fainelli 2019-02-10 17:51 ` [Bridge] " Florian Fainelli 2019-02-10 17:51 ` [PATCH net-next v2 13/16] net: mscc: ocelot: " Florian Fainelli 2019-02-10 17:51 ` [Bridge] " Florian Fainelli 2019-02-10 17:51 ` [PATCH net-next v2 14/16] staging: fsl-dpaa2: ethsw: " Florian Fainelli 2019-02-10 17:51 ` [Bridge] " Florian Fainelli 2019-02-10 17:51 ` [PATCH net-next v2 15/16] net: switchdev: Replace port attr set SDO with a notification Florian Fainelli 2019-02-10 17:51 ` [Bridge] " Florian Fainelli 2019-02-10 17:51 ` [PATCH net-next v2 16/16] net: Remove switchdev_ops Florian Fainelli 2019-02-10 17:51 ` [Bridge] " Florian Fainelli 2019-02-10 20:44 ` [PATCH net-next v2 00/16] " Florian Fainelli 2019-02-10 20:44 ` [Bridge] " Florian Fainelli 2019-02-10 20:51 ` David Miller 2019-02-10 20:51 ` [Bridge] " David Miller
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190212142022.GA18413@splinter \ --to=idosch@mellanox.com \ --cc=andrew@lunn.ch \ --cc=bridge@lists.linux-foundation.org \ --cc=devel@driverdev.osuosl.org \ --cc=f.fainelli@gmail.com \ --cc=jiri@mellanox.com \ --cc=linux-kernel@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=vivien.didelot@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.