* [Intel-wired-lan] [PATCH v2 0/1] i40e: additional safety check @ 2025-11-13 8:04 gregory.herrero 2025-11-13 8:04 ` [Intel-wired-lan] [PATCH v2 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero 0 siblings, 1 reply; 4+ messages in thread From: gregory.herrero @ 2025-11-13 8:04 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(). 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Intel-wired-lan] [PATCH v2 1/1] i40e: validate ring_len parameter against hardware-specific values. 2025-11-13 8:04 [Intel-wired-lan] [PATCH v2 0/1] i40e: additional safety check gregory.herrero @ 2025-11-13 8:04 ` gregory.herrero 2025-11-13 8:18 ` Loktionov, Aleksandr 0 siblings, 1 reply; 4+ messages in thread From: gregory.herrero @ 2025-11-13 8:04 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. Cast info->ring_len to u32 in i40e_config_vsi_tx_queue() as it's u16 in struct virtchnl_txq_info. Also cast it in i40e_config_vsi_rx_queue() even if it's u32 in virtchnl_rxq_info to ease stable backport in case this changed. By fixing an over-acceptance issue, behavior change could be seen where ring_len would now be rejected whereas it was not before. 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..5e058159057b 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) { + (u32)info->ring_len > i40e_get_max_num_descriptors(pf)) { ret = -EINVAL; goto error_param; } -- 2.51.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Intel-wired-lan] [PATCH v2 1/1] i40e: validate ring_len parameter against hardware-specific values. 2025-11-13 8:04 ` [Intel-wired-lan] [PATCH v2 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero @ 2025-11-13 8:18 ` Loktionov, Aleksandr 2025-11-13 8:36 ` Gregory Herrero 0 siblings, 1 reply; 4+ messages in thread From: Loktionov, Aleksandr @ 2025-11-13 8:18 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: Thursday, November 13, 2025 9:05 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 v2 1/1] i40e: validate ring_len parameter against > hardware-specific values. > Please drop the trailing period from the subject. > 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. > Also cast it in i40e_config_vsi_rx_queue() even if it's u32 in > virtchnl_rxq_info to ease stable backport in case this changed. > > By fixing an over-acceptance issue, behavior change could be seen > where ring_len would now be rejected whereas it was not before. > Please add a short “Tested:” explanation (what hw/flows, expected/actual before/after). > 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..5e058159057b 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) { > + (u32)info->ring_len > i40e_get_max_num_descriptors(pf)) { virtchnl_rxq_info.ring_len is already u32 (as noted in the commit message). Casting it to u32 before comparison is redundant and adds churn without value in mainline. The (u32) cast on info->ring_len can be dropped in mainline; if you need it only for a stable backport, consider keeping the mainline patch minimal and adding the backport‑only hunk when submitting to stable. With the best regards, Alex > ret = -EINVAL; > goto error_param; > } > -- > 2.51.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-wired-lan] [PATCH v2 1/1] i40e: validate ring_len parameter against hardware-specific values. 2025-11-13 8:18 ` Loktionov, Aleksandr @ 2025-11-13 8:36 ` Gregory Herrero 0 siblings, 0 replies; 4+ messages in thread From: Gregory Herrero @ 2025-11-13 8:36 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 Thu, Nov 13, 2025 at 08:18:30AM +0000, Loktionov, Aleksandr wrote: > > > > -----Original Message----- > > From: gregory.herrero@oracle.com <gregory.herrero@oracle.com> > > Sent: Thursday, November 13, 2025 9:05 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 v2 1/1] i40e: validate ring_len parameter against > > hardware-specific values. > > > Please drop the trailing period from the subject. > Ok > > 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. > > Also cast it in i40e_config_vsi_rx_queue() even if it's u32 in > > virtchnl_rxq_info to ease stable backport in case this changed. > > > > By fixing an over-acceptance issue, behavior change could be seen > > where ring_len would now be rejected whereas it was not before. > > > Please add a short “Tested:” explanation (what hw/flows, expected/actual before/after). > No real test was done, it was found only by code inspection. I could update the description though: 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..5e058159057b 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) { > > + (u32)info->ring_len > i40e_get_max_num_descriptors(pf)) { > virtchnl_rxq_info.ring_len is already u32 (as noted in the commit message). > Casting it to u32 before comparison is redundant and adds churn without value in mainline. > The (u32) cast on info->ring_len can be dropped in mainline; if you need it only for a stable backport, consider keeping the mainline patch minimal and adding the backport‑only hunk when submitting to stable. > I've dropped this cast and removed the comment about it from the commit description. Thanks for the review, Greg ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-13 8:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-13 8:04 [Intel-wired-lan] [PATCH v2 0/1] i40e: additional safety check gregory.herrero 2025-11-13 8:04 ` [Intel-wired-lan] [PATCH v2 1/1] i40e: validate ring_len parameter against hardware-specific values gregory.herrero 2025-11-13 8:18 ` Loktionov, Aleksandr 2025-11-13 8:36 ` Gregory Herrero
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).