diff mbox series

[v3,net-next,5/8] sfc: add devlink port support for ef100

Message ID 20230127093651.54035-6-alejandro.lucero-palau@amd.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series sfc: devlink support for ef100 | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 57 this patch: 57
netdev/source_inline success Was 0 now: 0

Commit Message

Lucero Palau, Alejandro Jan. 27, 2023, 9:36 a.m. UTC 
From: Alejandro Lucero <alejandro.lucero-palau@amd.com>

Using the data when enumerating mports, create devlink ports just before
netdevs are registered and remove those devlink ports after netdev has
been unregistered.

Signed-off-by: Alejandro Lucero <alejandro.lucero-palau@amd.com>
---
 drivers/net/ethernet/sfc/ef100_netdev.c |  9 +++
 drivers/net/ethernet/sfc/ef100_rep.c    | 22 ++++++
 drivers/net/ethernet/sfc/ef100_rep.h    |  7 ++
 drivers/net/ethernet/sfc/efx_devlink.c  | 97 +++++++++++++++++++++++++
 drivers/net/ethernet/sfc/efx_devlink.h  |  7 ++
 drivers/net/ethernet/sfc/mae.h          |  2 +
 drivers/net/ethernet/sfc/net_driver.h   |  2 +
 7 files changed, 146 insertions(+)

Comments

