Re: [Intel-wired-lan] [PATCH iwl-next v1 2/2] i40e: add mdd-auto-reset-vf private flag

[Tony Nguyen <anthony.l.nguyen@intel.com>]

On 8/11/2023 5:46 AM, Jan Sokolowski wrote:

#1 these patches are missing a cover letter.

> Since VF RX MDD events should disable the queue, add ethtool
> private flag mdd-auto-reset-vf to configure VF reset
> to re-enable the queue. This can be used by a system's administrator
> to select the desired level of security in running VF's.

#2 private flags are no longer allowed and/or highly discouraged. This 
should be RFC'd to netdev first to see if they are open to accepting 
this private flag.

> Signed-off-by: Jan Sokolowski <jan.sokolowski@intel.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e.h        |  2 +-
>   .../net/ethernet/intel/i40e/i40e_ethtool.c    |  1 +
>   drivers/net/ethernet/intel/i40e/i40e_main.c   | 75 ++++++++++++++++---
>   .../ethernet/intel/i40e/i40e_virtchnl_pf.h    |  2 +
>   4 files changed, 70 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index 6e310a539467..72bd45c4f9ba 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -603,7 +603,7 @@ struct i40e_pf {
>    *   in abilities field of i40e_aq_set_phy_config structure
>    */
>   #define I40E_FLAG_TOTAL_PORT_SHUTDOWN_ENABLED	BIT(27)
> -
> +#define I40E_FLAG_MDD_AUTO_RESET_VF		BIT(28)
>   	struct i40e_client_instance *cinst;
>   	bool stat_offsets_loaded;
>   	struct i40e_hw_port_stats stats;
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index afc4fa8c66af..54bdf477bcd6 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -457,6 +457,7 @@ static const struct i40e_priv_flags i40e_gstrings_priv_flags[] = {
>   	I40E_PRIV_FLAG("base-r-fec", I40E_FLAG_BASE_R_FEC, 0),
>   	I40E_PRIV_FLAG("vf-vlan-pruning",
>   		       I40E_FLAG_VF_VLAN_PRUNING, 0),
> +	I40E_PRIV_FLAG("mdd-auto-reset-vf", I40E_FLAG_MDD_AUTO_RESET_VF, 0),
>   };
>   
>   #define I40E_PRIV_FLAGS_STR_LEN ARRAY_SIZE(i40e_gstrings_priv_flags)
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index f346ba6ef7bf..6c483f7dd279 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -11153,6 +11153,52 @@ static void i40e_handle_reset_warning(struct i40e_pf *pf, bool lock_acquired)
>   	i40e_reset_and_rebuild(pf, false, lock_acquired);
>   }
>   
> +/**
> + * i40e_print_vf_rx_mdd_event - print VF Rx malicious driver detect event
> + * @vf: pointer to the VF structure
> + */
> +static void i40e_print_vf_rx_mdd_event(struct i40e_pf *pf, struct i40e_vf *vf)
> +{
> +	dev_err_ratelimited(&pf->pdev->dev, "%lld Rx Malicious Driver Detection events detected on PF %d VF %d MAC %pm. mdd-auto-reset-vfs=%s\n",
> +		vf->mdd_rx_events.count,
> +		pf->hw.pf_id,
> +		vf->vf_id,
> +		vf->default_lan_addr.addr,
> +		(I40E_FLAG_MDD_AUTO_RESET_VF & pf->flags) ? "on" : "off");
> +}
> +
> +/**
> + * i40e_print_vfs_mdd_events - print VFs malicious driver detect event
> + * @pf: pointer to the PF structure
> + *
> + * Called from i40e_handle_mdd_event to rate limit and print VFs MDD events.
> + */
> +static void i40e_print_vfs_mdd_events(struct i40e_pf *pf)
> +{
> +	struct i40e_vf *vf;
> +	unsigned int i;
> +
> +	for (i = 0; i < pf->num_alloc_vfs; i++) {
> +		vf = &pf->vf[i];
> +		/* only print Rx MDD event message if there are new events */
> +		if (vf->mdd_rx_events.count != vf->mdd_rx_events.last_printed) {
> +			vf->mdd_rx_events.last_printed = vf->mdd_rx_events.count;
> +			i40e_print_vf_rx_mdd_event(pf, vf);
> +		}
> +
> +		/* only print Tx MDD event message if there are new events */
> +		if (vf->mdd_tx_events.count != vf->mdd_tx_events.last_printed) {
> +			vf->mdd_tx_events.last_printed = vf->mdd_tx_events.count;
> +			dev_err_ratelimited(&pf->pdev->dev, "%lld Tx Malicious Driver Detection events detected on PF %d VF %d MAC %pM.\n",
> +				vf->mdd_tx_events.count,
> +				pf->hw.pf_id,
> +				vf->vf_id,
> +				vf->default_lan_addr.addr);
> +		}
> +	}
> +}
> +
> +
>   /**
>    * i40e_handle_mdd_event
>    * @pf: pointer to the PF structure
> @@ -11167,8 +11213,13 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
>   	u32 reg;
>   	int i;
>   
> -	if (!test_bit(__I40E_MDD_EVENT_PENDING, pf->state))
> +	if (!test_and_clear_bit(__I40E_MDD_EVENT_PENDING, pf->state)) {
> +		/* Since the VF MDD event logging is rate limited, check if
> +		 * there are pending MDD events.
> +		 */
> +		i40e_print_vfs_mdd_events(pf);
>   		return;
> +	}
>   
>   	/* find what triggered the MDD event */
>   	reg = rd32(hw, I40E_GL_MDET_TX);
> @@ -11224,10 +11275,6 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
>   		if (reg & I40E_VP_MDET_TX_VALID_MASK) {
>   			wr32(hw, I40E_VP_MDET_TX(i), 0xFFFF);
>   			vf->mdd_tx_events.count++;
> -			dev_info(&pf->pdev->dev, "TX driver issue detected on VF %d\n",
> -				 i);
> -			dev_info(&pf->pdev->dev,
> -				 "Use PF Control I/F to re-enable the VF\n");
>   			set_bit(I40E_VF_STATE_DISABLED, &vf->vf_states);
>   		}
>   
> @@ -11235,11 +11282,19 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
>   		if (reg & I40E_VP_MDET_RX_VALID_MASK) {
>   			wr32(hw, I40E_VP_MDET_RX(i), 0xFFFF);
>   			vf->mdd_rx_events.count++;
> -			dev_info(&pf->pdev->dev, "RX driver issue detected on VF %d\n",
> -				 i);
> -			dev_info(&pf->pdev->dev,
> -				 "Use PF Control I/F to re-enable the VF\n");
>   			set_bit(I40E_VF_STATE_DISABLED, &vf->vf_states);
> +
> +			if (pf->flags & I40E_FLAG_MDD_AUTO_RESET_VF) {
> +				/* VF MDD event counters will be cleared by
> +				 * reset, so print the event prior to reset.
> +				 */
> +				i40e_print_vf_rx_mdd_event(pf, vf);
> +				i40e_vc_notify_vf_reset(vf);
> +				/* Allow VF to process pending reset notification */
> +				msleep(20);
> +
> +				i40e_reset_vf(vf, false);
> +			}
>   		}
>   	}
>   
> @@ -11249,6 +11304,8 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
>   	reg |=  I40E_PFINT_ICR0_ENA_MAL_DETECT_MASK;
>   	wr32(hw, I40E_PFINT_ICR0_ENA, reg);
>   	i40e_flush(hw);
> +
> +	i40e_print_vfs_mdd_events(pf);
>   }
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
> index d75ba0a03169..dc127400ff1e 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
> @@ -64,6 +64,8 @@ struct i40evf_channel {
>   
>   struct i40e_mdd_vf_events {
>   	u64 count; /* total count of Rx|Tx events */
> +	/* count number of the last printed event */
> +	u64 last_printed;
>   };
>   
>   /* VF information structure */
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan