Re: [Intel-wired-lan] [PATCH net v2] ice: Fix ice VF reset during iavf initialization

[Petr Oros <poros@redhat.com>]

Kamil Maziarz píše v Čt 01. 01. 1970 v 00:00 +0000:
> From: Dawid Wesierski <dawidx.wesierski@intel.com>
> 
> Fix the current implementation that causes ice_trigger_vf_reset()
> to start resetting the VF even when the VF is still resetting itself
> and initializing adminq. 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
> asks for a reset. To avoid issues caused by the VF being reset too
> early, make sure to wait until receiving the message on the message
> box to know the exact state of the IAVF driver.
> 
> Fixes: c54d209c78b8 ("ice: Wait for VF to be reset/ready before
> configuration")
> Signed-off-by: Dawid Wesierski <dawidx.wesierski@intel.com>
> Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com>
> Acked-by: Jacob Keller <Jacob.e.keller@intel.com>
> ---
> v2: changed "reseted" to "reset", changed fixes tag, removed space
> ---
>  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(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 0cc05e54a781..d4206db7d6d5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -1181,7 +1181,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;
>  
> @@ -1296,7 +1296,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;
>  
> @@ -1350,7 +1350,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;
>  
> @@ -1663,7 +1663,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 0e57bd1b85fd..95878c95e953 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 ef30f05b5d02..3fc6a0a8d955 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 e24e3f5017ca..d8c66baf4eb4 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> @@ -3908,6 +3908,7 @@ void ice_vc_process_vf_msg(struct ice_pf *pf,
> struct ice_rq_event_info *event)
>                 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:


Hi,

How was this patch tested?

Why are we waiting for ICE_VF_STATE_ACTIVE? As I understand
ICE_VF_STATE_INIT is the state where we can start the reset again.

After this patch we are not able to attach VF to VM:

# virsh attach-interface v0 hostdev --managed 0000:41:01.0 --mac
52:54:00:b4:aa:bb
# error: Failed to attach interface
# error: Cannot set interface MAC to 52:54:00:b4:aa:bb for ifname
enp65s0f0np0 vf 0: Resource temporarily unavailable

Regards,
Petr

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan