mbox series

[net-next,v4,0/9] net: Remove switchdev_ops

Message ID 20190211191001.8623-1-f.fainelli@gmail.com
Headers show
Series net: Remove switchdev_ops | expand

Message

Florian Fainelli Feb. 11, 2019, 7:09 p.m. UTC
Hi all,

This patch series finishes by the removal of switchdev_ops. To get there
we convert the existing switchdev_port_attr_{set,get} switchdev_ops to
use a blocking notifier, thus making it consistent with how the objects
are pushed to the switchdev enabled devices.

Please review and let me know what you think!

David, I would like to get Ido's feedback on this to make sure I did not
miss something, thank you!

Changes in v4:
- removed double space in Documentation/networking/switchdev.txt
- added Jiri's Acked-by tags
- added fall through annotations where appropriate

Changes in v3:
- dropped patches removing te need to get the attribute since we
  still need that in order to support different sleeping vs.
  non-sleeping contexts

Changes in v2:
- fixed bisectability issues in patch #15
- added Acked-by from Jiri where necessary
- fixed a few minor issues according to Jiri's feedback:
	- rename port_attr_event -> port_attr_set_event
	- moved SWITCHDEV_PORT_ATTR_SET closer to other blocking events

Florian Fainelli (9):
  Documentation: networking: switchdev: Update port parent ID section
  switchdev: Add SWITCHDEV_PORT_ATTR_SET, SWITCHDEV_PORT_ATTR_GET
  rocker: Handle SWITCHDEV_PORT_ATTR_GET/SET
  mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_GET/SET
  net: mscc: ocelot: Handle SWITCHDEV_PORT_ATTR_GET/SET
  staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_ATTR_GET/SET
  net: dsa: Handle SWITCHDEV_PORT_ATTR_GET/SET
  net: switchdev: Replace port attr get/set SDO with a notification
  net: Remove switchdev_ops

 Documentation/networking/switchdev.txt        |  10 +-
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  12 --
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |   2 -
 .../mellanox/mlxsw/spectrum_switchdev.c       |  36 +++---
 drivers/net/ethernet/mscc/ocelot.c            |  26 ++++-
 drivers/net/ethernet/rocker/rocker_main.c     |  30 ++++-
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c       |  30 ++++-
 include/linux/netdevice.h                     |   3 -
 include/net/switchdev.h                       |  28 ++---
 net/dsa/slave.c                               |  30 ++++-
 net/switchdev/switchdev.c                     | 107 ++++++------------
 11 files changed, 168 insertions(+), 146 deletions(-)

Comments

David Miller Feb. 11, 2019, 8:16 p.m. UTC | #1
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 11 Feb 2019 11:09:52 -0800

> David, I would like to get Ido's feedback on this to make sure I did not
> miss something, thank you!

Ok, Ido please look at this when you can.
Ido Schimmel Feb. 11, 2019, 9:40 p.m. UTC | #2
On Mon, Feb 11, 2019 at 12:16:57PM -0800, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Mon, 11 Feb 2019 11:09:52 -0800
> 
> > David, I would like to get Ido's feedback on this to make sure I did not
> > miss something, thank you!
> 
> Ok, Ido please look at this when you can.

Will review tomorrow. I was occupied with other stuff last couple of
days. Sorry
Ido Schimmel Feb. 12, 2019, 1:14 p.m. UTC | #3
On Mon, Feb 11, 2019 at 11:09:52AM -0800, Florian Fainelli wrote:
> Hi all,
> 
> This patch series finishes by the removal of switchdev_ops. To get there
> we convert the existing switchdev_port_attr_{set,get} switchdev_ops to
> use a blocking notifier, thus making it consistent with how the objects
> are pushed to the switchdev enabled devices.
> 
> Please review and let me know what you think!
> 
> David, I would like to get Ido's feedback on this to make sure I did not
> miss something, thank you!

Hi Florian,

Why do you still keep switchdev_port_attr_get()? I believe we can remove
it and simplify things.

After your recent patchset to remove 'PORT_BRIDGE_FLAGS', the only
remaining user of get() is 'PORT_BRIDGE_FLAGS_SUPPORT'. It can be
converted to a blocking set() with 'PORT_PRE_BRIDGE_FLAGS' (or a similar
name).

I would like to make sure we're in sync with regards to future changes.
After this patchset to get rid of switchdev_ops we can continue to
completely removing switchdev (I believe Jiri approves). The
prepare-commit model is not really needed and the two switchdev
notification chains can be split into bridge and vxlan specific chains.

Notifications sent in an atomic context can be handled by drivers
directly in this context. Similar to how FDB/route/neighbour are
handled. It will really simplify things. No need for the defer flag
anymore and tricks like 'PORT_BRIDGE_FLAGS_SUPPORT' and
'PORT_PRE_BRIDGE_FLAGS'. In the atomic context the driver can veto the
requested bridge flags, but program the device from a blocking context
(using a workqueue).
Jiri Pirko Feb. 12, 2019, 1:53 p.m. UTC | #4
Tue, Feb 12, 2019 at 02:14:47PM CET, idosch@mellanox.com wrote:
>On Mon, Feb 11, 2019 at 11:09:52AM -0800, Florian Fainelli wrote:
>> Hi all,
>> 
>> This patch series finishes by the removal of switchdev_ops. To get there
>> we convert the existing switchdev_port_attr_{set,get} switchdev_ops to
>> use a blocking notifier, thus making it consistent with how the objects
>> are pushed to the switchdev enabled devices.
>> 
>> Please review and let me know what you think!
>> 
>> David, I would like to get Ido's feedback on this to make sure I did not
>> miss something, thank you!
>
>Hi Florian,
>
>Why do you still keep switchdev_port_attr_get()? I believe we can remove
>it and simplify things.
>
>After your recent patchset to remove 'PORT_BRIDGE_FLAGS', the only
>remaining user of get() is 'PORT_BRIDGE_FLAGS_SUPPORT'. It can be
>converted to a blocking set() with 'PORT_PRE_BRIDGE_FLAGS' (or a similar
>name).

Let's do that in a follow-up.


>
>I would like to make sure we're in sync with regards to future changes.
>After this patchset to get rid of switchdev_ops we can continue to
>completely removing switchdev (I believe Jiri approves). The

Yes.


>prepare-commit model is not really needed and the two switchdev
>notification chains can be split into bridge and vxlan specific chains.
>
>Notifications sent in an atomic context can be handled by drivers
>directly in this context. Similar to how FDB/route/neighbour are
>handled. It will really simplify things. No need for the defer flag
>anymore and tricks like 'PORT_BRIDGE_FLAGS_SUPPORT' and
>'PORT_PRE_BRIDGE_FLAGS'. In the atomic context the driver can veto the
>requested bridge flags, but program the device from a blocking context
>(using a workqueue).

Sounds good to me.