All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [PATCH net] ice: Fix MSI-X vector fallback logic
  2021-01-21  7:52 [Intel-wired-lan] [PATCH net] ice: Fix MSI-X vector fallback logic Brett Creeley
@ 2021-01-21  7:17 ` Paul Menzel
  2021-01-21 17:07   ` Venkataramanan, Anirudh
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Menzel @ 2021-01-21  7:17 UTC (permalink / raw)
  To: intel-wired-lan

Dear Brett,


Am 21.01.21 um 08:52 schrieb Brett Creeley:
> The current MSI-X enablement logic tries to enable best-case MSI-X
> vectors and if that fails we only support a bare-minimum set. This
> includes a single MSI-X for 1 Tx and 1 Rx queue and a single MSI-X
> for the OICR interrupt. Unfortunately, the driver fails to load when we
> don't get as many MSI-X as requested for a couple reasons.
> 
> First, the code to allocate MSI-X in the driver tries to allocate
> num_online_cpus() MSI-X for LAN traffic without caring about the number
> of MSI-X actually enabled/requested from the kernel for LAN traffic.
> So, when calling ice_get_res() for the PF VSI, it returns failure
> because the number of available vectors is less than requested. Fix
> this by not allowing the PF VSI to allocation  more than
> pf->num_lan_msix MSI-X vectors and pf->num_lan_msix Rx/Tx queues.
> Limiting the number of queues is done because we don't want more than
> 1 Tx/Rx queue per interrupt due to performance conerns.
> 
> Second, the driver assigns pf->num_lan_msix = 2, to account for LAN
> traffic and the OICR. However, pf->num_lan_msix is only meant for LAN
> MSI-X. This is causing a failure when the PF VSI tries to
> allocate/reserve the minimum pf->num_lan_msix because the OICR MSI-X has
> already been reserved, so there may not be enough MSI-X vectors left.
> Fix this by setting pf->num_lan_msix = 1 for the failure case. Then the
> ICE_MIN_MSIX accounts for the LAN MSI-X and the OICR MSI-X needed for
> the failure case.
> 
> Update the related defines used in ice_ena_msix_range() to align with
> the above behavior and remove the unused RDMA defines because RDMA is
> currently not supported. Also, remove the now incorrect comment.
> 
> Also, prevent users from enabling more combined queues than there are
> MSI-X available via ethtool.
> 
> Fixes: Commit 152b978a1f90 ("ice: Rework ice_ena_msix_range")
> Fixes: Commit 87324e747fde ("ice: Implement ethtool ops for channels")

The word *Commit* does not need to be put in there.

Could you split the ethtool change into a separate commit?

Also, can you document your test setup so the driver failed to load?


Kind regards,

Paul


