From 28756d9ec93e6588b1c3a00cc9123e238a71c709 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 22 Feb 2023 09:09:08 -0800 Subject: ice: convert ice_mbx_clear_malvf to void and use WARN The ice_mbx_clear_malvf function checks for a few error conditions before clearing the appropriate data. These error conditions are really warnings that should never occur in a properly initialized driver. Every caller of ice_mbx_clear_malvf just prints a dev_dbg message on failure which will generally be ignored. Convert this function to void and switch the error return values to WARN_ON. This will make any potentially misconfiguration more visible and makes future refactors that involve changing how we store the malicious VF data easier. Signed-off-by: Jacob Keller Reviewed-by: Michal Swiatkowski Tested-by: Marek Szlosek Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_vf_lib.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'drivers/net/ethernet/intel/ice/ice_vf_lib.c') diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c index 0e57bd1b85fd..116b43588389 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c @@ -496,10 +496,8 @@ void ice_reset_all_vfs(struct ice_pf *pf) /* clear all malicious info if the VFs are getting reset */ ice_for_each_vf(pf, bkt, vf) - if (ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, - ICE_MAX_SRIOV_VFS, vf->vf_id)) - dev_dbg(dev, "failed to clear malicious VF state for VF %u\n", - vf->vf_id); + ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, + ICE_MAX_SRIOV_VFS, vf->vf_id); /* If VFs have been disabled, there is no need to reset */ if (test_and_set_bit(ICE_VF_DIS, pf->state)) { @@ -705,10 +703,8 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags) ice_eswitch_replay_vf_mac_rule(vf); /* if the VF has been reset allow it to come up again */ - if (ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, - ICE_MAX_SRIOV_VFS, vf->vf_id)) - dev_dbg(dev, "failed to clear malicious VF state for VF %u\n", - vf->vf_id); + ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, + ICE_MAX_SRIOV_VFS, vf->vf_id); out_unlock: if (flags & ICE_VF_RESET_LOCK) -- cgit v1.2.3 From e4eaf8938852d092fa447b32adb8ec233621d86a Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 22 Feb 2023 09:09:09 -0800 Subject: ice: track malicious VFs in new ice_mbx_vf_info structure Currently the PF tracks malicious VFs in a malvfs bitmap which is used by the ice_mbx_clear_malvf and ice_mbx_report_malvf functions. This bitmap is used to ensure that we only report a VF as malicious once rather than continuously spamming the event log. This mechanism of storage for the malicious indication works well enough for SR-IOV. However, it will not work with Scalable IOV. This is because Scalable IOV VFs can be allocated dynamically and might change VF ID when their underlying VSI changes. To support this, the mailbox overflow logic will need to be refactored. First, introduce a new ice_mbx_vf_info structure which will be used to store data about a VF. Embed this structure in the struct ice_vf, and ensure it gets initialized when a new VF is created. For now this only stores the malicious indicator bit. Pass a pointer to the VF's mbx_info structure instead of using a bitmap to keep track of these bits. A future change will extend this structure and the rest of the logic associated with the overflow detection. Signed-off-by: Jacob Keller Reviewed-by: Michal Swiatkowski Tested-by: Marek Szlosek Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 7 ++- drivers/net/ethernet/intel/ice/ice_type.h | 7 +++ drivers/net/ethernet/intel/ice/ice_vf_lib.c | 10 ++-- drivers/net/ethernet/intel/ice/ice_vf_lib.h | 2 +- drivers/net/ethernet/intel/ice/ice_vf_mbx.c | 71 +++++++++++++---------------- drivers/net/ethernet/intel/ice/ice_vf_mbx.h | 9 ++-- 6 files changed, 53 insertions(+), 53 deletions(-) (limited to 'drivers/net/ethernet/intel/ice/ice_vf_lib.c') diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 7107c279752a..44b94276df91 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -204,8 +204,8 @@ void ice_free_vfs(struct ice_pf *pf) } /* clear malicious info since the VF is getting released */ - ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, - ICE_MAX_SRIOV_VFS, vf->vf_id); + ice_mbx_clear_malvf(&hw->mbx_snapshot, vf->vf_id, + &vf->mbx_info); mutex_unlock(&vf->cfg_lock); } @@ -1828,8 +1828,7 @@ ice_is_malicious_vf(struct ice_pf *pf, struct ice_rq_event_info *event, /* if the VF is malicious and we haven't let the user * know about it, then let them know now */ - status = ice_mbx_report_malvf(&pf->hw, pf->vfs.malvfs, - ICE_MAX_SRIOV_VFS, vf_id, + status = ice_mbx_report_malvf(&pf->hw, &vf->mbx_info, &report_vf); if (status) dev_dbg(dev, "Error reporting malicious VF\n"); diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h index e3f622cad425..d243a0c59ea4 100644 --- a/drivers/net/ethernet/intel/ice/ice_type.h +++ b/drivers/net/ethernet/intel/ice/ice_type.h @@ -794,6 +794,13 @@ struct ice_mbx_vf_counter { u32 vfcntr_len; }; +/* Structure used to track a single VF's messages on the mailbox: + * 1. malicious: whether this VF has been detected as malicious before + */ +struct ice_mbx_vf_info { + u8 malicious : 1; +}; + /* Structure to hold data relevant to the captured static snapshot * of the PF-VF mailbox. */ diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c index 116b43588389..69e89e960950 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c @@ -496,8 +496,8 @@ void ice_reset_all_vfs(struct ice_pf *pf) /* clear all malicious info if the VFs are getting reset */ ice_for_each_vf(pf, bkt, vf) - ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, - ICE_MAX_SRIOV_VFS, vf->vf_id); + ice_mbx_clear_malvf(&hw->mbx_snapshot, vf->vf_id, + &vf->mbx_info); /* If VFs have been disabled, there is no need to reset */ if (test_and_set_bit(ICE_VF_DIS, pf->state)) { @@ -703,8 +703,7 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags) ice_eswitch_replay_vf_mac_rule(vf); /* if the VF has been reset allow it to come up again */ - ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, - ICE_MAX_SRIOV_VFS, vf->vf_id); + ice_mbx_clear_malvf(&hw->mbx_snapshot, vf->vf_id, &vf->mbx_info); out_unlock: if (flags & ICE_VF_RESET_LOCK) @@ -760,6 +759,9 @@ void ice_initialize_vf_entry(struct ice_vf *vf) ice_vf_ctrl_invalidate_vsi(vf); ice_vf_fdir_init(vf); + /* Initialize mailbox info for this VF */ + ice_mbx_init_vf_info(&pf->hw, &vf->mbx_info); + mutex_init(&vf->cfg_lock); } diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h index ef30f05b5d02..e3cda6fb71ab 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h @@ -74,7 +74,6 @@ struct ice_vfs { u16 num_qps_per; /* number of queue pairs per VF */ u16 num_msix_per; /* number of MSI-X vectors per VF */ unsigned long last_printed_mdd_jiffies; /* MDD message rate limit */ - DECLARE_BITMAP(malvfs, ICE_MAX_SRIOV_VFS); /* malicious VF indicator */ }; /* VF information structure */ @@ -105,6 +104,7 @@ struct ice_vf { DECLARE_BITMAP(rxq_ena, ICE_MAX_RSS_QS_PER_VF); struct ice_vlan port_vlan_info; /* Port VLAN ID, QoS, and TPID */ struct virtchnl_vlan_caps vlan_v2_caps; + struct ice_mbx_vf_info mbx_info; u8 pf_set_mac:1; /* VF MAC address set by VMM admin */ u8 trusted:1; u8 spoofchk:1; diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c index 9f6acfeb0fc6..2e769bd0bf7e 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c @@ -345,35 +345,23 @@ ice_mbx_vf_state_handler(struct ice_hw *hw, /** * ice_mbx_report_malvf - Track and note malicious VF * @hw: pointer to the HW struct - * @all_malvfs: all malicious VFs tracked by PF - * @bitmap_len: length of bitmap in bits - * @vf_id: relative virtual function ID of the malicious VF + * @vf_info: the mailbox tracking info structure for a VF * @report_malvf: boolean to indicate if malicious VF must be reported * - * This function will update a bitmap that keeps track of the malicious - * VFs attached to the PF. A malicious VF must be reported only once if - * discovered between VF resets or loading so the function checks - * the input vf_id against the bitmap to verify if the VF has been - * detected in any previous mailbox iterations. + * This function updates the malicious indicator bit in the VF mailbox + * tracking structure. A malicious VF must be reported only once if discovered + * between VF resets or loading so the function first checks if the VF has + * already been detected in any previous mailbox iterations. */ int -ice_mbx_report_malvf(struct ice_hw *hw, unsigned long *all_malvfs, - u16 bitmap_len, u16 vf_id, bool *report_malvf) +ice_mbx_report_malvf(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info, + bool *report_malvf) { - if (!all_malvfs || !report_malvf) - return -EINVAL; - - *report_malvf = false; - - if (bitmap_len < hw->mbx_snapshot.mbx_vf.vfcntr_len) + if (!report_malvf) return -EINVAL; - if (vf_id >= bitmap_len) - return -EIO; - - /* If the vf_id is found in the bitmap set bit and boolean to true */ - if (!test_and_set_bit(vf_id, all_malvfs)) - *report_malvf = true; + *report_malvf = !vf_info->malicious; + vf_info->malicious = 1; return 0; } @@ -381,33 +369,24 @@ ice_mbx_report_malvf(struct ice_hw *hw, unsigned long *all_malvfs, /** * ice_mbx_clear_malvf - Clear VF bitmap and counter for VF ID * @snap: pointer to the mailbox snapshot structure - * @all_malvfs: all malicious VFs tracked by PF - * @bitmap_len: length of bitmap in bits * @vf_id: relative virtual function ID of the malicious VF + * @vf_info: mailbox tracking structure for this VF * - * In case of a VF reset, this function can be called to clear - * the bit corresponding to the VF ID in the bitmap tracking all - * malicious VFs attached to the PF. The function also clears the - * VF counter array at the index of the VF ID. This is to ensure - * that the new VF loaded is not considered malicious before going - * through the overflow detection algorithm. - */ +* In case of a VF reset, this function shall be called to clear the VF's +* current mailbox tracking state. +*/ void -ice_mbx_clear_malvf(struct ice_mbx_snapshot *snap, unsigned long *all_malvfs, - u16 bitmap_len, u16 vf_id) +ice_mbx_clear_malvf(struct ice_mbx_snapshot *snap, u16 vf_id, + struct ice_mbx_vf_info *vf_info) { - if (WARN_ON(!snap || !all_malvfs)) - return; - - if (WARN_ON(bitmap_len < snap->mbx_vf.vfcntr_len)) + if (WARN_ON(!snap)) return; /* Ensure VF ID value is not larger than bitmap or VF counter length */ - if (WARN_ON(vf_id >= bitmap_len || vf_id >= snap->mbx_vf.vfcntr_len)) + if (WARN_ON(vf_id >= snap->mbx_vf.vfcntr_len)) return; - /* Clear VF ID bit in the bitmap tracking malicious VFs attached to PF */ - clear_bit(vf_id, all_malvfs); + vf_info->malicious = 0; /* Clear the VF counter in the mailbox snapshot structure for that VF ID. * This is to ensure that if a VF is unloaded and a new one brought back @@ -418,6 +397,18 @@ ice_mbx_clear_malvf(struct ice_mbx_snapshot *snap, unsigned long *all_malvfs, snap->mbx_vf.vf_cntr[vf_id] = 0; } +/** + * ice_mbx_init_vf_info - Initialize a new VF mailbox tracking info + * @hw: pointer to the hardware structure + * @vf_info: the mailbox tracking info structure for a VF + * + * Initialize a VF mailbox tracking info structure. + */ +void ice_mbx_init_vf_info(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info) +{ + vf_info->malicious = 0; +} + /** * ice_mbx_init_snapshot - Initialize mailbox snapshot structure * @hw: pointer to the hardware structure diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.h b/drivers/net/ethernet/intel/ice/ice_vf_mbx.h index be593b951642..2613cba61ac7 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.h +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.h @@ -23,13 +23,14 @@ int ice_mbx_vf_state_handler(struct ice_hw *hw, struct ice_mbx_data *mbx_data, u16 vf_id, bool *is_mal_vf); void -ice_mbx_clear_malvf(struct ice_mbx_snapshot *snap, unsigned long *all_malvfs, - u16 bitmap_len, u16 vf_id); +ice_mbx_clear_malvf(struct ice_mbx_snapshot *snap, u16 vf_id, + struct ice_mbx_vf_info *vf_info); +void ice_mbx_init_vf_info(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info); int ice_mbx_init_snapshot(struct ice_hw *hw, u16 vf_count); void ice_mbx_deinit_snapshot(struct ice_hw *hw); int -ice_mbx_report_malvf(struct ice_hw *hw, unsigned long *all_malvfs, - u16 bitmap_len, u16 vf_id, bool *report_malvf); +ice_mbx_report_malvf(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info, + bool *report_malvf); #else /* CONFIG_PCI_IOV */ static inline int ice_aq_send_msg_to_vf(struct ice_hw __always_unused *hw, -- cgit v1.2.3 From 8cd8a6b17d275a45e3722d0215f6115b687c8c3e Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 22 Feb 2023 09:09:10 -0800 Subject: ice: move VF overflow message count into struct ice_mbx_vf_info The ice driver has some logic in ice_vf_mbx.c used to detect potentially malicious VF behavior with regards to overflowing the PF mailbox. This logic currently stores message counts in struct ice_mbx_vf_counter.vf_cntr as an array. This array is allocated during initialization with ice_mbx_init_snapshot. This logic makes sense for SR-IOV where all VFs are allocated at once up front. However, in the future with Scalable IOV this logic will not work. VFs can be added and removed dynamically. We could try to keep the vf_cntr array for the maximum possible number of VFs, but this is a waste of memory. Use the recently introduced struct ice_mbx_vf_info structure to store the message count. Pass a pointer to the mbx_info for a VF instead of using its VF ID. Replace the array of VF message counts with a linked list that tracks all currently active mailbox tracking info structures. Signed-off-by: Jacob Keller Reviewed-by: Michal Swiatkowski Tested-by: Marek Szlosek Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 9 +- drivers/net/ethernet/intel/ice/ice_type.h | 18 +-- drivers/net/ethernet/intel/ice/ice_vf_lib.c | 7 +- drivers/net/ethernet/intel/ice/ice_vf_mbx.c | 167 +++++++++------------------- drivers/net/ethernet/intel/ice/ice_vf_mbx.h | 8 +- 5 files changed, 69 insertions(+), 140 deletions(-) (limited to 'drivers/net/ethernet/intel/ice/ice_vf_lib.c') diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 44b94276df91..8820f269bfdf 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -204,8 +204,7 @@ void ice_free_vfs(struct ice_pf *pf) } /* clear malicious info since the VF is getting released */ - ice_mbx_clear_malvf(&hw->mbx_snapshot, vf->vf_id, - &vf->mbx_info); + list_del(&vf->mbx_info.list_entry); mutex_unlock(&vf->cfg_lock); } @@ -1025,9 +1024,7 @@ int ice_sriov_configure(struct pci_dev *pdev, int num_vfs) return -EBUSY; } - err = ice_mbx_init_snapshot(&pf->hw, num_vfs); - if (err) - return err; + ice_mbx_init_snapshot(&pf->hw); err = ice_pci_sriov_ena(pf, num_vfs); if (err) { @@ -1818,7 +1815,7 @@ ice_is_malicious_vf(struct ice_pf *pf, struct ice_rq_event_info *event, mbxdata.async_watermark_val = ICE_MBX_OVERFLOW_WATERMARK; /* check to see if we have a malicious VF */ - status = ice_mbx_vf_state_handler(&pf->hw, &mbxdata, vf_id, &malvf); + status = ice_mbx_vf_state_handler(&pf->hw, &mbxdata, &vf->mbx_info, &malvf); if (status) goto out_put_vf; diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h index d243a0c59ea4..a09556e57803 100644 --- a/drivers/net/ethernet/intel/ice/ice_type.h +++ b/drivers/net/ethernet/intel/ice/ice_type.h @@ -784,20 +784,14 @@ struct ice_mbx_snap_buffer_data { u16 max_num_msgs_mbx; }; -/* Structure to track messages sent by VFs on mailbox: - * 1. vf_cntr: a counter array of VFs to track the number of - * asynchronous messages sent by each VF - * 2. vfcntr_len: number of entries in VF counter array - */ -struct ice_mbx_vf_counter { - u32 *vf_cntr; - u32 vfcntr_len; -}; - /* Structure used to track a single VF's messages on the mailbox: - * 1. malicious: whether this VF has been detected as malicious before + * 1. list_entry: linked list entry node + * 2. msg_count: the number of asynchronous messages sent by this VF + * 3. malicious: whether this VF has been detected as malicious before */ struct ice_mbx_vf_info { + struct list_head list_entry; + u32 msg_count; u8 malicious : 1; }; @@ -806,7 +800,7 @@ struct ice_mbx_vf_info { */ struct ice_mbx_snapshot { struct ice_mbx_snap_buffer_data mbx_buf; - struct ice_mbx_vf_counter mbx_vf; + struct list_head mbx_vf; }; /* Structure to hold data to be used for capturing or updating a diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c index 69e89e960950..89fd6982df09 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c @@ -496,8 +496,7 @@ void ice_reset_all_vfs(struct ice_pf *pf) /* clear all malicious info if the VFs are getting reset */ ice_for_each_vf(pf, bkt, vf) - ice_mbx_clear_malvf(&hw->mbx_snapshot, vf->vf_id, - &vf->mbx_info); + ice_mbx_clear_malvf(&vf->mbx_info); /* If VFs have been disabled, there is no need to reset */ if (test_and_set_bit(ICE_VF_DIS, pf->state)) { @@ -599,12 +598,10 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags) struct ice_pf *pf = vf->pf; struct ice_vsi *vsi; struct device *dev; - struct ice_hw *hw; int err = 0; bool rsd; dev = ice_pf_to_dev(pf); - hw = &pf->hw; if (flags & ICE_VF_RESET_NOTIFY) ice_notify_vf_reset(vf); @@ -703,7 +700,7 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags) ice_eswitch_replay_vf_mac_rule(vf); /* if the VF has been reset allow it to come up again */ - ice_mbx_clear_malvf(&hw->mbx_snapshot, vf->vf_id, &vf->mbx_info); + ice_mbx_clear_malvf(&vf->mbx_info); out_unlock: if (flags & ICE_VF_RESET_LOCK) diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c index 2e769bd0bf7e..4bfed5fb3a88 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c @@ -93,36 +93,31 @@ u32 ice_conv_link_speed_to_virtchnl(bool adv_link_support, u16 link_speed) * * 2. When the caller starts processing its mailbox queue in response to an * interrupt, the structure ice_mbx_snapshot is expected to be cleared before - * the algorithm can be run for the first time for that interrupt. This can be - * done via ice_mbx_reset_snapshot(). + * the algorithm can be run for the first time for that interrupt. This + * requires calling ice_mbx_reset_snapshot() as well as calling + * ice_mbx_reset_vf_info() for each VF tracking structure. * * 3. For every message read by the caller from the MBX Queue, the caller must * call the detection algorithm's entry function ice_mbx_vf_state_handler(). * Before every call to ice_mbx_vf_state_handler() the struct ice_mbx_data is * filled as it is required to be passed to the algorithm. * - * 4. Every time a message is read from the MBX queue, a VFId is received which - * is passed to the state handler. The boolean output is_malvf of the state - * handler ice_mbx_vf_state_handler() serves as an indicator to the caller - * whether this VF is malicious or not. + * 4. Every time a message is read from the MBX queue, a tracking structure + * for the VF must be passed to the state handler. The boolean output + * report_malvf from ice_mbx_vf_state_handler() serves as an indicator to the + * caller whether it must report this VF as malicious or not. * * 5. When a VF is identified to be malicious, the caller can send a message - * to the system administrator. The caller can invoke ice_mbx_report_malvf() - * to help determine if a malicious VF is to be reported or not. This function - * requires the caller to maintain a global bitmap to track all malicious VFs - * and pass that to ice_mbx_report_malvf() along with the VFID which was identified - * to be malicious by ice_mbx_vf_state_handler(). + * to the system administrator. * - * 6. The global bitmap maintained by PF can be cleared completely if PF is in - * reset or the bit corresponding to a VF can be cleared if that VF is in reset. - * When a VF is shut down and brought back up, we assume that the new VF - * brought up is not malicious and hence report it if found malicious. + * 6. The PF is responsible for maintaining the struct ice_mbx_vf_info + * structure for each VF. The PF should clear the VF tracking structure if the + * VF is reset. When a VF is shut down and brought back up, we will then + * assume that the new VF is not malicious and may report it again if we + * detect it again. * * 7. The function ice_mbx_reset_snapshot() is called to reset the information * in ice_mbx_snapshot for every new mailbox interrupt handled. - * - * 8. The memory allocated for variables in ice_mbx_snapshot is de-allocated - * when driver is unloaded. */ #define ICE_RQ_DATA_MASK(rq_data) ((rq_data) & PF_MBX_ARQH_ARQH_M) /* Using the highest value for an unsigned 16-bit value 0xFFFF to indicate that @@ -132,26 +127,21 @@ u32 ice_conv_link_speed_to_virtchnl(bool adv_link_support, u16 link_speed) /** * ice_mbx_reset_snapshot - Reset mailbox snapshot structure - * @snap: pointer to mailbox snapshot structure in the ice_hw struct - * - * Reset the mailbox snapshot structure and clear VF counter array. + * @snap: pointer to the mailbox snapshot */ static void ice_mbx_reset_snapshot(struct ice_mbx_snapshot *snap) { - u32 vfcntr_len; - - if (!snap || !snap->mbx_vf.vf_cntr) - return; + struct ice_mbx_vf_info *vf_info; - /* Clear VF counters. */ - vfcntr_len = snap->mbx_vf.vfcntr_len; - if (vfcntr_len) - memset(snap->mbx_vf.vf_cntr, 0, - (vfcntr_len * sizeof(*snap->mbx_vf.vf_cntr))); - - /* Reset mailbox snapshot for a new capture. */ + /* Clear mbx_buf in the mailbox snaphot structure and setting the + * mailbox snapshot state to a new capture. + */ memset(&snap->mbx_buf, 0, sizeof(snap->mbx_buf)); snap->mbx_buf.state = ICE_MAL_VF_DETECT_STATE_NEW_SNAPSHOT; + + /* Reset message counts for all VFs to zero */ + list_for_each_entry(vf_info, &snap->mbx_vf, list_entry) + vf_info->msg_count = 0; } /** @@ -195,7 +185,7 @@ ice_mbx_traverse(struct ice_hw *hw, /** * ice_mbx_detect_malvf - Detect malicious VF in snapshot * @hw: pointer to the HW struct - * @vf_id: relative virtual function ID + * @vf_info: mailbox tracking structure for a VF * @new_state: new algorithm state * @is_malvf: boolean output to indicate if VF is malicious * @@ -204,19 +194,14 @@ ice_mbx_traverse(struct ice_hw *hw, * the permissible number of messages to send. */ static int -ice_mbx_detect_malvf(struct ice_hw *hw, u16 vf_id, +ice_mbx_detect_malvf(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info, enum ice_mbx_snapshot_state *new_state, bool *is_malvf) { - struct ice_mbx_snapshot *snap = &hw->mbx_snapshot; + /* increment the message count for this VF */ + vf_info->msg_count++; - if (vf_id >= snap->mbx_vf.vfcntr_len) - return -EIO; - - /* increment the message count in the VF array */ - snap->mbx_vf.vf_cntr[vf_id]++; - - if (snap->mbx_vf.vf_cntr[vf_id] >= ICE_ASYNC_VF_MSG_THRESHOLD) + if (vf_info->msg_count >= ICE_ASYNC_VF_MSG_THRESHOLD) *is_malvf = true; /* continue to iterate through the mailbox snapshot */ @@ -229,7 +214,7 @@ ice_mbx_detect_malvf(struct ice_hw *hw, u16 vf_id, * ice_mbx_vf_state_handler - Handle states of the overflow algorithm * @hw: pointer to the HW struct * @mbx_data: pointer to structure containing mailbox data - * @vf_id: relative virtual function (VF) ID + * @vf_info: mailbox tracking structure for the VF in question * @is_malvf: boolean output to indicate if VF is malicious * * The function serves as an entry point for the malicious VF @@ -250,7 +235,8 @@ ice_mbx_detect_malvf(struct ice_hw *hw, u16 vf_id, */ int ice_mbx_vf_state_handler(struct ice_hw *hw, - struct ice_mbx_data *mbx_data, u16 vf_id, + struct ice_mbx_data *mbx_data, + struct ice_mbx_vf_info *vf_info, bool *is_malvf) { struct ice_mbx_snapshot *snap = &hw->mbx_snapshot; @@ -315,7 +301,8 @@ ice_mbx_vf_state_handler(struct ice_hw *hw, if (snap_buf->num_pending_arq >= mbx_data->async_watermark_val) { new_state = ICE_MAL_VF_DETECT_STATE_DETECT; - status = ice_mbx_detect_malvf(hw, vf_id, &new_state, is_malvf); + status = ice_mbx_detect_malvf(hw, vf_info, &new_state, + is_malvf); } else { new_state = ICE_MAL_VF_DETECT_STATE_TRAVERSE; ice_mbx_traverse(hw, &new_state); @@ -329,7 +316,8 @@ ice_mbx_vf_state_handler(struct ice_hw *hw, case ICE_MAL_VF_DETECT_STATE_DETECT: new_state = ICE_MAL_VF_DETECT_STATE_DETECT; - status = ice_mbx_detect_malvf(hw, vf_id, &new_state, is_malvf); + status = ice_mbx_detect_malvf(hw, vf_info, &new_state, + is_malvf); break; default: @@ -367,34 +355,16 @@ ice_mbx_report_malvf(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info, } /** - * ice_mbx_clear_malvf - Clear VF bitmap and counter for VF ID - * @snap: pointer to the mailbox snapshot structure - * @vf_id: relative virtual function ID of the malicious VF - * @vf_info: mailbox tracking structure for this VF + * ice_mbx_clear_malvf - Clear VF mailbox info + * @vf_info: the mailbox tracking structure for a VF * -* In case of a VF reset, this function shall be called to clear the VF's -* current mailbox tracking state. -*/ -void -ice_mbx_clear_malvf(struct ice_mbx_snapshot *snap, u16 vf_id, - struct ice_mbx_vf_info *vf_info) + * In case of a VF reset, this function shall be called to clear the VF's + * current mailbox tracking state. + */ +void ice_mbx_clear_malvf(struct ice_mbx_vf_info *vf_info) { - if (WARN_ON(!snap)) - return; - - /* Ensure VF ID value is not larger than bitmap or VF counter length */ - if (WARN_ON(vf_id >= snap->mbx_vf.vfcntr_len)) - return; - vf_info->malicious = 0; - - /* Clear the VF counter in the mailbox snapshot structure for that VF ID. - * This is to ensure that if a VF is unloaded and a new one brought back - * up with the same VF ID for a snapshot currently in traversal or detect - * state the counter for that VF ID does not increment on top of existing - * values in the mailbox overflow detection algorithm. - */ - snap->mbx_vf.vf_cntr[vf_id] = 0; + vf_info->msg_count = 0; } /** @@ -402,55 +372,32 @@ ice_mbx_clear_malvf(struct ice_mbx_snapshot *snap, u16 vf_id, * @hw: pointer to the hardware structure * @vf_info: the mailbox tracking info structure for a VF * - * Initialize a VF mailbox tracking info structure. + * Initialize a VF mailbox tracking info structure and insert it into the + * snapshot list. + * + * If you remove the VF, you must also delete the associated VF info structure + * from the linked list. */ void ice_mbx_init_vf_info(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info) { - vf_info->malicious = 0; + struct ice_mbx_snapshot *snap = &hw->mbx_snapshot; + + ice_mbx_clear_malvf(vf_info); + list_add(&vf_info->list_entry, &snap->mbx_vf); } /** - * ice_mbx_init_snapshot - Initialize mailbox snapshot structure + * ice_mbx_init_snapshot - Initialize mailbox snapshot data * @hw: pointer to the hardware structure - * @vf_count: number of VFs allocated on a PF - * - * Clear the mailbox snapshot structure and allocate memory - * for the VF counter array based on the number of VFs allocated - * on that PF. * - * Assumption: This function will assume ice_get_caps() has already been - * called to ensure that the vf_count can be compared against the number - * of VFs supported as defined in the functional capabilities of the device. + * Clear the mailbox snapshot structure and initialize the VF mailbox list. */ -int ice_mbx_init_snapshot(struct ice_hw *hw, u16 vf_count) +void ice_mbx_init_snapshot(struct ice_hw *hw) { struct ice_mbx_snapshot *snap = &hw->mbx_snapshot; - /* Ensure that the number of VFs allocated is non-zero and - * is not greater than the number of supported VFs defined in - * the functional capabilities of the PF. - */ - if (!vf_count || vf_count > hw->func_caps.num_allocd_vfs) - return -EINVAL; - - snap->mbx_vf.vf_cntr = devm_kcalloc(ice_hw_to_dev(hw), vf_count, - sizeof(*snap->mbx_vf.vf_cntr), - GFP_KERNEL); - if (!snap->mbx_vf.vf_cntr) - return -ENOMEM; - - /* Setting the VF counter length to the number of allocated - * VFs for given PF's functional capabilities. - */ - snap->mbx_vf.vfcntr_len = vf_count; - - /* Clear mbx_buf in the mailbox snaphot structure and setting the - * mailbox snapshot state to a new capture. - */ - memset(&snap->mbx_buf, 0, sizeof(snap->mbx_buf)); - snap->mbx_buf.state = ICE_MAL_VF_DETECT_STATE_NEW_SNAPSHOT; - - return 0; + INIT_LIST_HEAD(&snap->mbx_vf); + ice_mbx_reset_snapshot(snap); } /** @@ -463,10 +410,6 @@ void ice_mbx_deinit_snapshot(struct ice_hw *hw) { struct ice_mbx_snapshot *snap = &hw->mbx_snapshot; - /* Free VF counter array and reset VF counter length */ - devm_kfree(ice_hw_to_dev(hw), snap->mbx_vf.vf_cntr); - snap->mbx_vf.vfcntr_len = 0; - /* Clear mbx_buf in the mailbox snaphot structure */ memset(&snap->mbx_buf, 0, sizeof(snap->mbx_buf)); } diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.h b/drivers/net/ethernet/intel/ice/ice_vf_mbx.h index 2613cba61ac7..a6d42f467dc5 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.h +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.h @@ -21,12 +21,10 @@ ice_aq_send_msg_to_vf(struct ice_hw *hw, u16 vfid, u32 v_opcode, u32 v_retval, u32 ice_conv_link_speed_to_virtchnl(bool adv_link_support, u16 link_speed); int ice_mbx_vf_state_handler(struct ice_hw *hw, struct ice_mbx_data *mbx_data, - u16 vf_id, bool *is_mal_vf); -void -ice_mbx_clear_malvf(struct ice_mbx_snapshot *snap, u16 vf_id, - struct ice_mbx_vf_info *vf_info); + struct ice_mbx_vf_info *vf_info, bool *is_mal_vf); +void ice_mbx_clear_malvf(struct ice_mbx_vf_info *vf_info); void ice_mbx_init_vf_info(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info); -int ice_mbx_init_snapshot(struct ice_hw *hw, u16 vf_count); +void ice_mbx_init_snapshot(struct ice_hw *hw); void ice_mbx_deinit_snapshot(struct ice_hw *hw); int ice_mbx_report_malvf(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info, -- cgit v1.2.3 From 7255355a0636b4eff08d5e8139c77d98f151c4fc Mon Sep 17 00:00:00 2001 From: Dawid Wesierski Date: Tue, 18 Apr 2023 11:52:55 +0200 Subject: ice: Fix ice VF reset during iavf initialization Fix the current implementation that causes ice_trigger_vf_reset() to start resetting the VF even when the VF-NIC is still initializing. When we reset NIC with ice driver it can interfere with iavf-vf initialization e.g. during consecutive resets induced by ice iavf ice | | |<-----------------| | ice resets vf iavf | reset | start | |<-----------------| | ice resets vf | causing iavf | initialization | error | | iavf reset end This leads to a series of -53 errors (failed to init adminq) from the IAVF. Change the state of the vf_state field to be not active when the IAVF is still initializing. Make sure to wait until receiving the message on the message box to ensure that the vf is ready and initializded. In simple terms we use the ACTIVE flag to make sure that the ice driver knows if the iavf is ready for another reset iavf ice | | | | |<------------- ice resets vf iavf vf_state != ACTIVE reset | start | | | | | iavf | reset-------> vf_state == ACTIVE end ice resets vf | | | | Fixes: c54d209c78b8 ("ice: Wait for VF to be reset/ready before configuration") Signed-off-by: Dawid Wesierski Signed-off-by: Kamil Maziarz Acked-by: Jacob Keller Tested-by: Rafal Romanowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 8 ++++---- drivers/net/ethernet/intel/ice/ice_vf_lib.c | 19 +++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_vf_lib.h | 1 + drivers/net/ethernet/intel/ice/ice_virtchnl.c | 1 + 4 files changed, 25 insertions(+), 4 deletions(-) (limited to 'drivers/net/ethernet/intel/ice/ice_vf_lib.c') diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index f1dca59bd844..588ad8696756 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -1171,7 +1171,7 @@ int ice_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool ena) if (!vf) return -EINVAL; - ret = ice_check_vf_ready_for_cfg(vf); + ret = ice_check_vf_ready_for_reset(vf); if (ret) goto out_put_vf; @@ -1286,7 +1286,7 @@ int ice_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac) goto out_put_vf; } - ret = ice_check_vf_ready_for_cfg(vf); + ret = ice_check_vf_ready_for_reset(vf); if (ret) goto out_put_vf; @@ -1340,7 +1340,7 @@ int ice_set_vf_trust(struct net_device *netdev, int vf_id, bool trusted) return -EOPNOTSUPP; } - ret = ice_check_vf_ready_for_cfg(vf); + ret = ice_check_vf_ready_for_reset(vf); if (ret) goto out_put_vf; @@ -1653,7 +1653,7 @@ ice_set_vf_port_vlan(struct net_device *netdev, int vf_id, u16 vlan_id, u8 qos, if (!vf) return -EINVAL; - ret = ice_check_vf_ready_for_cfg(vf); + ret = ice_check_vf_ready_for_reset(vf); if (ret) goto out_put_vf; diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c index 89fd6982df09..bf74a2f3a4f8 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c @@ -185,6 +185,25 @@ int ice_check_vf_ready_for_cfg(struct ice_vf *vf) return 0; } +/** + * ice_check_vf_ready_for_reset - check if VF is ready to be reset + * @vf: VF to check if it's ready to be reset + * + * The purpose of this function is to ensure that the VF is not in reset, + * disabled, and is both initialized and active, thus enabling us to safely + * initialize another reset. + */ +int ice_check_vf_ready_for_reset(struct ice_vf *vf) +{ + int ret; + + ret = ice_check_vf_ready_for_cfg(vf); + if (!ret && !test_bit(ICE_VF_STATE_ACTIVE, vf->vf_states)) + ret = -EAGAIN; + + return ret; +} + /** * ice_trigger_vf_reset - Reset a VF on HW * @vf: pointer to the VF structure diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h index e3cda6fb71ab..a38ef00a3679 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h @@ -215,6 +215,7 @@ u16 ice_get_num_vfs(struct ice_pf *pf); struct ice_vsi *ice_get_vf_vsi(struct ice_vf *vf); bool ice_is_vf_disabled(struct ice_vf *vf); int ice_check_vf_ready_for_cfg(struct ice_vf *vf); +int ice_check_vf_ready_for_reset(struct ice_vf *vf); void ice_set_vf_state_dis(struct ice_vf *vf); bool ice_is_any_vf_in_unicast_promisc(struct ice_pf *pf); void diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c index 97243c616d5d..f4a524f80b11 100644 --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c @@ -3955,6 +3955,7 @@ error_handler: ice_vc_notify_vf_link_state(vf); break; case VIRTCHNL_OP_RESET_VF: + clear_bit(ICE_VF_STATE_ACTIVE, vf->vf_states); ops->reset_vf(vf); break; case VIRTCHNL_OP_ADD_ETH_ADDR: -- cgit v1.2.3