diff mbox series

[net-next,3/7] sfc: add mport lookup based on driver's mport data

Message ID 20230119113140.20208-4-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: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: habetsm.xilinx@gmail.com
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: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 76 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Lucero Palau, Alejandro Jan. 19, 2023, 11:31 a.m. UTC
From: Alejandro Lucero <alejandro.lucero-palau@amd.com>

Obtaining mport id is based on asking the firmware about it. This is
still needed for mport initialization itself, but once the mport data is
now kept by the driver, further mport id request can be satisfied
internally without firmware interaction.

Previous function is just modified in name making clear the firmware
interaction. The new function uses the old name and looks for the data
in the mport data structure.

Signed-off-by: Alejandro Lucero <alejandro.lucero-palau@amd.com>
---
 drivers/net/ethernet/sfc/ef100_nic.c |  4 ++--
 drivers/net/ethernet/sfc/ef100_rep.c |  5 +----
 drivers/net/ethernet/sfc/mae.c       | 27 ++++++++++++++++++++++++++-
 drivers/net/ethernet/sfc/mae.h       |  2 ++
 4 files changed, 31 insertions(+), 7 deletions(-)

Comments

Jacob Keller Jan. 19, 2023, 11:57 p.m. UTC | #1
On 1/19/2023 3:31 AM, alejandro.lucero-palau@amd.com wrote:
> +int efx_mae_lookup_mport(struct efx_nic *efx, u32 vf_idx, u32 *id)
> +{
> +	struct ef100_nic_data *nic_data = efx->nic_data;
> +	struct efx_mae *mae = efx->mae;
> +	struct rhashtable_iter walk;
> +	struct mae_mport_desc *m;
> +	int rc = -ENOENT;
> +
> +	rhashtable_walk_enter(&mae->mports_ht, &walk);
> +	rhashtable_walk_start(&walk);
> +	while ((m = rhashtable_walk_next(&walk)) != NULL) {
> +		if (m->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC &&
> +		    m->interface_idx == nic_data->local_mae_intf &&
> +		    m->pf_idx == 0 &&
> +		    m->vf_idx == vf_idx) {
> +			*id = m->mport_id;
> +			rc = 0;
> +			break;
> +		}
> +	}
> +	rhashtable_walk_stop(&walk);
> +	rhashtable_walk_exit(&walk);

Curious if you have any reasoning for why you chose rhashtable vs
another structure (such as a simpler hash table of linked lists or xarray).

At any rate,

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Lucero Palau, Alejandro Jan. 20, 2023, 9:34 a.m. UTC | #2
On 1/19/23 23:57, Jacob Keller wrote:
>
> On 1/19/2023 3:31 AM, alejandro.lucero-palau@amd.com wrote:
>> +int efx_mae_lookup_mport(struct efx_nic *efx, u32 vf_idx, u32 *id)
>> +{
>> +	struct ef100_nic_data *nic_data = efx->nic_data;
>> +	struct efx_mae *mae = efx->mae;
>> +	struct rhashtable_iter walk;
>> +	struct mae_mport_desc *m;
>> +	int rc = -ENOENT;
>> +
>> +	rhashtable_walk_enter(&mae->mports_ht, &walk);
>> +	rhashtable_walk_start(&walk);
>> +	while ((m = rhashtable_walk_next(&walk)) != NULL) {
>> +		if (m->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC &&
>> +		    m->interface_idx == nic_data->local_mae_intf &&
>> +		    m->pf_idx == 0 &&
>> +		    m->vf_idx == vf_idx) {
>> +			*id = m->mport_id;
>> +			rc = 0;
>> +			break;
>> +		}
>> +	}
>> +	rhashtable_walk_stop(&walk);
>> +	rhashtable_walk_exit(&walk);
> Curious if you have any reasoning for why you chose rhashtable vs
> another structure (such as a simpler hash table of linked lists or xarray).


The mports can appear and disappear (although it is not supported by the 
code yet nor by current firmware/hardware) so something resizable was 
needed for supporting this in the near future.


> At any rate,
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Jacob Keller Jan. 20, 2023, 6:36 p.m. UTC | #3
> -----Original Message-----
> From: Lucero Palau, Alejandro <alejandro.lucero-palau@amd.com>
> Sent: Friday, January 20, 2023 1:35 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>; Lucero Palau, Alejandro
> <alejandro.lucero-palau@amd.com>; netdev@vger.kernel.org; linux-net-drivers
> (AMD-Xilinx) <linux-net-drivers@amd.com>
> Cc: davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com;
> edumazet@google.com; habetsm@gmail.com; ecree.xilinx@gmail.com
> Subject: Re: [PATCH net-next 3/7] sfc: add mport lookup based on driver's mport
> data
> 
> 
> On 1/19/23 23:57, Jacob Keller wrote:
> >
> > On 1/19/2023 3:31 AM, alejandro.lucero-palau@amd.com wrote:
> >> +int efx_mae_lookup_mport(struct efx_nic *efx, u32 vf_idx, u32 *id)
> >> +{
> >> +	struct ef100_nic_data *nic_data = efx->nic_data;
> >> +	struct efx_mae *mae = efx->mae;
> >> +	struct rhashtable_iter walk;
> >> +	struct mae_mport_desc *m;
> >> +	int rc = -ENOENT;
> >> +
> >> +	rhashtable_walk_enter(&mae->mports_ht, &walk);
> >> +	rhashtable_walk_start(&walk);
> >> +	while ((m = rhashtable_walk_next(&walk)) != NULL) {
> >> +		if (m->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC
> &&
> >> +		    m->interface_idx == nic_data->local_mae_intf &&
> >> +		    m->pf_idx == 0 &&
> >> +		    m->vf_idx == vf_idx) {
> >> +			*id = m->mport_id;
> >> +			rc = 0;
> >> +			break;
> >> +		}
> >> +	}
> >> +	rhashtable_walk_stop(&walk);
> >> +	rhashtable_walk_exit(&walk);
> > Curious if you have any reasoning for why you chose rhashtable vs
> > another structure (such as a simpler hash table of linked lists or xarray).
> 
> 
> The mports can appear and disappear (although it is not supported by the
> code yet nor by current firmware/hardware) so something resizable was
> needed for supporting this in the near future.
> 
> 

Right. Xarray feels like it would fit the bill too. I don't know what the advantage/disadvantage would be compared to rhashtable.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
index ee260b508377..38c809eb7828 100644
--- a/drivers/net/ethernet/sfc/ef100_nic.c
+++ b/drivers/net/ethernet/sfc/ef100_nic.c
@@ -737,7 +737,7 @@  static int efx_ef100_get_base_mport(struct efx_nic *efx)
 	/* Construct mport selector for "physical network port" */
 	efx_mae_mport_wire(efx, &selector);
 	/* Look up actual mport ID */
-	rc = efx_mae_lookup_mport(efx, selector, &id);
+	rc = efx_mae_fw_lookup_mport(efx, selector, &id);
 	if (rc)
 		return rc;
 	/* The ID should always fit in 16 bits, because that's how wide the
@@ -752,7 +752,7 @@  static int efx_ef100_get_base_mport(struct efx_nic *efx)
 	/* Construct mport selector for "calling PF" */
 	efx_mae_mport_uplink(efx, &selector);
 	/* Look up actual mport ID */
-	rc = efx_mae_lookup_mport(efx, selector, &id);
+	rc = efx_mae_fw_lookup_mport(efx, selector, &id);
 	if (rc)
 		return rc;
 	if (id >> 16)
diff --git a/drivers/net/ethernet/sfc/ef100_rep.c b/drivers/net/ethernet/sfc/ef100_rep.c
index ebe7b1275713..9cd1a3ac67e0 100644
--- a/drivers/net/ethernet/sfc/ef100_rep.c
+++ b/drivers/net/ethernet/sfc/ef100_rep.c
@@ -243,14 +243,11 @@  static struct efx_rep *efx_ef100_rep_create_netdev(struct efx_nic *efx,
 static int efx_ef100_configure_rep(struct efx_rep *efv)
 {
 	struct efx_nic *efx = efv->parent;
-	u32 selector;
 	int rc;
 
 	efv->rx_pring_size = EFX_REP_DEFAULT_PSEUDO_RING_SIZE;
-	/* Construct mport selector for corresponding VF */
-	efx_mae_mport_vf(efx, efv->idx, &selector);
 	/* Look up actual mport ID */
-	rc = efx_mae_lookup_mport(efx, selector, &efv->mport);
+	rc = efx_mae_lookup_mport(efx, efv->idx, &efv->mport);
 	if (rc)
 		return rc;
 	pci_dbg(efx->pci_dev, "VF %u has mport ID %#x\n", efv->idx, efv->mport);
diff --git a/drivers/net/ethernet/sfc/mae.c b/drivers/net/ethernet/sfc/mae.c
index bf039862ee14..6a5ab3a037db 100644
--- a/drivers/net/ethernet/sfc/mae.c
+++ b/drivers/net/ethernet/sfc/mae.c
@@ -97,7 +97,7 @@  void efx_mae_mport_mport(struct efx_nic *efx __always_unused, u32 mport_id, u32
 }
 
 /* id is really only 24 bits wide */
-int efx_mae_lookup_mport(struct efx_nic *efx, u32 selector, u32 *id)
+int efx_mae_fw_lookup_mport(struct efx_nic *efx, u32 selector, u32 *id)
 {
 	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_MPORT_LOOKUP_OUT_LEN);
 	MCDI_DECLARE_BUF(inbuf, MC_CMD_MAE_MPORT_LOOKUP_IN_LEN);
@@ -488,6 +488,31 @@  int efx_mae_free_counter(struct efx_nic *efx, struct efx_tc_counter *cnt)
 	return 0;
 }
 
+int efx_mae_lookup_mport(struct efx_nic *efx, u32 vf_idx, u32 *id)
+{
+	struct ef100_nic_data *nic_data = efx->nic_data;
+	struct efx_mae *mae = efx->mae;
+	struct rhashtable_iter walk;
+	struct mae_mport_desc *m;
+	int rc = -ENOENT;
+
+	rhashtable_walk_enter(&mae->mports_ht, &walk);
+	rhashtable_walk_start(&walk);
+	while ((m = rhashtable_walk_next(&walk)) != NULL) {
+		if (m->mport_type == MAE_MPORT_DESC_MPORT_TYPE_VNIC &&
+		    m->interface_idx == nic_data->local_mae_intf &&
+		    m->pf_idx == 0 &&
+		    m->vf_idx == vf_idx) {
+			*id = m->mport_id;
+			rc = 0;
+			break;
+		}
+	}
+	rhashtable_walk_stop(&walk);
+	rhashtable_walk_exit(&walk);
+	return rc;
+}
+
 static bool efx_mae_asl_id(u32 id)
 {
 	return !!(id & BIT(31));
diff --git a/drivers/net/ethernet/sfc/mae.h b/drivers/net/ethernet/sfc/mae.h
index e1f057f01f08..d9adeafc0654 100644
--- a/drivers/net/ethernet/sfc/mae.h
+++ b/drivers/net/ethernet/sfc/mae.h
@@ -97,4 +97,6 @@  int efx_mae_delete_rule(struct efx_nic *efx, u32 id);
 int efx_init_mae(struct efx_nic *efx);
 void efx_fini_mae(struct efx_nic *efx);
 void efx_mae_remove_mport(void *desc, void *arg);
+int efx_mae_fw_lookup_mport(struct efx_nic *efx, u32 selector, u32 *id);
+int efx_mae_lookup_mport(struct efx_nic *efx, u32 vf, u32 *id);
 #endif /* EF100_MAE_H */