mbox series

[net-next,v2,00/13] net: fix netdev to devlink_port linkage and expose to user

Message ID 20221003105204.3315337-1-jiri@resnulli.us (mailing list archive)
Headers show
Series net: fix netdev to devlink_port linkage and expose to user | expand

Message

Jiri Pirko Oct. 3, 2022, 10:51 a.m. UTC
From: Jiri Pirko <jiri@nvidia.com>

Currently, the info about linkage from netdev to the related
devlink_port instance is done using ndo_get_devlink_port().
This is not sufficient, as it is up to the driver to implement it and
some of them don't do that. Also it leads to a lot of unnecessary
boilerplate code in all the drivers.

Instead of that, introduce a possibility for driver to expose this
relationship by new SET_NETDEV_DEVLINK_PORT macro which stores it into
dev->devlink_port. It is ensured by the driver init/fini flows that
the devlink_port pointer does not change during the netdev lifetime.
Devlink port is always registered before netdev register and
unregistered after netdev unregister.

Benefit from this linkage setup and remove explicit calls from driver
to devlink_port_type_eth_set() and clear(). Many of the driver
didn't use it correctly anyway. Let the devlink.c to track associated
netdev events and adjust type and type pointer accordingly. Also
use this events to to keep track on ifname change and remove RTNL lock
taking from devlink_nl_port_fill().

Finally, remove the ndo_get_devlink_port() ndo which is no longer used
and expose devlink_port handle as a new netdev netlink attribute to the
user. That way, during the ifname->devlink_port lookup, userspace app
does not have to dump whole devlink port list and instead it can just
do a simple RTM_GETLINK query.

---
v1->v2:
- see patches 5 and 6 for changelogs

Jiri Pirko (13):
  net: devlink: convert devlink port type-specific pointers to union
  net: devlink: move port_type_warn_schedule() call to
    __devlink_port_type_set()
  net: devlink: move port_type_netdev_checks() call to
    __devlink_port_type_set()
  net: devlink: take RTNL in port_fill() function only if it is not held
  net: devlink: track netdev with devlink_port assigned
  net: make drivers to use SET_NETDEV_DEVLINK_PORT to set devlink_port
  net: devlink: remove netdev arg from devlink_port_type_eth_set()
  net: devlink: remove net namespace check from devlink_nl_port_fill()
  net: devlink: store copy netdevice ifindex and ifname to allow
    port_fill() without RTNL held
  net: devlink: add not cleared type warning to port unregister
  net: devlink: use devlink_port pointer instead of ndo_get_devlink_port
  net: remove unused ndo_get_devlink_port
  net: expose devlink port over rtnetlink

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  14 +-
 .../freescale/dpaa2/dpaa2-eth-devlink.c       |  11 +-
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |   1 +
 .../ethernet/fungible/funeth/funeth_main.c    |  13 +-
 drivers/net/ethernet/intel/ice/ice_devlink.c  |  14 +-
 drivers/net/ethernet/intel/ice/ice_main.c     |  18 +-
 drivers/net/ethernet/intel/ice/ice_repr.c     |  12 +-
 .../marvell/prestera/prestera_devlink.c       |  17 --
 .../marvell/prestera/prestera_devlink.h       |   5 -
 .../ethernet/marvell/prestera/prestera_main.c |   5 +-
 .../net/ethernet/mellanox/mlx4/en_netdev.c    |   9 +-
 drivers/net/ethernet/mellanox/mlx4/main.c     |   2 +-
 .../ethernet/mellanox/mlx5/core/en/devlink.c  |  17 --
 .../ethernet/mellanox/mlx5/core/en/devlink.h  |   2 -
 .../net/ethernet/mellanox/mlx5/core/en_main.c |   4 +-
 .../net/ethernet/mellanox/mlx5/core/en_rep.c  |  43 +---
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  20 +-
 drivers/net/ethernet/mellanox/mlxsw/core.h    |   7 +-
 drivers/net/ethernet/mellanox/mlxsw/minimal.c |  17 +-
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  16 +-
 drivers/net/ethernet/mscc/ocelot_net.c        |  11 +-
 drivers/net/ethernet/mscc/ocelot_vsc7514.c    |   6 -
 drivers/net/ethernet/netronome/nfp/nfp_app.h  |   2 -
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  23 +-
 .../ethernet/netronome/nfp/nfp_net_common.c   |   2 -
 .../net/ethernet/netronome/nfp/nfp_net_main.c |  11 +-
 .../net/ethernet/netronome/nfp/nfp_net_repr.c |   1 -
 drivers/net/ethernet/netronome/nfp/nfp_port.h |   2 -
 .../ethernet/pensando/ionic/ionic_devlink.c   |   2 +-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  14 +-
 drivers/net/netdevsim/dev.c                   |   2 -
 drivers/net/netdevsim/netdev.c                |  10 +-
 include/linux/netdevice.h                     |  24 +-
 include/net/devlink.h                         |  32 ++-
 include/uapi/linux/if_link.h                  |   2 +
 net/core/dev.c                                |  11 +-
 net/core/devlink.c                            | 205 +++++++++++++-----
 net/core/net-sysfs.c                          |   4 +-
 net/core/rtnetlink.c                          |  39 ++++
 net/dsa/dsa2.c                                |   9 -
 net/dsa/slave.c                               |   9 +-
 net/ethtool/ioctl.c                           |  11 +-
 42 files changed, 292 insertions(+), 387 deletions(-)

