mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5 0/1] i40e: additional safety check
@ 2025-12-12 21:06 gregory.herrero
  2025-12-12 21:06 ` [PATCH v5 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero
  2025-12-12 23:53 ` [PATCH v5 0/1] i40e: additional safety check Creeley, Brett
  0 siblings, 2 replies; 4+ messages in thread
From: gregory.herrero @ 2025-12-12 21:06 UTC (permalink / raw)
  To: aleksandr.loktionov, anthony.l.nguyen, przemyslaw.kitszel,
	andrew+netdev, davem, edumazet, kuba, pabeni
  Cc: intel-wired-lan, netdev, linux-kernel, 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().

v5:
- use "hardware-dependent" in commit description
- add Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
- get rid of i40e_get_max_num_descriptors() documentation as it's self explanatory
v4:
- remove u32 cast in i40e_config_vsi_tx_queue() too and don't mention it
  anymore in commit description.
- wrap i40e_get_max_num_descriptors() description
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             | 11 +++++++++++
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c     | 12 ------------
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |  4 ++--
 3 files changed, 13 insertions(+), 14 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v5 1/1] i40e: validate ring_len parameter against hardware-specific values
  2025-12-12 21:06 [PATCH v5 0/1] i40e: additional safety check gregory.herrero
@ 2025-12-12 21:06 ` gregory.herrero
  2025-12-12 21:12   ` Loktionov, Aleksandr
  2025-12-12 23:53 ` [PATCH v5 0/1] i40e: additional safety check Creeley, Brett
  1 sibling, 1 reply; 4+ messages in thread
From: gregory.herrero @ 2025-12-12 21:06 UTC (permalink / raw)
  To: aleksandr.loktionov, anthony.l.nguyen, przemyslaw.kitszel,
	andrew+netdev, davem, edumazet, kuba, pabeni
  Cc: intel-wired-lan, netdev, linux-kernel, Gregory Herrero, Rafal Romanowski

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.

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-dependent maximum number of
descriptors.

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

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index d2d03db2acec..dcb50c2e1aa2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -1422,4 +1422,15 @@ 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;
 }
 
+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 f2c2646ea298..6a47ea0927e9 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 8b30a3accd31..1fa877b52f61 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) {
+	    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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v5 1/1] i40e: validate ring_len parameter against hardware-specific values
  2025-12-12 21:06 ` [PATCH v5 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero
@ 2025-12-12 21:12   ` Loktionov, Aleksandr
  0 siblings, 0 replies; 4+ messages in thread
From: Loktionov, Aleksandr @ 2025-12-12 21:12 UTC (permalink / raw)
  To: gregory.herrero, Nguyen, Anthony L, Kitszel, Przemyslaw,
	andrew+netdev, davem, edumazet, kuba, pabeni
  Cc: intel-wired-lan, netdev, linux-kernel, Romanowski, Rafal



> -----Original Message-----
> From: gregory.herrero@oracle.com <gregory.herrero@oracle.com>
> Sent: Friday, December 12, 2025 10:07 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>;
> Romanowski, Rafal <rafal.romanowski@intel.com>
> Subject: [PATCH v5 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.
> 
> 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-dependent maximum number of
> descriptors.
> 
> Fixes: 55d225670def ("i40e: add validation for ring_len param")
> Signed-off-by: Gregory Herrero <gregory.herrero@oracle.com>
> Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e.h             | 11 +++++++++++
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c     | 12 ------------
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |  4 ++--
>  3 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
> b/drivers/net/ethernet/intel/i40e/i40e.h
> index d2d03db2acec..dcb50c2e1aa2 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -1422,4 +1422,15 @@ 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;
>  }
> 
> +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 f2c2646ea298..6a47ea0927e9 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 8b30a3accd31..1fa877b52f61 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) {
> +	    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

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v5 0/1] i40e: additional safety check
  2025-12-12 21:06 [PATCH v5 0/1] i40e: additional safety check gregory.herrero
  2025-12-12 21:06 ` [PATCH v5 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero
@ 2025-12-12 23:53 ` Creeley, Brett
  1 sibling, 0 replies; 4+ messages in thread
From: Creeley, Brett @ 2025-12-12 23:53 UTC (permalink / raw)
  To: gregory.herrero, aleksandr.loktionov, anthony.l.nguyen,
	przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni
  Cc: intel-wired-lan, netdev, linux-kernel

On 12/12/2025 1:06 PM, gregory.herrero@oracle.com wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> 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().
>
> v5:
> - use "hardware-dependent" in commit description
> - add Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
> - get rid of i40e_get_max_num_descriptors() documentation as it's self explanatory
> v4:
> - remove u32 cast in i40e_config_vsi_tx_queue() too and don't mention it
>    anymore in commit description.
> - wrap i40e_get_max_num_descriptors() description
> 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             | 11 +++++++++++
>   drivers/net/ethernet/intel/i40e/i40e_ethtool.c     | 12 ------------
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |  4 ++--
>   3 files changed, 13 insertions(+), 14 deletions(-)

Sorry I didn't notice this before, but I'm not used to seeing a cover 
letter for a single patch. Usually (always?) cover letters are only used 
for series (2 or more patches).

Thanks,

Brett
>
> --
> 2.51.0
>
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-12 23:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-12 21:06 [PATCH v5 0/1] i40e: additional safety check gregory.herrero
2025-12-12 21:06 ` [PATCH v5 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero
2025-12-12 21:12   ` Loktionov, Aleksandr
2025-12-12 23:53 ` [PATCH v5 0/1] i40e: additional safety check Creeley, Brett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®