netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Martin Habets <habetsm.xilinx@gmail.com>
To: Jiri Pirko <jiri@resnulli.us>
Cc: netdev@vger.kernel.org, kuba@kernel.org, pabeni@redhat.com,
	davem@davemloft.net, edumazet@google.com, leon@kernel.org,
	saeedm@nvidia.com, moshe@nvidia.com, jesse.brandeburg@intel.com,
	anthony.l.nguyen@intel.com, tariqt@nvidia.com, idosch@nvidia.com,
	petrm@nvidia.com, simon.horman@corigine.com,
	ecree.xilinx@gmail.com, michal.wilczynski@intel.com,
	jacob.e.keller@intel.com
Subject: Re: [patch net-next v2 10/15] devlink: move port_fn_hw_addr_get/set() to devlink_port_ops
Date: Tue, 30 May 2023 09:03:43 +0100	[thread overview]
Message-ID: <ZHWt357tA3QIN3CU@gmail.com> (raw)
In-Reply-To: <20230526102841.2226553-11-jiri@resnulli.us>

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

  reply	other threads:[~2023-05-30  8:03 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-26 10:28 [patch net-next v2 00/15] devlink: move port ops into separate structure Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 01/15] devlink: introduce port ops placeholder Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 02/15] ice: register devlink port for PF with ops Jiri Pirko
2023-05-26 10:52   ` Wilczynski, Michal
2023-05-26 11:35     ` Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 03/15] mlxsw_core: register devlink port " Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 04/15] nfp: devlink: " Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 05/15] devlink: move port_split/unsplit() ops into devlink_port_ops Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 06/15] mlx4: register devlink port with ops Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 07/15] devlink: move port_type_set() op into devlink_port_ops Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 08/15] sfc: register devlink port with ops Jiri Pirko
2023-05-30  8:02   ` Martin Habets
2023-05-26 10:28 ` [patch net-next v2 09/15] mlx5: register devlink ports " Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 10/15] devlink: move port_fn_hw_addr_get/set() to devlink_port_ops Jiri Pirko
2023-05-30  8:03   ` Martin Habets [this message]
2023-05-26 10:28 ` [patch net-next v2 11/15] devlink: move port_fn_roce_get/set() " Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 12/15] devlink: move port_fn_migratable_get/set() " Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 13/15] devlink: move port_fn_state_get/set() " Jiri Pirko
2023-05-26 10:28 ` [patch net-next v2 14/15] devlink: move port_del() " Jiri Pirko
2023-05-27  4:10   ` Jakub Kicinski
2023-05-27  7:42     ` Jiri Pirko
2023-05-29  6:33       ` Jakub Kicinski
2023-05-29  8:31         ` Jiri Pirko
2023-05-30  1:41           ` Jakub Kicinski
2023-05-30  6:33             ` Jiri Pirko
2023-05-30  6:58             ` Jiri Pirko
2023-05-30 17:34               ` Jakub Kicinski
2023-05-31  6:33                 ` Jiri Pirko
2023-06-01 22:16                   ` Jacob Keller
2023-05-26 10:28 ` [patch net-next v2 15/15] devlink: save devlink_port_ops into a variable in devlink_port_function_validate() Jiri Pirko
2023-05-30 18:00 ` [patch net-next v2 00/15] devlink: move port ops into separate structure patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZHWt357tA3QIN3CU@gmail.com \
    --to=habetsm.xilinx@gmail.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=ecree.xilinx@gmail.com \
    --cc=edumazet@google.com \
    --cc=idosch@nvidia.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=michal.wilczynski@intel.com \
    --cc=moshe@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petrm@nvidia.com \
    --cc=saeedm@nvidia.com \
    --cc=simon.horman@corigine.com \
    --cc=tariqt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).