> Signed-off-by: Brett Creeley <brett.creeley@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice.h         |  4 +++-
>   drivers/net/ethernet/intel/ice/ice_ethtool.c |  8 ++++----
>   drivers/net/ethernet/intel/ice/ice_lib.c     | 14 +++++++++-----
>   drivers/net/ethernet/intel/ice/ice_main.c    |  8 ++------
>   4 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 6efafe7d8a62..619d93f8b54c 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -68,7 +68,9 @@
>   #define ICE_INT_NAME_STR_LEN	(IFNAMSIZ + 16)
>   #define ICE_AQ_LEN		64
>   #define ICE_MBXSQ_LEN		64
> -#define ICE_MIN_MSIX		2
> +#define ICE_MIN_LAN_TXRX_MSIX	1
> +#define ICE_MIN_LAN_OICR_MSIX	1
> +#define ICE_MIN_MSIX		(ICE_MIN_LAN_TXRX_MSIX + ICE_MIN_LAN_OICR_MSIX)
>   #define ICE_FDIR_MSIX		1
>   #define ICE_NO_VSI		0xffff
>   #define ICE_VSI_MAP_CONTIG	0
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index 41427302332c..aebebd2102da 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3265,8 +3265,8 @@ ice_set_rxfh(struct net_device *netdev, const u32 *indir, const u8 *key,
>    */
>   static int ice_get_max_txq(struct ice_pf *pf)
>   {
> -	return min_t(int, num_online_cpus(),
> -		     pf->hw.func_caps.common_cap.num_txq);
> +	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
> +		    (u16)pf->hw.func_caps.common_cap.num_txq);
>   }
>   
>   /**
> @@ -3275,8 +3275,8 @@ static int ice_get_max_txq(struct ice_pf *pf)
>    */
>   static int ice_get_max_rxq(struct ice_pf *pf)
>   {
> -	return min_t(int, num_online_cpus(),
> -		     pf->hw.func_caps.common_cap.num_rxq);
> +	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
> +		    (u16)pf->hw.func_caps.common_cap.num_rxq);
>   }
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 3df67486d42d..ad9c22a1b97a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -161,8 +161,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id)
>   
>   	switch (vsi->type) {
>   	case ICE_VSI_PF:
> -		vsi->alloc_txq = min_t(int, ice_get_avail_txq_count(pf),
> -				       num_online_cpus());
> +		vsi->alloc_txq = min3(pf->num_lan_msix,
> +				      ice_get_avail_txq_count(pf),
> +				      (u16)num_online_cpus());
>   		if (vsi->req_txq) {
>   			vsi->alloc_txq = vsi->req_txq;
>   			vsi->num_txq = vsi->req_txq;
> @@ -174,8 +175,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id)
>   		if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) {
>   			vsi->alloc_rxq = 1;
>   		} else {
> -			vsi->alloc_rxq = min_t(int, ice_get_avail_rxq_count(pf),
> -					       num_online_cpus());
> +			vsi->alloc_rxq = min3(pf->num_lan_msix,
> +					      ice_get_avail_rxq_count(pf),
> +					      (u16)num_online_cpus());
>   			if (vsi->req_rxq) {
>   				vsi->alloc_rxq = vsi->req_rxq;
>   				vsi->num_rxq = vsi->req_rxq;
> @@ -184,7 +186,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id)
>   
>   		pf->num_lan_rx = vsi->alloc_rxq;
>   
> -		vsi->num_q_vectors = max_t(int, vsi->alloc_rxq, vsi->alloc_txq);
> +		vsi->num_q_vectors = min_t(int, pf->num_lan_msix,
> +					   max_t(int, vsi->alloc_rxq,
> +						 vsi->alloc_txq));
>   		break;
>   	case ICE_VSI_VF:
>   		vf = &pf->vf[vsi->vf_id];
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index fb81aa5979e3..e10ca8929f85 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -3430,18 +3430,14 @@ static int ice_ena_msix_range(struct ice_pf *pf)
>   	if (v_actual < v_budget) {
>   		dev_warn(dev, "not enough OS MSI-X vectors. requested = %d, obtained = %d\n",
>   			 v_budget, v_actual);
> -/* 2 vectors each for LAN and RDMA (traffic + OICR), one for flow director */
> -#define ICE_MIN_LAN_VECS 2
> -#define ICE_MIN_RDMA_VECS 2
> -#define ICE_MIN_VECS (ICE_MIN_LAN_VECS + ICE_MIN_RDMA_VECS + 1)
>   
> -		if (v_actual < ICE_MIN_LAN_VECS) {
> +		if (v_actual < ICE_MIN_MSIX) {
>   			/* error if we can't get minimum vectors */
>   			pci_disable_msix(pf->pdev);
>   			err = -ERANGE;
>   			goto msix_err;
>   		} else {
> -			pf->num_lan_msix = ICE_MIN_LAN_VECS;
> +			pf->num_lan_msix = ICE_MIN_LAN_TXRX_MSIX;
>   		}
>   	}
>   
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Fix MSI-X vector fallback logic
@ 2021-01-21  7:52 Brett Creeley
  2021-01-21  7:17 ` Paul Menzel
  0 siblings, 1 reply; 5+ messages in thread
From: Brett Creeley @ 2021-01-21  7:52 UTC (permalink / raw)
  To: intel-wired-lan

The current MSI-X enablement logic tries to enable best-case MSI-X
vectors and if that fails we only support a bare-minimum set. This
includes a single MSI-X for 1 Tx and 1 Rx queue and a single MSI-X
for the OICR interrupt. Unfortunately, the driver fails to load when we
don't get as many MSI-X as requested for a couple reasons.

First, the code to allocate MSI-X in the driver tries to allocate
num_online_cpus() MSI-X for LAN traffic without caring about the number
of MSI-X actually enabled/requested from the kernel for LAN traffic.
So, when calling ice_get_res() for the PF VSI, it returns failure
because the number of available vectors is less than requested. Fix
this by not allowing the PF VSI to allocation  more than
pf->num_lan_msix MSI-X vectors and pf->num_lan_msix Rx/Tx queues.
Limiting the number of queues is done because we don't want more than
1 Tx/Rx queue per interrupt due to performance conerns.

Second, the driver assigns pf->num_lan_msix = 2, to account for LAN
traffic and the OICR. However, pf->num_lan_msix is only meant for LAN
MSI-X. This is causing a failure when the PF VSI tries to
allocate/reserve the minimum pf->num_lan_msix because the OICR MSI-X has
already been reserved, so there may not be enough MSI-X vectors left.
Fix this by setting pf->num_lan_msix = 1 for the failure case. Then the
ICE_MIN_MSIX accounts for the LAN MSI-X and the OICR MSI-X needed for
the failure case.

Update the related defines used in ice_ena_msix_range() to align with
the above behavior and remove the unused RDMA defines because RDMA is
currently not supported. Also, remove the now incorrect comment.

Also, prevent users from enabling more combined queues than there are
MSI-X available via ethtool.

Fixes: Commit 152b978a1f90 ("ice: Rework ice_ena_msix_range")
Fixes: Commit 87324e747fde ("ice: Implement ethtool ops for channels")
Signed-off-by: Brett Creeley <brett.creeley@intel.com>
---
 drivers/net/ethernet/intel/ice/ice.h         |  4 +++-
 drivers/net/ethernet/intel/ice/ice_ethtool.c |  8 ++++----
 drivers/net/ethernet/intel/ice/ice_lib.c     | 14 +++++++++-----
 drivers/net/ethernet/intel/ice/ice_main.c    |  8 ++------
 4 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 6efafe7d8a62..619d93f8b54c 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -68,7 +68,9 @@
 #define ICE_INT_NAME_STR_LEN	(IFNAMSIZ + 16)
 #define ICE_AQ_LEN		64
 #define ICE_MBXSQ_LEN		64
-#define ICE_MIN_MSIX		2
+#define ICE_MIN_LAN_TXRX_MSIX	1
+#define ICE_MIN_LAN_OICR_MSIX	1
+#define ICE_MIN_MSIX		(ICE_MIN_LAN_TXRX_MSIX + ICE_MIN_LAN_OICR_MSIX)
 #define ICE_FDIR_MSIX		1
 #define ICE_NO_VSI		0xffff
 #define ICE_VSI_MAP_CONTIG	0
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 41427302332c..aebebd2102da 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3265,8 +3265,8 @@ ice_set_rxfh(struct net_device *netdev, const u32 *indir, const u8 *key,
  */
 static int ice_get_max_txq(struct ice_pf *pf)
 {
-	return min_t(int, num_online_cpus(),
-		     pf->hw.func_caps.common_cap.num_txq);
+	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
+		    (u16)pf->hw.func_caps.common_cap.num_txq);
 }
 
 /**
@@ -3275,8 +3275,8 @@ static int ice_get_max_txq(struct ice_pf *pf)
  */
 static int ice_get_max_rxq(struct ice_pf *pf)
 {
-	return min_t(int, num_online_cpus(),
-		     pf->hw.func_caps.common_cap.num_rxq);
+	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
+		    (u16)pf->hw.func_caps.common_cap.num_rxq);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 3df67486d42d..ad9c22a1b97a 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -161,8 +161,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id)
 
 	switch (vsi->type) {
 	case ICE_VSI_PF:
-		vsi->alloc_txq = min_t(int, ice_get_avail_txq_count(pf),
-				       num_online_cpus());
+		vsi->alloc_txq = min3(pf->num_lan_msix,
+				      ice_get_avail_txq_count(pf),
+				      (u16)num_online_cpus());
 		if (vsi->req_txq) {
 			vsi->alloc_txq = vsi->req_txq;
 			vsi->num_txq = vsi->req_txq;
@@ -174,8 +175,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id)
 		if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) {
 			vsi->alloc_rxq = 1;
 		} else {
-			vsi->alloc_rxq = min_t(int, ice_get_avail_rxq_count(pf),
-					       num_online_cpus());
+			vsi->alloc_rxq = min3(pf->num_lan_msix,
+					      ice_get_avail_rxq_count(pf),
+					      (u16)num_online_cpus());
 			if (vsi->req_rxq) {
 				vsi->alloc_rxq = vsi->req_rxq;
 				vsi->num_rxq = vsi->req_rxq;
@@ -184,7 +186,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id)
 
 		pf->num_lan_rx = vsi->alloc_rxq;
 
-		vsi->num_q_vectors = max_t(int, vsi->alloc_rxq, vsi->alloc_txq);
+		vsi->num_q_vectors = min_t(int, pf->num_lan_msix,
+					   max_t(int, vsi->alloc_rxq,
+						 vsi->alloc_txq));
 		break;
 	case ICE_VSI_VF:
 		vf = &pf->vf[vsi->vf_id];
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index fb81aa5979e3..e10ca8929f85 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3430,18 +3430,14 @@ static int ice_ena_msix_range(struct ice_pf *pf)
 	if (v_actual < v_budget) {
 		dev_warn(dev, "not enough OS MSI-X vectors. requested = %d, obtained = %d\n",
 			 v_budget, v_actual);
-/* 2 vectors each for LAN and RDMA (traffic + OICR), one for flow director */
-#define ICE_MIN_LAN_VECS 2
-#define ICE_MIN_RDMA_VECS 2
-#define ICE_MIN_VECS (ICE_MIN_LAN_VECS + ICE_MIN_RDMA_VECS + 1)
 
-		if (v_actual < ICE_MIN_LAN_VECS) {
+		if (v_actual < ICE_MIN_MSIX) {
 			/* error if we can't get minimum vectors */
 			pci_disable_msix(pf->pdev);
 			err = -ERANGE;
 			goto msix_err;
 		} else {
-			pf->num_lan_msix = ICE_MIN_LAN_VECS;
+			pf->num_lan_msix = ICE_MIN_LAN_TXRX_MSIX;
 		}
 	}
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Fix MSI-X vector fallback logic
  2021-01-21 17:07   ` Venkataramanan, Anirudh
@ 2021-01-21 17:04     ` Maciej Fijalkowski
  2021-01-21 17:27       ` Venkataramanan, Anirudh
  0 siblings, 1 reply; 5+ messages in thread
From: Maciej Fijalkowski @ 2021-01-21 17:04 UTC (permalink / raw)
  To: intel-wired-lan

On Thu, Jan 21, 2021 at 05:07:21PM +0000, Venkataramanan, Anirudh wrote:
> On Thu, 2021-01-21 at 08:17 +0100, Paul Menzel wrote:
> > Dear Brett,
> > 
> > 
> > Am 21.01.21 um 08:52 schrieb Brett Creeley:
> > > The current MSI-X enablement logic tries to enable best-case MSI-X
> > > vectors and if that fails we only support a bare-minimum set. This
> > > includes a single MSI-X for 1 Tx and 1 Rx queue and a single MSI-X
> > > for the OICR interrupt. Unfortunately, the driver fails to load
> > > when we
> > > don't get as many MSI-X as requested for a couple reasons.
> > > 
> > > First, the code to allocate MSI-X in the driver tries to allocate
> > > num_online_cpus() MSI-X for LAN traffic without caring about the
> > > number
> > > of MSI-X actually enabled/requested from the kernel for LAN
> > > traffic.
> > > So, when calling ice_get_res() for the PF VSI, it returns failure
> > > because the number of available vectors is less than requested. Fix
> > > this by not allowing the PF VSI to allocation  more than
> > > pf->num_lan_msix MSI-X vectors and pf->num_lan_msix Rx/Tx queues.
> > > Limiting the number of queues is done because we don't want more
> > > than
> > > 1 Tx/Rx queue per interrupt due to performance conerns.
> > > 
> > > Second, the driver assigns pf->num_lan_msix = 2, to account for LAN
> > > traffic and the OICR. However, pf->num_lan_msix is only meant for
> > > LAN
> > > MSI-X. This is causing a failure when the PF VSI tries to
> > > allocate/reserve the minimum pf->num_lan_msix because the OICR MSI-
> > > X has
> > > already been reserved, so there may not be enough MSI-X vectors
> > > left.
> > > Fix this by setting pf->num_lan_msix = 1 for the failure case. Then
> > > the
> > > ICE_MIN_MSIX accounts for the LAN MSI-X and the OICR MSI-X needed
> > > for
> > > the failure case.
> > > 
> > > Update the related defines used in ice_ena_msix_range() to align
> > > with
> > > the above behavior and remove the unused RDMA defines because RDMA
> > > is
> > > currently not supported. Also, remove the now incorrect comment.
> > > 
> > > Also, prevent users from enabling more combined queues than there
> > > are
> > > MSI-X available via ethtool.
> > > 
> > > Fixes: Commit 152b978a1f90 ("ice: Rework ice_ena_msix_range")
> > > Fixes: Commit 87324e747fde ("ice: Implement ethtool ops for
> > > channels")
> > 
> > The word *Commit* does not need to be put in there.
> 
> Will do.
> 
> > 
> > Could you split the ethtool change into a separate commit?
> 
> Will do.
> 
> > 
> > Also, can you document your test setup so the driver failed to load?
> 
> The issue (fixed by this patch) is triggered when the driver doesn't
> get adequate MSI-X vectors. We made a local change to the driver to
> simulate this situation and verified that with this patch, the driver
> comes up with a single queue.

What's diff from the version that was up on netdev?

> 
> > 
> > 
> > Kind regards,
> > 
> > Paul
> > 
> > 
> > > Signed-off-by: Brett Creeley <brett.creeley@intel.com>
> > > ---
> > >   drivers/net/ethernet/intel/ice/ice.h         |  4 +++-
> > >   drivers/net/ethernet/intel/ice/ice_ethtool.c |  8 ++++----
> > >   drivers/net/ethernet/intel/ice/ice_lib.c     | 14 +++++++++-----
> > >   drivers/net/ethernet/intel/ice/ice_main.c    |  8 ++------
> > >   4 files changed, 18 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/intel/ice/ice.h
> > > b/drivers/net/ethernet/intel/ice/ice.h
> > > index 6efafe7d8a62..619d93f8b54c 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice.h
> > > +++ b/drivers/net/ethernet/intel/ice/ice.h
> > > @@ -68,7 +68,9 @@
> > >   #define ICE_INT_NAME_STR_LEN	(IFNAMSIZ + 16)
> > >   #define ICE_AQ_LEN		64
> > >   #define ICE_MBXSQ_LEN		64
> > > -#define ICE_MIN_MSIX		2
> > > +#define ICE_MIN_LAN_TXRX_MSIX	1
> > > +#define ICE_MIN_LAN_OICR_MSIX	1
> > > +#define ICE_MIN_MSIX		(ICE_MIN_LAN_TXRX_MSIX +
> > > ICE_MIN_LAN_OICR_MSIX)
> > >   #define ICE_FDIR_MSIX		1
> > >   #define ICE_NO_VSI		0xffff
> > >   #define ICE_VSI_MAP_CONTIG	0
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > > b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > > index 41427302332c..aebebd2102da 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > > @@ -3265,8 +3265,8 @@ ice_set_rxfh(struct net_device *netdev, const
> > > u32 *indir, const u8 *key,
> > >    */
> > >   static int ice_get_max_txq(struct ice_pf *pf)
> > >   {
> > > -	return min_t(int, num_online_cpus(),
> > > -		     pf->hw.func_caps.common_cap.num_txq);
> > > +	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
> > > +		    (u16)pf->hw.func_caps.common_cap.num_txq);
> > >   }
> > >   
> > >   /**
> > > @@ -3275,8 +3275,8 @@ static int ice_get_max_txq(struct ice_pf *pf)
> > >    */
> > >   static int ice_get_max_rxq(struct ice_pf *pf)
> > >   {
> > > -	return min_t(int, num_online_cpus(),
> > > -		     pf->hw.func_caps.common_cap.num_rxq);
> > > +	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
> > > +		    (u16)pf->hw.func_caps.common_cap.num_rxq);
> > >   }
> > >   
> > >   /**
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> > > b/drivers/net/ethernet/intel/ice/ice_lib.c
> > > index 3df67486d42d..ad9c22a1b97a 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> > > @@ -161,8 +161,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi
> > > *vsi, u16 vf_id)
> > >   
> > >   	switch (vsi->type) {
> > >   	case ICE_VSI_PF:
> > > -		vsi->alloc_txq = min_t(int,
> > > ice_get_avail_txq_count(pf),
> > > -				       num_online_cpus());
> > > +		vsi->alloc_txq = min3(pf->num_lan_msix,
> > > +				      ice_get_avail_txq_count(pf),
> > > +				      (u16)num_online_cpus());
> > >   		if (vsi->req_txq) {
> > >   			vsi->alloc_txq = vsi->req_txq;
> > >   			vsi->num_txq = vsi->req_txq;
> > > @@ -174,8 +175,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi
> > > *vsi, u16 vf_id)
> > >   		if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) {
> > >   			vsi->alloc_rxq = 1;
> > >   		} else {
> > > -			vsi->alloc_rxq = min_t(int,
> > > ice_get_avail_rxq_count(pf),
> > > -					       num_online_cpus());
> > > +			vsi->alloc_rxq = min3(pf->num_lan_msix,
> > > +					      ice_get_avail_rxq_count(p
> > > f),
> > > +					      (u16)num_online_cpus());
> > >   			if (vsi->req_rxq) {
> > >   				vsi->alloc_rxq = vsi->req_rxq;
> > >   				vsi->num_rxq = vsi->req_rxq;
> > > @@ -184,7 +186,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi
> > > *vsi, u16 vf_id)
> > >   
> > >   		pf->num_lan_rx = vsi->alloc_rxq;
> > >   
> > > -		vsi->num_q_vectors = max_t(int, vsi->alloc_rxq, vsi-
> > > >alloc_txq);
> > > +		vsi->num_q_vectors = min_t(int, pf->num_lan_msix,
> > > +					   max_t(int, vsi->alloc_rxq,
> > > +						 vsi->alloc_txq));
> > >   		break;
> > >   	case ICE_VSI_VF:
> > >   		vf = &pf->vf[vsi->vf_id];
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> > > b/drivers/net/ethernet/intel/ice/ice_main.c
> > > index fb81aa5979e3..e10ca8929f85 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > > @@ -3430,18 +3430,14 @@ static int ice_ena_msix_range(struct ice_pf
> > > *pf)
> > >   	if (v_actual < v_budget) {
> > >   		dev_warn(dev, "not enough OS MSI-X vectors. requested =
> > > %d, obtained = %d\n",
> > >   			 v_budget, v_actual);
> > > -/* 2 vectors each for LAN and RDMA (traffic + OICR), one for flow
> > > director */
> > > -#define ICE_MIN_LAN_VECS 2
> > > -#define ICE_MIN_RDMA_VECS 2
> > > -#define ICE_MIN_VECS (ICE_MIN_LAN_VECS + ICE_MIN_RDMA_VECS + 1)
> > >   
> > > -		if (v_actual < ICE_MIN_LAN_VECS) {
> > > +		if (v_actual < ICE_MIN_MSIX) {
> > >   			/* error if we can't get minimum vectors */
> > >   			pci_disable_msix(pf->pdev);
> > >   			err = -ERANGE;
> > >   			goto msix_err;
> > >   		} else {
> > > -			pf->num_lan_msix = ICE_MIN_LAN_VECS;
> > > +			pf->num_lan_msix = ICE_MIN_LAN_TXRX_MSIX;
> > >   		}
> > >   	}
> > >   
> > > 
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan at osuosl.org
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Fix MSI-X vector fallback logic
  2021-01-21  7:17 ` Paul Menzel
@ 2021-01-21 17:07   ` Venkataramanan, Anirudh
  2021-01-21 17:04     ` Maciej Fijalkowski
  0 siblings, 1 reply; 5+ messages in thread
From: Venkataramanan, Anirudh @ 2021-01-21 17:07 UTC (permalink / raw)
  To: intel-wired-lan

On Thu, 2021-01-21 at 08:17 +0100, Paul Menzel wrote:
> Dear Brett,
> 
> 
> Am 21.01.21 um 08:52 schrieb Brett Creeley:
> > The current MSI-X enablement logic tries to enable best-case MSI-X
> > vectors and if that fails we only support a bare-minimum set. This
> > includes a single MSI-X for 1 Tx and 1 Rx queue and a single MSI-X
> > for the OICR interrupt. Unfortunately, the driver fails to load
> > when we
> > don't get as many MSI-X as requested for a couple reasons.
> > 
> > First, the code to allocate MSI-X in the driver tries to allocate
> > num_online_cpus() MSI-X for LAN traffic without caring about the
> > number
> > of MSI-X actually enabled/requested from the kernel for LAN
> > traffic.
> > So, when calling ice_get_res() for the PF VSI, it returns failure
> > because the number of available vectors is less than requested. Fix
> > this by not allowing the PF VSI to allocation  more than
> > pf->num_lan_msix MSI-X vectors and pf->num_lan_msix Rx/Tx queues.
> > Limiting the number of queues is done because we don't want more
> > than
> > 1 Tx/Rx queue per interrupt due to performance conerns.
> > 
> > Second, the driver assigns pf->num_lan_msix = 2, to account for LAN
> > traffic and the OICR. However, pf->num_lan_msix is only meant for
> > LAN
> > MSI-X. This is causing a failure when the PF VSI tries to
> > allocate/reserve the minimum pf->num_lan_msix because the OICR MSI-
> > X has
> > already been reserved, so there may not be enough MSI-X vectors
> > left.
> > Fix this by setting pf->num_lan_msix = 1 for the failure case. Then
> > the
> > ICE_MIN_MSIX accounts for the LAN MSI-X and the OICR MSI-X needed
> > for
> > the failure case.
> > 
> > Update the related defines used in ice_ena_msix_range() to align
> > with
> > the above behavior and remove the unused RDMA defines because RDMA
> > is
> > currently not supported. Also, remove the now incorrect comment.
> > 
> > Also, prevent users from enabling more combined queues than there
> > are
> > MSI-X available via ethtool.
> > 
> > Fixes: Commit 152b978a1f90 ("ice: Rework ice_ena_msix_range")
> > Fixes: Commit 87324e747fde ("ice: Implement ethtool ops for
> > channels")
> 
> The word *Commit* does not need to be put in there.

Will do.

> 
> Could you split the ethtool change into a separate commit?

Will do.

> 
> Also, can you document your test setup so the driver failed to load?

The issue (fixed by this patch) is triggered when the driver doesn't
get adequate MSI-X vectors. We made a local change to the driver to
simulate this situation and verified that with this patch, the driver
comes up with a single queue.

> 
> 
> Kind regards,
> 
> Paul
> 
> 
> > Signed-off-by: Brett Creeley <brett.creeley@intel.com>
> > ---
> >   drivers/net/ethernet/intel/ice/ice.h         |  4 +++-
> >   drivers/net/ethernet/intel/ice/ice_ethtool.c |  8 ++++----
> >   drivers/net/ethernet/intel/ice/ice_lib.c     | 14 +++++++++-----
> >   drivers/net/ethernet/intel/ice/ice_main.c    |  8 ++------
> >   4 files changed, 18 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice.h
> > b/drivers/net/ethernet/intel/ice/ice.h
> > index 6efafe7d8a62..619d93f8b54c 100644
> > --- a/drivers/net/ethernet/intel/ice/ice.h
> > +++ b/drivers/net/ethernet/intel/ice/ice.h
> > @@ -68,7 +68,9 @@
> >   #define ICE_INT_NAME_STR_LEN	(IFNAMSIZ + 16)
> >   #define ICE_AQ_LEN		64
> >   #define ICE_MBXSQ_LEN		64
> > -#define ICE_MIN_MSIX		2
> > +#define ICE_MIN_LAN_TXRX_MSIX	1
> > +#define ICE_MIN_LAN_OICR_MSIX	1
> > +#define ICE_MIN_MSIX		(ICE_MIN_LAN_TXRX_MSIX +
> > ICE_MIN_LAN_OICR_MSIX)
> >   #define ICE_FDIR_MSIX		1
> >   #define ICE_NO_VSI		0xffff
> >   #define ICE_VSI_MAP_CONTIG	0
> > diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > index 41427302332c..aebebd2102da 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > @@ -3265,8 +3265,8 @@ ice_set_rxfh(struct net_device *netdev, const
> > u32 *indir, const u8 *key,
> >    */
> >   static int ice_get_max_txq(struct ice_pf *pf)
> >   {
> > -	return min_t(int, num_online_cpus(),
> > -		     pf->hw.func_caps.common_cap.num_txq);
> > +	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
> > +		    (u16)pf->hw.func_caps.common_cap.num_txq);
> >   }
> >   
> >   /**
> > @@ -3275,8 +3275,8 @@ static int ice_get_max_txq(struct ice_pf *pf)
> >    */
> >   static int ice_get_max_rxq(struct ice_pf *pf)
> >   {
> > -	return min_t(int, num_online_cpus(),
> > -		     pf->hw.func_caps.common_cap.num_rxq);
> > +	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
> > +		    (u16)pf->hw.func_caps.common_cap.num_rxq);
> >   }
> >   
> >   /**
> > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> > b/drivers/net/ethernet/intel/ice/ice_lib.c
> > index 3df67486d42d..ad9c22a1b97a 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> > @@ -161,8 +161,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi
> > *vsi, u16 vf_id)
> >   
> >   	switch (vsi->type) {
> >   	case ICE_VSI_PF:
> > -		vsi->alloc_txq = min_t(int,
> > ice_get_avail_txq_count(pf),
> > -				       num_online_cpus());
> > +		vsi->alloc_txq = min3(pf->num_lan_msix,
> > +				      ice_get_avail_txq_count(pf),
> > +				      (u16)num_online_cpus());
> >   		if (vsi->req_txq) {
> >   			vsi->alloc_txq = vsi->req_txq;
> >   			vsi->num_txq = vsi->req_txq;
> > @@ -174,8 +175,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi
> > *vsi, u16 vf_id)
> >   		if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) {
> >   			vsi->alloc_rxq = 1;
> >   		} else {
> > -			vsi->alloc_rxq = min_t(int,
> > ice_get_avail_rxq_count(pf),
> > -					       num_online_cpus());
> > +			vsi->alloc_rxq = min3(pf->num_lan_msix,
> > +					      ice_get_avail_rxq_count(p
> > f),
> > +					      (u16)num_online_cpus());
> >   			if (vsi->req_rxq) {
> >   				vsi->alloc_rxq = vsi->req_rxq;
> >   				vsi->num_rxq = vsi->req_rxq;
> > @@ -184,7 +186,9 @@ static void ice_vsi_set_num_qs(struct ice_vsi
> > *vsi, u16 vf_id)
> >   
> >   		pf->num_lan_rx = vsi->alloc_rxq;
> >   
> > -		vsi->num_q_vectors = max_t(int, vsi->alloc_rxq, vsi-
> > >alloc_txq);
> > +		vsi->num_q_vectors = min_t(int, pf->num_lan_msix,
> > +					   max_t(int, vsi->alloc_rxq,
> > +						 vsi->alloc_txq));
> >   		break;
> >   	case ICE_VSI_VF:
> >   		vf = &pf->vf[vsi->vf_id];
> > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> > b/drivers/net/ethernet/intel/ice/ice_main.c
> > index fb81aa5979e3..e10ca8929f85 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > @@ -3430,18 +3430,14 @@ static int ice_ena_msix_range(struct ice_pf
> > *pf)
> >   	if (v_actual < v_budget) {
> >   		dev_warn(dev, "not enough OS MSI-X vectors. requested =
> > %d, obtained = %d\n",
> >   			 v_budget, v_actual);
> > -/* 2 vectors each for LAN and RDMA (traffic + OICR), one for flow
> > director */
> > -#define ICE_MIN_LAN_VECS 2
> > -#define ICE_MIN_RDMA_VECS 2
> > -#define ICE_MIN_VECS (ICE_MIN_LAN_VECS + ICE_MIN_RDMA_VECS + 1)
> >   
> > -		if (v_actual < ICE_MIN_LAN_VECS) {
> > +		if (v_actual < ICE_MIN_MSIX) {
> >   			/* error if we can't get minimum vectors */
> >   			pci_disable_msix(pf->pdev);
> >   			err = -ERANGE;
> >   			goto msix_err;
> >   		} else {
> > -			pf->num_lan_msix = ICE_MIN_LAN_VECS;
> > +			pf->num_lan_msix = ICE_MIN_LAN_TXRX_MSIX;
> >   		}
> >   	}
> >   
> > 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Fix MSI-X vector fallback logic
  2021-01-21 17:04     ` Maciej Fijalkowski
@ 2021-01-21 17:27       ` Venkataramanan, Anirudh
  0 siblings, 0 replies; 5+ messages in thread
From: Venkataramanan, Anirudh @ 2021-01-21 17:27 UTC (permalink / raw)
  To: intel-wired-lan

On Thu, 2021-01-21 at 18:04 +0100, Maciej Fijalkowski wrote:
> On Thu, Jan 21, 2021 at 05:07:21PM +0000, Venkataramanan, Anirudh
> wrote:
> > On Thu, 2021-01-21 at 08:17 +0100, Paul Menzel wrote:
> > > Dear Brett,
> > > 
> > > 
> > > Am 21.01.21 um 08:52 schrieb Brett Creeley:
> > > > The current MSI-X enablement logic tries to enable best-case
> > > > MSI-X
> > > > vectors and if that fails we only support a bare-minimum set.
> > > > This
> > > > includes a single MSI-X for 1 Tx and 1 Rx queue and a single
> > > > MSI-X
> > > > for the OICR interrupt. Unfortunately, the driver fails to load
> > > > when we
> > > > don't get as many MSI-X as requested for a couple reasons.
> > > > 
> > > > First, the code to allocate MSI-X in the driver tries to
> > > > allocate
> > > > num_online_cpus() MSI-X for LAN traffic without caring about
> > > > the
> > > > number
> > > > of MSI-X actually enabled/requested from the kernel for LAN
> > > > traffic.
> > > > So, when calling ice_get_res() for the PF VSI, it returns
> > > > failure
> > > > because the number of available vectors is less than requested.
> > > > Fix
> > > > this by not allowing the PF VSI to allocation  more than
> > > > pf->num_lan_msix MSI-X vectors and pf->num_lan_msix Rx/Tx
> > > > queues.
> > > > Limiting the number of queues is done because we don't want
> > > > more
> > > > than
> > > > 1 Tx/Rx queue per interrupt due to performance conerns.
> > > > 
> > > > Second, the driver assigns pf->num_lan_msix = 2, to account for
> > > > LAN
> > > > traffic and the OICR. However, pf->num_lan_msix is only meant
> > > > for
> > > > LAN
> > > > MSI-X. This is causing a failure when the PF VSI tries to
> > > > allocate/reserve the minimum pf->num_lan_msix because the OICR
> > > > MSI-
> > > > X has
> > > > already been reserved, so there may not be enough MSI-X vectors
> > > > left.
> > > > Fix this by setting pf->num_lan_msix = 1 for the failure case.
> > > > Then
> > > > the
> > > > ICE_MIN_MSIX accounts for the LAN MSI-X and the OICR MSI-X
> > > > needed
> > > > for
> > > > the failure case.
> > > > 
> > > > Update the related defines used in ice_ena_msix_range() to
> > > > align
> > > > with
> > > > the above behavior and remove the unused RDMA defines because
> > > > RDMA
> > > > is
> > > > currently not supported. Also, remove the now incorrect
> > > > comment.
> > > > 
> > > > Also, prevent users from enabling more combined queues than
> > > > there
> > > > are
> > > > MSI-X available via ethtool.
> > > > 
> > > > Fixes: Commit 152b978a1f90 ("ice: Rework ice_ena_msix_range")
> > > > Fixes: Commit 87324e747fde ("ice: Implement ethtool ops for
> > > > channels")
> > > 
> > > The word *Commit* does not need to be put in there.
> > 
> > Will do.
> > 
> > > Could you split the ethtool change into a separate commit?
> > 
> > Will do.
> > 
> > > Also, can you document your test setup so the driver failed to
> > > load?
> > 
> > The issue (fixed by this patch) is triggered when the driver
> > doesn't
> > get adequate MSI-X vectors. We made a local change to the driver to
> > simulate this situation and verified that with this patch, the
> > driver
> > comes up with a single queue.
> 
> What's diff from the version that was up on netdev?

The one on netdev was an enhancement to the fallback logic. There was
feedback on this patch from Jakub so we are going to rework this.

The current patch is just a fix to the existing fallback logic that's
buggy.

> 
> > > 
> > > Kind regards,
> > > 
> > > Paul
> > > 
> > > 
> > > > Signed-off-by: Brett Creeley <brett.creeley@intel.com>
> > > > ---
> > > >   drivers/net/ethernet/intel/ice/ice.h         |  4 +++-
> > > >   drivers/net/ethernet/intel/ice/ice_ethtool.c |  8 ++++----
> > > >   drivers/net/ethernet/intel/ice/ice_lib.c     | 14 +++++++++
> > > > -----
> > > >   drivers/net/ethernet/intel/ice/ice_main.c    |  8 ++------
> > > >   4 files changed, 18 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/ethernet/intel/ice/ice.h
> > > > b/drivers/net/ethernet/intel/ice/ice.h
> > > > index 6efafe7d8a62..619d93f8b54c 100644
> > > > --- a/drivers/net/ethernet/intel/ice/ice.h
> > > > +++ b/drivers/net/ethernet/intel/ice/ice.h
> > > > @@ -68,7 +68,9 @@
> > > >   #define ICE_INT_NAME_STR_LEN	(IFNAMSIZ + 16)
> > > >   #define ICE_AQ_LEN		64
> > > >   #define ICE_MBXSQ_LEN		64
> > > > -#define ICE_MIN_MSIX		2
> > > > +#define ICE_MIN_LAN_TXRX_MSIX	1
> > > > +#define ICE_MIN_LAN_OICR_MSIX	1
> > > > +#define ICE_MIN_MSIX		(ICE_MIN_LAN_TXRX_MSIX +
> > > > ICE_MIN_LAN_OICR_MSIX)
> > > >   #define ICE_FDIR_MSIX		1
> > > >   #define ICE_NO_VSI		0xffff
> > > >   #define ICE_VSI_MAP_CONTIG	0
> > > > diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > > > b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > > > index 41427302332c..aebebd2102da 100644
> > > > --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > > > +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > > > @@ -3265,8 +3265,8 @@ ice_set_rxfh(struct net_device *netdev,
> > > > const
> > > > u32 *indir, const u8 *key,
> > > >    */
> > > >   static int ice_get_max_txq(struct ice_pf *pf)
> > > >   {
> > > > -	return min_t(int, num_online_cpus(),
> > > > -		     pf->hw.func_caps.common_cap.num_txq);
> > > > +	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
> > > > +		    (u16)pf->hw.func_caps.common_cap.num_txq);
> > > >   }
> > > >   
> > > >   /**
> > > > @@ -3275,8 +3275,8 @@ static int ice_get_max_txq(struct ice_pf
> > > > *pf)
> > > >    */
> > > >   static int ice_get_max_rxq(struct ice_pf *pf)
> > > >   {
> > > > -	return min_t(int, num_online_cpus(),
> > > > -		     pf->hw.func_caps.common_cap.num_rxq);
> > > > +	return min3(pf->num_lan_msix, (u16)num_online_cpus(),
> > > > +		    (u16)pf->hw.func_caps.common_cap.num_rxq);
> > > >   }
> > > >   
> > > >   /**
> > > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> > > > b/drivers/net/ethernet/intel/ice/ice_lib.c
> > > > index 3df67486d42d..ad9c22a1b97a 100644
> > > > --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> > > > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> > > > @@ -161,8 +161,9 @@ static void ice_vsi_set_num_qs(struct
> > > > ice_vsi
> > > > *vsi, u16 vf_id)
> > > >   
> > > >   	switch (vsi->type) {
> > > >   	case ICE_VSI_PF:
> > > > -		vsi->alloc_txq = min_t(int,
> > > > ice_get_avail_txq_count(pf),
> > > > -				       num_online_cpus());
> > > > +		vsi->alloc_txq = min3(pf->num_lan_msix,
> > > > +				      ice_get_avail_txq_count(p
> > > > f),
> > > > +				      (u16)num_online_cpus());
> > > >   		if (vsi->req_txq) {
> > > >   			vsi->alloc_txq = vsi->req_txq;
> > > >   			vsi->num_txq = vsi->req_txq;
> > > > @@ -174,8 +175,9 @@ static void ice_vsi_set_num_qs(struct
> > > > ice_vsi
> > > > *vsi, u16 vf_id)
> > > >   		if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) {
> > > >   			vsi->alloc_rxq = 1;
> > > >   		} else {
> > > > -			vsi->alloc_rxq = min_t(int,
> > > > ice_get_avail_rxq_count(pf),
> > > > -					       num_online_cpus(
> > > > ));
> > > > +			vsi->alloc_rxq = min3(pf->num_lan_msix,
> > > > +					      ice_get_avail_rxq
> > > > _count(p
> > > > f),
> > > > +					      (u16)num_online_c
> > > > pus());
> > > >   			if (vsi->req_rxq) {
> > > >   				vsi->alloc_rxq = vsi->req_rxq;
> > > >   				vsi->num_rxq = vsi->req_rxq;
> > > > @@ -184,7 +186,9 @@ static void ice_vsi_set_num_qs(struct
> > > > ice_vsi
> > > > *vsi, u16 vf_id)
> > > >   
> > > >   		pf->num_lan_rx = vsi->alloc_rxq;
> > > >   
> > > > -		vsi->num_q_vectors = max_t(int, vsi->alloc_rxq, 
> > > > vsi-
> > > > > alloc_txq);
> > > > +		vsi->num_q_vectors = min_t(int, pf-
> > > > >num_lan_msix,
> > > > +					   max_t(int, vsi-
> > > > >alloc_rxq,
> > > > +						 vsi-
> > > > >alloc_txq));
> > > >   		break;
> > > >   	case ICE_VSI_VF:
> > > >   		vf = &pf->vf[vsi->vf_id];
> > > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> > > > b/drivers/net/ethernet/intel/ice/ice_main.c
> > > > index fb81aa5979e3..e10ca8929f85 100644
> > > > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > > > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > > > @@ -3430,18 +3430,14 @@ static int ice_ena_msix_range(struct
> > > > ice_pf
> > > > *pf)
> > > >   	if (v_actual < v_budget) {
> > > >   		dev_warn(dev, "not enough OS MSI-X vectors.
> > > > requested =
> > > > %d, obtained = %d\n",
> > > >   			 v_budget, v_actual);
> > > > -/* 2 vectors each for LAN and RDMA (traffic + OICR), one for
> > > > flow
> > > > director */
> > > > -#define ICE_MIN_LAN_VECS 2
> > > > -#define ICE_MIN_RDMA_VECS 2
> > > > -#define ICE_MIN_VECS (ICE_MIN_LAN_VECS + ICE_MIN_RDMA_VECS +
> > > > 1)
> > > >   
> > > > -		if (v_actual < ICE_MIN_LAN_VECS) {
> > > > +		if (v_actual < ICE_MIN_MSIX) {
> > > >   			/* error if we can't get minimum
> > > > vectors */
> > > >   			pci_disable_msix(pf->pdev);
> > > >   			err = -ERANGE;
> > > >   			goto msix_err;
> > > >   		} else {
> > > > -			pf->num_lan_msix = ICE_MIN_LAN_VECS;
> > > > +			pf->num_lan_msix =
> > > > ICE_MIN_LAN_TXRX_MSIX;
> > > >   		}
> > > >   	}
> > > >   
> > > > 
> > > _______________________________________________
> > > Intel-wired-lan mailing list
> > > Intel-wired-lan at osuosl.org
> > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan at osuosl.org
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-21 17:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21  7:52 [Intel-wired-lan] [PATCH net] ice: Fix MSI-X vector fallback logic Brett Creeley
2021-01-21  7:17 ` Paul Menzel
2021-01-21 17:07   ` Venkataramanan, Anirudh
2021-01-21 17:04     ` Maciej Fijalkowski
2021-01-21 17:27       ` Venkataramanan, Anirudh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.