Martin Habets Jan. 27, 2023, 11:35 a.m. UTC | #1 
On Fri, Jan 27, 2023 at 09:36:48AM +0000, alejandro.lucero-palau@amd.com wrote:
> From: Alejandro Lucero <alejandro.lucero-palau@amd.com>
> 
> Using the data when enumerating mports, create devlink ports just before
> netdevs are registered and remove those devlink ports after netdev has
> been unregistered.
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero-palau@amd.com>
> ---
>  drivers/net/ethernet/sfc/ef100_netdev.c |  9 +++
>  drivers/net/ethernet/sfc/ef100_rep.c    | 22 ++++++
>  drivers/net/ethernet/sfc/ef100_rep.h    |  7 ++
>  drivers/net/ethernet/sfc/efx_devlink.c  | 97 +++++++++++++++++++++++++
>  drivers/net/ethernet/sfc/efx_devlink.h  |  7 ++
>  drivers/net/ethernet/sfc/mae.h          |  2 +
>  drivers/net/ethernet/sfc/net_driver.h   |  2 +
>  7 files changed, 146 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
> index b10a226f4a07..36774b55d413 100644
> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
> @@ -335,7 +335,9 @@ void ef100_remove_netdev(struct efx_probe_data *probe_data)
>  
>  	/* devlink lock */
>  	efx_fini_devlink_start(efx);
> +
>  	ef100_unregister_netdev(efx);
> +	ef100_pf_unset_devlink_port(efx);
>  
>  #ifdef CONFIG_SFC_SRIOV
>  	efx_fini_tc(efx);
> @@ -423,6 +425,8 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>  		rc = ef100_probe_netdev_pf(efx);
>  		if (rc)
>  			goto fail;
> +
> +		ef100_pf_set_devlink_port(efx);
>  	}
>  
>  	efx->netdev_notifier.notifier_call = ef100_netdev_event;
> @@ -433,7 +437,12 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>  		goto fail;
>  	}
>  
> +	/* devlink unlock */
> +	efx_probe_devlink_done(efx);
> +	return rc;
>  fail:
> +	/* remove devlink port if does exist */
> +	ef100_pf_unset_devlink_port(efx);
>  	/* devlink unlock */
>  	efx_probe_devlink_done(efx);
>  	return rc;
> diff --git a/drivers/net/ethernet/sfc/ef100_rep.c b/drivers/net/ethernet/sfc/ef100_rep.c
> index 9cd1a3ac67e0..6b5bc5d6955d 100644
> --- a/drivers/net/ethernet/sfc/ef100_rep.c
> +++ b/drivers/net/ethernet/sfc/ef100_rep.c
> @@ -16,6 +16,7 @@
>  #include "mae.h"
>  #include "rx_common.h"
>  #include "tc_bindings.h"
> +#include "efx_devlink.h"
>  
>  #define EFX_EF100_REP_DRIVER	"efx_ef100_rep"
>  
> @@ -297,6 +298,7 @@ int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
>  			i, rc);
>  		goto fail1;
>  	}
> +	ef100_rep_set_devlink_port(efv);
>  	rc = register_netdev(efv->net_dev);
>  	if (rc) {
>  		pci_err(efx->pci_dev,
> @@ -308,6 +310,7 @@ int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
>  		efv->net_dev->name);
>  	return 0;
>  fail2:
> +	ef100_rep_unset_devlink_port(efv);
>  	efx_ef100_deconfigure_rep(efv);
>  fail1:
>  	efx_ef100_rep_destroy_netdev(efv);
> @@ -323,6 +326,7 @@ void efx_ef100_vfrep_destroy(struct efx_nic *efx, struct efx_rep *efv)
>  		return;
>  	netif_dbg(efx, drv, rep_dev, "Removing VF representor\n");
>  	unregister_netdev(rep_dev);
> +	ef100_rep_unset_devlink_port(efv);
>  	efx_ef100_deconfigure_rep(efv);
>  	efx_ef100_rep_destroy_netdev(efv);
>  }
> @@ -339,6 +343,24 @@ void efx_ef100_fini_vfreps(struct efx_nic *efx)
>  		efx_ef100_vfrep_destroy(efx, efv);
>  }
>  
> +static bool ef100_mport_is_pcie_vnic(struct mae_mport_desc *mport_desc)
> +{
> +	return mport_desc->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC &&
> +	       mport_desc->vnic_client_type == MAE_MPORT_DESC_VNIC_CLIENT_TYPE_FUNCTION;
> +}
> +
> +bool ef100_mport_on_local_intf(struct efx_nic *efx,
> +			       struct mae_mport_desc *mport_desc)
> +{
> +	struct ef100_nic_data *nic_data = efx->nic_data;
> +	bool pcie_func;
> +
> +	pcie_func = ef100_mport_is_pcie_vnic(mport_desc);
> +
> +	return nic_data->have_local_intf && pcie_func &&
> +		     mport_desc->interface_idx == nic_data->local_mae_intf;
> +}
> +
>  void efx_ef100_init_reps(struct efx_nic *efx)
>  {
>  	struct ef100_nic_data *nic_data = efx->nic_data;
> diff --git a/drivers/net/ethernet/sfc/ef100_rep.h b/drivers/net/ethernet/sfc/ef100_rep.h
> index 328ac0cbb532..ae6add4b0855 100644
> --- a/drivers/net/ethernet/sfc/ef100_rep.h
> +++ b/drivers/net/ethernet/sfc/ef100_rep.h
> @@ -22,6 +22,8 @@ struct efx_rep_sw_stats {
>  	atomic64_t rx_dropped, tx_errors;
>  };
>  
> +struct devlink_port;
> +
>  /**
>   * struct efx_rep - Private data for an Efx representor
>   *
> @@ -39,6 +41,7 @@ struct efx_rep_sw_stats {
>   * @rx_lock: protects @rx_list
>   * @napi: NAPI control structure
>   * @stats: software traffic counters for netdev stats
> + * @dl_port: devlink port associated to this netdev representor
>   */
>  struct efx_rep {
>  	struct efx_nic *parent;
> @@ -54,6 +57,7 @@ struct efx_rep {
>  	spinlock_t rx_lock;
>  	struct napi_struct napi;
>  	struct efx_rep_sw_stats stats;
> +	struct devlink_port *dl_port;
>  };
>  
>  int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i);
> @@ -69,4 +73,7 @@ struct efx_rep *efx_ef100_find_rep_by_mport(struct efx_nic *efx, u16 mport);
>  extern const struct net_device_ops efx_ef100_rep_netdev_ops;
>  void efx_ef100_init_reps(struct efx_nic *efx);
>  void efx_ef100_fini_reps(struct efx_nic *efx);
> +struct mae_mport_desc;
> +bool ef100_mport_on_local_intf(struct efx_nic *efx,
> +			       struct mae_mport_desc *mport_desc);
>  #endif /* EF100_REP_H */
> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
> index ff5adfe3905e..b1637eb372ad 100644
> --- a/drivers/net/ethernet/sfc/efx_devlink.c
> +++ b/drivers/net/ethernet/sfc/efx_devlink.c
> @@ -16,11 +16,48 @@
>  #include "mcdi.h"
>  #include "mcdi_functions.h"
>  #include "mcdi_pcol.h"
> +#include "mae.h"
> +#include "ef100_rep.h"
>  
>  struct efx_devlink {
>  	struct efx_nic *efx;
>  };
>  
> +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(efx->devlink, &mport->dl_port, mport->mport_id);
> +}
> +
>  static int efx_devlink_info_nvram_partition(struct efx_nic *efx,
>  					    struct devlink_info_req *req,
>  					    unsigned int partition_type,
> @@ -428,6 +465,66 @@ static const struct devlink_ops sfc_devlink_ops = {
>  	.info_get			= efx_devlink_info_get,
>  };
>  
> +static struct devlink_port *ef100_set_devlink_port(struct efx_nic *efx, u32 idx)
> +{
> +	struct mae_mport_desc *mport;
> +	u32 id;
> +
> +	if (efx_mae_lookup_mport(efx, idx, &id)) {
> +		/* This should not happen. */
> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
> +			pci_warn(efx->pci_dev, "No mport ID found for PF.\n");
> +		else
> +			pci_warn(efx->pci_dev, "No mport ID found for VF %u.\n",
> +				 idx);
> +		return NULL;
> +	}
> +
> +	mport = efx_mae_get_mport(efx, id);
> +	if (!mport) {
> +		/* This should not happen. */
> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
> +			pci_warn(efx->pci_dev, "No mport found for PF.\n");
> +		else
> +			pci_warn(efx->pci_dev, "No mport found for VF %u.\n",
> +				 idx);
> +		return NULL;
> +	}
> +
> +	if (efx_devlink_add_port(efx, mport)) {
> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
> +			pci_warn(efx->pci_dev,
> +				 "devlink port creation for PF failed.\n");
> +		else
> +			pci_warn(efx->pci_dev,
> +				 "devlink_port creationg for VF %u failed.\n",
> +				 idx);
> +		return NULL;
> +	}
> +
> +	return &mport->dl_port;
> +}
> +
> +void ef100_rep_set_devlink_port(struct efx_rep *efv)
> +{
> +	efv->dl_port = ef100_set_devlink_port(efv->parent, efv->idx);
> +}
> +
> +void ef100_pf_set_devlink_port(struct efx_nic *efx)
> +{
> +	efx->dl_port = ef100_set_devlink_port(efx, MAE_MPORT_DESC_VF_IDX_NULL);
> +}
> +
> +void ef100_rep_unset_devlink_port(struct efx_rep *efv)
> +{
> +	efx_devlink_del_port(efv->dl_port);
> +}
> +
> +void ef100_pf_unset_devlink_port(struct efx_nic *efx)
> +{
> +	efx_devlink_del_port(efx->dl_port);
> +}

I'd rather see these as static inline functions in the .h file.

> +
>  void efx_fini_devlink_start(struct efx_nic *efx)
>  {
>  	if (efx->devlink)
> diff --git a/drivers/net/ethernet/sfc/efx_devlink.h b/drivers/net/ethernet/sfc/efx_devlink.h
> index 8bcd077d8d8d..8d993ac2572b 100644
> --- a/drivers/net/ethernet/sfc/efx_devlink.h
> +++ b/drivers/net/ethernet/sfc/efx_devlink.h
> @@ -36,4 +36,11 @@ void efx_probe_devlink_done(struct efx_nic *efx);
>  void efx_fini_devlink_start(struct efx_nic *efx);
>  void efx_fini_devlink(struct efx_nic *efx);
>  
> +struct mae_mport_desc;

Why do you need this here? The new APIs below don't need it.

Martin

> +struct efx_rep;
> +
> +void ef100_pf_set_devlink_port(struct efx_nic *efx);
> +void ef100_rep_set_devlink_port(struct efx_rep *efv);
> +void ef100_pf_unset_devlink_port(struct efx_nic *efx);
> +void ef100_rep_unset_devlink_port(struct efx_rep *efv);
>  #endif	/* _EFX_DEVLINK_H */
> diff --git a/drivers/net/ethernet/sfc/mae.h b/drivers/net/ethernet/sfc/mae.h
> index d9adeafc0654..e1b7967132ad 100644
> --- a/drivers/net/ethernet/sfc/mae.h
> +++ b/drivers/net/ethernet/sfc/mae.h
> @@ -13,6 +13,7 @@
>  #define EF100_MAE_H
>  /* MCDI interface for the ef100 Match-Action Engine */
>  
> +#include <net/devlink.h>
>  #include "net_driver.h"
>  #include "tc.h"
>  #include "mcdi_pcol.h" /* needed for various MC_CMD_MAE_*_NULL defines */
> @@ -44,6 +45,7 @@ struct mae_mport_desc {
>  	};
>  	struct rhash_head linkage;
>  	struct efx_rep *efv;
> +	struct devlink_port dl_port;
>  };
>  
>  int efx_mae_enumerate_mports(struct efx_nic *efx);
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index bc9efbfb3d6b..fcd51d3992fa 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -998,6 +998,7 @@ struct efx_mae;
>   * @netdev_notifier: Netdevice notifier.
>   * @tc: state for TC offload (EF100).
>   * @devlink: reference to devlink structure owned by this device
> + * @dl_port: devlink port associated with the PF
>   * @mem_bar: The BAR that is mapped into membase.
>   * @reg_base: Offset from the start of the bar to the function control window.
>   * @monitor_work: Hardware monitor workitem
> @@ -1185,6 +1186,7 @@ struct efx_nic {
>  	struct efx_tc_state *tc;
>  
>  	struct devlink *devlink;
> +	struct devlink_port *dl_port;
>  	unsigned int mem_bar;
>  	u32 reg_base;
>  
> -- 
> 2.17.1
Lucero Palau, Alejandro Jan. 31, 2023, 9:36 a.m. UTC | #2 
On 1/27/23 11:35, Martin Habets wrote:
> On Fri, Jan 27, 2023 at 09:36:48AM +0000, alejandro.lucero-palau@amd.com wrote:
>> From: Alejandro Lucero <alejandro.lucero-palau@amd.com>
>>
>> Using the data when enumerating mports, create devlink ports just before
>> netdevs are registered and remove those devlink ports after netdev has
>> been unregistered.
>>
>> Signed-off-by: Alejandro Lucero <alejandro.lucero-palau@amd.com>
>> ---
>>   drivers/net/ethernet/sfc/ef100_netdev.c |  9 +++
>>   drivers/net/ethernet/sfc/ef100_rep.c    | 22 ++++++
>>   drivers/net/ethernet/sfc/ef100_rep.h    |  7 ++
>>   drivers/net/ethernet/sfc/efx_devlink.c  | 97 +++++++++++++++++++++++++
>>   drivers/net/ethernet/sfc/efx_devlink.h  |  7 ++
>>   drivers/net/ethernet/sfc/mae.h          |  2 +
>>   drivers/net/ethernet/sfc/net_driver.h   |  2 +
>>   7 files changed, 146 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
>> index b10a226f4a07..36774b55d413 100644
>> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
>> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
>> @@ -335,7 +335,9 @@ void ef100_remove_netdev(struct efx_probe_data *probe_data)
>>   
>>   	/* devlink lock */
>>   	efx_fini_devlink_start(efx);
>> +
>>   	ef100_unregister_netdev(efx);
>> +	ef100_pf_unset_devlink_port(efx);
>>   
>>   #ifdef CONFIG_SFC_SRIOV
>>   	efx_fini_tc(efx);
>> @@ -423,6 +425,8 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>>   		rc = ef100_probe_netdev_pf(efx);
>>   		if (rc)
>>   			goto fail;
>> +
>> +		ef100_pf_set_devlink_port(efx);
>>   	}
>>   
>>   	efx->netdev_notifier.notifier_call = ef100_netdev_event;
>> @@ -433,7 +437,12 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>>   		goto fail;
>>   	}
>>   
>> +	/* devlink unlock */
>> +	efx_probe_devlink_done(efx);
>> +	return rc;
>>   fail:
>> +	/* remove devlink port if does exist */
>> +	ef100_pf_unset_devlink_port(efx);
>>   	/* devlink unlock */
>>   	efx_probe_devlink_done(efx);
>>   	return rc;
>> diff --git a/drivers/net/ethernet/sfc/ef100_rep.c b/drivers/net/ethernet/sfc/ef100_rep.c
>> index 9cd1a3ac67e0..6b5bc5d6955d 100644
>> --- a/drivers/net/ethernet/sfc/ef100_rep.c
>> +++ b/drivers/net/ethernet/sfc/ef100_rep.c
>> @@ -16,6 +16,7 @@
>>   #include "mae.h"
>>   #include "rx_common.h"
>>   #include "tc_bindings.h"
>> +#include "efx_devlink.h"
>>   
>>   #define EFX_EF100_REP_DRIVER	"efx_ef100_rep"
>>   
>> @@ -297,6 +298,7 @@ int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
>>   			i, rc);
>>   		goto fail1;
>>   	}
>> +	ef100_rep_set_devlink_port(efv);
>>   	rc = register_netdev(efv->net_dev);
>>   	if (rc) {
>>   		pci_err(efx->pci_dev,
>> @@ -308,6 +310,7 @@ int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
>>   		efv->net_dev->name);
>>   	return 0;
>>   fail2:
>> +	ef100_rep_unset_devlink_port(efv);
>>   	efx_ef100_deconfigure_rep(efv);
>>   fail1:
>>   	efx_ef100_rep_destroy_netdev(efv);
>> @@ -323,6 +326,7 @@ void efx_ef100_vfrep_destroy(struct efx_nic *efx, struct efx_rep *efv)
>>   		return;
>>   	netif_dbg(efx, drv, rep_dev, "Removing VF representor\n");
>>   	unregister_netdev(rep_dev);
>> +	ef100_rep_unset_devlink_port(efv);
>>   	efx_ef100_deconfigure_rep(efv);
>>   	efx_ef100_rep_destroy_netdev(efv);
>>   }
>> @@ -339,6 +343,24 @@ void efx_ef100_fini_vfreps(struct efx_nic *efx)
>>   		efx_ef100_vfrep_destroy(efx, efv);
>>   }
>>   
>> +static bool ef100_mport_is_pcie_vnic(struct mae_mport_desc *mport_desc)
>> +{
>> +	return mport_desc->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC &&
>> +	       mport_desc->vnic_client_type == MAE_MPORT_DESC_VNIC_CLIENT_TYPE_FUNCTION;
>> +}
>> +
>> +bool ef100_mport_on_local_intf(struct efx_nic *efx,
>> +			       struct mae_mport_desc *mport_desc)
>> +{
>> +	struct ef100_nic_data *nic_data = efx->nic_data;
>> +	bool pcie_func;
>> +
>> +	pcie_func = ef100_mport_is_pcie_vnic(mport_desc);
>> +
>> +	return nic_data->have_local_intf && pcie_func &&
>> +		     mport_desc->interface_idx == nic_data->local_mae_intf;
>> +}
>> +
>>   void efx_ef100_init_reps(struct efx_nic *efx)
>>   {
>>   	struct ef100_nic_data *nic_data = efx->nic_data;
>> diff --git a/drivers/net/ethernet/sfc/ef100_rep.h b/drivers/net/ethernet/sfc/ef100_rep.h
>> index 328ac0cbb532..ae6add4b0855 100644
>> --- a/drivers/net/ethernet/sfc/ef100_rep.h
>> +++ b/drivers/net/ethernet/sfc/ef100_rep.h
>> @@ -22,6 +22,8 @@ struct efx_rep_sw_stats {
>>   	atomic64_t rx_dropped, tx_errors;
>>   };
>>   
>> +struct devlink_port;
>> +
>>   /**
>>    * struct efx_rep - Private data for an Efx representor
>>    *
>> @@ -39,6 +41,7 @@ struct efx_rep_sw_stats {
>>    * @rx_lock: protects @rx_list
>>    * @napi: NAPI control structure
>>    * @stats: software traffic counters for netdev stats
>> + * @dl_port: devlink port associated to this netdev representor
>>    */
>>   struct efx_rep {
>>   	struct efx_nic *parent;
>> @@ -54,6 +57,7 @@ struct efx_rep {
>>   	spinlock_t rx_lock;
>>   	struct napi_struct napi;
>>   	struct efx_rep_sw_stats stats;
>> +	struct devlink_port *dl_port;
>>   };
>>   
>>   int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i);
>> @@ -69,4 +73,7 @@ struct efx_rep *efx_ef100_find_rep_by_mport(struct efx_nic *efx, u16 mport);
>>   extern const struct net_device_ops efx_ef100_rep_netdev_ops;
>>   void efx_ef100_init_reps(struct efx_nic *efx);
>>   void efx_ef100_fini_reps(struct efx_nic *efx);
>> +struct mae_mport_desc;
>> +bool ef100_mport_on_local_intf(struct efx_nic *efx,
>> +			       struct mae_mport_desc *mport_desc);
>>   #endif /* EF100_REP_H */
>> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
>> index ff5adfe3905e..b1637eb372ad 100644
>> --- a/drivers/net/ethernet/sfc/efx_devlink.c
>> +++ b/drivers/net/ethernet/sfc/efx_devlink.c
>> @@ -16,11 +16,48 @@
>>   #include "mcdi.h"
>>   #include "mcdi_functions.h"
>>   #include "mcdi_pcol.h"
>> +#include "mae.h"
>> +#include "ef100_rep.h"
>>   
>>   struct efx_devlink {
>>   	struct efx_nic *efx;
>>   };
>>   
>> +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(efx->devlink, &mport->dl_port, mport->mport_id);
>> +}
>> +
>>   static int efx_devlink_info_nvram_partition(struct efx_nic *efx,
>>   					    struct devlink_info_req *req,
>>   					    unsigned int partition_type,
>> @@ -428,6 +465,66 @@ static const struct devlink_ops sfc_devlink_ops = {
>>   	.info_get			= efx_devlink_info_get,
>>   };
>>   
>> +static struct devlink_port *ef100_set_devlink_port(struct efx_nic *efx, u32 idx)
>> +{
>> +	struct mae_mport_desc *mport;
>> +	u32 id;
>> +
>> +	if (efx_mae_lookup_mport(efx, idx, &id)) {
>> +		/* This should not happen. */
>> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
>> +			pci_warn(efx->pci_dev, "No mport ID found for PF.\n");
>> +		else
>> +			pci_warn(efx->pci_dev, "No mport ID found for VF %u.\n",
>> +				 idx);
>> +		return NULL;
>> +	}
>> +
>> +	mport = efx_mae_get_mport(efx, id);
>> +	if (!mport) {
>> +		/* This should not happen. */
>> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
>> +			pci_warn(efx->pci_dev, "No mport found for PF.\n");
>> +		else
>> +			pci_warn(efx->pci_dev, "No mport found for VF %u.\n",
>> +				 idx);
>> +		return NULL;
>> +	}
>> +
>> +	if (efx_devlink_add_port(efx, mport)) {
>> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
>> +			pci_warn(efx->pci_dev,
>> +				 "devlink port creation for PF failed.\n");
>> +		else
>> +			pci_warn(efx->pci_dev,
>> +				 "devlink_port creationg for VF %u failed.\n",
>> +				 idx);
>> +		return NULL;
>> +	}
>> +
>> +	return &mport->dl_port;
>> +}
>> +
>> +void ef100_rep_set_devlink_port(struct efx_rep *efv)
>> +{
>> +	efv->dl_port = ef100_set_devlink_port(efv->parent, efv->idx);
>> +}
>> +
>> +void ef100_pf_set_devlink_port(struct efx_nic *efx)
>> +{
>> +	efx->dl_port = ef100_set_devlink_port(efx, MAE_MPORT_DESC_VF_IDX_NULL);
>> +}
>> +
>> +void ef100_rep_unset_devlink_port(struct efx_rep *efv)
>> +{
>> +	efx_devlink_del_port(efv->dl_port);
>> +}
>> +
>> +void ef100_pf_unset_devlink_port(struct efx_nic *efx)
>> +{
>> +	efx_devlink_del_port(efx->dl_port);
>> +}
> I'd rather see these as static inline functions in the .h file.

I disagree. They call a static function which I prefer to keep in this 
file with its complementary efx_devlink_add_port.


>> +
>>   void efx_fini_devlink_start(struct efx_nic *efx)
>>   {
>>   	if (efx->devlink)
>> diff --git a/drivers/net/ethernet/sfc/efx_devlink.h b/drivers/net/ethernet/sfc/efx_devlink.h
>> index 8bcd077d8d8d..8d993ac2572b 100644
>> --- a/drivers/net/ethernet/sfc/efx_devlink.h
>> +++ b/drivers/net/ethernet/sfc/efx_devlink.h
>> @@ -36,4 +36,11 @@ void efx_probe_devlink_done(struct efx_nic *efx);
>>   void efx_fini_devlink_start(struct efx_nic *efx);
>>   void efx_fini_devlink(struct efx_nic *efx);
>>   
>> +struct mae_mport_desc;
> Why do you need this here? The new APIs below don't need it.


Right. I forgot to remove it after the changes.

I'll remove it.

Thanks.


> Martin
>
>> +struct efx_rep;
>> +
>> +void ef100_pf_set_devlink_port(struct efx_nic *efx);
>> +void ef100_rep_set_devlink_port(struct efx_rep *efv);
>> +void ef100_pf_unset_devlink_port(struct efx_nic *efx);
>> +void ef100_rep_unset_devlink_port(struct efx_rep *efv);
>>   #endif	/* _EFX_DEVLINK_H */
>> diff --git a/drivers/net/ethernet/sfc/mae.h b/drivers/net/ethernet/sfc/mae.h
>> index d9adeafc0654..e1b7967132ad 100644
>> --- a/drivers/net/ethernet/sfc/mae.h
>> +++ b/drivers/net/ethernet/sfc/mae.h
>> @@ -13,6 +13,7 @@
>>   #define EF100_MAE_H
>>   /* MCDI interface for the ef100 Match-Action Engine */
>>   
>> +#include <net/devlink.h>
>>   #include "net_driver.h"
>>   #include "tc.h"
>>   #include "mcdi_pcol.h" /* needed for various MC_CMD_MAE_*_NULL defines */
>> @@ -44,6 +45,7 @@ struct mae_mport_desc {
>>   	};
>>   	struct rhash_head linkage;
>>   	struct efx_rep *efv;
>> +	struct devlink_port dl_port;
>>   };
>>   
>>   int efx_mae_enumerate_mports(struct efx_nic *efx);
>> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
>> index bc9efbfb3d6b..fcd51d3992fa 100644
>> --- a/drivers/net/ethernet/sfc/net_driver.h
>> +++ b/drivers/net/ethernet/sfc/net_driver.h
>> @@ -998,6 +998,7 @@ struct efx_mae;
>>    * @netdev_notifier: Netdevice notifier.
>>    * @tc: state for TC offload (EF100).
>>    * @devlink: reference to devlink structure owned by this device
>> + * @dl_port: devlink port associated with the PF
>>    * @mem_bar: The BAR that is mapped into membase.
>>    * @reg_base: Offset from the start of the bar to the function control window.
>>    * @monitor_work: Hardware monitor workitem
>> @@ -1185,6 +1186,7 @@ struct efx_nic {
>>   	struct efx_tc_state *tc;
>>   
>>   	struct devlink *devlink;
>> +	struct devlink_port *dl_port;
>>   	unsigned int mem_bar;
>>   	u32 reg_base;
>>   
>> -- 
>> 2.17.1
Martin Habets Jan. 31, 2023, 12:19 p.m. UTC | #3 
On Tue, Jan 31, 2023 at 09:36:17AM +0000, Lucero Palau, Alejandro wrote:
> 
> On 1/27/23 11:35, Martin Habets wrote:
> > On Fri, Jan 27, 2023 at 09:36:48AM +0000, alejandro.lucero-palau@amd.com wrote:
> >> From: Alejandro Lucero <alejandro.lucero-palau@amd.com>
> >>
> >> Using the data when enumerating mports, create devlink ports just before
> >> netdevs are registered and remove those devlink ports after netdev has
> >> been unregistered.
> >>
> >> Signed-off-by: Alejandro Lucero <alejandro.lucero-palau@amd.com>
> >> ---
> >>   drivers/net/ethernet/sfc/ef100_netdev.c |  9 +++
> >>   drivers/net/ethernet/sfc/ef100_rep.c    | 22 ++++++
> >>   drivers/net/ethernet/sfc/ef100_rep.h    |  7 ++
> >>   drivers/net/ethernet/sfc/efx_devlink.c  | 97 +++++++++++++++++++++++++
> >>   drivers/net/ethernet/sfc/efx_devlink.h  |  7 ++
> >>   drivers/net/ethernet/sfc/mae.h          |  2 +
> >>   drivers/net/ethernet/sfc/net_driver.h   |  2 +
> >>   7 files changed, 146 insertions(+)
> >>
> >> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
> >> index b10a226f4a07..36774b55d413 100644
> >> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
> >> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
> >> @@ -335,7 +335,9 @@ void ef100_remove_netdev(struct efx_probe_data *probe_data)
> >>   
> >>   	/* devlink lock */
> >>   	efx_fini_devlink_start(efx);
> >> +
> >>   	ef100_unregister_netdev(efx);
> >> +	ef100_pf_unset_devlink_port(efx);
> >>   
> >>   #ifdef CONFIG_SFC_SRIOV
> >>   	efx_fini_tc(efx);
> >> @@ -423,6 +425,8 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
> >>   		rc = ef100_probe_netdev_pf(efx);
> >>   		if (rc)
> >>   			goto fail;
> >> +
> >> +		ef100_pf_set_devlink_port(efx);
> >>   	}
> >>   
> >>   	efx->netdev_notifier.notifier_call = ef100_netdev_event;
> >> @@ -433,7 +437,12 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
> >>   		goto fail;
> >>   	}
> >>   
> >> +	/* devlink unlock */
> >> +	efx_probe_devlink_done(efx);
> >> +	return rc;
> >>   fail:
> >> +	/* remove devlink port if does exist */
> >> +	ef100_pf_unset_devlink_port(efx);
> >>   	/* devlink unlock */
> >>   	efx_probe_devlink_done(efx);
> >>   	return rc;
> >> diff --git a/drivers/net/ethernet/sfc/ef100_rep.c b/drivers/net/ethernet/sfc/ef100_rep.c
> >> index 9cd1a3ac67e0..6b5bc5d6955d 100644
> >> --- a/drivers/net/ethernet/sfc/ef100_rep.c
> >> +++ b/drivers/net/ethernet/sfc/ef100_rep.c
> >> @@ -16,6 +16,7 @@
> >>   #include "mae.h"
> >>   #include "rx_common.h"
> >>   #include "tc_bindings.h"
> >> +#include "efx_devlink.h"
> >>   
> >>   #define EFX_EF100_REP_DRIVER	"efx_ef100_rep"
> >>   
> >> @@ -297,6 +298,7 @@ int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
> >>   			i, rc);
> >>   		goto fail1;
> >>   	}
> >> +	ef100_rep_set_devlink_port(efv);
> >>   	rc = register_netdev(efv->net_dev);
> >>   	if (rc) {
> >>   		pci_err(efx->pci_dev,
> >> @@ -308,6 +310,7 @@ int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
> >>   		efv->net_dev->name);
> >>   	return 0;
> >>   fail2:
> >> +	ef100_rep_unset_devlink_port(efv);
> >>   	efx_ef100_deconfigure_rep(efv);
> >>   fail1:
> >>   	efx_ef100_rep_destroy_netdev(efv);
> >> @@ -323,6 +326,7 @@ void efx_ef100_vfrep_destroy(struct efx_nic *efx, struct efx_rep *efv)
> >>   		return;
> >>   	netif_dbg(efx, drv, rep_dev, "Removing VF representor\n");
> >>   	unregister_netdev(rep_dev);
> >> +	ef100_rep_unset_devlink_port(efv);
> >>   	efx_ef100_deconfigure_rep(efv);
> >>   	efx_ef100_rep_destroy_netdev(efv);
> >>   }
> >> @@ -339,6 +343,24 @@ void efx_ef100_fini_vfreps(struct efx_nic *efx)
> >>   		efx_ef100_vfrep_destroy(efx, efv);
> >>   }
> >>   
> >> +static bool ef100_mport_is_pcie_vnic(struct mae_mport_desc *mport_desc)
> >> +{
> >> +	return mport_desc->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC &&
> >> +	       mport_desc->vnic_client_type == MAE_MPORT_DESC_VNIC_CLIENT_TYPE_FUNCTION;
> >> +}
> >> +
> >> +bool ef100_mport_on_local_intf(struct efx_nic *efx,
> >> +			       struct mae_mport_desc *mport_desc)
> >> +{
> >> +	struct ef100_nic_data *nic_data = efx->nic_data;
> >> +	bool pcie_func;
> >> +
> >> +	pcie_func = ef100_mport_is_pcie_vnic(mport_desc);
> >> +
> >> +	return nic_data->have_local_intf && pcie_func &&
> >> +		     mport_desc->interface_idx == nic_data->local_mae_intf;
> >> +}
> >> +
> >>   void efx_ef100_init_reps(struct efx_nic *efx)
> >>   {
> >>   	struct ef100_nic_data *nic_data = efx->nic_data;
> >> diff --git a/drivers/net/ethernet/sfc/ef100_rep.h b/drivers/net/ethernet/sfc/ef100_rep.h
> >> index 328ac0cbb532..ae6add4b0855 100644
> >> --- a/drivers/net/ethernet/sfc/ef100_rep.h
> >> +++ b/drivers/net/ethernet/sfc/ef100_rep.h
> >> @@ -22,6 +22,8 @@ struct efx_rep_sw_stats {
> >>   	atomic64_t rx_dropped, tx_errors;
> >>   };
> >>   
> >> +struct devlink_port;
> >> +
> >>   /**
> >>    * struct efx_rep - Private data for an Efx representor
> >>    *
> >> @@ -39,6 +41,7 @@ struct efx_rep_sw_stats {
> >>    * @rx_lock: protects @rx_list
> >>    * @napi: NAPI control structure
> >>    * @stats: software traffic counters for netdev stats
> >> + * @dl_port: devlink port associated to this netdev representor
> >>    */
> >>   struct efx_rep {
> >>   	struct efx_nic *parent;
> >> @@ -54,6 +57,7 @@ struct efx_rep {
> >>   	spinlock_t rx_lock;
> >>   	struct napi_struct napi;
> >>   	struct efx_rep_sw_stats stats;
> >> +	struct devlink_port *dl_port;
> >>   };
> >>   
> >>   int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i);
> >> @@ -69,4 +73,7 @@ struct efx_rep *efx_ef100_find_rep_by_mport(struct efx_nic *efx, u16 mport);
> >>   extern const struct net_device_ops efx_ef100_rep_netdev_ops;
> >>   void efx_ef100_init_reps(struct efx_nic *efx);
> >>   void efx_ef100_fini_reps(struct efx_nic *efx);
> >> +struct mae_mport_desc;
> >> +bool ef100_mport_on_local_intf(struct efx_nic *efx,
> >> +			       struct mae_mport_desc *mport_desc);
> >>   #endif /* EF100_REP_H */
> >> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
> >> index ff5adfe3905e..b1637eb372ad 100644
> >> --- a/drivers/net/ethernet/sfc/efx_devlink.c
> >> +++ b/drivers/net/ethernet/sfc/efx_devlink.c
> >> @@ -16,11 +16,48 @@
> >>   #include "mcdi.h"
> >>   #include "mcdi_functions.h"
> >>   #include "mcdi_pcol.h"
> >> +#include "mae.h"
> >> +#include "ef100_rep.h"
> >>   
> >>   struct efx_devlink {
> >>   	struct efx_nic *efx;
> >>   };
> >>   
> >> +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(efx->devlink, &mport->dl_port, mport->mport_id);
> >> +}
> >> +
> >>   static int efx_devlink_info_nvram_partition(struct efx_nic *efx,
> >>   					    struct devlink_info_req *req,
> >>   					    unsigned int partition_type,
> >> @@ -428,6 +465,66 @@ static const struct devlink_ops sfc_devlink_ops = {
> >>   	.info_get			= efx_devlink_info_get,
> >>   };
> >>   
> >> +static struct devlink_port *ef100_set_devlink_port(struct efx_nic *efx, u32 idx)
> >> +{
> >> +	struct mae_mport_desc *mport;
> >> +	u32 id;
> >> +
> >> +	if (efx_mae_lookup_mport(efx, idx, &id)) {
> >> +		/* This should not happen. */
> >> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
> >> +			pci_warn(efx->pci_dev, "No mport ID found for PF.\n");
> >> +		else
> >> +			pci_warn(efx->pci_dev, "No mport ID found for VF %u.\n",
> >> +				 idx);
> >> +		return NULL;
> >> +	}
> >> +
> >> +	mport = efx_mae_get_mport(efx, id);
> >> +	if (!mport) {
> >> +		/* This should not happen. */
> >> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
> >> +			pci_warn(efx->pci_dev, "No mport found for PF.\n");
> >> +		else
> >> +			pci_warn(efx->pci_dev, "No mport found for VF %u.\n",
> >> +				 idx);
> >> +		return NULL;
> >> +	}
> >> +
> >> +	if (efx_devlink_add_port(efx, mport)) {
> >> +		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
> >> +			pci_warn(efx->pci_dev,
> >> +				 "devlink port creation for PF failed.\n");
> >> +		else
> >> +			pci_warn(efx->pci_dev,
> >> +				 "devlink_port creationg for VF %u failed.\n",
> >> +				 idx);
> >> +		return NULL;
> >> +	}
> >> +
> >> +	return &mport->dl_port;
> >> +}
> >> +
> >> +void ef100_rep_set_devlink_port(struct efx_rep *efv)
> >> +{
> >> +	efv->dl_port = ef100_set_devlink_port(efv->parent, efv->idx);
> >> +}
> >> +
> >> +void ef100_pf_set_devlink_port(struct efx_nic *efx)
> >> +{
> >> +	efx->dl_port = ef100_set_devlink_port(efx, MAE_MPORT_DESC_VF_IDX_NULL);
> >> +}
> >> +
> >> +void ef100_rep_unset_devlink_port(struct efx_rep *efv)
> >> +{
> >> +	efx_devlink_del_port(efv->dl_port);
> >> +}
> >> +
> >> +void ef100_pf_unset_devlink_port(struct efx_nic *efx)
> >> +{
> >> +	efx_devlink_del_port(efx->dl_port);
> >> +}
> > I'd rather see these as static inline functions in the .h file.
> 
> I disagree. They call a static function which I prefer to keep in this 
> file with its complementary efx_devlink_add_port.

That was not quite what I meant. At the moment we have 4 non-static
APIs here, which call 2 static functions.
We can change that to have 2 non-static APIs here, and 4 static inline
functions in the .h file. That is better in my view because we have
fewer non-static APIs, and the compiler will be able to optimise the
static inline functions away.
But this is no biggie for me, so keep it. Po-ta-to, po-tah-to. :)

