Messages in this thread | | | Date | Mon, 20 Nov 2023 11:42:24 +0000 | From | Simon Horman <> | Subject | Re: [PATCH iwl-next v3 3/5] i40e: Add helpers to find VSI and VEB by SEID and use them |
| |
On Thu, Nov 16, 2023 at 04:21:12PM +0100, Ivan Vecera wrote: > Add two helpers i40e_(veb|vsi)_get_by_seid() to find corresponding > VEB or VSI by their SEID value and use these helpers to replace > existing open-coded loops. > > Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Hi Ivan,
some minor feedback from my side.
...
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h > index 1e9266de270b..ca8997d29c02 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e.h > +++ b/drivers/net/ethernet/intel/i40e/i40e.h > @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw) > > struct device *i40e_hw_to_dev(struct i40e_hw *hw); > > +/** > + * i40e_pf_get_vsi_by_seid - find VSI by SEID > + * @pf: pointer to a PF
nit: @seid is missing here
> + **/ > +static inline struct i40e_vsi * > +i40e_pf_get_vsi_by_seid(struct i40e_pf *pf, u16 seid) > +{ > + struct i40e_vsi *vsi; > + int i; > + > + i40e_pf_for_each_vsi(pf, i, vsi) > + if (vsi->seid == seid) > + return vsi; > + > + return NULL; > +} > + > +/** > + * i40e_pf_get_veb_by_seid - find VEB by SEID > + * @pf: pointer to a PF
Ditto
...
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
...
> @@ -14848,23 +14831,16 @@ struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags, > } > > /* make sure there is such a vsi and uplink */ > - i40e_pf_for_each_vsi(pf, vsi_idx, vsi) > - if (vsi->seid == vsi_seid) > - break; > - > - if (vsi_idx == pf->num_alloc_vsi && vsi_seid != 0) { > - dev_info(&pf->pdev->dev, "vsi seid %d not found\n", > - vsi_seid); > - return NULL; > + if (vsi_seid) { > + vsi = i40e_pf_get_vsi_by_seid(pf, vsi_seid); > + if (!vsi) { > + dev_err(&pf->pdev->dev, "vsi seid %d not found\n", > + vsi_seid); > + return NULL; > + } > } > - > if (uplink_seid && uplink_seid != pf->mac_seid) { > - i40e_pf_for_each_veb(pf, veb_idx, veb) { > - if (veb->seid == uplink_seid) { > - uplink_veb = veb; > - break; > - } > - } > + uplink_veb = i40e_pf_get_veb_by_seid(pf, uplink_seid); > if (!uplink_veb) { > dev_info(&pf->pdev->dev, > "uplink seid %d not found\n", uplink_seid);
The next part of this function looks like this:
if (!uplink_veb) { dev_info(&pf->pdev->dev, "uplink seid %d not found\n", uplink_seid); return NULL; } } /* get veb sw struct */ veb_idx = i40e_veb_mem_alloc(pf); if (veb_idx < 0) goto err_alloc; veb = pf->veb[veb_idx]; veb->flags = flags; veb->uplink_seid = uplink_seid; veb->veb_idx = (uplink_veb ? uplink_veb->idx : I40E_NO_VEB); veb->enabled_tc = (enabled_tc ? enabled_tc : 0x1);
/* create the VEB in the switch */ ret = i40e_add_veb(veb, vsi);
Smatch complains that vsi may be used uninitialised here. Which does seem possible to me if vsi_seid is 0.
...
| |