Commit b03d519d authored by Jacob Keller's avatar Jacob Keller Committed by Tony Nguyen

ice: store VF pointer instead of VF ID

The VSI structure contains a vf_id field used to associate a VSI with a
VF. This is used mainly for ICE_VSI_VF as well as partially for
ICE_VSI_CTRL associated with the VFs.

This API was designed with the idea that VFs are stored in a simple
array that was expected to be static throughout most of the driver's
life.

We plan on refactoring VF storage in a few key ways:

  1) converting from a simple static array to a hash table
  2) using krefs to track VF references obtained from the hash table
  3) use RCU to delay release of VF memory until after all references
     are dropped

This is motivated by the goal to ensure that the lifetime of VF
structures is accounted for, and prevent various use-after-free bugs.

With the existing vsi->vf_id, the reference tracking for VFs would
become somewhat convoluted, because each VSI maintains a vf_id field
which will then require performing a look up. This means all these flows
will require reference tracking and proper usage of rcu_read_lock, etc.

We know that the VF VSI will always be backed by a valid VF structure,
because the VSI is created during VF initialization and removed before
the VF is destroyed. Rely on this and store a reference to the VF in the
VSI structure instead of storing a VF ID. This will simplify the usage
and avoid the need to perform lookups on the hash table in the future.

For ICE_VSI_VF, it is expected that vsi->vf is always non-NULL after
ice_vsi_alloc succeeds. Because of this, use WARN_ON when checking if a
vsi->vf pointer is valid when dealing with VF VSIs. This will aid in
debugging code which violates this assumption and avoid more disastrous
panics.
Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Tested-by: default avatarKonrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent df830543
...@@ -109,7 +109,6 @@ ...@@ -109,7 +109,6 @@
/* All VF control VSIs share the same IRQ, so assign a unique ID for them */ /* All VF control VSIs share the same IRQ, so assign a unique ID for them */
#define ICE_RES_VF_CTRL_VEC_ID (ICE_RES_RDMA_VEC_ID - 1) #define ICE_RES_VF_CTRL_VEC_ID (ICE_RES_RDMA_VEC_ID - 1)
#define ICE_INVAL_Q_INDEX 0xffff #define ICE_INVAL_Q_INDEX 0xffff
#define ICE_INVAL_VFID 256
#define ICE_MAX_RXQS_PER_TC 256 /* Used when setting VSI context per TC Rx queues */ #define ICE_MAX_RXQS_PER_TC 256 /* Used when setting VSI context per TC Rx queues */
...@@ -333,7 +332,7 @@ struct ice_vsi { ...@@ -333,7 +332,7 @@ struct ice_vsi {
u16 vsi_num; /* HW (absolute) index of this VSI */ u16 vsi_num; /* HW (absolute) index of this VSI */
u16 idx; /* software index in pf->vsi[] */ u16 idx; /* software index in pf->vsi[] */
s16 vf_id; /* VF ID for SR-IOV VSIs */ struct ice_vf *vf; /* VF associated with this VSI */
u16 ethtype; /* Ethernet protocol for pause frame */ u16 ethtype; /* Ethernet protocol for pause frame */
u16 num_gfltr; u16 num_gfltr;
......
...@@ -323,7 +323,7 @@ ice_setup_tx_ctx(struct ice_tx_ring *ring, struct ice_tlan_ctx *tlan_ctx, u16 pf ...@@ -323,7 +323,7 @@ ice_setup_tx_ctx(struct ice_tx_ring *ring, struct ice_tlan_ctx *tlan_ctx, u16 pf
break; break;
case ICE_VSI_VF: case ICE_VSI_VF:
/* Firmware expects vmvf_num to be absolute VF ID */ /* Firmware expects vmvf_num to be absolute VF ID */
tlan_ctx->vmvf_num = hw->func_caps.vf_base_id + vsi->vf_id; tlan_ctx->vmvf_num = hw->func_caps.vf_base_id + vsi->vf->vf_id;
tlan_ctx->vmvf_type = ICE_TLAN_CTX_VMVF_TYPE_VF; tlan_ctx->vmvf_type = ICE_TLAN_CTX_VMVF_TYPE_VF;
break; break;
case ICE_VSI_SWITCHDEV_CTRL: case ICE_VSI_SWITCHDEV_CTRL:
...@@ -429,7 +429,7 @@ static int ice_setup_rx_ctx(struct ice_rx_ring *ring) ...@@ -429,7 +429,7 @@ static int ice_setup_rx_ctx(struct ice_rx_ring *ring)
*/ */
if (ice_is_dvm_ena(hw)) if (ice_is_dvm_ena(hw))
if (vsi->type == ICE_VSI_VF && if (vsi->type == ICE_VSI_VF &&
ice_vf_is_port_vlan_ena(&vsi->back->vf[vsi->vf_id])) ice_vf_is_port_vlan_ena(vsi->vf))
rlan_ctx.l2tsel = 1; rlan_ctx.l2tsel = 1;
else else
rlan_ctx.l2tsel = 0; rlan_ctx.l2tsel = 0;
......
...@@ -315,7 +315,7 @@ void ice_eswitch_update_repr(struct ice_vsi *vsi) ...@@ -315,7 +315,7 @@ void ice_eswitch_update_repr(struct ice_vsi *vsi)
if (!ice_is_switchdev_running(pf)) if (!ice_is_switchdev_running(pf))
return; return;
vf = &pf->vf[vsi->vf_id]; vf = vsi->vf;
repr = vf->repr; repr = vf->repr;
repr->src_vsi = vsi; repr->src_vsi = vsi;
repr->dst->u.port_info.port_id = vsi->vsi_num; repr->dst->u.port_info.port_id = vsi->vsi_num;
...@@ -323,7 +323,8 @@ void ice_eswitch_update_repr(struct ice_vsi *vsi) ...@@ -323,7 +323,8 @@ void ice_eswitch_update_repr(struct ice_vsi *vsi)
ret = ice_vsi_update_security(vsi, ice_vsi_ctx_clear_antispoof); ret = ice_vsi_update_security(vsi, ice_vsi_ctx_clear_antispoof);
if (ret) { if (ret) {
ice_fltr_add_mac_and_broadcast(vsi, vf->hw_lan_addr.addr, ICE_FWD_TO_VSI); ice_fltr_add_mac_and_broadcast(vsi, vf->hw_lan_addr.addr, ICE_FWD_TO_VSI);
dev_err(ice_pf_to_dev(pf), "Failed to update VF %d port representor", vsi->vf_id); dev_err(ice_pf_to_dev(pf), "Failed to update VF %d port representor",
vsi->vf->vf_id);
} }
} }
...@@ -407,7 +408,7 @@ static void ice_eswitch_release_env(struct ice_pf *pf) ...@@ -407,7 +408,7 @@ static void ice_eswitch_release_env(struct ice_pf *pf)
static struct ice_vsi * static struct ice_vsi *
ice_eswitch_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi) ice_eswitch_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
{ {
return ice_vsi_setup(pf, pi, ICE_VSI_SWITCHDEV_CTRL, ICE_INVAL_VFID, NULL); return ice_vsi_setup(pf, pi, ICE_VSI_SWITCHDEV_CTRL, NULL, NULL);
} }
/** /**
......
This diff is collapsed.
...@@ -52,7 +52,8 @@ void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc); ...@@ -52,7 +52,8 @@ void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc);
struct ice_vsi * struct ice_vsi *
ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi, ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
enum ice_vsi_type vsi_type, u16 vf_id, struct ice_channel *ch); enum ice_vsi_type vsi_type, struct ice_vf *vf,
struct ice_channel *ch);
void ice_napi_del(struct ice_vsi *vsi); void ice_napi_del(struct ice_vsi *vsi);
......
...@@ -2439,7 +2439,7 @@ static int ice_vsi_req_irq_msix(struct ice_vsi *vsi, char *basename) ...@@ -2439,7 +2439,7 @@ static int ice_vsi_req_irq_msix(struct ice_vsi *vsi, char *basename)
/* skip this unused q_vector */ /* skip this unused q_vector */
continue; continue;
} }
if (vsi->type == ICE_VSI_CTRL && vsi->vf_id != ICE_INVAL_VFID) if (vsi->type == ICE_VSI_CTRL && vsi->vf)
err = devm_request_irq(dev, irq_num, vsi->irq_handler, err = devm_request_irq(dev, irq_num, vsi->irq_handler,
IRQF_SHARED, q_vector->name, IRQF_SHARED, q_vector->name,
q_vector); q_vector);
...@@ -3386,14 +3386,14 @@ void ice_fill_rss_lut(u8 *lut, u16 rss_table_size, u16 rss_size) ...@@ -3386,14 +3386,14 @@ void ice_fill_rss_lut(u8 *lut, u16 rss_table_size, u16 rss_size)
static struct ice_vsi * static struct ice_vsi *
ice_pf_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi) ice_pf_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
{ {
return ice_vsi_setup(pf, pi, ICE_VSI_PF, ICE_INVAL_VFID, NULL); return ice_vsi_setup(pf, pi, ICE_VSI_PF, NULL, NULL);
} }
static struct ice_vsi * static struct ice_vsi *
ice_chnl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi, ice_chnl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
struct ice_channel *ch) struct ice_channel *ch)
{ {
return ice_vsi_setup(pf, pi, ICE_VSI_CHNL, ICE_INVAL_VFID, ch); return ice_vsi_setup(pf, pi, ICE_VSI_CHNL, NULL, ch);
} }
/** /**
...@@ -3407,7 +3407,7 @@ ice_chnl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi, ...@@ -3407,7 +3407,7 @@ ice_chnl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
static struct ice_vsi * static struct ice_vsi *
ice_ctrl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi) ice_ctrl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
{ {
return ice_vsi_setup(pf, pi, ICE_VSI_CTRL, ICE_INVAL_VFID, NULL); return ice_vsi_setup(pf, pi, ICE_VSI_CTRL, NULL, NULL);
} }
/** /**
...@@ -3421,7 +3421,7 @@ ice_ctrl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi) ...@@ -3421,7 +3421,7 @@ ice_ctrl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
struct ice_vsi * struct ice_vsi *
ice_lb_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi) ice_lb_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
{ {
return ice_vsi_setup(pf, pi, ICE_VSI_LB, ICE_INVAL_VFID, NULL); return ice_vsi_setup(pf, pi, ICE_VSI_LB, NULL, NULL);
} }
/** /**
......
...@@ -1165,7 +1165,7 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget) ...@@ -1165,7 +1165,7 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
struct ice_vsi *ctrl_vsi = rx_ring->vsi; struct ice_vsi *ctrl_vsi = rx_ring->vsi;
if (rx_desc->wb.rxdid == FDIR_DESC_RXDID && if (rx_desc->wb.rxdid == FDIR_DESC_RXDID &&
ctrl_vsi->vf_id != ICE_INVAL_VFID) ctrl_vsi->vf)
ice_vc_fdir_irq_handler(ctrl_vsi, rx_desc); ice_vc_fdir_irq_handler(ctrl_vsi, rx_desc);
ice_put_rx_buf(rx_ring, NULL, 0); ice_put_rx_buf(rx_ring, NULL, 0);
cleaned_count++; cleaned_count++;
......
...@@ -34,9 +34,10 @@ void ice_vf_vsi_init_vlan_ops(struct ice_vsi *vsi) ...@@ -34,9 +34,10 @@ void ice_vf_vsi_init_vlan_ops(struct ice_vsi *vsi)
{ {
struct ice_vsi_vlan_ops *vlan_ops; struct ice_vsi_vlan_ops *vlan_ops;
struct ice_pf *pf = vsi->back; struct ice_pf *pf = vsi->back;
struct ice_vf *vf; struct ice_vf *vf = vsi->vf;
vf = &pf->vf[vsi->vf_id]; if (WARN_ON(!vf))
return;
if (ice_is_dvm_ena(&pf->hw)) { if (ice_is_dvm_ena(&pf->hw)) {
vlan_ops = &vsi->outer_vlan_ops; vlan_ops = &vsi->outer_vlan_ops;
...@@ -126,9 +127,14 @@ void ice_vf_vsi_init_vlan_ops(struct ice_vsi *vsi) ...@@ -126,9 +127,14 @@ void ice_vf_vsi_init_vlan_ops(struct ice_vsi *vsi)
*/ */
void ice_vf_vsi_cfg_dvm_legacy_vlan_mode(struct ice_vsi *vsi) void ice_vf_vsi_cfg_dvm_legacy_vlan_mode(struct ice_vsi *vsi)
{ {
struct ice_vf *vf = &vsi->back->vf[vsi->vf_id];
struct device *dev = ice_pf_to_dev(vf->pf);
struct ice_vsi_vlan_ops *vlan_ops; struct ice_vsi_vlan_ops *vlan_ops;
struct ice_vf *vf = vsi->vf;
struct device *dev;
if (WARN_ON(!vf))
return;
dev = ice_pf_to_dev(vf->pf);
if (!ice_is_dvm_ena(&vsi->back->hw) || ice_vf_is_port_vlan_ena(vf)) if (!ice_is_dvm_ena(&vsi->back->hw) || ice_vf_is_port_vlan_ena(vf))
return; return;
...@@ -192,7 +198,10 @@ void ice_vf_vsi_cfg_dvm_legacy_vlan_mode(struct ice_vsi *vsi) ...@@ -192,7 +198,10 @@ void ice_vf_vsi_cfg_dvm_legacy_vlan_mode(struct ice_vsi *vsi)
*/ */
void ice_vf_vsi_cfg_svm_legacy_vlan_mode(struct ice_vsi *vsi) void ice_vf_vsi_cfg_svm_legacy_vlan_mode(struct ice_vsi *vsi)
{ {
struct ice_vf *vf = &vsi->back->vf[vsi->vf_id]; struct ice_vf *vf = vsi->vf;
if (WARN_ON(!vf))
return;
if (ice_is_dvm_ena(&vsi->back->hw) || ice_vf_is_port_vlan_ena(vf)) if (ice_is_dvm_ena(&vsi->back->hw) || ice_vf_is_port_vlan_ena(vf))
return; return;
......
...@@ -1288,15 +1288,16 @@ ice_vc_fdir_irq_handler(struct ice_vsi *ctrl_vsi, ...@@ -1288,15 +1288,16 @@ ice_vc_fdir_irq_handler(struct ice_vsi *ctrl_vsi,
union ice_32b_rx_flex_desc *rx_desc) union ice_32b_rx_flex_desc *rx_desc)
{ {
struct ice_pf *pf = ctrl_vsi->back; struct ice_pf *pf = ctrl_vsi->back;
struct ice_vf *vf = ctrl_vsi->vf;
struct ice_vf_fdir_ctx *ctx_done; struct ice_vf_fdir_ctx *ctx_done;
struct ice_vf_fdir_ctx *ctx_irq; struct ice_vf_fdir_ctx *ctx_irq;
struct ice_vf_fdir *fdir; struct ice_vf_fdir *fdir;
unsigned long flags; unsigned long flags;
struct device *dev; struct device *dev;
struct ice_vf *vf;
int ret; int ret;
vf = &pf->vf[ctrl_vsi->vf_id]; if (WARN_ON(!vf))
return;
fdir = &vf->fdir; fdir = &vf->fdir;
ctx_done = &fdir->ctx_done; ctx_done = &fdir->ctx_done;
......
...@@ -667,7 +667,7 @@ static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf *vf) ...@@ -667,7 +667,7 @@ static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf *vf)
struct ice_pf *pf = vf->pf; struct ice_pf *pf = vf->pf;
struct ice_vsi *vsi; struct ice_vsi *vsi;
vsi = ice_vsi_setup(pf, pi, ICE_VSI_VF, vf->vf_id, NULL); vsi = ice_vsi_setup(pf, pi, ICE_VSI_VF, vf, NULL);
if (!vsi) { if (!vsi) {
dev_err(ice_pf_to_dev(pf), "Failed to create VF VSI\n"); dev_err(ice_pf_to_dev(pf), "Failed to create VF VSI\n");
...@@ -694,7 +694,7 @@ struct ice_vsi *ice_vf_ctrl_vsi_setup(struct ice_vf *vf) ...@@ -694,7 +694,7 @@ struct ice_vsi *ice_vf_ctrl_vsi_setup(struct ice_vf *vf)
struct ice_pf *pf = vf->pf; struct ice_pf *pf = vf->pf;
struct ice_vsi *vsi; struct ice_vsi *vsi;
vsi = ice_vsi_setup(pf, pi, ICE_VSI_CTRL, vf->vf_id, NULL); vsi = ice_vsi_setup(pf, pi, ICE_VSI_CTRL, vf, NULL);
if (!vsi) { if (!vsi) {
dev_err(ice_pf_to_dev(pf), "Failed to create VF control VSI\n"); dev_err(ice_pf_to_dev(pf), "Failed to create VF control VSI\n");
ice_vf_ctrl_invalidate_vsi(vf); ice_vf_ctrl_invalidate_vsi(vf);
...@@ -2526,7 +2526,7 @@ bool ice_vc_isvalid_vsi_id(struct ice_vf *vf, u16 vsi_id) ...@@ -2526,7 +2526,7 @@ bool ice_vc_isvalid_vsi_id(struct ice_vf *vf, u16 vsi_id)
vsi = ice_find_vsi_from_id(pf, vsi_id); vsi = ice_find_vsi_from_id(pf, vsi_id);
return (vsi && (vsi->vf_id == vf->vf_id)); return (vsi && (vsi->vf == vf));
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment