* [PATCH v4 0/1] i40e: additional safety check @ 2025-11-17 8:33 gregory.herrero 2025-11-17 8:33 ` [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero 0 siblings, 1 reply; 8+ messages in thread From: gregory.herrero @ 2025-11-17 8:33 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(). 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 | 18 ++++++++++++++++++ drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 ------------ .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 4 ++-- 3 files changed, 20 insertions(+), 14 deletions(-) -- 2.51.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values 2025-11-17 8:33 [PATCH v4 0/1] i40e: additional safety check gregory.herrero @ 2025-11-17 8:33 ` gregory.herrero 2025-11-17 11:58 ` Loktionov, Aleksandr 2025-12-12 17:37 ` Creeley, Brett 0 siblings, 2 replies; 8+ messages in thread From: gregory.herrero @ 2025-11-17 8:33 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> 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-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 | 18 ++++++++++++++++++ drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 ------------ .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 4 ++-- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index 801a57a925da..5b367397ae43 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -1418,4 +1418,22 @@ 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..cf831c649c9c 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 related [flat|nested] 8+ messages in thread
* RE: [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values 2025-11-17 8:33 ` [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero @ 2025-11-17 11:58 ` Loktionov, Aleksandr 2025-12-11 8:38 ` Romanowski, Rafal 2025-12-12 20:20 ` Gregory Herrero 2025-12-12 17:37 ` Creeley, Brett 1 sibling, 2 replies; 8+ messages in thread From: Loktionov, Aleksandr @ 2025-11-17 11:58 UTC (permalink / raw) To: gregory.herrero@oracle.com, Nguyen, Anthony L, Kitszel, Przemyslaw, 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 > -----Original Message----- > From: gregory.herrero@oracle.com <gregory.herrero@oracle.com> > Sent: Monday, November 17, 2025 9:33 AM > 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 v4 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 First paragraph uses “hardware dependent” (no hyphen) while later text uses “hardware‑specific” (hyphenated). Prefer “hardware‑dependent” for consistency. > 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-specific maximum number > of descriptors. > The message explains the behavioral change but does not state how the change was tested (e.g., which MAC types exercised, ethtool -G paths, VF configuration via virtchnl, acceptance/rejection boundaries). Netdev routinely asks for this when behavior changes. > 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 | 18 > ++++++++++++++++++ > drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 ------------ > .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 4 ++-- > 3 files changed, 20 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h > b/drivers/net/ethernet/intel/i40e/i40e.h > index 801a57a925da..5b367397ae43 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e.h > +++ b/drivers/net/ethernet/intel/i40e/i40e.h > @@ -1418,4 +1418,22 @@ 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; } ... > -- > 2.51.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values 2025-11-17 11:58 ` Loktionov, Aleksandr @ 2025-12-11 8:38 ` Romanowski, Rafal 2025-12-12 20:20 ` Gregory Herrero 1 sibling, 0 replies; 8+ messages in thread From: Romanowski, Rafal @ 2025-12-11 8:38 UTC (permalink / raw) To: Loktionov, Aleksandr, gregory.herrero@oracle.com, Nguyen, Anthony L, Kitszel, Przemyslaw, 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 > -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Loktionov, Aleksandr > Sent: Monday, November 17, 2025 12:59 > To: gregory.herrero@oracle.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 > Subject: Re: [Intel-wired-lan] [PATCH v4 1/1] i40e: validate ring_len parameter > against hardware-specific values > > > > > -----Original Message----- > > From: gregory.herrero@oracle.com <gregory.herrero@oracle.com> > > Sent: Monday, November 17, 2025 9:33 AM > > 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 v4 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 > First paragraph uses “hardware dependent” (no hyphen) while later text uses > “hardware‑specific” (hyphenated). > Prefer “hardware‑dependent” for consistency. > > > 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-specific maximum number > > of descriptors. > > > The message explains the behavioral change but does not state how the change > was tested (e.g., which MAC types exercised, ethtool -G paths, VF configuration > via virtchnl, acceptance/rejection boundaries). > Netdev routinely asks for this when behavior changes. > > > 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 | 18 > > ++++++++++++++++++ > > drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 ------------ > > .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 4 ++-- > > 3 files changed, 20 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h > > b/drivers/net/ethernet/intel/i40e/i40e.h > > index 801a57a925da..5b367397ae43 100644 > > --- a/drivers/net/ethernet/intel/i40e/i40e.h > > +++ b/drivers/net/ethernet/intel/i40e/i40e.h > > @@ -1418,4 +1418,22 @@ 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; } > > ... > > > -- > > 2.51.0 Tested-by: Rafal Romanowski <rafal.romanowski@intel.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values 2025-11-17 11:58 ` Loktionov, Aleksandr 2025-12-11 8:38 ` Romanowski, Rafal @ 2025-12-12 20:20 ` Gregory Herrero 1 sibling, 0 replies; 8+ messages in thread From: Gregory Herrero @ 2025-12-12 20:20 UTC (permalink / raw) To: Loktionov, Aleksandr Cc: Nguyen, Anthony L, Kitszel, Przemyslaw, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org On Mon, Nov 17, 2025 at 11:58:48AM +0000, Loktionov, Aleksandr wrote: > > > > -----Original Message----- > > From: gregory.herrero@oracle.com <gregory.herrero@oracle.com> > > Sent: Monday, November 17, 2025 9:33 AM > > 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 v4 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 > First paragraph uses “hardware dependent” (no hyphen) while later text uses “hardware‑specific” (hyphenated). > Prefer “hardware‑dependent” for consistency. > I will address in v5 > > 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-specific maximum number > > of descriptors. > > > The message explains the behavioral change but does not state how the change was tested > (e.g., which MAC types exercised, ethtool -G paths, VF configuration via virtchnl, acceptance/rejection boundaries). > Netdev routinely asks for this when behavior changes. > In the meantime, Rafal Romanowski tested it so I will add: Tested-by: Rafal Romanowski <rafal.romanowski@intel.com> Let me know if that's enough or if more details are required. Thanks, Gregory ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values 2025-11-17 8:33 ` [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero 2025-11-17 11:58 ` Loktionov, Aleksandr @ 2025-12-12 17:37 ` Creeley, Brett 2025-12-12 20:22 ` Gregory Herrero 1 sibling, 1 reply; 8+ messages in thread From: Creeley, Brett @ 2025-12-12 17:37 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 11/17/2025 12:33 AM, 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> > > 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-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 | 18 ++++++++++++++++++ > drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 ------------ > .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 4 ++-- > 3 files changed, 20 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h > index 801a57a925da..5b367397ae43 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e.h > +++ b/drivers/net/ethernet/intel/i40e/i40e.h > @@ -1418,4 +1418,22 @@ 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. > + **/ Nit, but the function name is descriptive enough without the documentation. I think the purpose of the function would be even more obvious if the argument was a pointer to the hw structure instead of a pointer to the pf since the max is based on the hw not the pf. Brett > +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..cf831c649c9c 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] 8+ messages in thread
* Re: [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values 2025-12-12 17:37 ` Creeley, Brett @ 2025-12-12 20:22 ` Gregory Herrero 2025-12-12 20:45 ` Creeley, Brett 0 siblings, 1 reply; 8+ messages in thread From: Gregory Herrero @ 2025-12-12 20:22 UTC (permalink / raw) To: Creeley, Brett Cc: aleksandr.loktionov, anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni, intel-wired-lan, netdev, linux-kernel On Fri, Dec 12, 2025 at 09:37:20AM -0800, Creeley, Brett wrote: > > On 11/17/2025 12:33 AM, 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> > > > > 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-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 | 18 ++++++++++++++++++ > > drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 ------------ > > .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 4 ++-- > > 3 files changed, 20 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h > > index 801a57a925da..5b367397ae43 100644 > > --- a/drivers/net/ethernet/intel/i40e/i40e.h > > +++ b/drivers/net/ethernet/intel/i40e/i40e.h > > @@ -1418,4 +1418,22 @@ 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. > > + **/ > > Nit, but the function name is descriptive enough without the documentation. > > I think the purpose of the function would be even more obvious if the > argument was a pointer to the hw structure instead of a pointer to the pf > since the max is based on the hw not the pf. > I agree, it's just that it will require changing the 5 callers of this function and invalidate the testing from Rafal Romanowski. Please let me know what you think, I can wait before sending v5. Thanks, Gregory > > +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..cf831c649c9c 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] 8+ messages in thread
* Re: [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values 2025-12-12 20:22 ` Gregory Herrero @ 2025-12-12 20:45 ` Creeley, Brett 0 siblings, 0 replies; 8+ messages in thread From: Creeley, Brett @ 2025-12-12 20:45 UTC (permalink / raw) To: Gregory Herrero Cc: aleksandr.loktionov, anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni, intel-wired-lan, netdev, linux-kernel On 12/12/2025 12:22 PM, Gregory Herrero wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Fri, Dec 12, 2025 at 09:37:20AM -0800, Creeley, Brett wrote: >> On 11/17/2025 12:33 AM, 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> >>> >>> 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-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 | 18 ++++++++++++++++++ >>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 12 ------------ >>> .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 4 ++-- >>> 3 files changed, 20 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h >>> index 801a57a925da..5b367397ae43 100644 >>> --- a/drivers/net/ethernet/intel/i40e/i40e.h >>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h >>> @@ -1418,4 +1418,22 @@ 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. >>> + **/ >> Nit, but the function name is descriptive enough without the documentation. >> >> I think the purpose of the function would be even more obvious if the >> argument was a pointer to the hw structure instead of a pointer to the pf >> since the max is based on the hw not the pf. >> > I agree, it's just that it will require changing the 5 callers of this > function and invalidate the testing from Rafal Romanowski. > Please let me know what you think, I can wait before sending v5. I'm fine with the PF being the parameter. However, the documentation should really be removed IMO. Thanks, Brett > > Thanks, > Gregory >>> +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..cf831c649c9c 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] 8+ messages in thread
end of thread, other threads:[~2025-12-12 20:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-17 8:33 [PATCH v4 0/1] i40e: additional safety check gregory.herrero 2025-11-17 8:33 ` [PATCH v4 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero 2025-11-17 11:58 ` Loktionov, Aleksandr 2025-12-11 8:38 ` Romanowski, Rafal 2025-12-12 20:20 ` Gregory Herrero 2025-12-12 17:37 ` Creeley, Brett 2025-12-12 20:22 ` Gregory Herrero 2025-12-12 20:45 ` Creeley, Brett
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox