[PATCH v3 0/1] i40e: additional safety check

[PATCH v3 0/1] i40e: additional safety check

[gregory.herrero]

From: Gregory Herrero <gregory.herrero@oracle.com>

On code inspection, I realized we may want to check ring_len parameter
against hardware specific values in i40e_config_vsi_tx_queue() and
i40e_config_vsi_rx_queue().

v3:
- drop trailing period from the subject
- reword commit description
- remove u32 cast in i40e_config_vsi_rx_queue()
v2:
- make i40e_get_max_num_descriptors() 'pf' argument const.
- reword i40e_get_max_num_descriptors() description.
- modify commit description to explain potential behavior change.

Gregory Herrero (1):
  i40e: validate ring_len parameter against hardware-specific values

 drivers/net/ethernet/intel/i40e/i40e.h          | 17 +++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c  | 12 ------------
 .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c  |  4 ++--
 3 files changed, 19 insertions(+), 14 deletions(-)

-- 
2.51.0

[PATCH v3 1/1] i40e: validate ring_len parameter against hardware-specific values

[gregory.herrero]

From: Gregory Herrero <gregory.herrero@oracle.com>

The maximum number of descriptors supported by the hardware is hardware
dependent and can be retrieved using i40e_get_max_num_descriptors().
Move this function to a shared header and use it when checking for valid
ring_len parameter rather than using hardcoded value.
Cast info->ring_len to u32 in i40e_config_vsi_tx_queue() as it's u16 in
struct virtchnl_txq_info.

By fixing an over-acceptance issue, behavior change could be seen where
ring_len could now be rejected while configuring rx and tx queues if its
size is larger than the hardware-specific maximum number of descriptors.

Fixes: 55d225670def ("i40e: add validation for ring_len param")
Signed-off-by: Gregory Herrero <gregory.herrero@oracle.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h          | 17 +++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c  | 12 ------------
 .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c  |  4 ++--
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 801a57a925da..a953cce008f4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -1418,4 +1418,21 @@ static inline struct i40e_veb *i40e_pf_get_main_veb(struct i40e_pf *pf)
 	return (pf->lan_veb != I40E_NO_VEB) ? pf->veb[pf->lan_veb] : NULL;
 }
 
+/**
+ * i40e_get_max_num_descriptors - get maximum number of descriptors for this hardware.
+ * @pf: pointer to a PF
+ *
+ * Return: u32 value corresponding to the maximum number of descriptors.
+ **/
+static inline u32 i40e_get_max_num_descriptors(const struct i40e_pf *pf)
+{
+	const struct i40e_hw *hw = &pf->hw;
+
+	switch (hw->mac.type) {
+	case I40E_MAC_XL710:
+		return I40E_MAX_NUM_DESCRIPTORS_XL710;
+	default:
+		return I40E_MAX_NUM_DESCRIPTORS;
+	}
+}
 #endif /* _I40E_H_ */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 86c72596617a..61c39e881b00 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -2013,18 +2013,6 @@ static void i40e_get_drvinfo(struct net_device *netdev,
 		drvinfo->n_priv_flags += I40E_GL_PRIV_FLAGS_STR_LEN;
 }
 
-static u32 i40e_get_max_num_descriptors(struct i40e_pf *pf)
-{
-	struct i40e_hw *hw = &pf->hw;
-
-	switch (hw->mac.type) {
-	case I40E_MAC_XL710:
-		return I40E_MAX_NUM_DESCRIPTORS_XL710;
-	default:
-		return I40E_MAX_NUM_DESCRIPTORS;
-	}
-}
-
 static void i40e_get_ringparam(struct net_device *netdev,
 			       struct ethtool_ringparam *ring,
 			       struct kernel_ethtool_ringparam *kernel_ring,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 081a4526a2f0..5ecc9bb908f9 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -656,7 +656,7 @@ static int i40e_config_vsi_tx_queue(struct i40e_vf *vf, u16 vsi_id,
 
 	/* ring_len has to be multiple of 8 */
 	if (!IS_ALIGNED(info->ring_len, 8) ||
-	    info->ring_len > I40E_MAX_NUM_DESCRIPTORS_XL710) {
+	    (u32)info->ring_len > i40e_get_max_num_descriptors(pf)) {
 		ret = -EINVAL;
 		goto error_context;
 	}
@@ -726,7 +726,7 @@ static int i40e_config_vsi_rx_queue(struct i40e_vf *vf, u16 vsi_id,
 
 	/* ring_len has to be multiple of 32 */
 	if (!IS_ALIGNED(info->ring_len, 32) ||
-	    info->ring_len > I40E_MAX_NUM_DESCRIPTORS_XL710) {
+	    info->ring_len > i40e_get_max_num_descriptors(pf)) {
 		ret = -EINVAL;
 		goto error_param;
 	}
-- 
2.51.0

RE: [PATCH v3 1/1] i40e: validate ring_len parameter against hardware-specific values

[Loktionov, Aleksandr]

> -----Original Message-----
> From: gregory.herrero@oracle.com <gregory.herrero@oracle.com>
> Sent: Friday, November 14, 2025 5:03 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Nguyen,
> Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Gregory Herrero <gregory.herrero@oracle.com>
> Subject: [PATCH v3 1/1] i40e: validate ring_len parameter against
> hardware-specific values
> 
> From: Gregory Herrero <gregory.herrero@oracle.com>
> 
> The maximum number of descriptors supported by the hardware is
> hardware dependent and can be retrieved using
> i40e_get_max_num_descriptors().
> Move this function to a shared header and use it when checking for
> valid ring_len parameter rather than using hardcoded value.
> Cast info->ring_len to u32 in i40e_config_vsi_tx_queue() as it's u16
> in struct virtchnl_txq_info.
> 
> By fixing an over-acceptance issue, behavior change could be seen
> where ring_len could now be rejected while configuring rx and tx
> queues if its size is larger than the hardware-specific maximum number
> of descriptors.
> 
> Fixes: 55d225670def ("i40e: add validation for ring_len param")
> Signed-off-by: Gregory Herrero <gregory.herrero@oracle.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e.h          | 17
> +++++++++++++++++
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c  | 12 ------------
> .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c  |  4 ++--
>  3 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
> b/drivers/net/ethernet/intel/i40e/i40e.h
> index 801a57a925da..a953cce008f4 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -1418,4 +1418,21 @@ static inline struct i40e_veb
> *i40e_pf_get_main_veb(struct i40e_pf *pf)
>  	return (pf->lan_veb != I40E_NO_VEB) ? pf->veb[pf->lan_veb] :
> NULL;  }
> 
> +/**
> + * i40e_get_max_num_descriptors - get maximum number of descriptors
> for this hardware.
Please wrap this line to keep within ~80 columns (which is still enforced in net/* reviews and expected for drivers/net as well).

> + * @pf: pointer to a PF
> + *
> + * Return: u32 value corresponding to the maximum number of
> descriptors.
> + **/
> +static inline u32 i40e_get_max_num_descriptors(const struct i40e_pf
> +*pf) {
> +	const struct i40e_hw *hw = &pf->hw;
> +
> +	switch (hw->mac.type) {
> +	case I40E_MAC_XL710:
> +		return I40E_MAX_NUM_DESCRIPTORS_XL710;
> +	default:
> +		return I40E_MAX_NUM_DESCRIPTORS;
> +	}
> +}
>  #endif /* _I40E_H_ */
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 86c72596617a..61c39e881b00 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -2013,18 +2013,6 @@ static void i40e_get_drvinfo(struct net_device
> *netdev,
>  		drvinfo->n_priv_flags += I40E_GL_PRIV_FLAGS_STR_LEN;  }
> 
> -static u32 i40e_get_max_num_descriptors(struct i40e_pf *pf) -{
> -	struct i40e_hw *hw = &pf->hw;
> -
> -	switch (hw->mac.type) {
> -	case I40E_MAC_XL710:
> -		return I40E_MAX_NUM_DESCRIPTORS_XL710;
> -	default:
> -		return I40E_MAX_NUM_DESCRIPTORS;
> -	}
> -}
> -
>  static void i40e_get_ringparam(struct net_device *netdev,
>  			       struct ethtool_ringparam *ring,
>  			       struct kernel_ethtool_ringparam
> *kernel_ring, diff --git
> a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 081a4526a2f0..5ecc9bb908f9 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -656,7 +656,7 @@ static int i40e_config_vsi_tx_queue(struct i40e_vf
> *vf, u16 vsi_id,
> 
>  	/* ring_len has to be multiple of 8 */
>  	if (!IS_ALIGNED(info->ring_len, 8) ||
> -	    info->ring_len > I40E_MAX_NUM_DESCRIPTORS_XL710) {
> +	    (u32)info->ring_len > i40e_get_max_num_descriptors(pf)) {
The cast to u32 is not needed: info->ring_len (per your commit message, u16 in struct virtchnl_txq_info) will undergo the usual integer promotions and be compared as u32 against the function's u32 return value. Dropping the cast reduces churn and matches the RX path style (which doesn't cast)

>  		ret = -EINVAL;
>  		goto error_context;
>  	}
> @@ -726,7 +726,7 @@ static int i40e_config_vsi_rx_queue(struct i40e_vf
> *vf, u16 vsi_id,
> 
>  	/* ring_len has to be multiple of 32 */
>  	if (!IS_ALIGNED(info->ring_len, 32) ||
> -	    info->ring_len > I40E_MAX_NUM_DESCRIPTORS_XL710) {
> +	    info->ring_len > i40e_get_max_num_descriptors(pf)) {
>  		ret = -EINVAL;
>  		goto error_param;
>  	}
> --
> 2.51.0