Martin

> 
> 
> >> +
> >>   void efx_fini_devlink_start(struct efx_nic *efx)
> >>   {
> >>   	if (efx->devlink)
> >> diff --git a/drivers/net/ethernet/sfc/efx_devlink.h b/drivers/net/ethernet/sfc/efx_devlink.h
> >> index 8bcd077d8d8d..8d993ac2572b 100644
> >> --- a/drivers/net/ethernet/sfc/efx_devlink.h
> >> +++ b/drivers/net/ethernet/sfc/efx_devlink.h
> >> @@ -36,4 +36,11 @@ void efx_probe_devlink_done(struct efx_nic *efx);
> >>   void efx_fini_devlink_start(struct efx_nic *efx);
> >>   void efx_fini_devlink(struct efx_nic *efx);
> >>   
> >> +struct mae_mport_desc;
> > Why do you need this here? The new APIs below don't need it.
> 
> 
> Right. I forgot to remove it after the changes.
> 
> I'll remove it.
> 
> Thanks.
> 
> 
> > Martin
> >
> >> +struct efx_rep;
> >> +
> >> +void ef100_pf_set_devlink_port(struct efx_nic *efx);
> >> +void ef100_rep_set_devlink_port(struct efx_rep *efv);
> >> +void ef100_pf_unset_devlink_port(struct efx_nic *efx);
> >> +void ef100_rep_unset_devlink_port(struct efx_rep *efv);
> >>   #endif	/* _EFX_DEVLINK_H */
> >> diff --git a/drivers/net/ethernet/sfc/mae.h b/drivers/net/ethernet/sfc/mae.h
> >> index d9adeafc0654..e1b7967132ad 100644
> >> --- a/drivers/net/ethernet/sfc/mae.h
> >> +++ b/drivers/net/ethernet/sfc/mae.h
> >> @@ -13,6 +13,7 @@
> >>   #define EF100_MAE_H
> >>   /* MCDI interface for the ef100 Match-Action Engine */
> >>   
> >> +#include <net/devlink.h>
> >>   #include "net_driver.h"
> >>   #include "tc.h"
> >>   #include "mcdi_pcol.h" /* needed for various MC_CMD_MAE_*_NULL defines */
> >> @@ -44,6 +45,7 @@ struct mae_mport_desc {
> >>   	};
> >>   	struct rhash_head linkage;
> >>   	struct efx_rep *efv;
> >> +	struct devlink_port dl_port;
> >>   };
> >>   
> >>   int efx_mae_enumerate_mports(struct efx_nic *efx);
> >> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> >> index bc9efbfb3d6b..fcd51d3992fa 100644
> >> --- a/drivers/net/ethernet/sfc/net_driver.h
> >> +++ b/drivers/net/ethernet/sfc/net_driver.h
> >> @@ -998,6 +998,7 @@ struct efx_mae;
> >>    * @netdev_notifier: Netdevice notifier.
> >>    * @tc: state for TC offload (EF100).
> >>    * @devlink: reference to devlink structure owned by this device
> >> + * @dl_port: devlink port associated with the PF
> >>    * @mem_bar: The BAR that is mapped into membase.
> >>    * @reg_base: Offset from the start of the bar to the function control window.
> >>    * @monitor_work: Hardware monitor workitem
> >> @@ -1185,6 +1186,7 @@ struct efx_nic {
> >>   	struct efx_tc_state *tc;
> >>   
> >>   	struct devlink *devlink;
> >> +	struct devlink_port *dl_port;
> >>   	unsigned int mem_bar;
> >>   	u32 reg_base;
> >>   
> >> -- 
> >> 2.17.1
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
index b10a226f4a07..36774b55d413 100644
--- a/drivers/net/ethernet/sfc/ef100_netdev.c
+++ b/drivers/net/ethernet/sfc/ef100_netdev.c
@@ -335,7 +335,9 @@  void ef100_remove_netdev(struct efx_probe_data *probe_data)
 
 	/* devlink lock */
 	efx_fini_devlink_start(efx);
+
 	ef100_unregister_netdev(efx);
+	ef100_pf_unset_devlink_port(efx);
 
 #ifdef CONFIG_SFC_SRIOV
 	efx_fini_tc(efx);
@@ -423,6 +425,8 @@  int ef100_probe_netdev(struct efx_probe_data *probe_data)
 		rc = ef100_probe_netdev_pf(efx);
 		if (rc)
 			goto fail;
+
+		ef100_pf_set_devlink_port(efx);
 	}
 
 	efx->netdev_notifier.notifier_call = ef100_netdev_event;