Comments

Jakub Kicinski Oct. 3, 2022, 4:45 p.m. UTC | #1
On Mon,  3 Oct 2022 12:51:51 +0200 Jiri Pirko wrote:
> Currently, the info about linkage from netdev to the related
> devlink_port instance is done using ndo_get_devlink_port().
> This is not sufficient, as it is up to the driver to implement it and
> some of them don't do that. Also it leads to a lot of unnecessary
> boilerplate code in all the drivers.
> 
> Instead of that, introduce a possibility for driver to expose this
> relationship by new SET_NETDEV_DEVLINK_PORT macro which stores it into
> dev->devlink_port. It is ensured by the driver init/fini flows that
> the devlink_port pointer does not change during the netdev lifetime.
> Devlink port is always registered before netdev register and
> unregistered after netdev unregister.
> 
> Benefit from this linkage setup and remove explicit calls from driver
> to devlink_port_type_eth_set() and clear(). Many of the driver
> didn't use it correctly anyway. Let the devlink.c to track associated
> netdev events and adjust type and type pointer accordingly. Also
> use this events to to keep track on ifname change and remove RTNL lock
> taking from devlink_nl_port_fill().
> 
> Finally, remove the ndo_get_devlink_port() ndo which is no longer used
> and expose devlink_port handle as a new netdev netlink attribute to the
> user. That way, during the ifname->devlink_port lookup, userspace app
> does not have to dump whole devlink port list and instead it can just
> do a simple RTM_GETLINK query.

Would you be okay if we deferred until 6.2?

It's technically past the deadline and some odd driver could regress.
Jiri Pirko Oct. 4, 2022, 6:56 a.m. UTC | #2
Mon, Oct 03, 2022 at 06:45:56PM CEST, kuba@kernel.org wrote:
>On Mon,  3 Oct 2022 12:51:51 +0200 Jiri Pirko wrote:
>> Currently, the info about linkage from netdev to the related
>> devlink_port instance is done using ndo_get_devlink_port().
>> This is not sufficient, as it is up to the driver to implement it and
>> some of them don't do that. Also it leads to a lot of unnecessary
>> boilerplate code in all the drivers.
>> 
>> Instead of that, introduce a possibility for driver to expose this
>> relationship by new SET_NETDEV_DEVLINK_PORT macro which stores it into
>> dev->devlink_port. It is ensured by the driver init/fini flows that
>> the devlink_port pointer does not change during the netdev lifetime.
>> Devlink port is always registered before netdev register and
>> unregistered after netdev unregister.
>> 
>> Benefit from this linkage setup and remove explicit calls from driver
>> to devlink_port_type_eth_set() and clear(). Many of the driver
>> didn't use it correctly anyway. Let the devlink.c to track associated
>> netdev events and adjust type and type pointer accordingly. Also
>> use this events to to keep track on ifname change and remove RTNL lock
>> taking from devlink_nl_port_fill().
>> 
>> Finally, remove the ndo_get_devlink_port() ndo which is no longer used
>> and expose devlink_port handle as a new netdev netlink attribute to the
>> user. That way, during the ifname->devlink_port lookup, userspace app
>> does not have to dump whole devlink port list and instead it can just
>> do a simple RTM_GETLINK query.
>
>Would you be okay if we deferred until 6.2?
>
>It's technically past the deadline and some odd driver could regress.

Sure.