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 |
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>
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>
> -----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 --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 */