@@ -433,7 +437,12 @@  int ef100_probe_netdev(struct efx_probe_data *probe_data)
 		goto fail;
 	}
 
+	/* devlink unlock */
+	efx_probe_devlink_done(efx);
+	return rc;
 fail:
+	/* remove devlink port if does exist */
+	ef100_pf_unset_devlink_port(efx);
 	/* devlink unlock */
 	efx_probe_devlink_done(efx);
 	return rc;
diff --git a/drivers/net/ethernet/sfc/ef100_rep.c b/drivers/net/ethernet/sfc/ef100_rep.c
index 9cd1a3ac67e0..6b5bc5d6955d 100644
--- a/drivers/net/ethernet/sfc/ef100_rep.c
+++ b/drivers/net/ethernet/sfc/ef100_rep.c
@@ -16,6 +16,7 @@ 
 #include "mae.h"
 #include "rx_common.h"
 #include "tc_bindings.h"
+#include "efx_devlink.h"
 
 #define EFX_EF100_REP_DRIVER	"efx_ef100_rep"
 
@@ -297,6 +298,7 @@  int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
 			i, rc);
 		goto fail1;
 	}
+	ef100_rep_set_devlink_port(efv);
 	rc = register_netdev(efv->net_dev);
 	if (rc) {
 		pci_err(efx->pci_dev,
@@ -308,6 +310,7 @@  int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i)
 		efv->net_dev->name);
 	return 0;
 fail2:
+	ef100_rep_unset_devlink_port(efv);
 	efx_ef100_deconfigure_rep(efv);
 fail1:
 	efx_ef100_rep_destroy_netdev(efv);
@@ -323,6 +326,7 @@  void efx_ef100_vfrep_destroy(struct efx_nic *efx, struct efx_rep *efv)
 		return;
 	netif_dbg(efx, drv, rep_dev, "Removing VF representor\n");
 	unregister_netdev(rep_dev);
+	ef100_rep_unset_devlink_port(efv);
 	efx_ef100_deconfigure_rep(efv);
 	efx_ef100_rep_destroy_netdev(efv);
 }
@@ -339,6 +343,24 @@  void efx_ef100_fini_vfreps(struct efx_nic *efx)
 		efx_ef100_vfrep_destroy(efx, efv);
 }
 
+static bool ef100_mport_is_pcie_vnic(struct mae_mport_desc *mport_desc)
+{
+	return mport_desc->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC &&
+	       mport_desc->vnic_client_type == MAE_MPORT_DESC_VNIC_CLIENT_TYPE_FUNCTION;
+}
+
+bool ef100_mport_on_local_intf(struct efx_nic *efx,
+			       struct mae_mport_desc *mport_desc)
+{
+	struct ef100_nic_data *nic_data = efx->nic_data;
+	bool pcie_func;
+
+	pcie_func = ef100_mport_is_pcie_vnic(mport_desc);
+
+	return nic_data->have_local_intf && pcie_func &&
+		     mport_desc->interface_idx == nic_data->local_mae_intf;
+}
+
 void efx_ef100_init_reps(struct efx_nic *efx)
 {
 	struct ef100_nic_data *nic_data = efx->nic_data;
diff --git a/drivers/net/ethernet/sfc/ef100_rep.h b/drivers/net/ethernet/sfc/ef100_rep.h
index 328ac0cbb532..ae6add4b0855 100644
--- a/drivers/net/ethernet/sfc/ef100_rep.h
+++ b/drivers/net/ethernet/sfc/ef100_rep.h
@@ -22,6 +22,8 @@  struct efx_rep_sw_stats {
 	atomic64_t rx_dropped, tx_errors;
 };
 
+struct devlink_port;
+
 /**
  * struct efx_rep - Private data for an Efx representor
  *
@@ -39,6 +41,7 @@  struct efx_rep_sw_stats {
  * @rx_lock: protects @rx_list
  * @napi: NAPI control structure
  * @stats: software traffic counters for netdev stats
+ * @dl_port: devlink port associated to this netdev representor
  */
 struct efx_rep {
 	struct efx_nic *parent;
@@ -54,6 +57,7 @@  struct efx_rep {
 	spinlock_t rx_lock;
 	struct napi_struct napi;
 	struct efx_rep_sw_stats stats;
+	struct devlink_port *dl_port;
 };
 
 int efx_ef100_vfrep_create(struct efx_nic *efx, unsigned int i);
@@ -69,4 +73,7 @@  struct efx_rep *efx_ef100_find_rep_by_mport(struct efx_nic *efx, u16 mport);
 extern const struct net_device_ops efx_ef100_rep_netdev_ops;
 void efx_ef100_init_reps(struct efx_nic *efx);
 void efx_ef100_fini_reps(struct efx_nic *efx);
+struct mae_mport_desc;
+bool ef100_mport_on_local_intf(struct efx_nic *efx,
+			       struct mae_mport_desc *mport_desc);
 #endif /* EF100_REP_H */
diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
index ff5adfe3905e..b1637eb372ad 100644
--- a/drivers/net/ethernet/sfc/efx_devlink.c
+++ b/drivers/net/ethernet/sfc/efx_devlink.c
@@ -16,11 +16,48 @@ 
 #include "mcdi.h"
 #include "mcdi_functions.h"
 #include "mcdi_pcol.h"
+#include "mae.h"
+#include "ef100_rep.h"
 
 struct efx_devlink {
 	struct efx_nic *efx;
 };
 
+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(efx->devlink, &mport->dl_port, mport->mport_id);
+}
+
 static int efx_devlink_info_nvram_partition(struct efx_nic *efx,
 					    struct devlink_info_req *req,
 					    unsigned int partition_type,
@@ -428,6 +465,66 @@  static const struct devlink_ops sfc_devlink_ops = {
 	.info_get			= efx_devlink_info_get,
 };
 
+static struct devlink_port *ef100_set_devlink_port(struct efx_nic *efx, u32 idx)
+{
+	struct mae_mport_desc *mport;
+	u32 id;
+
+	if (efx_mae_lookup_mport(efx, idx, &id)) {
+		/* This should not happen. */
+		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
+			pci_warn(efx->pci_dev, "No mport ID found for PF.\n");
+		else
+			pci_warn(efx->pci_dev, "No mport ID found for VF %u.\n",
+				 idx);
+		return NULL;
+	}
+
+	mport = efx_mae_get_mport(efx, id);
+	if (!mport) {
+		/* This should not happen. */
+		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
+			pci_warn(efx->pci_dev, "No mport found for PF.\n");
+		else
+			pci_warn(efx->pci_dev, "No mport found for VF %u.\n",
+				 idx);
+		return NULL;
+	}
+
+	if (efx_devlink_add_port(efx, mport)) {
+		if (idx == MAE_MPORT_DESC_VF_IDX_NULL)
+			pci_warn(efx->pci_dev,
+				 "devlink port creation for PF failed.\n");
+		else
+			pci_warn(efx->pci_dev,
+				 "devlink_port creationg for VF %u failed.\n",
+				 idx);
+		return NULL;
+	}
+
+	return &mport->dl_port;
+}
+
+void ef100_rep_set_devlink_port(struct efx_rep *efv)
+{
+	efv->dl_port = ef100_set_devlink_port(efv->parent, efv->idx);
+}
+
+void ef100_pf_set_devlink_port(struct efx_nic *efx)
+{
+	efx->dl_port = ef100_set_devlink_port(efx, MAE_MPORT_DESC_VF_IDX_NULL);
+}
+
+void ef100_rep_unset_devlink_port(struct efx_rep *efv)
+{
+	efx_devlink_del_port(efv->dl_port);
+}
+
+void ef100_pf_unset_devlink_port(struct efx_nic *efx)
+{
+	efx_devlink_del_port(efx->dl_port);
+}
+
 void efx_fini_devlink_start(struct efx_nic *efx)
 {
 	if (efx->devlink)
diff --git a/drivers/net/ethernet/sfc/efx_devlink.h b/drivers/net/ethernet/sfc/efx_devlink.h
index 8bcd077d8d8d..8d993ac2572b 100644
--- a/drivers/net/ethernet/sfc/efx_devlink.h
+++ b/drivers/net/ethernet/sfc/efx_devlink.h
@@ -36,4 +36,11 @@  void efx_probe_devlink_done(struct efx_nic *efx);
 void efx_fini_devlink_start(struct efx_nic *efx);
 void efx_fini_devlink(struct efx_nic *efx);
 
+struct mae_mport_desc;
+struct efx_rep;
+
+void ef100_pf_set_devlink_port(struct efx_nic *efx);
+void ef100_rep_set_devlink_port(struct efx_rep *efv);
+void ef100_pf_unset_devlink_port(struct efx_nic *efx);
+void ef100_rep_unset_devlink_port(struct efx_rep *efv);
 #endif	/* _EFX_DEVLINK_H */
diff --git a/drivers/net/ethernet/sfc/mae.h b/drivers/net/ethernet/sfc/mae.h
index d9adeafc0654..e1b7967132ad 100644
--- a/drivers/net/ethernet/sfc/mae.h
+++ b/drivers/net/ethernet/sfc/mae.h
@@ -13,6 +13,7 @@ 
 #define EF100_MAE_H
 /* MCDI interface for the ef100 Match-Action Engine */
 
+#include <net/devlink.h>
 #include "net_driver.h"
 #include "tc.h"
 #include "mcdi_pcol.h" /* needed for various MC_CMD_MAE_*_NULL defines */
@@ -44,6 +45,7 @@  struct mae_mport_desc {
 	};
 	struct rhash_head linkage;
 	struct efx_rep *efv;
+	struct devlink_port dl_port;
 };
 
 int efx_mae_enumerate_mports(struct efx_nic *efx);
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index bc9efbfb3d6b..fcd51d3992fa 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -998,6 +998,7 @@  struct efx_mae;
  * @netdev_notifier: Netdevice notifier.
  * @tc: state for TC offload (EF100).
  * @devlink: reference to devlink structure owned by this device
+ * @dl_port: devlink port associated with the PF
  * @mem_bar: The BAR that is mapped into membase.
  * @reg_base: Offset from the start of the bar to the function control window.
  * @monitor_work: Hardware monitor workitem
@@ -1185,6 +1186,7 @@  struct efx_nic {
 	struct efx_tc_state *tc;
 
 	struct devlink *devlink;
+	struct devlink_port *dl_port;
 	unsigned int mem_bar;
 	u32 reg_base;