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

[patch net-next v2 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.

---
v1->v2:
- see individual patches, there are 2 cosmetical changes basically:
    - fixed function names in kdoc comments
    - use dummy empty ops in case ops is null

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                        | 119 +++++----
 11 files changed, 298 insertions(+), 279 deletions(-)

-- 
2.39.2

[patch net-next v2 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>
---
v1->v2:
- fixed function names in kdoc comments
- use dummy empty ops in case ops is null
---
 include/net/devlink.h  | 41 +++++++++++++++++++++++++++++++++++------
 net/devlink/leftover.c | 30 +++++++++++++++++++-----------
 2 files changed, 54 insertions(+), 17 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..14bb82403c2d 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -6793,12 +6793,15 @@ void devlink_port_fini(struct devlink_port *devlink_port)
 }
 EXPORT_SYMBOL_GPL(devlink_port_fini);
 
+static const struct devlink_port_ops devlink_port_dummy_ops = {};
+
 /**
- * devl_port_register() - Register devlink port
+ * devl_port_register_with_ops() - Register devlink port
  *
  * @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 +6809,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 +6823,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 ? ops : &devlink_port_dummy_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,14 +6835,15 @@ 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
+ *	devlink_port_register_with_ops - Register devlink port
  *
  *	@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 +6853,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

[patch net-next v2 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>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.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 v2 02/15] ice: register devlink port for PF with ops

[Wilczynski, Michal]

On 5/26/2023 12:28 PM, 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>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.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 = {
> +};

I can see that you're doing this everywhere, but aren't those braces redundant ?
This struct would be initialized to zero anyway.

> +
>  /**
>   * 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: Michal Wilczynski <michal.wilczynski@intel.com>

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

[Jiri Pirko]

Fri, May 26, 2023 at 12:52:21PM CEST, michal.wilczynski@intel.com wrote:
>
>
>On 5/26/2023 12:28 PM, 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>
>> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.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 = {
>> +};
>
>I can see that you're doing this everywhere, but aren't those braces redundant ?
>This struct would be initialized to zero anyway.

Well, yeah, but 3 patches later, they are not going to be empty anymore.


>
>> +
>>  /**
>>   * 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: Michal Wilczynski <michal.wilczynski@intel.com>
>
>

[patch net-next v2 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>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Petr Machata <petrm@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

[patch net-next v2 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 v2 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>
---
v1->v2:
- avoid ops check as they no longer could be null
---
 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 14bb82403c2d..43c97271f299 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->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->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

[patch net-next v2 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 28c435ce98d8..369642478fab 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3024,13 +3024,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 v2 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>
---
v1->v2:
- avoid ops check as they no longer could be null
---
 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 369642478fab..61286b0d9b0c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3024,7 +3024,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)
@@ -3878,31 +3904,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);
@@ -3987,7 +3988,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 43c97271f299..7fbcd58fb21e 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->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 v2 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 ef9971cbb695..e74f74037405 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 v2 08/15] sfc: register devlink port with ops

[Martin Habets]

On Fri, May 26, 2023 at 12:28:34PM +0200, 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>

Acked-by: Martin Habets <habetsm.xilinx@gmail.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 ef9971cbb695..e74f74037405 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

[patch net-next v2 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 v2 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>
---
v1->v2:
- avoid ops check as they no longer could be null
---
 .../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 280dc71b032c..f70124ad71cf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -506,12 +506,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 7a65dcf01dba..1b2f5e273525 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -3957,9 +3957,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;
@@ -3986,9 +3986,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 e74f74037405..b82dad50a5b1 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,
@@ -615,10 +617,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 7fbcd58fb21e..861d9c2a80aa 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_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->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

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

[Martin Habets]

On Fri, May 26, 2023 at 12:28:36PM +0200, Jiri Pirko wrote:
> 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>

For sfc:
Acked-by: Martin Habets <habetsm.xilinx@gmail.com>

> ---
> v1->v2:
> - avoid ops check as they no longer could be null
> ---
>  .../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 280dc71b032c..f70124ad71cf 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> @@ -506,12 +506,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 7a65dcf01dba..1b2f5e273525 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -3957,9 +3957,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;
> @@ -3986,9 +3986,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 e74f74037405..b82dad50a5b1 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,
> @@ -615,10 +617,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 7fbcd58fb21e..861d9c2a80aa 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_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->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 v2 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>
---
v1->v2:
- avoid ops check as they no longer could be null
---
 .../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                        | 17 +++++-----
 4 files changed, 25 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 861d9c2a80aa..22d904bb600a 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->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,7 @@ 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->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 v2 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>
---
v1->v2:
- avoid ops check as they no longer could be null
---
 .../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                        | 23 ++++++------
 4 files changed, 26 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 22d904bb600a..3526e85bc8cb 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->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);
@@ -1219,7 +1216,7 @@ 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->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 v2 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>
---
v1->v2:
- avoid ops check as they no longer could be null
---
 .../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 3526e85bc8cb..03ce24a1397e 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_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->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 v2 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>
---
v1->v2:
- avoid ops check as they no longer could be null
---
 .../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 03ce24a1397e..52aaa439caa5 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -1348,7 +1348,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] ||
@@ -1387,10 +1387,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->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

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

[Jakub Kicinski]

On Fri, 26 May 2023 12:28:40 +0200 Jiri Pirko wrote:
> Move port_del() from devlink_ops into newly introduced devlink_port_ops.

I didn't think this thru last time, I thought port_new will move 
in another patch, but that's impossible (obviously?).

Isn't it kinda weird that the new callback is in one place and del
callback is in another? Asymmetric ?

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

[Jiri Pirko]

Sat, May 27, 2023 at 06:10:08AM CEST, kuba@kernel.org wrote:
>On Fri, 26 May 2023 12:28:40 +0200 Jiri Pirko wrote:
>> Move port_del() from devlink_ops into newly introduced devlink_port_ops.
>
>I didn't think this thru last time, I thought port_new will move 
>in another patch, but that's impossible (obviously?).
>
>Isn't it kinda weird that the new callback is in one place and del
>callback is in another? Asymmetric ?

Yeah, I don't know how to do it differently. port_new() has to be
devlink op, as it operates not on the port but on the device. However,
port_del() operates on device. I was thinking about changing the name of
port_del() to port_destructor() or something like that which would make
the symmetricity issue bit less visible. IDK, up to you. One way or
another, I think this could be easily done as a follow-up (I have 15
patches now already anyway).

Thanks!

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

[Jakub Kicinski]

On Sat, 27 May 2023 09:42:45 +0200 Jiri Pirko wrote:
> >I didn't think this thru last time, I thought port_new will move 
> >in another patch, but that's impossible (obviously?).
> >
> >Isn't it kinda weird that the new callback is in one place and del
> >callback is in another? Asymmetric ?  
> 
> Yeah, I don't know how to do it differently. port_new() has to be
> devlink op, as it operates not on the port but on the device. However,
> port_del() operates on device. I was thinking about changing the name of
> port_del() to port_destructor() or something like that which would make
> the symmetricity issue bit less visible. IDK, up to you. One way or
> another, I think this could be easily done as a follow-up (I have 15
> patches now already anyway).

One could argue logically removing a port is also an operation of 
the parent (i.e. the devlink instance). The fact that the port gets
destroyed in the process is secondary. Ergo maybe we should skip 
this patch?

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

[Jiri Pirko]

Mon, May 29, 2023 at 08:33:34AM CEST, kuba@kernel.org wrote:
>On Sat, 27 May 2023 09:42:45 +0200 Jiri Pirko wrote:
>> >I didn't think this thru last time, I thought port_new will move 
>> >in another patch, but that's impossible (obviously?).
>> >
>> >Isn't it kinda weird that the new callback is in one place and del
>> >callback is in another? Asymmetric ?  
>> 
>> Yeah, I don't know how to do it differently. port_new() has to be
>> devlink op, as it operates not on the port but on the device. However,
>> port_del() operates on device. I was thinking about changing the name of
>> port_del() to port_destructor() or something like that which would make
>> the symmetricity issue bit less visible. IDK, up to you. One way or
>> another, I think this could be easily done as a follow-up (I have 15
>> patches now already anyway).
>
>One could argue logically removing a port is also an operation of 
>the parent (i.e. the devlink instance). The fact that the port gets
>destroyed in the process is secondary. Ergo maybe we should skip 
>this patch?

Well, the port_del() could differ for different port flavours. The
embedding structure of struct devlink_port is also different.

Makes sense to me to skip the flavour switch and have one port_del() for
each port.

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

[Jakub Kicinski]

On Mon, 29 May 2023 10:31:14 +0200 Jiri Pirko wrote:
> >One could argue logically removing a port is also an operation of 
> >the parent (i.e. the devlink instance). The fact that the port gets
> >destroyed in the process is secondary. Ergo maybe we should skip 
> >this patch?  
> 
> Well, the port_del() could differ for different port flavours. The
> embedding structure of struct devlink_port is also different.
> 
> Makes sense to me to skip the flavour switch and have one port_del() for
> each port.

The asymmetry bothers me. It's hard to comment on what the best
approach is given this series shows no benefit of moving port_del().
Maybe even a loss, as mlx5 now has an ifdef in two places:

> 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,

Is it okay if we deferred the port_del() patch until there's some
clear benefit?

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

[Jiri Pirko]

Tue, May 30, 2023 at 03:41:19AM CEST, kuba@kernel.org wrote:
>On Mon, 29 May 2023 10:31:14 +0200 Jiri Pirko wrote:
>> >One could argue logically removing a port is also an operation of 
>> >the parent (i.e. the devlink instance). The fact that the port gets
>> >destroyed in the process is secondary. Ergo maybe we should skip 
>> >this patch?  
>> 
>> Well, the port_del() could differ for different port flavours. The
>> embedding structure of struct devlink_port is also different.
>> 
>> Makes sense to me to skip the flavour switch and have one port_del() for
>> each port.
>
>The asymmetry bothers me. It's hard to comment on what the best
>approach is given this series shows no benefit of moving port_del().
>Maybe even a loss, as mlx5 now has an ifdef in two places:
>
>> 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,
>
>Is it okay if we deferred the port_del() patch until there's some
>clear benefit?

Sure.

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

[Jiri Pirko]

Tue, May 30, 2023 at 03:41:19AM CEST, kuba@kernel.org wrote:
>On Mon, 29 May 2023 10:31:14 +0200 Jiri Pirko wrote:
>> >One could argue logically removing a port is also an operation of 
>> >the parent (i.e. the devlink instance). The fact that the port gets
>> >destroyed in the process is secondary. Ergo maybe we should skip 
>> >this patch?  
>> 
>> Well, the port_del() could differ for different port flavours. The
>> embedding structure of struct devlink_port is also different.
>> 
>> Makes sense to me to skip the flavour switch and have one port_del() for
>> each port.
>
>The asymmetry bothers me. It's hard to comment on what the best

Yeah, I had the same problem with that, but after a lots of thinking,
it is a best I could think of. Please see below for the reasoning.


>approach is given this series shows no benefit of moving port_del().
>Maybe even a loss, as mlx5 now has an ifdef in two places:
>
>> 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

Btw, this ifdef is going to go away in a follow-up patchset.


>>  	.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,
>
>Is it okay if we deferred the port_del() patch until there's some
>clear benefit?

Well actually, there is a clear benefit even in this patchset:

We have 2 flavours of ports each with different ops in mlx5:
VF:
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,
        .port_fn_migratable_get = mlx5_devlink_port_fn_migratable_get,
        .port_fn_migratable_set = mlx5_devlink_port_fn_migratable_set,
};

SF:
static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
        .port_del = mlx5_devlink_sf_port_del,
        .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,
        .port_fn_state_get = mlx5_devlink_sf_port_fn_state_get,
        .port_fn_state_set = mlx5_devlink_sf_port_fn_state_set,
};

You can see that the port_del() op is supported only on the SF flavour.
VF does not support it and therefore port_del() is not defined on it.

Without this patch, I would have to have a helper
mlx5_devlink_port_del() that would check if the port is SF and call
mlx5_devlink_sf_port_del() in that case. This patch reduces the
boilerplate.


Btw if you look at the cmd line api, it also aligns:
$ devlink port add pci/0000:08:00.0 flavour pcisf pfnum 0 sfnum 101
pci/0000:08:00.0/32768: type eth netdev eth4 flavour pcisf controller 0 pfnum 0 sfnum 101 splittable false
  function:
    hw_addr 00:00:00:00:00:00 state inactive opstate detached
$ devlink port del pci/0000:08:00.0/32768

You use pci/0000:08:00.0/32768 as a delete handle.

port_del() is basically an object destructor. Would it perhaps help to
rename is to .port_destructor()? That would somehow ease the asymmetry
:) IDK. I would leave the name as it is a and move to port_ops.

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

[Jakub Kicinski]

On Tue, 30 May 2023 08:58:47 +0200 Jiri Pirko wrote:
> >>  	.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,  
> >
> >Is it okay if we deferred the port_del() patch until there's some
> >clear benefit?  
> 
> Well actually, there is a clear benefit even in this patchset:
> 
> We have 2 flavours of ports each with different ops in mlx5:
> VF:
> 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,
>         .port_fn_migratable_get = mlx5_devlink_port_fn_migratable_get,
>         .port_fn_migratable_set = mlx5_devlink_port_fn_migratable_set,
> };
> 
> SF:
> static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
>         .port_del = mlx5_devlink_sf_port_del,
>         .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,
>         .port_fn_state_get = mlx5_devlink_sf_port_fn_state_get,
>         .port_fn_state_set = mlx5_devlink_sf_port_fn_state_set,
> };
> 
> You can see that the port_del() op is supported only on the SF flavour.
> VF does not support it and therefore port_del() is not defined on it.

This is what I started thinking as well yesterday. Is there any reason
to delete a port which isn't an SF? Similarly - is there any reason to
delete a port which wasn't allocated via port_new?

> Without this patch, I would have to have a helper
> mlx5_devlink_port_del() that would check if the port is SF and call
> mlx5_devlink_sf_port_del() in that case. This patch reduces the
> boilerplate.

... Because if port_del can only happen on port_new'd ports, we should
try to move that check into the core. It'd prevent misuse of the API.

> Btw if you look at the cmd line api, it also aligns:
> $ devlink port add pci/0000:08:00.0 flavour pcisf pfnum 0 sfnum 101
> pci/0000:08:00.0/32768: type eth netdev eth4 flavour pcisf controller 0 pfnum 0 sfnum 101 splittable false
>   function:
>     hw_addr 00:00:00:00:00:00 state inactive opstate detached
> $ devlink port del pci/0000:08:00.0/32768
> 
> You use pci/0000:08:00.0/32768 as a delete handle.
> 
> port_del() is basically an object destructor. Would it perhaps help to
> rename is to .port_destructor()? That would somehow ease the asymmetry
> :) IDK. I would leave the name as it is a and move to port_ops.

Meh.

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

[Jiri Pirko]

Tue, May 30, 2023 at 07:34:00PM CEST, kuba@kernel.org wrote:
>On Tue, 30 May 2023 08:58:47 +0200 Jiri Pirko wrote:
>> >>  	.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,  
>> >
>> >Is it okay if we deferred the port_del() patch until there's some
>> >clear benefit?  
>> 
>> Well actually, there is a clear benefit even in this patchset:
>> 
>> We have 2 flavours of ports each with different ops in mlx5:
>> VF:
>> 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,
>>         .port_fn_migratable_get = mlx5_devlink_port_fn_migratable_get,
>>         .port_fn_migratable_set = mlx5_devlink_port_fn_migratable_set,
>> };
>> 
>> SF:
>> static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
>>         .port_del = mlx5_devlink_sf_port_del,
>>         .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,
>>         .port_fn_state_get = mlx5_devlink_sf_port_fn_state_get,
>>         .port_fn_state_set = mlx5_devlink_sf_port_fn_state_set,
>> };
>> 
>> You can see that the port_del() op is supported only on the SF flavour.
>> VF does not support it and therefore port_del() is not defined on it.
>
>This is what I started thinking as well yesterday. Is there any reason
>to delete a port which isn't an SF? Similarly - is there any reason to
>delete a port which wasn't allocated via port_new?

Good question. Not possible atm. For SR-IOV VFs it probably does not make
sense as there are PCI sysfs knobs to do that. Not sure about SIOV.


>
>> Without this patch, I would have to have a helper
>> mlx5_devlink_port_del() that would check if the port is SF and call
>> mlx5_devlink_sf_port_del() in that case. This patch reduces the
>> boilerplate.
>
>... Because if port_del can only happen on port_new'd ports, we should
>try to move that check into the core. It'd prevent misuse of the API.

Okay, that is fair. Will make this change now. If the future brings
different needs, easy to change.


>
>> Btw if you look at the cmd line api, it also aligns:
>> $ devlink port add pci/0000:08:00.0 flavour pcisf pfnum 0 sfnum 101
>> pci/0000:08:00.0/32768: type eth netdev eth4 flavour pcisf controller 0 pfnum 0 sfnum 101 splittable false
>>   function:
>>     hw_addr 00:00:00:00:00:00 state inactive opstate detached
>> $ devlink port del pci/0000:08:00.0/32768
>> 
>> You use pci/0000:08:00.0/32768 as a delete handle.
>> 
>> port_del() is basically an object destructor. Would it perhaps help to
>> rename is to .port_destructor()? That would somehow ease the asymmetry
>> :) IDK. I would leave the name as it is a and move to port_ops.
>
>Meh.

Yeah :)

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

[Jacob Keller]

On 5/30/2023 11:33 PM, Jiri Pirko wrote:
> Tue, May 30, 2023 at 07:34:00PM CEST, kuba@kernel.org wrote:
>> On Tue, 30 May 2023 08:58:47 +0200 Jiri Pirko wrote:
>>>>>  	.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,  
>>>>
>>>> Is it okay if we deferred the port_del() patch until there's some
>>>> clear benefit?  
>>>
>>> Well actually, there is a clear benefit even in this patchset:
>>>
>>> We have 2 flavours of ports each with different ops in mlx5:
>>> VF:
>>> 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,
>>>         .port_fn_migratable_get = mlx5_devlink_port_fn_migratable_get,
>>>         .port_fn_migratable_set = mlx5_devlink_port_fn_migratable_set,
>>> };
>>>
>>> SF:
>>> static const struct devlink_port_ops mlx5_esw_dl_sf_port_ops = {
>>>         .port_del = mlx5_devlink_sf_port_del,
>>>         .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,
>>>         .port_fn_state_get = mlx5_devlink_sf_port_fn_state_get,
>>>         .port_fn_state_set = mlx5_devlink_sf_port_fn_state_set,
>>> };
>>>
>>> You can see that the port_del() op is supported only on the SF flavour.
>>> VF does not support it and therefore port_del() is not defined on it.
>>
>> This is what I started thinking as well yesterday. Is there any reason
>> to delete a port which isn't an SF? Similarly - is there any reason to
>> delete a port which wasn't allocated via port_new?
> 
> Good question. Not possible atm. For SR-IOV VFs it probably does not make
> sense as there are PCI sysfs knobs to do that. Not sure about SIOV.
> 
> 

At least for ice, the current plan for Scalable IOV I had was creating
ports via port_new, so port_del makes sense there. I don't know how else
others were planning on doing this. We're a bit delayed on being able to
post actual patches just yet though :(

>>
>>> Without this patch, I would have to have a helper
>>> mlx5_devlink_port_del() that would check if the port is SF and call
>>> mlx5_devlink_sf_port_del() in that case. This patch reduces the
>>> boilerplate.
>>
>> ... Because if port_del can only happen on port_new'd ports, we should
>> try to move that check into the core. It'd prevent misuse of the API.
> 
> Okay, that is fair. Will make this change now. If the future brings
> different needs, easy to change.
> 
> 
>>
>>> Btw if you look at the cmd line api, it also aligns:
>>> $ devlink port add pci/0000:08:00.0 flavour pcisf pfnum 0 sfnum 101
>>> pci/0000:08:00.0/32768: type eth netdev eth4 flavour pcisf controller 0 pfnum 0 sfnum 101 splittable false
>>>   function:
>>>     hw_addr 00:00:00:00:00:00 state inactive opstate detached
>>> $ devlink port del pci/0000:08:00.0/32768
>>>
>>> You use pci/0000:08:00.0/32768 as a delete handle.
>>>
>>> port_del() is basically an object destructor. Would it perhaps help to
>>> rename is to .port_destructor()? That would somehow ease the asymmetry
>>> :) IDK. I would leave the name as it is a and move to port_ops.
>>
>> Meh.
> 

i like thinking about it in terms of destructor, but that sort of
implies its called whenever the port is removed (i.e. even if removed by
the driver via something like devlink_port_unregister or whatever its
called).

> Yeah :)
> 

[patch net-next v2 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>
---
v1->v2:
- rebased on top of removed ops checks
---
 net/devlink/leftover.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 52aaa439caa5..bfec0a744280 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -1185,16 +1185,16 @@ 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->port_fn_hw_addr_set) {
+	    !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->port_fn_state_set) {
+	if (tb[DEVLINK_PORT_FN_ATTR_STATE] && !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,13 +1205,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->port_fn_roce_set) {
+		    !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->port_fn_migratable_set) {
+			if (!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 v2 00/15] devlink: move port ops into separate structure

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 26 May 2023 12:28:26 +0200 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,01/15] devlink: introduce port ops placeholder
    https://git.kernel.org/netdev/net-next/c/6acdf43d8abe
  - [net-next,v2,02/15] ice: register devlink port for PF with ops
    https://git.kernel.org/netdev/net-next/c/b2857685372b
  - [net-next,v2,03/15] mlxsw_core: register devlink port with ops
    https://git.kernel.org/netdev/net-next/c/865a1a1b97b6
  - [net-next,v2,04/15] nfp: devlink: register devlink port with ops
    https://git.kernel.org/netdev/net-next/c/ab8ccc6c1347
  - [net-next,v2,05/15] devlink: move port_split/unsplit() ops into devlink_port_ops
    https://git.kernel.org/netdev/net-next/c/f58a3e4dfe24
  - [net-next,v2,06/15] mlx4: register devlink port with ops
    https://git.kernel.org/netdev/net-next/c/8a756d91d26c
  - [net-next,v2,07/15] devlink: move port_type_set() op into devlink_port_ops
    https://git.kernel.org/netdev/net-next/c/65a4c44bf937
  - [net-next,v2,08/15] sfc: register devlink port with ops
    https://git.kernel.org/netdev/net-next/c/7bfb3d0a83b6
  - [net-next,v2,09/15] mlx5: register devlink ports with ops
    https://git.kernel.org/netdev/net-next/c/aa3aff8264f2
  - [net-next,v2,10/15] devlink: move port_fn_hw_addr_get/set() to devlink_port_ops
    https://git.kernel.org/netdev/net-next/c/71c93e37cf3d
  - [net-next,v2,11/15] devlink: move port_fn_roce_get/set() to devlink_port_ops
    https://git.kernel.org/netdev/net-next/c/933c13275c49
  - [net-next,v2,12/15] devlink: move port_fn_migratable_get/set() to devlink_port_ops
    https://git.kernel.org/netdev/net-next/c/4a490d7154b3
  - [net-next,v2,13/15] devlink: move port_fn_state_get/set() to devlink_port_ops
    https://git.kernel.org/netdev/net-next/c/216aa67f3e98
  - [net-next,v2,14/15] devlink: move port_del() to devlink_port_ops
    https://git.kernel.org/netdev/net-next/c/216ba9f4adc8
  - [net-next,v2,15/15] devlink: save devlink_port_ops into a variable in devlink_port_function_validate()
    https://git.kernel.org/netdev/net-next/c/4b5ed2b5a145

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html