[patch net-next 00/15] devlink: move port ops into separate structure

[patch net-next 00/15] devlink: move port ops into separate structure

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

In devlink, some of the objects have separate ops registered alongside
with the object itself. Port however have ops in devlink_ops structure.
For drivers what register multiple kinds of ports with different ops
this is not convenient.

This patchset changes does following changes:
1) Introduces devlink_port_ops with functions that allow devlink port
   to be registered passing a pointer to driver port ops. (patch #1)
2) Converts drivers to define port_ops and register ports passing the
   ops pointer. (patches #2, #3, #4, #6, #8, and #9)
3) Moves ops from devlink_ops struct to devlink_port_ops.
   (patches #5, #7, #10-15)

No functional changes.

Jiri Pirko (15):
  devlink: introduce port ops placeholder
  ice: register devlink port for PF with ops
  mlxsw_core: register devlink port with ops
  nfp: devlink: register devlink port with ops
  devlink: move port_split/unsplit() ops into devlink_port_ops
  mlx4: register devlink port with ops
  devlink: move port_type_set() op into devlink_port_ops
  sfc: register devlink port with ops
  mlx5: register devlink ports with ops
  devlink: move port_fn_hw_addr_get/set() to devlink_port_ops
  devlink: move port_fn_roce_get/set() to devlink_port_ops
  devlink: move port_fn_migratable_get/set() to devlink_port_ops
  devlink: move port_fn_state_get/set() to devlink_port_ops
  devlink: move port_del() to devlink_port_ops
  devlink: save devlink_port_ops into a variable in
    devlink_port_function_validate()

 drivers/net/ethernet/intel/ice/ice_devlink.c  |  10 +-
 drivers/net/ethernet/mellanox/mlx4/main.c     |  58 ++---
 .../net/ethernet/mellanox/mlx5/core/devlink.c |   9 -
 .../mellanox/mlx5/core/esw/devlink_port.c     |  29 ++-
 .../net/ethernet/mellanox/mlx5/core/eswitch.h |  12 +-
 .../mellanox/mlx5/core/eswitch_offloads.c     |  12 +-
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  10 +-
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  10 +-
 drivers/net/ethernet/sfc/efx_devlink.c        |  80 +++---
 include/net/devlink.h                         | 228 +++++++++---------
 net/devlink/leftover.c                        | 120 +++++----
 11 files changed, 298 insertions(+), 280 deletions(-)

-- 
2.39.2

[patch net-next 01/15] devlink: introduce port ops placeholder

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

In devlink, some of the objects have separate ops registered alongside
with the object itself. Port however have ops in devlink_ops structure.
For drivers what register multiple kinds of ports with different ops
this is not convenient. Introduce devlink_port_ops and a set
of functions that allow drivers to pass ops pointer during
port registration.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 include/net/devlink.h  | 41 +++++++++++++++++++++++++++++++++++------
 net/devlink/leftover.c | 24 +++++++++++++++---------
 2 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 1bd56c8d6f3c..850148b98f70 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -123,6 +123,7 @@ struct devlink_port {
 	struct list_head list;
 	struct list_head region_list;
 	struct devlink *devlink;
+	const struct devlink_port_ops *ops;
 	unsigned int index;
 	spinlock_t type_lock; /* Protects type and type_eth/ib
 			       * structures consistency.
@@ -1649,15 +1650,43 @@ void devl_unregister(struct devlink *devlink);
 void devlink_register(struct devlink *devlink);
 void devlink_unregister(struct devlink *devlink);
 void devlink_free(struct devlink *devlink);
+
+/**
+ * struct devlink_port_ops - Port operations
+ */
+struct devlink_port_ops {
+};
+
 void devlink_port_init(struct devlink *devlink,
 		       struct devlink_port *devlink_port);
 void devlink_port_fini(struct devlink_port *devlink_port);
-int devl_port_register(struct devlink *devlink,
-		       struct devlink_port *devlink_port,
-		       unsigned int port_index);
-int devlink_port_register(struct devlink *devlink,
-			  struct devlink_port *devlink_port,
-			  unsigned int port_index);
+
+int devl_port_register_with_ops(struct devlink *devlink,
+				struct devlink_port *devlink_port,
+				unsigned int port_index,
+				const struct devlink_port_ops *ops);
+
+static inline int devl_port_register(struct devlink *devlink,
+				     struct devlink_port *devlink_port,
+				     unsigned int port_index)
+{
+	return devl_port_register_with_ops(devlink, devlink_port,
+					   port_index, NULL);
+}
+
+int devlink_port_register_with_ops(struct devlink *devlink,
+				   struct devlink_port *devlink_port,
+				   unsigned int port_index,
+				   const struct devlink_port_ops *ops);
+
+static inline int devlink_port_register(struct devlink *devlink,
+					struct devlink_port *devlink_port,
+					unsigned int port_index)
+{
+	return devlink_port_register_with_ops(devlink, devlink_port,
+					      port_index, NULL);
+}
+
 void devl_port_unregister(struct devlink_port *devlink_port);
 void devlink_port_unregister(struct devlink_port *devlink_port);
 void devlink_port_type_eth_set(struct devlink_port *devlink_port);
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 0410137a4a31..ff1c2ed84aba 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -6799,6 +6799,7 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
  * @devlink: devlink
  * @devlink_port: devlink port
  * @port_index: driver-specific numerical identifier of the port
+ * @ops: port ops
  *
  * Register devlink port with provided port index. User can use
  * any indexing, even hw-related one. devlink_port structure
@@ -6806,9 +6807,10 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
  * Note that the caller should take care of zeroing the devlink_port
  * structure.
  */
-int devl_port_register(struct devlink *devlink,
-		       struct devlink_port *devlink_port,
-		       unsigned int port_index)
+int devl_port_register_with_ops(struct devlink *devlink,
+				struct devlink_port *devlink_port,
+				unsigned int port_index,
+				const struct devlink_port_ops *ops)
 {
 	int err;
 
@@ -6819,6 +6821,7 @@ int devl_port_register(struct devlink *devlink,
 	devlink_port_init(devlink, devlink_port);
 	devlink_port->registered = true;
 	devlink_port->index = port_index;
+	devlink_port->ops = ops;
 	spin_lock_init(&devlink_port->type_lock);
 	INIT_LIST_HEAD(&devlink_port->reporter_list);
 	err = xa_insert(&devlink->ports, port_index, devlink_port, GFP_KERNEL);
@@ -6830,7 +6833,7 @@ int devl_port_register(struct devlink *devlink,
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
 	return 0;
 }
-EXPORT_SYMBOL_GPL(devl_port_register);
+EXPORT_SYMBOL_GPL(devl_port_register_with_ops);
 
 /**
  *	devlink_port_register - Register devlink port
@@ -6838,6 +6841,7 @@ EXPORT_SYMBOL_GPL(devl_port_register);
  *	@devlink: devlink
  *	@devlink_port: devlink port
  *	@port_index: driver-specific numerical identifier of the port
+ *	@ops: port ops
  *
  *	Register devlink port with provided port index. User can use
  *	any indexing, even hw-related one. devlink_port structure
@@ -6847,18 +6851,20 @@ EXPORT_SYMBOL_GPL(devl_port_register);
  *
  *	Context: Takes and release devlink->lock <mutex>.
  */
-int devlink_port_register(struct devlink *devlink,
-			  struct devlink_port *devlink_port,
-			  unsigned int port_index)
+int devlink_port_register_with_ops(struct devlink *devlink,
+				   struct devlink_port *devlink_port,
+				   unsigned int port_index,
+				   const struct devlink_port_ops *ops)
 {
 	int err;
 
 	devl_lock(devlink);
-	err = devl_port_register(devlink, devlink_port, port_index);
+	err = devl_port_register_with_ops(devlink, devlink_port,
+					  port_index, ops);
 	devl_unlock(devlink);
 	return err;
 }
-EXPORT_SYMBOL_GPL(devlink_port_register);
+EXPORT_SYMBOL_GPL(devlink_port_register_with_ops);
 
 /**
  * devl_port_unregister() - Unregister devlink port
-- 
2.39.2

Re: [patch net-next 01/15] devlink: introduce port ops placeholder

[Jakub Kicinski]

On Wed, 24 May 2023 14:18:22 +0200 Jiri Pirko wrote:
> @@ -6799,6 +6799,7 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
>   * @devlink: devlink
>   * @devlink_port: devlink port
>   * @port_index: driver-specific numerical identifier of the port
> + * @ops: port ops
>   *
>   * Register devlink port with provided port index. User can use
>   * any indexing, even hw-related one. devlink_port structure
> @@ -6806,9 +6807,10 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
>   * Note that the caller should take care of zeroing the devlink_port
>   * structure.
>   */
> -int devl_port_register(struct devlink *devlink,
> -		       struct devlink_port *devlink_port,
> -		       unsigned int port_index)
> +int devl_port_register_with_ops(struct devlink *devlink,
> +				struct devlink_port *devlink_port,
> +				unsigned int port_index,
> +				const struct devlink_port_ops *ops)
>  {
>  	int err;

function name in kdoc needs an update

> @@ -6819,6 +6821,7 @@ int devl_port_register(struct devlink *devlink,
>  	devlink_port_init(devlink, devlink_port);
>  	devlink_port->registered = true;
>  	devlink_port->index = port_index;
> +	devlink_port->ops = ops;
>  	spin_lock_init(&devlink_port->type_lock);
>  	INIT_LIST_HEAD(&devlink_port->reporter_list);
>  	err = xa_insert(&devlink->ports, port_index, devlink_port, GFP_KERNEL);
> @@ -6830,7 +6833,7 @@ int devl_port_register(struct devlink *devlink,
>  	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(devl_port_register);
> +EXPORT_SYMBOL_GPL(devl_port_register_with_ops);
>  
>  /**
>   *	devlink_port_register - Register devlink port
> @@ -6838,6 +6841,7 @@ EXPORT_SYMBOL_GPL(devl_port_register);
>   *	@devlink: devlink
>   *	@devlink_port: devlink port
>   *	@port_index: driver-specific numerical identifier of the port
> + *	@ops: port ops
>   *
>   *	Register devlink port with provided port index. User can use
>   *	any indexing, even hw-related one. devlink_port structure
> @@ -6847,18 +6851,20 @@ EXPORT_SYMBOL_GPL(devl_port_register);
>   *
>   *	Context: Takes and release devlink->lock <mutex>.
>   */
> -int devlink_port_register(struct devlink *devlink,
> -			  struct devlink_port *devlink_port,
> -			  unsigned int port_index)
> +int devlink_port_register_with_ops(struct devlink *devlink,
> +				   struct devlink_port *devlink_port,
> +				   unsigned int port_index,
> +				   const struct devlink_port_ops *ops)

same here.

BTW do we need to provide the "devlink_*" form of this API or can we
use this as an opportunity to move everyone to devl_*. Even if the
driver just wraps the call with devl_lock(), sooner or later people
will coalesce the locking in the drivers, I hope.
-- 
pw-bot: cr

Re: [patch net-next 01/15] devlink: introduce port ops placeholder

[Jiri Pirko]

Thu, May 25, 2023 at 06:48:11AM CEST, kuba@kernel.org wrote:
>On Wed, 24 May 2023 14:18:22 +0200 Jiri Pirko wrote:
>> @@ -6799,6 +6799,7 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
>>   * @devlink: devlink
>>   * @devlink_port: devlink port
>>   * @port_index: driver-specific numerical identifier of the port
>> + * @ops: port ops
>>   *
>>   * Register devlink port with provided port index. User can use
>>   * any indexing, even hw-related one. devlink_port structure
>> @@ -6806,9 +6807,10 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
>>   * Note that the caller should take care of zeroing the devlink_port
>>   * structure.
>>   */
>> -int devl_port_register(struct devlink *devlink,
>> -		       struct devlink_port *devlink_port,
>> -		       unsigned int port_index)
>> +int devl_port_register_with_ops(struct devlink *devlink,
>> +				struct devlink_port *devlink_port,
>> +				unsigned int port_index,
>> +				const struct devlink_port_ops *ops)
>>  {
>>  	int err;
>
>function name in kdoc needs an update

Ah, will fix.


>
>> @@ -6819,6 +6821,7 @@ int devl_port_register(struct devlink *devlink,
>>  	devlink_port_init(devlink, devlink_port);
>>  	devlink_port->registered = true;
>>  	devlink_port->index = port_index;
>> +	devlink_port->ops = ops;
>>  	spin_lock_init(&devlink_port->type_lock);
>>  	INIT_LIST_HEAD(&devlink_port->reporter_list);
>>  	err = xa_insert(&devlink->ports, port_index, devlink_port, GFP_KERNEL);
>> @@ -6830,7 +6833,7 @@ int devl_port_register(struct devlink *devlink,
>>  	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
>>  	return 0;
>>  }
>> -EXPORT_SYMBOL_GPL(devl_port_register);
>> +EXPORT_SYMBOL_GPL(devl_port_register_with_ops);
>>  
>>  /**
>>   *	devlink_port_register - Register devlink port
>> @@ -6838,6 +6841,7 @@ EXPORT_SYMBOL_GPL(devl_port_register);
>>   *	@devlink: devlink
>>   *	@devlink_port: devlink port
>>   *	@port_index: driver-specific numerical identifier of the port
>> + *	@ops: port ops
>>   *
>>   *	Register devlink port with provided port index. User can use
>>   *	any indexing, even hw-related one. devlink_port structure
>> @@ -6847,18 +6851,20 @@ EXPORT_SYMBOL_GPL(devl_port_register);
>>   *
>>   *	Context: Takes and release devlink->lock <mutex>.
>>   */
>> -int devlink_port_register(struct devlink *devlink,
>> -			  struct devlink_port *devlink_port,
>> -			  unsigned int port_index)
>> +int devlink_port_register_with_ops(struct devlink *devlink,
>> +				   struct devlink_port *devlink_port,
>> +				   unsigned int port_index,
>> +				   const struct devlink_port_ops *ops)
>
>same here.

Yeah, will fix.

>
>BTW do we need to provide the "devlink_*" form of this API or can we
>use this as an opportunity to move everyone to devl_*. Even if the
>driver just wraps the call with devl_lock(), sooner or later people
>will coalesce the locking in the drivers, I hope.

It is on my list, stay tuned, will address in a follow-up.


>-- 
>pw-bot: cr

[patch net-next 02/15] ice: register devlink port for PF with ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Use newly introduce devlink port registration function variant and
register devlink port passing ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/intel/ice/ice_devlink.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index bc44cc220818..6661d12772a3 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -1512,6 +1512,9 @@ ice_devlink_set_port_split_options(struct ice_pf *pf,
 	ice_active_port_option = active_idx;
 }
 
+static const struct devlink_port_ops ice_devlink_port_ops = {
+};
+
 /**
  * ice_devlink_create_pf_port - Create a devlink port for this PF
  * @pf: the PF to create a devlink port for
@@ -1551,7 +1554,8 @@ int ice_devlink_create_pf_port(struct ice_pf *pf)
 	devlink_port_attrs_set(devlink_port, &attrs);
 	devlink = priv_to_devlink(pf);
 
-	err = devlink_port_register(devlink, devlink_port, vsi->idx);
+	err = devlink_port_register_with_ops(devlink, devlink_port, vsi->idx,
+					     &ice_devlink_port_ops);
 	if (err) {
 		dev_err(dev, "Failed to create devlink port for PF %d, error %d\n",
 			pf->hw.pf_id, err);
-- 
2.39.2

Re: [patch net-next 02/15] ice: register devlink port for PF with ops

[Jesse Brandeburg]

On 5/24/2023 5:18 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Use newly introduce devlink port registration function variant and
> register devlink port passing ops.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_devlink.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> index bc44cc220818..6661d12772a3 100644
> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> @@ -1512,6 +1512,9 @@ ice_devlink_set_port_split_options(struct ice_pf *pf,
>  	ice_active_port_option = active_idx;
>  }
>  
> +static const struct devlink_port_ops ice_devlink_port_ops = {
> +};
> +
>  /**
>   * ice_devlink_create_pf_port - Create a devlink port for this PF
>   * @pf: the PF to create a devlink port for
> @@ -1551,7 +1554,8 @@ int ice_devlink_create_pf_port(struct ice_pf *pf)
>  	devlink_port_attrs_set(devlink_port, &attrs);
>  	devlink = priv_to_devlink(pf);
>  
> -	err = devlink_port_register(devlink, devlink_port, vsi->idx);
> +	err = devlink_port_register_with_ops(devlink, devlink_port, vsi->idx,
> +					     &ice_devlink_port_ops);
>  	if (err) {
>  		dev_err(dev, "Failed to create devlink port for PF %d, error %d\n",
>  			pf->hw.pf_id, err);

Looks good to me.

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

[patch net-next 03/15] mlxsw_core: register devlink port with ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Use newly introduce devlink port registration function variant and
register devlink port passing ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 22db0bb15c45..605881b17ccc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -3116,6 +3116,9 @@ u64 mlxsw_core_res_get(struct mlxsw_core *mlxsw_core,
 }
 EXPORT_SYMBOL(mlxsw_core_res_get);
 
+static const struct devlink_port_ops mlxsw_devlink_port_ops = {
+};
+
 static int __mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u16 local_port,
 				  enum devlink_port_flavour flavour,
 				  u8 slot_index, u32 port_number, bool split,
@@ -3150,7 +3153,8 @@ static int __mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u16 local_port,
 		devlink_port_linecard_set(devlink_port,
 					  linecard->devlink_linecard);
 	}
-	err = devl_port_register(devlink, devlink_port, local_port);
+	err = devl_port_register_with_ops(devlink, devlink_port, local_port,
+					  &mlxsw_devlink_port_ops);
 	if (err)
 		memset(mlxsw_core_port, 0, sizeof(*mlxsw_core_port));
 	return err;
-- 
2.39.2

Re: [patch net-next 03/15] mlxsw_core: register devlink port with ops

[Petr Machata]

Jiri Pirko <jiri@resnulli.us> writes:

> From: Jiri Pirko <jiri@nvidia.com>
>
> Use newly introduce devlink port registration function variant and
> register devlink port passing ops.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Petr Machata <petrm@nvidia.com>

[patch net-next 04/15] nfp: devlink: register devlink port with ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Use newly introduce devlink port registration function variant and
register devlink port passing ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index bf6bae557158..4e4296ecae7c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -321,6 +321,9 @@ const struct devlink_ops nfp_devlink_ops = {
 	.flash_update		= nfp_devlink_flash_update,
 };
 
+static const struct devlink_port_ops nfp_devlink_port_ops = {
+};
+
 int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 {
 	struct devlink_port_attrs attrs = {};
@@ -351,7 +354,8 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 
 	devlink = priv_to_devlink(app->pf);
 
-	return devl_port_register(devlink, &port->dl_port, port->eth_id);
+	return devl_port_register_with_ops(devlink, &port->dl_port,
+					   port->eth_id, &nfp_devlink_port_ops);
 }
 
 void nfp_devlink_port_unregister(struct nfp_port *port)
-- 
2.39.2

[patch net-next 05/15] devlink: move port_split/unsplit() ops into devlink_port_ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move port_split/unsplit() from devlink_ops into newly introduced
devlink_port_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/intel/ice/ice_devlink.c     |  4 ++--
 drivers/net/ethernet/mellanox/mlxsw/core.c       |  4 ++--
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c |  4 ++--
 include/net/devlink.h                            | 11 +++++++----
 net/devlink/leftover.c                           | 10 +++++-----
 5 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 6661d12772a3..80dc5445b50d 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -1256,8 +1256,6 @@ static const struct devlink_ops ice_devlink_ops = {
 			  BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
 	.reload_down = ice_devlink_reload_down,
 	.reload_up = ice_devlink_reload_up,
-	.port_split = ice_devlink_port_split,
-	.port_unsplit = ice_devlink_port_unsplit,
 	.eswitch_mode_get = ice_eswitch_mode_get,
 	.eswitch_mode_set = ice_eswitch_mode_set,
 	.info_get = ice_devlink_info_get,
@@ -1513,6 +1511,8 @@ ice_devlink_set_port_split_options(struct ice_pf *pf,
 }
 
 static const struct devlink_port_ops ice_devlink_port_ops = {
+	.port_split = ice_devlink_port_split,
+	.port_unsplit = ice_devlink_port_unsplit,
 };
 
 /**
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 605881b17ccc..1ccf3b73ed72 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1723,8 +1723,6 @@ static const struct devlink_ops mlxsw_devlink_ops = {
 				  BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
 	.reload_down		= mlxsw_devlink_core_bus_device_reload_down,
 	.reload_up		= mlxsw_devlink_core_bus_device_reload_up,
-	.port_split			= mlxsw_devlink_port_split,
-	.port_unsplit			= mlxsw_devlink_port_unsplit,
 	.sb_pool_get			= mlxsw_devlink_sb_pool_get,
 	.sb_pool_set			= mlxsw_devlink_sb_pool_set,
 	.sb_port_pool_get		= mlxsw_devlink_sb_port_pool_get,
@@ -3117,6 +3115,8 @@ u64 mlxsw_core_res_get(struct mlxsw_core *mlxsw_core,
 EXPORT_SYMBOL(mlxsw_core_res_get);
 
 static const struct devlink_port_ops mlxsw_devlink_port_ops = {
+	.port_split			= mlxsw_devlink_port_split,
+	.port_unsplit			= mlxsw_devlink_port_unsplit,
 };
 
 static int __mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u16 local_port,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 4e4296ecae7c..8c6954c58a88 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -311,8 +311,6 @@ nfp_devlink_flash_update(struct devlink *devlink,
 }
 
 const struct devlink_ops nfp_devlink_ops = {
-	.port_split		= nfp_devlink_port_split,
-	.port_unsplit		= nfp_devlink_port_unsplit,
 	.sb_pool_get		= nfp_devlink_sb_pool_get,
 	.sb_pool_set		= nfp_devlink_sb_pool_set,
 	.eswitch_mode_get	= nfp_devlink_eswitch_mode_get,
@@ -322,6 +320,8 @@ const struct devlink_ops nfp_devlink_ops = {
 };
 
 static const struct devlink_port_ops nfp_devlink_port_ops = {
+	.port_split		= nfp_devlink_port_split,
+	.port_unsplit		= nfp_devlink_port_unsplit,
 };
 
 int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 850148b98f70..6ddb1fb905b2 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1276,10 +1276,6 @@ struct devlink_ops {
 			 struct netlink_ext_ack *extack);
 	int (*port_type_set)(struct devlink_port *devlink_port,
 			     enum devlink_port_type port_type);
-	int (*port_split)(struct devlink *devlink, struct devlink_port *port,
-			  unsigned int count, struct netlink_ext_ack *extack);
-	int (*port_unsplit)(struct devlink *devlink, struct devlink_port *port,
-			    struct netlink_ext_ack *extack);
 	int (*sb_pool_get)(struct devlink *devlink, unsigned int sb_index,
 			   u16 pool_index,
 			   struct devlink_sb_pool_info *pool_info);
@@ -1653,8 +1649,15 @@ void devlink_free(struct devlink *devlink);
 
 /**
  * struct devlink_port_ops - Port operations
+ * @port_split: Callback used to split the port into multiple ones.
+ * @port_unsplit: Callback used to unsplit the port group back into
+ *		  a single port.
  */
 struct devlink_port_ops {
+	int (*port_split)(struct devlink *devlink, struct devlink_port *port,
+			  unsigned int count, struct netlink_ext_ack *extack);
+	int (*port_unsplit)(struct devlink *devlink, struct devlink_port *port,
+			    struct netlink_ext_ack *extack);
 };
 
 void devlink_port_init(struct devlink *devlink,
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index ff1c2ed84aba..8646861127a0 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -1320,7 +1320,7 @@ static int devlink_nl_cmd_port_split_doit(struct sk_buff *skb,
 
 	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_PORT_SPLIT_COUNT))
 		return -EINVAL;
-	if (!devlink->ops->port_split)
+	if (!devlink_port->ops || !devlink_port->ops->port_split)
 		return -EOPNOTSUPP;
 
 	count = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_SPLIT_COUNT]);
@@ -1339,8 +1339,8 @@ static int devlink_nl_cmd_port_split_doit(struct sk_buff *skb,
 		return -EINVAL;
 	}
 
-	return devlink->ops->port_split(devlink, devlink_port, count,
-					info->extack);
+	return devlink_port->ops->port_split(devlink, devlink_port, count,
+					     info->extack);
 }
 
 static int devlink_nl_cmd_port_unsplit_doit(struct sk_buff *skb,
@@ -1349,9 +1349,9 @@ static int devlink_nl_cmd_port_unsplit_doit(struct sk_buff *skb,
 	struct devlink_port *devlink_port = info->user_ptr[1];
 	struct devlink *devlink = info->user_ptr[0];
 
-	if (!devlink->ops->port_unsplit)
+	if (!devlink_port->ops || !devlink_port->ops->port_unsplit)
 		return -EOPNOTSUPP;
-	return devlink->ops->port_unsplit(devlink, devlink_port, info->extack);
+	return devlink_port->ops->port_unsplit(devlink, devlink_port, info->extack);
 }
 
 static int devlink_nl_cmd_port_new_doit(struct sk_buff *skb,
-- 
2.39.2

Re: [patch net-next 05/15] devlink: move port_split/unsplit() ops into devlink_port_ops

[Jesse Brandeburg]

On 5/24/2023 5:18 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Move port_split/unsplit() from devlink_ops into newly introduced
> devlink_port_ops.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_devlink.c     |  4 ++--
>  drivers/net/ethernet/mellanox/mlxsw/core.c       |  4 ++--
>  drivers/net/ethernet/netronome/nfp/nfp_devlink.c |  4 ++--
>  include/net/devlink.h                            | 11 +++++++----
>  net/devlink/leftover.c                           | 10 +++++-----
>  5 files changed, 18 insertions(+), 15 deletions(-)

For the ice driver changes (and topically reviewed the other drivers in
this patch)

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Re: [patch net-next 05/15] devlink: move port_split/unsplit() ops into devlink_port_ops

[Jakub Kicinski]

On Wed, 24 May 2023 14:18:26 +0200 Jiri Pirko wrote:
>  /**
>   * struct devlink_port_ops - Port operations
> + * @port_split: Callback used to split the port into multiple ones.
> + * @port_unsplit: Callback used to unsplit the port group back into
> + *		  a single port.
>   */
>  struct devlink_port_ops {
> +	int (*port_split)(struct devlink *devlink, struct devlink_port *port,
> +			  unsigned int count, struct netlink_ext_ack *extack);
> +	int (*port_unsplit)(struct devlink *devlink, struct devlink_port *port,
> +			    struct netlink_ext_ack *extack);
>  };

Two random take it or leave it comments: (1) since these are port ops
now do they need the port_* prefix? (2) I've become partial to adding
the kdoc inline in op structs:

/**
 * struct x - it is the X struct
 */
struct x {
	/** @an_op: its an op */
	int (*an_op)(int arg);
};

I think it's because every time I look at struct net_device_ops 
a little part of me gives up.

Re: [patch net-next 05/15] devlink: move port_split/unsplit() ops into devlink_port_ops

[Jiri Pirko]

Thu, May 25, 2023 at 06:53:01AM CEST, kuba@kernel.org wrote:
>On Wed, 24 May 2023 14:18:26 +0200 Jiri Pirko wrote:
>>  /**
>>   * struct devlink_port_ops - Port operations
>> + * @port_split: Callback used to split the port into multiple ones.
>> + * @port_unsplit: Callback used to unsplit the port group back into
>> + *		  a single port.
>>   */
>>  struct devlink_port_ops {
>> +	int (*port_split)(struct devlink *devlink, struct devlink_port *port,
>> +			  unsigned int count, struct netlink_ext_ack *extack);
>> +	int (*port_unsplit)(struct devlink *devlink, struct devlink_port *port,
>> +			    struct netlink_ext_ack *extack);
>>  };
>
>Two random take it or leave it comments: (1) since these are port ops
>now do they need the port_* prefix? (2) I've become partial to adding

Yeah, I'd like to leave them, for grepability purposes.


>the kdoc inline in op structs:
>
>/**
> * struct x - it is the X struct
> */
>struct x {
>	/** @an_op: its an op */
>	int (*an_op)(int arg);
>};
>
>I think it's because every time I look at struct net_device_ops 
>a little part of me gives up.

Does this work? I checked the existing layout of devlink_ops and the
internal comments are ignored by kdoc. Actually the whole devlink_ops
struct is omitted in kdoc. See:
$ scripts/kernel-doc include/net/devlink.h

I followed the format we have for the other ops structures in devlink.h
which works with kernel-doc script.

Re: [patch net-next 05/15] devlink: move port_split/unsplit() ops into devlink_port_ops

[Jakub Kicinski]

On Thu, 25 May 2023 08:05:22 +0200 Jiri Pirko wrote:
> >I think it's because every time I look at struct net_device_ops 
> >a little part of me gives up.  
> 
> Does this work? I checked the existing layout of devlink_ops and the
> internal comments are ignored by kdoc. Actually the whole devlink_ops
> struct is omitted in kdoc. See:
> $ scripts/kernel-doc include/net/devlink.h

Hm, it should work, it's documented as a legal way to kdoc:

https://www.kernel.org/doc/html/next/doc-guide/kernel-doc.html#in-line-member-documentation-comments

We should ask Jon if you're sure it's broken.

Re: [patch net-next 05/15] devlink: move port_split/unsplit() ops into devlink_port_ops

[Jiri Pirko]

Thu, May 25, 2023 at 05:27:03PM CEST, kuba@kernel.org wrote:
>On Thu, 25 May 2023 08:05:22 +0200 Jiri Pirko wrote:
>> >I think it's because every time I look at struct net_device_ops 
>> >a little part of me gives up.  
>> 
>> Does this work? I checked the existing layout of devlink_ops and the
>> internal comments are ignored by kdoc. Actually the whole devlink_ops
>> struct is omitted in kdoc. See:
>> $ scripts/kernel-doc include/net/devlink.h
>
>Hm, it should work, it's documented as a legal way to kdoc:
>
>https://www.kernel.org/doc/html/next/doc-guide/kernel-doc.html#in-line-member-documentation-comments
>
>We should ask Jon if you're sure it's broken.

IDK, you can try, I don't see it there.

Re: [patch net-next 05/15] devlink: move port_split/unsplit() ops into devlink_port_ops

[Petr Machata]

Jiri Pirko <jiri@resnulli.us> writes:

> From: Jiri Pirko <jiri@nvidia.com>
>
> Move port_split/unsplit() from devlink_ops into newly introduced
> devlink_port_ops.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

Reviewed-by: Petr Machata <petrm@nvidia.com> # for mlxsw
Tested-by: Petr Machata <petrm@nvidia.com> # for mlxsw

[patch net-next 06/15] mlx4: register devlink port with ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Use newly introduce devlink port registration function variant and
register devlink port passing ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 277738c50c56..fd81c0b7191d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3027,13 +3027,17 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
 	}
 }
 
+static const struct devlink_port_ops mlx4_devlink_port_ops = {
+};
+
 static int mlx4_init_port_info(struct mlx4_dev *dev, int port)
 {
 	struct devlink *devlink = priv_to_devlink(mlx4_priv(dev));
 	struct mlx4_port_info *info = &mlx4_priv(dev)->port[port];
 	int err;
 
-	err = devl_port_register(devlink, &info->devlink_port, port);
+	err = devl_port_register_with_ops(devlink, &info->devlink_port, port,
+					  &mlx4_devlink_port_ops);
 	if (err)
 		return err;
 
-- 
2.39.2

[patch net-next 07/15] devlink: move port_type_set() op into devlink_port_ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move port_type_set() from devlink_ops into newly introduced
devlink_port_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 52 +++++++++++------------
 include/net/devlink.h                     |  5 ++-
 net/devlink/leftover.c                    |  5 +--
 3 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index fd81c0b7191d..9c652e044c0f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3027,7 +3027,33 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
 	}
 }
 
+static int mlx4_devlink_port_type_set(struct devlink_port *devlink_port,
+				      enum devlink_port_type port_type)
+{
+	struct mlx4_port_info *info = container_of(devlink_port,
+						   struct mlx4_port_info,
+						   devlink_port);
+	enum mlx4_port_type mlx4_port_type;
+
+	switch (port_type) {
+	case DEVLINK_PORT_TYPE_AUTO:
+		mlx4_port_type = MLX4_PORT_TYPE_AUTO;
+		break;
+	case DEVLINK_PORT_TYPE_ETH:
+		mlx4_port_type = MLX4_PORT_TYPE_ETH;
+		break;
+	case DEVLINK_PORT_TYPE_IB:
+		mlx4_port_type = MLX4_PORT_TYPE_IB;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return __set_port_type(info, mlx4_port_type);
+}
+
 static const struct devlink_port_ops mlx4_devlink_port_ops = {
+	.port_type_set = mlx4_devlink_port_type_set,
 };
 
 static int mlx4_init_port_info(struct mlx4_dev *dev, int port)
@@ -3881,31 +3907,6 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data,
 	return err;
 }
 
-static int mlx4_devlink_port_type_set(struct devlink_port *devlink_port,
-				      enum devlink_port_type port_type)
-{
-	struct mlx4_port_info *info = container_of(devlink_port,
-						   struct mlx4_port_info,
-						   devlink_port);
-	enum mlx4_port_type mlx4_port_type;
-
-	switch (port_type) {
-	case DEVLINK_PORT_TYPE_AUTO:
-		mlx4_port_type = MLX4_PORT_TYPE_AUTO;
-		break;
-	case DEVLINK_PORT_TYPE_ETH:
-		mlx4_port_type = MLX4_PORT_TYPE_ETH;
-		break;
-	case DEVLINK_PORT_TYPE_IB:
-		mlx4_port_type = MLX4_PORT_TYPE_IB;
-		break;
-	default:
-		return -EOPNOTSUPP;
-	}
-
-	return __set_port_type(info, mlx4_port_type);
-}
-
 static void mlx4_devlink_param_load_driverinit_values(struct devlink *devlink)
 {
 	struct mlx4_priv *priv = devlink_priv(devlink);
@@ -3990,7 +3991,6 @@ static int mlx4_devlink_reload_up(struct devlink *devlink, enum devlink_reload_a
 }
 
 static const struct devlink_ops mlx4_devlink_ops = {
-	.port_type_set	= mlx4_devlink_port_type_set,
 	.reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT),
 	.reload_down	= mlx4_devlink_reload_down,
 	.reload_up	= mlx4_devlink_reload_up,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 6ddb1fb905b2..6fd1697d0443 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1274,8 +1274,6 @@ struct devlink_ops {
 	int (*reload_up)(struct devlink *devlink, enum devlink_reload_action action,
 			 enum devlink_reload_limit limit, u32 *actions_performed,
 			 struct netlink_ext_ack *extack);
-	int (*port_type_set)(struct devlink_port *devlink_port,
-			     enum devlink_port_type port_type);
 	int (*sb_pool_get)(struct devlink *devlink, unsigned int sb_index,
 			   u16 pool_index,
 			   struct devlink_sb_pool_info *pool_info);
@@ -1652,12 +1650,15 @@ void devlink_free(struct devlink *devlink);
  * @port_split: Callback used to split the port into multiple ones.
  * @port_unsplit: Callback used to unsplit the port group back into
  *		  a single port.
+ * @port_type_set: Callback used to set a type of a port.
  */
 struct devlink_port_ops {
 	int (*port_split)(struct devlink *devlink, struct devlink_port *port,
 			  unsigned int count, struct netlink_ext_ack *extack);
 	int (*port_unsplit)(struct devlink *devlink, struct devlink_port *port,
 			    struct netlink_ext_ack *extack);
+	int (*port_type_set)(struct devlink_port *devlink_port,
+			     enum devlink_port_type port_type);
 };
 
 void devlink_port_init(struct devlink *devlink,
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 8646861127a0..b8fb96a8f4d6 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -1137,14 +1137,13 @@ static int devlink_port_type_set(struct devlink_port *devlink_port,
 {
 	int err;
 
-	if (!devlink_port->devlink->ops->port_type_set)
+	if (!devlink_port->ops || !devlink_port->ops->port_type_set)
 		return -EOPNOTSUPP;
 
 	if (port_type == devlink_port->type)
 		return 0;
 
-	err = devlink_port->devlink->ops->port_type_set(devlink_port,
-							port_type);
+	err = devlink_port->ops->port_type_set(devlink_port, port_type);
 	if (err)
 		return err;
 
-- 
2.39.2

[patch net-next 08/15] sfc: register devlink port with ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Use newly introduce devlink port registration function variant and
register devlink port passing ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/sfc/efx_devlink.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
index 381b805659d3..f93437757ba3 100644
--- a/drivers/net/ethernet/sfc/efx_devlink.c
+++ b/drivers/net/ethernet/sfc/efx_devlink.c
@@ -25,6 +25,10 @@ struct efx_devlink {
 };
 
 #ifdef CONFIG_SFC_SRIOV
+
+static const struct devlink_port_ops sfc_devlink_port_ops = {
+};
+
 static void efx_devlink_del_port(struct devlink_port *dl_port)
 {
 	if (!dl_port)
@@ -57,7 +61,9 @@ static int efx_devlink_add_port(struct efx_nic *efx,
 
 	mport->dl_port.index = mport->mport_id;
 
-	return devl_port_register(efx->devlink, &mport->dl_port, mport->mport_id);
+	return devl_port_register_with_ops(efx->devlink, &mport->dl_port,
+					   mport->mport_id,
+					   &sfc_devlink_port_ops);
 }
 
 static int efx_devlink_port_addr_get(struct devlink_port *port, u8 *hw_addr,
-- 
2.39.2

Re: [patch net-next 08/15] sfc: register devlink port with ops

[Alejandro Lucero Palau]

Hi Jiri,

On 5/24/23 13:18, Jiri Pirko wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>
>
> From: Jiri Pirko <jiri@nvidia.com>
>
> Use newly introduce devlink port registration function variant and
> register devlink port passing ops.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>   drivers/net/ethernet/sfc/efx_devlink.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
> index 381b805659d3..f93437757ba3 100644
> --- a/drivers/net/ethernet/sfc/efx_devlink.c
> +++ b/drivers/net/ethernet/sfc/efx_devlink.c
> @@ -25,6 +25,10 @@ struct efx_devlink {
>   };
>
>   #ifdef CONFIG_SFC_SRIOV
> +
> +static const struct devlink_port_ops sfc_devlink_port_ops = {
> +};
> +

We can have devlink port without SRIOV, so we need this outside the 
previous ifdef.

Apart from that, it looks OK. I'll test it and report back.

>   static void efx_devlink_del_port(struct devlink_port *dl_port)
>   {
>          if (!dl_port)
> @@ -57,7 +61,9 @@ static int efx_devlink_add_port(struct efx_nic *efx,
>
>          mport->dl_port.index = mport->mport_id;
>
> -       return devl_port_register(efx->devlink, &mport->dl_port, mport->mport_id);
> +       return devl_port_register_with_ops(efx->devlink, &mport->dl_port,
> +                                          mport->mport_id,
> +                                          &sfc_devlink_port_ops);
>   }
>
>   static int efx_devlink_port_addr_get(struct devlink_port *port, u8 *hw_addr,
> --
> 2.39.2
>
>

Re: [patch net-next 08/15] sfc: register devlink port with ops

[Jiri Pirko]

Wed, May 24, 2023 at 07:57:50PM CEST, alucerop@amd.com wrote:
>Hi Jiri,
>
>On 5/24/23 13:18, Jiri Pirko wrote:
>> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.

Awesome, could you manage to remove this irrelevant text in your mailer?


>> 
>> 
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> Use newly introduce devlink port registration function variant and
>> register devlink port passing ops.
>> 
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>>   drivers/net/ethernet/sfc/efx_devlink.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
>> index 381b805659d3..f93437757ba3 100644
>> --- a/drivers/net/ethernet/sfc/efx_devlink.c
>> +++ b/drivers/net/ethernet/sfc/efx_devlink.c
>> @@ -25,6 +25,10 @@ struct efx_devlink {
>>   };
>> 
>>   #ifdef CONFIG_SFC_SRIOV
>> +
>> +static const struct devlink_port_ops sfc_devlink_port_ops = {
>> +};
>> +
>
>We can have devlink port without SRIOV, so we need this outside the previous
>ifdef.

Nope, in the original code, efx_devlink_add_port() is under this ifdef
too. So ifdef-wise, this patch does not change anything. Why do you
think so?


>
>Apart from that, it looks OK. I'll test it and report back.
>
>>   static void efx_devlink_del_port(struct devlink_port *dl_port)
>>   {
>>          if (!dl_port)
>> @@ -57,7 +61,9 @@ static int efx_devlink_add_port(struct efx_nic *efx,
>> 
>>          mport->dl_port.index = mport->mport_id;
>> 
>> -       return devl_port_register(efx->devlink, &mport->dl_port, mport->mport_id);
>> +       return devl_port_register_with_ops(efx->devlink, &mport->dl_port,
>> +                                          mport->mport_id,
>> +                                          &sfc_devlink_port_ops);
>>   }
>> 
>>   static int efx_devlink_port_addr_get(struct devlink_port *port, u8 *hw_addr,
>> --
>> 2.39.2
>> 
>> 

Re: [patch net-next 08/15] sfc: register devlink port with ops

[Alejandro Lucero Palau]

On 5/24/23 18:57, Alejandro Lucero Palau wrote:
> Hi Jiri,
>
> On 5/24/23 13:18, Jiri Pirko wrote:
>>
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Use newly introduce devlink port registration function variant and
>> register devlink port passing ops.
>>
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>>   drivers/net/ethernet/sfc/efx_devlink.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c 
>> b/drivers/net/ethernet/sfc/efx_devlink.c
>> index 381b805659d3..f93437757ba3 100644
>> --- a/drivers/net/ethernet/sfc/efx_devlink.c
>> +++ b/drivers/net/ethernet/sfc/efx_devlink.c
>> @@ -25,6 +25,10 @@ struct efx_devlink {
>>   };
>>
>>   #ifdef CONFIG_SFC_SRIOV
>> +
>> +static const struct devlink_port_ops sfc_devlink_port_ops = {
>> +};
>> +
>
> We can have devlink port without SRIOV, so we need this outside the 
> previous ifdef.
>
> Apart from that, it looks OK. I'll test it and report back.
>
Apart from the change requested:

Reviewed-by: Alejandro Lucero<alucerop@amd.com>
Tested-by: Alejandro Lucero<alucerop@amd.com>

>>   static void efx_devlink_del_port(struct devlink_port *dl_port)
>>   {
>>          if (!dl_port)
>> @@ -57,7 +61,9 @@ static int efx_devlink_add_port(struct efx_nic *efx,
>>
>>          mport->dl_port.index = mport->mport_id;
>>
>> -       return devl_port_register(efx->devlink, &mport->dl_port, 
>> mport->mport_id);
>> +       return devl_port_register_with_ops(efx->devlink, 
>> &mport->dl_port,
>> +                                          mport->mport_id,
>> + &sfc_devlink_port_ops);
>>   }
>>
>>   static int efx_devlink_port_addr_get(struct devlink_port *port, u8 
>> *hw_addr,
>> -- 
>> 2.39.2
>>
>>
>

Re: [patch net-next 08/15] sfc: register devlink port with ops

[Jiri Pirko]

Thu, May 25, 2023 at 05:13:57PM CEST, alucerop@amd.com wrote:
>
>On 5/24/23 18:57, Alejandro Lucero Palau wrote:
>> Hi Jiri,
>> 
>> On 5/24/23 13:18, Jiri Pirko wrote:
>> > 
>> > From: Jiri Pirko <jiri@nvidia.com>
>> > 
>> > Use newly introduce devlink port registration function variant and
>> > register devlink port passing ops.
>> > 
>> > Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> > ---
>> >   drivers/net/ethernet/sfc/efx_devlink.c | 8 +++++++-
>> >   1 file changed, 7 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/net/ethernet/sfc/efx_devlink.c
>> > b/drivers/net/ethernet/sfc/efx_devlink.c
>> > index 381b805659d3..f93437757ba3 100644
>> > --- a/drivers/net/ethernet/sfc/efx_devlink.c
>> > +++ b/drivers/net/ethernet/sfc/efx_devlink.c
>> > @@ -25,6 +25,10 @@ struct efx_devlink {
>> >   };
>> > 
>> >   #ifdef CONFIG_SFC_SRIOV
>> > +
>> > +static const struct devlink_port_ops sfc_devlink_port_ops = {
>> > +};
>> > +
>> 
>> We can have devlink port without SRIOV, so we need this outside the
>> previous ifdef.
>> 
>> Apart from that, it looks OK. I'll test it and report back.
>> 
>Apart from the change requested:

Did you see the reply to your email?

>
>Reviewed-by: Alejandro Lucero<alucerop@amd.com>
>Tested-by: Alejandro Lucero<alucerop@amd.com>
>
>> >   static void efx_devlink_del_port(struct devlink_port *dl_port)
>> >   {
>> >          if (!dl_port)
>> > @@ -57,7 +61,9 @@ static int efx_devlink_add_port(struct efx_nic *efx,
>> > 
>> >          mport->dl_port.index = mport->mport_id;
>> > 
>> > -       return devl_port_register(efx->devlink, &mport->dl_port,
>> > mport->mport_id);
>> > +       return devl_port_register_with_ops(efx->devlink,
>> > &mport->dl_port,
>> > +                                          mport->mport_id,
>> > + &sfc_devlink_port_ops);
>> >   }
>> > 
>> >   static int efx_devlink_port_addr_get(struct devlink_port *port, u8
>> > *hw_addr,
>> > -- 
>> > 2.39.2
>> > 
>> > 
>> 

[patch net-next 09/15] mlx5: register devlink ports with ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Use newly introduce devlink port registration function variant and
register devlink port passing ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/esw/devlink_port.c   | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index 084a910bb4e7..d9c17481b972 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -65,6 +65,9 @@ static void mlx5_esw_dl_port_free(struct devlink_port *dl_port)
 	kfree(dl_port);
 }
 
+static const struct devlink_port_ops mlx5_esw_dl_port_ops = {
+};
+
 int mlx5_esw_offloads_devlink_port_register(struct mlx5_eswitch *esw, u16 vport_num)
 {
 	struct mlx5_core_dev *dev = esw->dev;
@@ -87,7 +90,8 @@ int mlx5_esw_offloads_devlink_port_register(struct mlx5_eswitch *esw, u16 vport_
 
 	devlink = priv_to_devlink(dev);
 	dl_port_index = mlx5_esw_vport_to_devlink_port_index(dev, vport_num);
-	err = devl_port_register(devlink, dl_port, dl_port_index);
+	err = devl_port_register_with_ops(devlink, dl_port, dl_port_index,
+					  &mlx5_esw_dl_port_ops);
 	if (err)
 		goto reg_err;
 
@@ -134,6 +138,9 @@ struct devlink_port *mlx5_esw_offloads_devlink_port(struct mlx5_eswitch *esw, u1
 	return IS_ERR(vport) ? ERR_CAST(vport) : vport->dl_port;
 }
 
+static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
+};
+
 int mlx5_esw_devlink_sf_port_register(struct mlx5_eswitch *esw, struct devlink_port *dl_port,
 				      u16 vport_num, u32 controller, u32 sfnum)
 {
@@ -156,7 +163,8 @@ int mlx5_esw_devlink_sf_port_register(struct mlx5_eswitch *esw, struct devlink_p
 	devlink_port_attrs_pci_sf_set(dl_port, controller, pfnum, sfnum, !!controller);
 	devlink = priv_to_devlink(dev);
 	dl_port_index = mlx5_esw_vport_to_devlink_port_index(dev, vport_num);
-	err = devl_port_register(devlink, dl_port, dl_port_index);
+	err = devl_port_register_with_ops(devlink, dl_port, dl_port_index,
+					  &mlx5_esw_dl_sf_port_ops);
 	if (err)
 		return err;
 
-- 
2.39.2

[patch net-next 10/15] devlink: move port_fn_hw_addr_get/set() to devlink_port_ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move port_fn_hw_addr_get/set() from devlink_ops into newly introduced
devlink_port_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/devlink.c |  2 -
 .../mellanox/mlx5/core/esw/devlink_port.c     |  4 +
 .../net/ethernet/mellanox/mlx5/core/eswitch.h | 12 +--
 .../mellanox/mlx5/core/eswitch_offloads.c     | 12 +--
 drivers/net/ethernet/sfc/efx_devlink.c        | 86 +++++++++----------
 include/net/devlink.h                         | 38 ++++----
 net/devlink/leftover.c                        | 15 ++--
 7 files changed, 80 insertions(+), 89 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index bfaec67abf0d..1e96f32bd1b5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -310,8 +310,6 @@ static const struct devlink_ops mlx5_devlink_ops = {
 	.eswitch_inline_mode_get = mlx5_devlink_eswitch_inline_mode_get,
 	.eswitch_encap_mode_set = mlx5_devlink_eswitch_encap_mode_set,
 	.eswitch_encap_mode_get = mlx5_devlink_eswitch_encap_mode_get,
-	.port_function_hw_addr_get = mlx5_devlink_port_function_hw_addr_get,
-	.port_function_hw_addr_set = mlx5_devlink_port_function_hw_addr_set,
 	.rate_leaf_tx_share_set = mlx5_esw_devlink_rate_leaf_tx_share_set,
 	.rate_leaf_tx_max_set = mlx5_esw_devlink_rate_leaf_tx_max_set,
 	.rate_node_tx_share_set = mlx5_esw_devlink_rate_node_tx_share_set,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index d9c17481b972..78d12c377900 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -66,6 +66,8 @@ static void mlx5_esw_dl_port_free(struct devlink_port *dl_port)
 }
 
 static const struct devlink_port_ops mlx5_esw_dl_port_ops = {
+	.port_fn_hw_addr_get = mlx5_devlink_port_fn_hw_addr_get,
+	.port_fn_hw_addr_set = mlx5_devlink_port_fn_hw_addr_set,
 };
 
 int mlx5_esw_offloads_devlink_port_register(struct mlx5_eswitch *esw, u16 vport_num)
@@ -139,6 +141,8 @@ struct devlink_port *mlx5_esw_offloads_devlink_port(struct mlx5_eswitch *esw, u1
 }
 
 static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
+	.port_fn_hw_addr_get = mlx5_devlink_port_fn_hw_addr_get,
+	.port_fn_hw_addr_set = mlx5_devlink_port_fn_hw_addr_set,
 };
 
 int mlx5_esw_devlink_sf_port_register(struct mlx5_eswitch *esw, struct devlink_port *dl_port,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 6c5c05c414fb..9cbededdbe9c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -503,12 +503,12 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
 					struct netlink_ext_ack *extack);
 int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
 					enum devlink_eswitch_encap_mode *encap);
-int mlx5_devlink_port_function_hw_addr_get(struct devlink_port *port,
-					   u8 *hw_addr, int *hw_addr_len,
-					   struct netlink_ext_ack *extack);
-int mlx5_devlink_port_function_hw_addr_set(struct devlink_port *port,
-					   const u8 *hw_addr, int hw_addr_len,
-					   struct netlink_ext_ack *extack);
+int mlx5_devlink_port_fn_hw_addr_get(struct devlink_port *port,
+				     u8 *hw_addr, int *hw_addr_len,
+				     struct netlink_ext_ack *extack);
+int mlx5_devlink_port_fn_hw_addr_set(struct devlink_port *port,
+				     const u8 *hw_addr, int hw_addr_len,
+				     struct netlink_ext_ack *extack);
 int mlx5_devlink_port_fn_roce_get(struct devlink_port *port, bool *is_enabled,
 				  struct netlink_ext_ack *extack);
 int mlx5_devlink_port_fn_roce_set(struct devlink_port *port, bool enable,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 76db72f339d6..7cbdb23c0e18 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -3953,9 +3953,9 @@ is_port_function_supported(struct mlx5_eswitch *esw, u16 vport_num)
 	       mlx5_esw_is_sf_vport(esw, vport_num);
 }
 
-int mlx5_devlink_port_function_hw_addr_get(struct devlink_port *port,
-					   u8 *hw_addr, int *hw_addr_len,
-					   struct netlink_ext_ack *extack)
+int mlx5_devlink_port_fn_hw_addr_get(struct devlink_port *port,
+				     u8 *hw_addr, int *hw_addr_len,
+				     struct netlink_ext_ack *extack)
 {
 	struct mlx5_eswitch *esw;
 	struct mlx5_vport *vport;
@@ -3982,9 +3982,9 @@ int mlx5_devlink_port_function_hw_addr_get(struct devlink_port *port,
 	return 0;
 }
 
-int mlx5_devlink_port_function_hw_addr_set(struct devlink_port *port,
-					   const u8 *hw_addr, int hw_addr_len,
-					   struct netlink_ext_ack *extack)
+int mlx5_devlink_port_fn_hw_addr_set(struct devlink_port *port,
+				     const u8 *hw_addr, int hw_addr_len,
+				     struct netlink_ext_ack *extack)
 {
 	struct mlx5_eswitch *esw;
 	u16 vport_num;
diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
index f93437757ba3..0c5677edc9b6 100644
--- a/drivers/net/ethernet/sfc/efx_devlink.c
+++ b/drivers/net/ethernet/sfc/efx_devlink.c
@@ -26,46 +26,6 @@ struct efx_devlink {
 
 #ifdef CONFIG_SFC_SRIOV
 
-static const struct devlink_port_ops sfc_devlink_port_ops = {
-};
-
-static void efx_devlink_del_port(struct devlink_port *dl_port)
-{
-	if (!dl_port)
-		return;
-	devl_port_unregister(dl_port);
-}
-
-static int efx_devlink_add_port(struct efx_nic *efx,
-				struct mae_mport_desc *mport)
-{
-	bool external = false;
-
-	if (!ef100_mport_on_local_intf(efx, mport))
-		external = true;
-
-	switch (mport->mport_type) {
-	case MAE_MPORT_DESC_MPORT_TYPE_VNIC:
-		if (mport->vf_idx != MAE_MPORT_DESC_VF_IDX_NULL)
-			devlink_port_attrs_pci_vf_set(&mport->dl_port, 0, mport->pf_idx,
-						      mport->vf_idx,
-						      external);
-		else
-			devlink_port_attrs_pci_pf_set(&mport->dl_port, 0, mport->pf_idx,
-						      external);
-		break;
-	default:
-		/* MAE_MPORT_DESC_MPORT_ALIAS and UNDEFINED */
-		return 0;
-	}
-
-	mport->dl_port.index = mport->mport_id;
-
-	return devl_port_register_with_ops(efx->devlink, &mport->dl_port,
-					   mport->mport_id,
-					   &sfc_devlink_port_ops);
-}
-
 static int efx_devlink_port_addr_get(struct devlink_port *port, u8 *hw_addr,
 				     int *hw_addr_len,
 				     struct netlink_ext_ack *extack)
@@ -164,6 +124,48 @@ static int efx_devlink_port_addr_set(struct devlink_port *port,
 	return rc;
 }
 
+static const struct devlink_port_ops sfc_devlink_port_ops = {
+	.port_fn_hw_addr_get = efx_devlink_port_addr_get,
+	.port_fn_hw_addr_set = efx_devlink_port_addr_set,
+};
+
+static void efx_devlink_del_port(struct devlink_port *dl_port)
+{
+	if (!dl_port)
+		return;
+	devl_port_unregister(dl_port);
+}
+
+static int efx_devlink_add_port(struct efx_nic *efx,
+				struct mae_mport_desc *mport)
+{
+	bool external = false;
+
+	if (!ef100_mport_on_local_intf(efx, mport))
+		external = true;
+
+	switch (mport->mport_type) {
+	case MAE_MPORT_DESC_MPORT_TYPE_VNIC:
+		if (mport->vf_idx != MAE_MPORT_DESC_VF_IDX_NULL)
+			devlink_port_attrs_pci_vf_set(&mport->dl_port, 0, mport->pf_idx,
+						      mport->vf_idx,
+						      external);
+		else
+			devlink_port_attrs_pci_pf_set(&mport->dl_port, 0, mport->pf_idx,
+						      external);
+		break;
+	default:
+		/* MAE_MPORT_DESC_MPORT_ALIAS and UNDEFINED */
+		return 0;
+	}
+
+	mport->dl_port.index = mport->mport_id;
+
+	return devl_port_register_with_ops(efx->devlink, &mport->dl_port,
+					   mport->mport_id,
+					   &sfc_devlink_port_ops);
+}
+
 #endif
 
 static int efx_devlink_info_nvram_partition(struct efx_nic *efx,
@@ -620,10 +622,6 @@ static int efx_devlink_info_get(struct devlink *devlink,
 
 static const struct devlink_ops sfc_devlink_ops = {
 	.info_get			= efx_devlink_info_get,
-#ifdef CONFIG_SFC_SRIOV
-	.port_function_hw_addr_get	= efx_devlink_port_addr_get,
-	.port_function_hw_addr_set	= efx_devlink_port_addr_set,
-#endif
 };
 
 #ifdef CONFIG_SFC_SRIOV
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 6fd1697d0443..984829e9239e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1429,28 +1429,6 @@ struct devlink_ops {
 	int (*trap_policer_counter_get)(struct devlink *devlink,
 					const struct devlink_trap_policer *policer,
 					u64 *p_drops);
-	/**
-	 * @port_function_hw_addr_get: Port function's hardware address get function.
-	 *
-	 * Should be used by device drivers to report the hardware address of a function managed
-	 * by the devlink port. Driver should return -EOPNOTSUPP if it doesn't support port
-	 * function handling for a particular port.
-	 *
-	 * Note: @extack can be NULL when port notifier queries the port function.
-	 */
-	int (*port_function_hw_addr_get)(struct devlink_port *port, u8 *hw_addr,
-					 int *hw_addr_len,
-					 struct netlink_ext_ack *extack);
-	/**
-	 * @port_function_hw_addr_set: Port function's hardware address set function.
-	 *
-	 * Should be used by device drivers to set the hardware address of a function managed
-	 * by the devlink port. Driver should return -EOPNOTSUPP if it doesn't support port
-	 * function handling for a particular port.
-	 */
-	int (*port_function_hw_addr_set)(struct devlink_port *port,
-					 const u8 *hw_addr, int hw_addr_len,
-					 struct netlink_ext_ack *extack);
 	/**
 	 * @port_fn_roce_get: Port function's roce get function.
 	 *
@@ -1651,6 +1629,16 @@ void devlink_free(struct devlink *devlink);
  * @port_unsplit: Callback used to unsplit the port group back into
  *		  a single port.
  * @port_type_set: Callback used to set a type of a port.
+ * @port_fn_hw_addr_get: Callback used to set port function's hardware address.
+ *			 Should be used by device drivers to report
+ *			 the hardware address of a function managed
+ *			 by the devlink port.
+ * @port_fn_hw_addr_set: Callback used to set port function's hardware address.
+ *			 Should be used by device drivers to set the hardware
+ *			 address of a function managed by the devlink port.
+ *
+ * Note: Driver should return -EOPNOTSUPP if it doesn't support
+ * port function (@port_fn_*) handling for a particular port.
  */
 struct devlink_port_ops {
 	int (*port_split)(struct devlink *devlink, struct devlink_port *port,
@@ -1659,6 +1647,12 @@ struct devlink_port_ops {
 			    struct netlink_ext_ack *extack);
 	int (*port_type_set)(struct devlink_port *devlink_port,
 			     enum devlink_port_type port_type);
+	int (*port_fn_hw_addr_get)(struct devlink_port *port, u8 *hw_addr,
+				   int *hw_addr_len,
+				   struct netlink_ext_ack *extack);
+	int (*port_fn_hw_addr_set)(struct devlink_port *port,
+				   const u8 *hw_addr, int hw_addr_len,
+				   struct netlink_ext_ack *extack);
 };
 
 void devlink_port_init(struct devlink *devlink,
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index b8fb96a8f4d6..fe38c4b2ab14 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -691,8 +691,7 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 	return 0;
 }
 
-static int devlink_port_fn_hw_addr_fill(const struct devlink_ops *ops,
-					struct devlink_port *port,
+static int devlink_port_fn_hw_addr_fill(struct devlink_port *port,
 					struct sk_buff *msg,
 					struct netlink_ext_ack *extack,
 					bool *msg_updated)
@@ -701,10 +700,10 @@ static int devlink_port_fn_hw_addr_fill(const struct devlink_ops *ops,
 	int hw_addr_len;
 	int err;
 
-	if (!ops->port_function_hw_addr_get)
+	if (!port->ops || !port->ops->port_fn_hw_addr_get)
 		return 0;
 
-	err = ops->port_function_hw_addr_get(port, hw_addr, &hw_addr_len,
+	err = port->ops->port_fn_hw_addr_get(port, hw_addr, &hw_addr_len,
 					     extack);
 	if (err) {
 		if (err == -EOPNOTSUPP)
@@ -884,8 +883,7 @@ devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *por
 		return -EMSGSIZE;
 
 	ops = port->devlink->ops;
-	err = devlink_port_fn_hw_addr_fill(ops, port, msg, extack,
-					   &msg_updated);
+	err = devlink_port_fn_hw_addr_fill(port, msg, extack, &msg_updated);
 	if (err)
 		goto out;
 	err = devlink_port_fn_caps_fill(ops, port, msg, extack,
@@ -1156,7 +1154,6 @@ static int devlink_port_function_hw_addr_set(struct devlink_port *port,
 					     const struct nlattr *attr,
 					     struct netlink_ext_ack *extack)
 {
-	const struct devlink_ops *ops = port->devlink->ops;
 	const u8 *hw_addr;
 	int hw_addr_len;
 
@@ -1177,7 +1174,7 @@ static int devlink_port_function_hw_addr_set(struct devlink_port *port,
 		}
 	}
 
-	return ops->port_function_hw_addr_set(port, hw_addr, hw_addr_len,
+	return port->ops->port_fn_hw_addr_set(port, hw_addr, hw_addr_len,
 					      extack);
 }
 
@@ -1201,7 +1198,7 @@ static int devlink_port_function_validate(struct devlink_port *devlink_port,
 	struct nlattr *attr;
 
 	if (tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] &&
-	    !ops->port_function_hw_addr_set) {
+	    (!devlink_port->ops || !devlink_port->ops->port_fn_hw_addr_set)) {
 		NL_SET_ERR_MSG_ATTR(extack, tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR],
 				    "Port doesn't support function attributes");
 		return -EOPNOTSUPP;
-- 
2.39.2

[patch net-next 11/15] devlink: move port_fn_roce_get/set() to devlink_port_ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move port_fn_roce_get/set() from devlink_ops into newly introduced
devlink_port_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/devlink.c |  2 --
 .../mellanox/mlx5/core/esw/devlink_port.c     |  4 +++
 include/net/devlink.h                         | 31 ++++++++-----------
 net/devlink/leftover.c                        | 18 +++++------
 4 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 1e96f32bd1b5..d63ec466dcd6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -317,8 +317,6 @@ static const struct devlink_ops mlx5_devlink_ops = {
 	.rate_node_new = mlx5_esw_devlink_rate_node_new,
 	.rate_node_del = mlx5_esw_devlink_rate_node_del,
 	.rate_leaf_parent_set = mlx5_esw_devlink_rate_parent_set,
-	.port_fn_roce_get = mlx5_devlink_port_fn_roce_get,
-	.port_fn_roce_set = mlx5_devlink_port_fn_roce_set,
 	.port_fn_migratable_get = mlx5_devlink_port_fn_migratable_get,
 	.port_fn_migratable_set = mlx5_devlink_port_fn_migratable_set,
 #endif
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index 78d12c377900..9011619e1fdd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -68,6 +68,8 @@ static void mlx5_esw_dl_port_free(struct devlink_port *dl_port)
 static const struct devlink_port_ops mlx5_esw_dl_port_ops = {
 	.port_fn_hw_addr_get = mlx5_devlink_port_fn_hw_addr_get,
 	.port_fn_hw_addr_set = mlx5_devlink_port_fn_hw_addr_set,
+	.port_fn_roce_get = mlx5_devlink_port_fn_roce_get,
+	.port_fn_roce_set = mlx5_devlink_port_fn_roce_set,
 };
 
 int mlx5_esw_offloads_devlink_port_register(struct mlx5_eswitch *esw, u16 vport_num)
@@ -143,6 +145,8 @@ struct devlink_port *mlx5_esw_offloads_devlink_port(struct mlx5_eswitch *esw, u1
 static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
 	.port_fn_hw_addr_get = mlx5_devlink_port_fn_hw_addr_get,
 	.port_fn_hw_addr_set = mlx5_devlink_port_fn_hw_addr_set,
+	.port_fn_roce_get = mlx5_devlink_port_fn_roce_get,
+	.port_fn_roce_set = mlx5_devlink_port_fn_roce_set,
 };
 
 int mlx5_esw_devlink_sf_port_register(struct mlx5_eswitch *esw, struct devlink_port *dl_port,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 984829e9239e..5ceedd279a1d 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1429,24 +1429,6 @@ struct devlink_ops {
 	int (*trap_policer_counter_get)(struct devlink *devlink,
 					const struct devlink_trap_policer *policer,
 					u64 *p_drops);
-	/**
-	 * @port_fn_roce_get: Port function's roce get function.
-	 *
-	 * Query RoCE state of a function managed by the devlink port.
-	 * Return -EOPNOTSUPP if port function RoCE handling is not supported.
-	 */
-	int (*port_fn_roce_get)(struct devlink_port *devlink_port,
-				bool *is_enable,
-				struct netlink_ext_ack *extack);
-	/**
-	 * @port_fn_roce_set: Port function's roce set function.
-	 *
-	 * Enable/Disable the RoCE state of a function managed by the devlink
-	 * port.
-	 * Return -EOPNOTSUPP if port function RoCE handling is not supported.
-	 */
-	int (*port_fn_roce_set)(struct devlink_port *devlink_port,
-				bool enable, struct netlink_ext_ack *extack);
 	/**
 	 * @port_fn_migratable_get: Port function's migratable get function.
 	 *
@@ -1636,6 +1618,14 @@ void devlink_free(struct devlink *devlink);
  * @port_fn_hw_addr_set: Callback used to set port function's hardware address.
  *			 Should be used by device drivers to set the hardware
  *			 address of a function managed by the devlink port.
+ * @port_fn_roce_get: Callback used to get port function's RoCE capability.
+ *		      Should be used by device drivers to report
+ *		      the current state of RoCE capability of a function
+ *		      managed by the devlink port.
+ * @port_fn_roce_set: Callback used to set port function's RoCE capability.
+ *		      Should be used by device drivers to enable/disable
+ *		      RoCE capability of a function managed
+ *		      by the devlink port.
  *
  * Note: Driver should return -EOPNOTSUPP if it doesn't support
  * port function (@port_fn_*) handling for a particular port.
@@ -1653,6 +1643,11 @@ struct devlink_port_ops {
 	int (*port_fn_hw_addr_set)(struct devlink_port *port,
 				   const u8 *hw_addr, int hw_addr_len,
 				   struct netlink_ext_ack *extack);
+	int (*port_fn_roce_get)(struct devlink_port *devlink_port,
+				bool *is_enable,
+				struct netlink_ext_ack *extack);
+	int (*port_fn_roce_set)(struct devlink_port *devlink_port,
+				bool enable, struct netlink_ext_ack *extack);
 };
 
 void devlink_port_init(struct devlink *devlink,
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index fe38c4b2ab14..a72586c5b1ad 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -447,18 +447,18 @@ static void devlink_port_fn_cap_fill(struct nla_bitfield32 *caps,
 		caps->value |= cap;
 }
 
-static int devlink_port_fn_roce_fill(const struct devlink_ops *ops,
-				     struct devlink_port *devlink_port,
+static int devlink_port_fn_roce_fill(struct devlink_port *devlink_port,
 				     struct nla_bitfield32 *caps,
 				     struct netlink_ext_ack *extack)
 {
 	bool is_enable;
 	int err;
 
-	if (!ops->port_fn_roce_get)
+	if (!devlink_port->ops || !devlink_port->ops->port_fn_roce_get)
 		return 0;
 
-	err = ops->port_fn_roce_get(devlink_port, &is_enable, extack);
+	err = devlink_port->ops->port_fn_roce_get(devlink_port, &is_enable,
+						  extack);
 	if (err) {
 		if (err == -EOPNOTSUPP)
 			return 0;
@@ -501,7 +501,7 @@ static int devlink_port_fn_caps_fill(const struct devlink_ops *ops,
 	struct nla_bitfield32 caps = {};
 	int err;
 
-	err = devlink_port_fn_roce_fill(ops, devlink_port, &caps, extack);
+	err = devlink_port_fn_roce_fill(devlink_port, &caps, extack);
 	if (err)
 		return err;
 
@@ -837,9 +837,8 @@ static int
 devlink_port_fn_roce_set(struct devlink_port *devlink_port, bool enable,
 			 struct netlink_ext_ack *extack)
 {
-	const struct devlink_ops *ops = devlink_port->devlink->ops;
-
-	return ops->port_fn_roce_set(devlink_port, enable, extack);
+	return devlink_port->ops->port_fn_roce_set(devlink_port, enable,
+						   extack);
 }
 
 static int devlink_port_fn_caps_set(struct devlink_port *devlink_port,
@@ -1214,7 +1213,8 @@ static int devlink_port_function_validate(struct devlink_port *devlink_port,
 
 		caps = nla_get_bitfield32(attr);
 		if (caps.selector & DEVLINK_PORT_FN_CAP_ROCE &&
-		    !ops->port_fn_roce_set) {
+		    (!devlink_port->ops ||
+		     !devlink_port->ops->port_fn_roce_set)) {
 			NL_SET_ERR_MSG_ATTR(extack, attr,
 					    "Port doesn't support RoCE function attribute");
 			return -EOPNOTSUPP;
-- 
2.39.2

[patch net-next 12/15] devlink: move port_fn_migratable_get/set() to devlink_port_ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move port_fn_migratable_get/set() from devlink_ops into newly introduced
devlink_port_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/devlink.c |  2 --
 .../mellanox/mlx5/core/esw/devlink_port.c     |  2 ++
 include/net/devlink.h                         | 35 ++++++++-----------
 net/devlink/leftover.c                        | 24 ++++++-------
 4 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index d63ec466dcd6..678bae618769 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -317,8 +317,6 @@ static const struct devlink_ops mlx5_devlink_ops = {
 	.rate_node_new = mlx5_esw_devlink_rate_node_new,
 	.rate_node_del = mlx5_esw_devlink_rate_node_del,
 	.rate_leaf_parent_set = mlx5_esw_devlink_rate_parent_set,
-	.port_fn_migratable_get = mlx5_devlink_port_fn_migratable_get,
-	.port_fn_migratable_set = mlx5_devlink_port_fn_migratable_set,
 #endif
 #ifdef CONFIG_MLX5_SF_MANAGER
 	.port_new = mlx5_devlink_sf_port_new,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index 9011619e1fdd..2ececd2b86c8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -70,6 +70,8 @@ static const struct devlink_port_ops mlx5_esw_dl_port_ops = {
 	.port_fn_hw_addr_set = mlx5_devlink_port_fn_hw_addr_set,
 	.port_fn_roce_get = mlx5_devlink_port_fn_roce_get,
 	.port_fn_roce_set = mlx5_devlink_port_fn_roce_set,
+	.port_fn_migratable_get = mlx5_devlink_port_fn_migratable_get,
+	.port_fn_migratable_set = mlx5_devlink_port_fn_migratable_set,
 };
 
 int mlx5_esw_offloads_devlink_port_register(struct mlx5_eswitch *esw, u16 vport_num)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 5ceedd279a1d..94a1fdb4105d 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1429,27 +1429,6 @@ struct devlink_ops {
 	int (*trap_policer_counter_get)(struct devlink *devlink,
 					const struct devlink_trap_policer *policer,
 					u64 *p_drops);
-	/**
-	 * @port_fn_migratable_get: Port function's migratable get function.
-	 *
-	 * Query migratable state of a function managed by the devlink port.
-	 * Return -EOPNOTSUPP if port function migratable handling is not
-	 * supported.
-	 */
-	int (*port_fn_migratable_get)(struct devlink_port *devlink_port,
-				      bool *is_enable,
-				      struct netlink_ext_ack *extack);
-	/**
-	 * @port_fn_migratable_set: Port function's migratable set function.
-	 *
-	 * Enable/Disable migratable state of a function managed by the devlink
-	 * port.
-	 * Return -EOPNOTSUPP if port function migratable handling is not
-	 * supported.
-	 */
-	int (*port_fn_migratable_set)(struct devlink_port *devlink_port,
-				      bool enable,
-				      struct netlink_ext_ack *extack);
 	/**
 	 * port_new() - Add a new port function of a specified flavor
 	 * @devlink: Devlink instance
@@ -1626,6 +1605,14 @@ void devlink_free(struct devlink *devlink);
  *		      Should be used by device drivers to enable/disable
  *		      RoCE capability of a function managed
  *		      by the devlink port.
+ * @port_fn_migratable_get: Callback used to get port function's migratable
+ *			    capability. Should be used by device drivers
+ *			    to report the current state of migratable capability
+ *			    of a function managed by the devlink port.
+ * @port_fn_migratable_set: Callback used to set port function's migratable
+ *			    capability. Should be used by device drivers
+ *			    to enable/disable migratable capability of
+ *			    a function managed by the devlink port.
  *
  * Note: Driver should return -EOPNOTSUPP if it doesn't support
  * port function (@port_fn_*) handling for a particular port.
@@ -1648,6 +1635,12 @@ struct devlink_port_ops {
 				struct netlink_ext_ack *extack);
 	int (*port_fn_roce_set)(struct devlink_port *devlink_port,
 				bool enable, struct netlink_ext_ack *extack);
+	int (*port_fn_migratable_get)(struct devlink_port *devlink_port,
+				      bool *is_enable,
+				      struct netlink_ext_ack *extack);
+	int (*port_fn_migratable_set)(struct devlink_port *devlink_port,
+				      bool enable,
+				      struct netlink_ext_ack *extack);
 };
 
 void devlink_port_init(struct devlink *devlink,
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index a72586c5b1ad..e19554187362 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -469,19 +469,19 @@ static int devlink_port_fn_roce_fill(struct devlink_port *devlink_port,
 	return 0;
 }
 
-static int devlink_port_fn_migratable_fill(const struct devlink_ops *ops,
-					   struct devlink_port *devlink_port,
+static int devlink_port_fn_migratable_fill(struct devlink_port *devlink_port,
 					   struct nla_bitfield32 *caps,
 					   struct netlink_ext_ack *extack)
 {
 	bool is_enable;
 	int err;
 
-	if (!ops->port_fn_migratable_get ||
+	if (!devlink_port->ops || !devlink_port->ops->port_fn_migratable_get ||
 	    devlink_port->attrs.flavour != DEVLINK_PORT_FLAVOUR_PCI_VF)
 		return 0;
 
-	err = ops->port_fn_migratable_get(devlink_port, &is_enable, extack);
+	err = devlink_port->ops->port_fn_migratable_get(devlink_port,
+							&is_enable, extack);
 	if (err) {
 		if (err == -EOPNOTSUPP)
 			return 0;
@@ -492,8 +492,7 @@ static int devlink_port_fn_migratable_fill(const struct devlink_ops *ops,
 	return 0;
 }
 
-static int devlink_port_fn_caps_fill(const struct devlink_ops *ops,
-				     struct devlink_port *devlink_port,
+static int devlink_port_fn_caps_fill(struct devlink_port *devlink_port,
 				     struct sk_buff *msg,
 				     struct netlink_ext_ack *extack,
 				     bool *msg_updated)
@@ -505,7 +504,7 @@ static int devlink_port_fn_caps_fill(const struct devlink_ops *ops,
 	if (err)
 		return err;
 
-	err = devlink_port_fn_migratable_fill(ops, devlink_port, &caps, extack);
+	err = devlink_port_fn_migratable_fill(devlink_port, &caps, extack);
 	if (err)
 		return err;
 
@@ -828,9 +827,8 @@ static int
 devlink_port_fn_mig_set(struct devlink_port *devlink_port, bool enable,
 			struct netlink_ext_ack *extack)
 {
-	const struct devlink_ops *ops = devlink_port->devlink->ops;
-
-	return ops->port_fn_migratable_set(devlink_port, enable, extack);
+	return devlink_port->ops->port_fn_migratable_set(devlink_port, enable,
+							 extack);
 }
 
 static int
@@ -885,8 +883,7 @@ devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *por
 	err = devlink_port_fn_hw_addr_fill(port, msg, extack, &msg_updated);
 	if (err)
 		goto out;
-	err = devlink_port_fn_caps_fill(ops, port, msg, extack,
-					&msg_updated);
+	err = devlink_port_fn_caps_fill(port, msg, extack, &msg_updated);
 	if (err)
 		goto out;
 	err = devlink_port_fn_state_fill(ops, port, msg, extack, &msg_updated);
@@ -1220,7 +1217,8 @@ static int devlink_port_function_validate(struct devlink_port *devlink_port,
 			return -EOPNOTSUPP;
 		}
 		if (caps.selector & DEVLINK_PORT_FN_CAP_MIGRATABLE) {
-			if (!ops->port_fn_migratable_set) {
+			if (!devlink_port->ops ||
+			    !devlink_port->ops->port_fn_migratable_set) {
 				NL_SET_ERR_MSG_ATTR(extack, attr,
 						    "Port doesn't support migratable function attribute");
 				return -EOPNOTSUPP;
-- 
2.39.2

[patch net-next 13/15] devlink: move port_fn_state_get/set() to devlink_port_ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move port_fn_state_get/set() from devlink_ops into newly introduced
devlink_port_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/devlink.c |  2 -
 .../mellanox/mlx5/core/esw/devlink_port.c     |  4 ++
 include/net/devlink.h                         | 45 +++++++------------
 net/devlink/leftover.c                        | 19 +++-----
 4 files changed, 26 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 678bae618769..e39fd85ea2f9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -321,8 +321,6 @@ static const struct devlink_ops mlx5_devlink_ops = {
 #ifdef CONFIG_MLX5_SF_MANAGER
 	.port_new = mlx5_devlink_sf_port_new,
 	.port_del = mlx5_devlink_sf_port_del,
-	.port_fn_state_get = mlx5_devlink_sf_port_fn_state_get,
-	.port_fn_state_set = mlx5_devlink_sf_port_fn_state_set,
 #endif
 	.flash_update = mlx5_devlink_flash_update,
 	.info_get = mlx5_devlink_info_get,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index 2ececd2b86c8..76c5d6e9d47f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -149,6 +149,10 @@ static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
 	.port_fn_hw_addr_set = mlx5_devlink_port_fn_hw_addr_set,
 	.port_fn_roce_get = mlx5_devlink_port_fn_roce_get,
 	.port_fn_roce_set = mlx5_devlink_port_fn_roce_set,
+#ifdef CONFIG_MLX5_SF_MANAGER
+	.port_fn_state_get = mlx5_devlink_sf_port_fn_state_get,
+	.port_fn_state_set = mlx5_devlink_sf_port_fn_state_set,
+#endif
 };
 
 int mlx5_esw_devlink_sf_port_register(struct mlx5_eswitch *esw, struct devlink_port *dl_port,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 94a1fdb4105d..21254d6884ee 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1464,36 +1464,6 @@ struct devlink_ops {
 	 */
 	int (*port_del)(struct devlink *devlink, struct devlink_port *port,
 			struct netlink_ext_ack *extack);
-	/**
-	 * port_fn_state_get() - Get the state of a port function
-	 * @devlink: Devlink instance
-	 * @port: The devlink port
-	 * @state: Admin configured state
-	 * @opstate: Current operational state
-	 * @extack: extack for reporting error messages
-	 *
-	 * Reports the admin and operational state of a devlink port function
-	 *
-	 * Return: 0 on success, negative value otherwise.
-	 */
-	int (*port_fn_state_get)(struct devlink_port *port,
-				 enum devlink_port_fn_state *state,
-				 enum devlink_port_fn_opstate *opstate,
-				 struct netlink_ext_ack *extack);
-	/**
-	 * port_fn_state_set() - Set the admin state of a port function
-	 * @devlink: Devlink instance
-	 * @port: The devlink port
-	 * @state: Admin state
-	 * @extack: extack for reporting error messages
-	 *
-	 * Set the admin state of a devlink port function
-	 *
-	 * Return: 0 on success, negative value otherwise.
-	 */
-	int (*port_fn_state_set)(struct devlink_port *port,
-				 enum devlink_port_fn_state state,
-				 struct netlink_ext_ack *extack);
 
 	/**
 	 * Rate control callbacks.
@@ -1613,6 +1583,14 @@ void devlink_free(struct devlink *devlink);
  *			    capability. Should be used by device drivers
  *			    to enable/disable migratable capability of
  *			    a function managed by the devlink port.
+ * @port_fn_state_get: Callback used to get port function's state.
+ *		       Should be used by device drivers to report
+ *		       the current admin and operational state of a
+ *		       function managed by the devlink port.
+ * @port_fn_state_set: Callback used to get port function's state.
+ *		       Should be used by device drivers set
+ *		       the admin state of a function managed
+ *		       by the devlink port.
  *
  * Note: Driver should return -EOPNOTSUPP if it doesn't support
  * port function (@port_fn_*) handling for a particular port.
@@ -1641,6 +1619,13 @@ struct devlink_port_ops {
 	int (*port_fn_migratable_set)(struct devlink_port *devlink_port,
 				      bool enable,
 				      struct netlink_ext_ack *extack);
+	int (*port_fn_state_get)(struct devlink_port *port,
+				 enum devlink_port_fn_state *state,
+				 enum devlink_port_fn_opstate *opstate,
+				 struct netlink_ext_ack *extack);
+	int (*port_fn_state_set)(struct devlink_port *port,
+				 enum devlink_port_fn_state state,
+				 struct netlink_ext_ack *extack);
 };
 
 void devlink_port_init(struct devlink *devlink,
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index e19554187362..ddff9815c651 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -787,8 +787,7 @@ devlink_port_fn_opstate_valid(enum devlink_port_fn_opstate opstate)
 	       opstate == DEVLINK_PORT_FN_OPSTATE_ATTACHED;
 }
 
-static int devlink_port_fn_state_fill(const struct devlink_ops *ops,
-				      struct devlink_port *port,
+static int devlink_port_fn_state_fill(struct devlink_port *port,
 				      struct sk_buff *msg,
 				      struct netlink_ext_ack *extack,
 				      bool *msg_updated)
@@ -797,10 +796,10 @@ static int devlink_port_fn_state_fill(const struct devlink_ops *ops,
 	enum devlink_port_fn_state state;
 	int err;
 
-	if (!ops->port_fn_state_get)
+	if (!port->ops || !port->ops->port_fn_state_get)
 		return 0;
 
-	err = ops->port_fn_state_get(port, &state, &opstate, extack);
+	err = port->ops->port_fn_state_get(port, &state, &opstate, extack);
 	if (err) {
 		if (err == -EOPNOTSUPP)
 			return 0;
@@ -870,7 +869,6 @@ static int
 devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *port,
 				   struct netlink_ext_ack *extack)
 {
-	const struct devlink_ops *ops;
 	struct nlattr *function_attr;
 	bool msg_updated = false;
 	int err;
@@ -879,14 +877,13 @@ devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *por
 	if (!function_attr)
 		return -EMSGSIZE;
 
-	ops = port->devlink->ops;
 	err = devlink_port_fn_hw_addr_fill(port, msg, extack, &msg_updated);
 	if (err)
 		goto out;
 	err = devlink_port_fn_caps_fill(port, msg, extack, &msg_updated);
 	if (err)
 		goto out;
-	err = devlink_port_fn_state_fill(ops, port, msg, extack, &msg_updated);
+	err = devlink_port_fn_state_fill(port, msg, extack, &msg_updated);
 out:
 	if (err || !msg_updated)
 		nla_nest_cancel(msg, function_attr);
@@ -1179,18 +1176,15 @@ static int devlink_port_fn_state_set(struct devlink_port *port,
 				     struct netlink_ext_ack *extack)
 {
 	enum devlink_port_fn_state state;
-	const struct devlink_ops *ops;
 
 	state = nla_get_u8(attr);
-	ops = port->devlink->ops;
-	return ops->port_fn_state_set(port, state, extack);
+	return port->ops->port_fn_state_set(port, state, extack);
 }
 
 static int devlink_port_function_validate(struct devlink_port *devlink_port,
 					  struct nlattr **tb,
 					  struct netlink_ext_ack *extack)
 {
-	const struct devlink_ops *ops = devlink_port->devlink->ops;
 	struct nlattr *attr;
 
 	if (tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] &&
@@ -1199,7 +1193,8 @@ static int devlink_port_function_validate(struct devlink_port *devlink_port,
 				    "Port doesn't support function attributes");
 		return -EOPNOTSUPP;
 	}
-	if (tb[DEVLINK_PORT_FN_ATTR_STATE] && !ops->port_fn_state_set) {
+	if (tb[DEVLINK_PORT_FN_ATTR_STATE] &&
+	    (!devlink_port->ops || !devlink_port->ops->port_fn_state_set)) {
 		NL_SET_ERR_MSG_ATTR(extack, tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR],
 				    "Function does not support state setting");
 		return -EOPNOTSUPP;
-- 
2.39.2

[patch net-next 14/15] devlink: move port_del() to devlink_port_ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Move port_del() from devlink_ops into newly introduced devlink_port_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/devlink.c |  1 -
 .../mellanox/mlx5/core/esw/devlink_port.c     |  3 +++
 include/net/devlink.h                         | 22 +++++--------------
 net/devlink/leftover.c                        |  6 ++---
 4 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index e39fd85ea2f9..63635cc44479 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -320,7 +320,6 @@ static const struct devlink_ops mlx5_devlink_ops = {
 #endif
 #ifdef CONFIG_MLX5_SF_MANAGER
 	.port_new = mlx5_devlink_sf_port_new,
-	.port_del = mlx5_devlink_sf_port_del,
 #endif
 	.flash_update = mlx5_devlink_flash_update,
 	.info_get = mlx5_devlink_info_get,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index 76c5d6e9d47f..f370f67d9e33 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -145,6 +145,9 @@ struct devlink_port *mlx5_esw_offloads_devlink_port(struct mlx5_eswitch *esw, u1
 }
 
 static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
+#ifdef CONFIG_MLX5_SF_MANAGER
+	.port_del = mlx5_devlink_sf_port_del,
+#endif
 	.port_fn_hw_addr_get = mlx5_devlink_port_fn_hw_addr_get,
 	.port_fn_hw_addr_set = mlx5_devlink_port_fn_hw_addr_set,
 	.port_fn_roce_get = mlx5_devlink_port_fn_roce_get,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 21254d6884ee..aeccab044358 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1447,23 +1447,6 @@ struct devlink_ops {
 	int (*port_new)(struct devlink *devlink,
 			const struct devlink_port_new_attrs *attrs,
 			struct netlink_ext_ack *extack);
-	/**
-	 * port_del() - Delete a port function
-	 * @devlink: Devlink instance
-	 * @port: The devlink port
-	 * @extack: extack for reporting error messages
-	 *
-	 * Devlink core will call this device driver function upon user request
-	 * to delete a previously created port function
-	 *
-	 * Notes:
-	 *	- On success, drivers must unregister the corresponding devlink
-	 *	  port
-	 *
-	 * Return: 0 on success, negative value otherwise.
-	 */
-	int (*port_del)(struct devlink *devlink, struct devlink_port *port,
-			struct netlink_ext_ack *extack);
 
 	/**
 	 * Rate control callbacks.
@@ -1560,6 +1543,9 @@ void devlink_free(struct devlink *devlink);
  * @port_unsplit: Callback used to unsplit the port group back into
  *		  a single port.
  * @port_type_set: Callback used to set a type of a port.
+ * @port_del: Callback used to delete selected port along with related function.
+ *	      Devlink core calls this upon user request to delete
+ *	      a port previously created by devlink_ops->port_new().
  * @port_fn_hw_addr_get: Callback used to set port function's hardware address.
  *			 Should be used by device drivers to report
  *			 the hardware address of a function managed
@@ -1602,6 +1588,8 @@ struct devlink_port_ops {
 			    struct netlink_ext_ack *extack);
 	int (*port_type_set)(struct devlink_port *devlink_port,
 			     enum devlink_port_type port_type);
+	int (*port_del)(struct devlink *devlink, struct devlink_port *port,
+			struct netlink_ext_ack *extack);
 	int (*port_fn_hw_addr_get)(struct devlink_port *port, u8 *hw_addr,
 				   int *hw_addr_len,
 				   struct netlink_ext_ack *extack);
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index ddff9815c651..b35dee4dddbc 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -1350,7 +1350,7 @@ static int devlink_nl_cmd_port_new_doit(struct sk_buff *skb,
 	struct devlink_port_new_attrs new_attrs = {};
 	struct devlink *devlink = info->user_ptr[0];
 
-	if (!devlink->ops->port_new || !devlink->ops->port_del)
+	if (!devlink->ops->port_new)
 		return -EOPNOTSUPP;
 
 	if (!info->attrs[DEVLINK_ATTR_PORT_FLAVOUR] ||
@@ -1389,10 +1389,10 @@ static int devlink_nl_cmd_port_del_doit(struct sk_buff *skb,
 	struct netlink_ext_ack *extack = info->extack;
 	struct devlink *devlink = info->user_ptr[0];
 
-	if (!devlink->ops->port_del)
+	if (!devlink_port->ops || !devlink_port->ops->port_del)
 		return -EOPNOTSUPP;
 
-	return devlink->ops->port_del(devlink, devlink_port, extack);
+	return devlink_port->ops->port_del(devlink, devlink_port, extack);
 }
 
 static int
-- 
2.39.2

[patch net-next 15/15] devlink: save devlink_port_ops into a variable in devlink_port_function_validate()

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Now when the original ops variable is removed, introduce it again
but this time for devlink_port_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/devlink/leftover.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index b35dee4dddbc..fd2b1a40b61e 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -1185,16 +1185,17 @@ static int devlink_port_function_validate(struct devlink_port *devlink_port,
 					  struct nlattr **tb,
 					  struct netlink_ext_ack *extack)
 {
+	const struct devlink_port_ops *ops = devlink_port->ops;
 	struct nlattr *attr;
 
 	if (tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] &&
-	    (!devlink_port->ops || !devlink_port->ops->port_fn_hw_addr_set)) {
+	    (!ops || !ops->port_fn_hw_addr_set)) {
 		NL_SET_ERR_MSG_ATTR(extack, tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR],
 				    "Port doesn't support function attributes");
 		return -EOPNOTSUPP;
 	}
 	if (tb[DEVLINK_PORT_FN_ATTR_STATE] &&
-	    (!devlink_port->ops || !devlink_port->ops->port_fn_state_set)) {
+	    (!ops || !ops->port_fn_state_set)) {
 		NL_SET_ERR_MSG_ATTR(extack, tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR],
 				    "Function does not support state setting");
 		return -EOPNOTSUPP;
@@ -1205,15 +1206,13 @@ static int devlink_port_function_validate(struct devlink_port *devlink_port,
 
 		caps = nla_get_bitfield32(attr);
 		if (caps.selector & DEVLINK_PORT_FN_CAP_ROCE &&
-		    (!devlink_port->ops ||
-		     !devlink_port->ops->port_fn_roce_set)) {
+		    (!ops || !ops->port_fn_roce_set)) {
 			NL_SET_ERR_MSG_ATTR(extack, attr,
 					    "Port doesn't support RoCE function attribute");
 			return -EOPNOTSUPP;
 		}
 		if (caps.selector & DEVLINK_PORT_FN_CAP_MIGRATABLE) {
-			if (!devlink_port->ops ||
-			    !devlink_port->ops->port_fn_migratable_set) {
+			if (!ops || !ops->port_fn_migratable_set) {
 				NL_SET_ERR_MSG_ATTR(extack, attr,
 						    "Port doesn't support migratable function attribute");
 				return -EOPNOTSUPP;
-- 
2.39.2

Re: [patch net-next 15/15] devlink: save devlink_port_ops into a variable in devlink_port_function_validate()

[Jakub Kicinski]

On Wed, 24 May 2023 14:18:36 +0200 Jiri Pirko wrote:
> +	const struct devlink_port_ops *ops = devlink_port->ops;
>  	struct nlattr *attr;
>  
>  	if (tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] &&
> -	    (!devlink_port->ops || !devlink_port->ops->port_fn_hw_addr_set)) {
> +	    (!ops || !ops->port_fn_hw_addr_set)) {

I was kinda expected last patch will remove the !ops checks.
Another series comes after this to convert more drivers?

Re: [patch net-next 15/15] devlink: save devlink_port_ops into a variable in devlink_port_function_validate()

[Jiri Pirko]

Thu, May 25, 2023 at 06:55:35AM CEST, kuba@kernel.org wrote:
>On Wed, 24 May 2023 14:18:36 +0200 Jiri Pirko wrote:
>> +	const struct devlink_port_ops *ops = devlink_port->ops;
>>  	struct nlattr *attr;
>>  
>>  	if (tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] &&
>> -	    (!devlink_port->ops || !devlink_port->ops->port_fn_hw_addr_set)) {
>> +	    (!ops || !ops->port_fn_hw_addr_set)) {
>
>I was kinda expected last patch will remove the !ops checks.
>Another series comes after this to convert more drivers?

Well, there are still drivers that don't use the port at all ops. I can
have them register with empty struct if you like, no strong opinition. I
can do that as follow-up (this set has 15 patches already anyway). Let
me know.

Re: [patch net-next 15/15] devlink: save devlink_port_ops into a variable in devlink_port_function_validate()

[Jakub Kicinski]

On Thu, 25 May 2023 07:58:09 +0200 Jiri Pirko wrote:
> >I was kinda expected last patch will remove the !ops checks.
> >Another series comes after this to convert more drivers?  
> 
> Well, there are still drivers that don't use the port at all ops. I can
> have them register with empty struct if you like, no strong opinition. I
> can do that as follow-up (this set has 15 patches already anyway). Let
> me know.

Hm. Or maybe we can hook in an empty ops struct in the core when driver
passes NULL? No strong preference.

Re: [patch net-next 15/15] devlink: save devlink_port_ops into a variable in devlink_port_function_validate()

[Jiri Pirko]

Thu, May 25, 2023 at 05:29:33PM CEST, kuba@kernel.org wrote:
>On Thu, 25 May 2023 07:58:09 +0200 Jiri Pirko wrote:
>> >I was kinda expected last patch will remove the !ops checks.
>> >Another series comes after this to convert more drivers?  
>> 
>> Well, there are still drivers that don't use the port at all ops. I can
>> have them register with empty struct if you like, no strong opinition. I
>> can do that as follow-up (this set has 15 patches already anyway). Let
>> me know.
>
>Hm. Or maybe we can hook in an empty ops struct in the core when driver
>passes NULL? No strong preference.

Okay, will check, thx!

Re: [patch net-next 00/15] devlink: move port ops into separate structure

[Petr Machata]

Jiri Pirko <jiri@resnulli.us> writes:

> From: Jiri Pirko <jiri@nvidia.com>
>
> In devlink, some of the objects have separate ops registered alongside
> with the object itself. Port however have ops in devlink_ops structure.
> For drivers what register multiple kinds of ports with different ops
> this is not convenient.
>
> This patchset changes does following changes:
> 1) Introduces devlink_port_ops with functions that allow devlink port
>    to be registered passing a pointer to driver port ops. (patch #1)
> 2) Converts drivers to define port_ops and register ports passing the
>    ops pointer. (patches #2, #3, #4, #6, #8, and #9)
> 3) Moves ops from devlink_ops struct to devlink_port_ops.
>    (patches #5, #7, #10-15)
>
> No functional changes.

The mlxsw bits look good to me. Will take this for a spin in our
regression and report back tomorrow.