All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc
@ 2023-01-11 14:20 Kamil Maziarz
  2023-01-12  7:11 ` Michal Swiatkowski
  2023-01-18 18:01 ` Tony Nguyen
  0 siblings, 2 replies; 6+ messages in thread
From: Kamil Maziarz @ 2023-01-11 14:20 UTC (permalink / raw)
  To: intel-wired-lan; +Cc: Kamil Maziarz, Rafal Rogalski

From: Rafal Rogalski <rafalx.rogalski@intel.com>

Commit f70b9d5f4426 ("ice: check for a leaf node presence") prevents
removal of VSI with leaf nodes. This is an expectation of driver action
induced by FW requirements. However, this caused RDMA scheduler config
removal to fail every time a qdisc was added or deleted.

Fix this by ignoring errors from RDMA configuration removal when qdisc are
being reconfigured.

Fixes: ff7e93219442 ("ice: Fix failure to re-add LAN/RDMA Tx queues")
Signed-off-by: Rafal Rogalski <rafalx.rogalski@intel.com>
Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com>
---
 drivers/net/ethernet/intel/ice/ice.h      | 1 +
 drivers/net/ethernet/intel/ice/ice_lib.c  | 2 +-
 drivers/net/ethernet/intel/ice/ice_main.c | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 2f0b604abc5e..b572d07bc126 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -306,6 +306,7 @@ enum ice_pf_state {
 	ICE_PHY_INIT_COMPLETE,
 	ICE_FD_VF_FLUSH_CTX,		/* set at FD Rx IRQ or timeout */
 	ICE_AUX_ERR_PENDING,
+	ICE_SETUP_TC_QDISC,
 	ICE_STATE_NBITS		/* must be last */
 };
 
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 22bcb414546a..0ee3acbea108 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3448,7 +3448,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)
 
 	ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx);
 	ret = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx);
-	if (ret)
+	if (ret && !(test_bit(ICE_SETUP_TC_QDISC, pf->state) && ret == -EBUSY))
 		dev_err(ice_pf_to_dev(vsi->back), "Failed to remove RDMA scheduler config for VSI %u, err %d\n",
 			vsi->vsi_num, ret);
 	ice_vsi_free_q_vectors(vsi);
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index a9a7f8b52140..5ff137645f08 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -8706,6 +8706,7 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
 	cur_txq = vsi->num_txq;
 	cur_rxq = vsi->num_rxq;
 
+	set_bit(ICE_SETUP_TC_QDISC, pf->state);
 	/* proceed with rebuild main VSI using correct number of queues */
 	ret = ice_vsi_rebuild(vsi, false);
 	if (ret) {
@@ -8716,9 +8717,11 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
 		clear_bit(ICE_RESET_FAILED, pf->state);
 		if (ice_vsi_rebuild(vsi, false)) {
 			dev_err(dev, "Rebuild of main VSI failed again\n");
+			clear_bit(ICE_SETUP_TC_QDISC, pf->state);
 			return ret;
 		}
 	}
+	clear_bit(ICE_SETUP_TC_QDISC, pf->state);
 
 	vsi->all_numtc = num_tcf;
 	vsi->all_enatc = ena_tc_qdisc;
-- 
2.31.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc
  2023-01-11 14:20 [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc Kamil Maziarz
@ 2023-01-12  7:11 ` Michal Swiatkowski
  2023-01-12 13:54   ` Rogalski, RafalX
  2023-01-18 18:01 ` Tony Nguyen
  1 sibling, 1 reply; 6+ messages in thread
From: Michal Swiatkowski @ 2023-01-12  7:11 UTC (permalink / raw)
  To: Kamil Maziarz; +Cc: Rafal Rogalski, intel-wired-lan

On Wed, Jan 11, 2023 at 03:20:29PM +0100, Kamil Maziarz wrote:
> From: Rafal Rogalski <rafalx.rogalski@intel.com>
> 
> Commit f70b9d5f4426 ("ice: check for a leaf node presence") prevents
> removal of VSI with leaf nodes. This is an expectation of driver action
> induced by FW requirements. However, this caused RDMA scheduler config
> removal to fail every time a qdisc was added or deleted.
> 
> Fix this by ignoring errors from RDMA configuration removal when qdisc are
> being reconfigured.
> 
> Fixes: ff7e93219442 ("ice: Fix failure to re-add LAN/RDMA Tx queues")
> Signed-off-by: Rafal Rogalski <rafalx.rogalski@intel.com>
> Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com>
> ---
Thanks for changes. It is a good practise to include changelog here.
Something like:
v1 --> v2:
 - check for -EBUSY etc.
 ---

 Is Your patch rebased on dev-queue? I am pretty sure that
 ice_vsi_rebuild looks different on current dev-queue, please check
 that.

>  drivers/net/ethernet/intel/ice/ice.h      | 1 +
>  drivers/net/ethernet/intel/ice/ice_lib.c  | 2 +-
>  drivers/net/ethernet/intel/ice/ice_main.c | 3 +++
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 2f0b604abc5e..b572d07bc126 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -306,6 +306,7 @@ enum ice_pf_state {
>  	ICE_PHY_INIT_COMPLETE,
>  	ICE_FD_VF_FLUSH_CTX,		/* set at FD Rx IRQ or timeout */
>  	ICE_AUX_ERR_PENDING,
> +	ICE_SETUP_TC_QDISC,
>  	ICE_STATE_NBITS		/* must be last */
>  };
>  
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 22bcb414546a..0ee3acbea108 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -3448,7 +3448,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)
>  
>  	ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx);
>  	ret = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx);
> -	if (ret)
> +	if (ret && !(test_bit(ICE_SETUP_TC_QDISC, pf->state) && ret == -EBUSY))
It is fine for me in current state, but do we really need to do this
check only in setting TC qdisc? Doesn't the same thing happen when the
number of queues is changed from ethtool? (maybe it is not because the
changes is blocked when RDMA is active). Maybe whole
ice_setup_tc_mqprio_qdisc() should return -EBOUSY or inval if RDMA is
active? Is it valid to a new qdisc when RDMA is active?

>  		dev_err(ice_pf_to_dev(vsi->back), "Failed to remove RDMA scheduler config for VSI %u, err %d\n",
>  			vsi->vsi_num, ret);
>  	ice_vsi_free_q_vectors(vsi);
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index a9a7f8b52140..5ff137645f08 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -8706,6 +8706,7 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
>  	cur_txq = vsi->num_txq;
>  	cur_rxq = vsi->num_rxq;
>  
> +	set_bit(ICE_SETUP_TC_QDISC, pf->state);
>  	/* proceed with rebuild main VSI using correct number of queues */
>  	ret = ice_vsi_rebuild(vsi, false);
>  	if (ret) {
> @@ -8716,9 +8717,11 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
>  		clear_bit(ICE_RESET_FAILED, pf->state);
>  		if (ice_vsi_rebuild(vsi, false)) {
>  			dev_err(dev, "Rebuild of main VSI failed again\n");
> +			clear_bit(ICE_SETUP_TC_QDISC, pf->state);
>  			return ret;
>  		}
>  	}
> +	clear_bit(ICE_SETUP_TC_QDISC, pf->state);
>  
>  	vsi->all_numtc = num_tcf;
>  	vsi->all_enatc = ena_tc_qdisc;
> -- 
> 2.31.1
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc
  2023-01-12  7:11 ` Michal Swiatkowski
@ 2023-01-12 13:54   ` Rogalski, RafalX
  2023-01-13  6:30     ` Michal Swiatkowski
  0 siblings, 1 reply; 6+ messages in thread
From: Rogalski, RafalX @ 2023-01-12 13:54 UTC (permalink / raw)
  To: Michal Swiatkowski, Maziarz, Kamil; +Cc: intel-wired-lan

>-----Original Message-----
>From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> 
>Sent: Thursday, January 12, 2023 8:12 AM
>To: Maziarz, Kamil <kamil.maziarz@intel.com>
>Cc: intel-wired-lan@lists.osuosl.org; Rogalski, RafalX <rafalx.rogalski@intel.com>
>Subject: Re: [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc
>
>On Wed, Jan 11, 2023 at 03:20:29PM +0100, Kamil Maziarz wrote:
>> From: Rafal Rogalski <rafalx.rogalski@intel.com>
>> 
>> Commit f70b9d5f4426 ("ice: check for a leaf node presence") prevents 
>> removal of VSI with leaf nodes. This is an expectation of driver 
>> action induced by FW requirements. However, this caused RDMA scheduler 
>> config removal to fail every time a qdisc was added or deleted.
>> 
>> Fix this by ignoring errors from RDMA configuration removal when qdisc 
>> are being reconfigured.
>> 
>> Fixes: ff7e93219442 ("ice: Fix failure to re-add LAN/RDMA Tx queues")
>> Signed-off-by: Rafal Rogalski <rafalx.rogalski@intel.com>
>> Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com>
>> ---
>Thanks for changes. It is a good practise to include changelog here.
>Something like:
>v1 --> v2:
> - check for -EBUSY etc.
> ---
>
> Is Your patch rebased on dev-queue? I am pretty sure that  ice_vsi_rebuild looks different on current dev-queue, please check  that.
>
Thank you for vigilance. Yes, my changes are pushed at the top of dev-queue branch to the net-queue repository.

>>  drivers/net/ethernet/intel/ice/ice.h      | 1 +
>>  drivers/net/ethernet/intel/ice/ice_lib.c  | 2 +-  
>> drivers/net/ethernet/intel/ice/ice_main.c | 3 +++
>>  3 files changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/ice/ice.h 
>> b/drivers/net/ethernet/intel/ice/ice.h
>> index 2f0b604abc5e..b572d07bc126 100644
>> --- a/drivers/net/ethernet/intel/ice/ice.h
>> +++ b/drivers/net/ethernet/intel/ice/ice.h
>> @@ -306,6 +306,7 @@ enum ice_pf_state {
>>  	ICE_PHY_INIT_COMPLETE,
>>  	ICE_FD_VF_FLUSH_CTX,		/* set at FD Rx IRQ or timeout */
>>  	ICE_AUX_ERR_PENDING,
>> +	ICE_SETUP_TC_QDISC,
>>  	ICE_STATE_NBITS		/* must be last */
>>  };
>>  
>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c 
>> b/drivers/net/ethernet/intel/ice/ice_lib.c
>> index 22bcb414546a..0ee3acbea108 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
>> @@ -3448,7 +3448,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool 
>> init_vsi)
>>  
>>  	ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx);
>>  	ret = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx);
>> -	if (ret)
>> +	if (ret && !(test_bit(ICE_SETUP_TC_QDISC, pf->state) && ret == 
>> +-EBUSY))
>It is fine for me in current state, but do we really need to do this check only in setting TC qdisc? Doesn't the same thing happen when the number of queues is changed from ethtool? (maybe it is not because the changes is blocked when RDMA is active). Maybe whole
>ice_setup_tc_mqprio_qdisc() should return -EBOUSY or inval if RDMA is active? Is it valid to a new qdisc when RDMA is active?
>
You are right about ethtool. However, I'm not sure if we should ignore -EBOUSY for every other scenario.

>>  		dev_err(ice_pf_to_dev(vsi->back), "Failed to remove RDMA scheduler config for VSI %u, err %d\n",
>>  			vsi->vsi_num, ret);
>>  	ice_vsi_free_q_vectors(vsi);
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c 
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>> index a9a7f8b52140..5ff137645f08 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -8706,6 +8706,7 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
>>  	cur_txq = vsi->num_txq;
>>  	cur_rxq = vsi->num_rxq;
>>  
>> +	set_bit(ICE_SETUP_TC_QDISC, pf->state);
>>  	/* proceed with rebuild main VSI using correct number of queues */
>>  	ret = ice_vsi_rebuild(vsi, false);
>>  	if (ret) {
>> @@ -8716,9 +8717,11 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
>>  		clear_bit(ICE_RESET_FAILED, pf->state);
>>  		if (ice_vsi_rebuild(vsi, false)) {
>>  			dev_err(dev, "Rebuild of main VSI failed again\n");
>> +			clear_bit(ICE_SETUP_TC_QDISC, pf->state);
>>  			return ret;
>>  		}
>>  	}
>> +	clear_bit(ICE_SETUP_TC_QDISC, pf->state);
>>  
>>  	vsi->all_numtc = num_tcf;
>>  	vsi->all_enatc = ena_tc_qdisc;
>> --
>> 2.31.1
>> 
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc
  2023-01-12 13:54   ` Rogalski, RafalX
@ 2023-01-13  6:30     ` Michal Swiatkowski
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Swiatkowski @ 2023-01-13  6:30 UTC (permalink / raw)
  To: Rogalski, RafalX; +Cc: intel-wired-lan, Maziarz, Kamil

On Thu, Jan 12, 2023 at 01:54:00PM +0000, Rogalski, RafalX wrote:
> >-----Original Message-----
> >From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> 
> >Sent: Thursday, January 12, 2023 8:12 AM
> >To: Maziarz, Kamil <kamil.maziarz@intel.com>
> >Cc: intel-wired-lan@lists.osuosl.org; Rogalski, RafalX <rafalx.rogalski@intel.com>
> >Subject: Re: [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc
> >
> >On Wed, Jan 11, 2023 at 03:20:29PM +0100, Kamil Maziarz wrote:
> >> From: Rafal Rogalski <rafalx.rogalski@intel.com>
> >> 
> >> Commit f70b9d5f4426 ("ice: check for a leaf node presence") prevents 
> >> removal of VSI with leaf nodes. This is an expectation of driver 
> >> action induced by FW requirements. However, this caused RDMA scheduler 
> >> config removal to fail every time a qdisc was added or deleted.
> >> 
> >> Fix this by ignoring errors from RDMA configuration removal when qdisc 
> >> are being reconfigured.
> >> 
> >> Fixes: ff7e93219442 ("ice: Fix failure to re-add LAN/RDMA Tx queues")
> >> Signed-off-by: Rafal Rogalski <rafalx.rogalski@intel.com>
> >> Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com>
> >> ---
> >Thanks for changes. It is a good practise to include changelog here.
> >Something like:
> >v1 --> v2:
> > - check for -EBUSY etc.
> > ---
> >
> > Is Your patch rebased on dev-queue? I am pretty sure that  ice_vsi_rebuild looks different on current dev-queue, please check  that.
> >
> Thank you for vigilance. Yes, my changes are pushed at the top of dev-queue branch to the net-queue repository.
> 
Yeah, sorry for confusion, didn't notice that it is a fix to net. Thanks
for clarification.

> >>  drivers/net/ethernet/intel/ice/ice.h      | 1 +
> >>  drivers/net/ethernet/intel/ice/ice_lib.c  | 2 +-  
> >> drivers/net/ethernet/intel/ice/ice_main.c | 3 +++
> >>  3 files changed, 5 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/net/ethernet/intel/ice/ice.h 
> >> b/drivers/net/ethernet/intel/ice/ice.h
> >> index 2f0b604abc5e..b572d07bc126 100644
> >> --- a/drivers/net/ethernet/intel/ice/ice.h
> >> +++ b/drivers/net/ethernet/intel/ice/ice.h
> >> @@ -306,6 +306,7 @@ enum ice_pf_state {
> >>  	ICE_PHY_INIT_COMPLETE,
> >>  	ICE_FD_VF_FLUSH_CTX,		/* set at FD Rx IRQ or timeout */
> >>  	ICE_AUX_ERR_PENDING,
> >> +	ICE_SETUP_TC_QDISC,
> >>  	ICE_STATE_NBITS		/* must be last */
> >>  };
> >>  
> >> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c 
> >> b/drivers/net/ethernet/intel/ice/ice_lib.c
> >> index 22bcb414546a..0ee3acbea108 100644
> >> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> >> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> >> @@ -3448,7 +3448,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool 
> >> init_vsi)
> >>  
> >>  	ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx);
> >>  	ret = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx);
> >> -	if (ret)
> >> +	if (ret && !(test_bit(ICE_SETUP_TC_QDISC, pf->state) && ret == 
> >> +-EBUSY))
> >It is fine for me in current state, but do we really need to do this check only in setting TC qdisc? Doesn't the same thing happen when the number of queues is changed from ethtool? (maybe it is not because the changes is blocked when RDMA is active). Maybe whole
> >ice_setup_tc_mqprio_qdisc() should return -EBOUSY or inval if RDMA is active? Is it valid to a new qdisc when RDMA is active?
> >
> You are right about ethtool. However, I'm not sure if we should ignore -EBOUSY for every other scenario.
> 
Dave recently added a patchset thath blocks any queue changes when RDMA
driver is bound to aux [1]. I think following the same logic in
ice_setup_tc_mqprio_qdisc() the same check should happen (because it is
changing queues).

Adding Dave

Thanks, Michal
> >>  		dev_err(ice_pf_to_dev(vsi->back), "Failed to remove RDMA scheduler config for VSI %u, err %d\n",
> >>  			vsi->vsi_num, ret);
> >>  	ice_vsi_free_q_vectors(vsi);
> >> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c 
> >> b/drivers/net/ethernet/intel/ice/ice_main.c
> >> index a9a7f8b52140..5ff137645f08 100644
> >> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> >> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> >> @@ -8706,6 +8706,7 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
> >>  	cur_txq = vsi->num_txq;
> >>  	cur_rxq = vsi->num_rxq;
> >>  
> >> +	set_bit(ICE_SETUP_TC_QDISC, pf->state);
> >>  	/* proceed with rebuild main VSI using correct number of queues */
> >>  	ret = ice_vsi_rebuild(vsi, false);
> >>  	if (ret) {
> >> @@ -8716,9 +8717,11 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
> >>  		clear_bit(ICE_RESET_FAILED, pf->state);
> >>  		if (ice_vsi_rebuild(vsi, false)) {
> >>  			dev_err(dev, "Rebuild of main VSI failed again\n");
> >> +			clear_bit(ICE_SETUP_TC_QDISC, pf->state);
> >>  			return ret;
> >>  		}
> >>  	}
> >> +	clear_bit(ICE_SETUP_TC_QDISC, pf->state);
> >>  
> >>  	vsi->all_numtc = num_tcf;
> >>  	vsi->all_enatc = ena_tc_qdisc;
> >> --
> >> 2.31.1
> >> 
> >> _______________________________________________
> >> Intel-wired-lan mailing list
> >> Intel-wired-lan@osuosl.org
> >> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc
  2023-01-11 14:20 [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc Kamil Maziarz
  2023-01-12  7:11 ` Michal Swiatkowski
@ 2023-01-18 18:01 ` Tony Nguyen
  2023-01-23 11:43   ` Rogalski, RafalX
  1 sibling, 1 reply; 6+ messages in thread
From: Tony Nguyen @ 2023-01-18 18:01 UTC (permalink / raw)
  To: Kamil Maziarz, intel-wired-lan; +Cc: Rafal Rogalski

On 1/11/2023 6:20 AM, Kamil Maziarz wrote:
> From: Rafal Rogalski <rafalx.rogalski@intel.com>
> 
> Commit f70b9d5f4426 ("ice: check for a leaf node presence") prevents
> removal of VSI with leaf nodes. This is an expectation of driver action
> induced by FW requirements. However, this caused RDMA scheduler config
> removal to fail every time a qdisc was added or deleted.
> 
> Fix this by ignoring errors from RDMA configuration removal when qdisc are
> being reconfigured.

Would this same issue occur [1] in this situation? Have you tested that 
RDMA works?

> Fixes: ff7e93219442 ("ice: Fix failure to re-add LAN/RDMA Tx queues")
> Signed-off-by: Rafal Rogalski <rafalx.rogalski@intel.com>
> Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice.h      | 1 +
>   drivers/net/ethernet/intel/ice/ice_lib.c  | 2 +-
>   drivers/net/ethernet/intel/ice/ice_main.c | 3 +++
>   3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 2f0b604abc5e..b572d07bc126 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -306,6 +306,7 @@ enum ice_pf_state {
>   	ICE_PHY_INIT_COMPLETE,
>   	ICE_FD_VF_FLUSH_CTX,		/* set at FD Rx IRQ or timeout */
>   	ICE_AUX_ERR_PENDING,
> +	ICE_SETUP_TC_QDISC,
>   	ICE_STATE_NBITS		/* must be last */
>   };
>   
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 22bcb414546a..0ee3acbea108 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -3448,7 +3448,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)
>   
>   	ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx);
>   	ret = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx);
> -	if (ret)
> +	if (ret && !(test_bit(ICE_SETUP_TC_QDISC, pf->state) && ret == -EBUSY))
>   		dev_err(ice_pf_to_dev(vsi->back), "Failed to remove RDMA scheduler config for VSI %u, err %d\n",
>   			vsi->vsi_num, ret);
>   	ice_vsi_free_q_vectors(vsi);
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index a9a7f8b52140..5ff137645f08 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -8706,6 +8706,7 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
>   	cur_txq = vsi->num_txq;
>   	cur_rxq = vsi->num_rxq;
>   
> +	set_bit(ICE_SETUP_TC_QDISC, pf->state);
>   	/* proceed with rebuild main VSI using correct number of queues */
>   	ret = ice_vsi_rebuild(vsi, false);
>   	if (ret) {
> @@ -8716,9 +8717,11 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
>   		clear_bit(ICE_RESET_FAILED, pf->state);
>   		if (ice_vsi_rebuild(vsi, false)) {
>   			dev_err(dev, "Rebuild of main VSI failed again\n");
> +			clear_bit(ICE_SETUP_TC_QDISC, pf->state);
>   			return ret;
>   		}
>   	}
> +	clear_bit(ICE_SETUP_TC_QDISC, pf->state);
>   
>   	vsi->all_numtc = num_tcf;
>   	vsi->all_enatc = ena_tc_qdisc;

[1] 
https://lore.kernel.org/netdev/MW5PR11MB5811E652D63BC5CC934F256DDD1C9@MW5PR11MB5811.namprd11.prod.outlook.com/
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc
  2023-01-18 18:01 ` Tony Nguyen
@ 2023-01-23 11:43   ` Rogalski, RafalX
  0 siblings, 0 replies; 6+ messages in thread
From: Rogalski, RafalX @ 2023-01-23 11:43 UTC (permalink / raw)
  To: Nguyen, Anthony L, Maziarz, Kamil, intel-wired-lan

>On 1/11/2023 6:20 AM, Kamil Maziarz wrote:
>> From: Rafal Rogalski <rafalx.rogalski@intel.com>
>> 
>> Commit f70b9d5f4426 ("ice: check for a leaf node presence") prevents 
>> removal of VSI with leaf nodes. This is an expectation of driver 
>> action induced by FW requirements. However, this caused RDMA scheduler 
>> config removal to fail every time a qdisc was added or deleted.
>> 
>> Fix this by ignoring errors from RDMA configuration removal when qdisc 
>> are being reconfigured.
>
>Would this same issue occur [1] in this situation? Have you tested that RDMA works?
>
Yes, RDMA works. Our patch has no effect on RDMA.
[1] You mean, are the queues reallocated the same way for this problem? As for that, yes. In this case, VSI reallocation is also made.

>> Fixes: ff7e93219442 ("ice: Fix failure to re-add LAN/RDMA Tx queues")
>> Signed-off-by: Rafal Rogalski <rafalx.rogalski@intel.com>
>> Signed-off-by: Kamil Maziarz <kamil.maziarz@intel.com>
>> ---
>>   drivers/net/ethernet/intel/ice/ice.h      | 1 +
>>   drivers/net/ethernet/intel/ice/ice_lib.c  | 2 +-
>>   drivers/net/ethernet/intel/ice/ice_main.c | 3 +++
>>   3 files changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/ice/ice.h 
>> b/drivers/net/ethernet/intel/ice/ice.h
>> index 2f0b604abc5e..b572d07bc126 100644
>> --- a/drivers/net/ethernet/intel/ice/ice.h
>> +++ b/drivers/net/ethernet/intel/ice/ice.h
>> @@ -306,6 +306,7 @@ enum ice_pf_state {
>>   	ICE_PHY_INIT_COMPLETE,
>>   	ICE_FD_VF_FLUSH_CTX,		/* set at FD Rx IRQ or timeout */
>>   	ICE_AUX_ERR_PENDING,
>> +	ICE_SETUP_TC_QDISC,
>>   	ICE_STATE_NBITS		/* must be last */
>>   };
>>   
>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c 
>> b/drivers/net/ethernet/intel/ice/ice_lib.c
>> index 22bcb414546a..0ee3acbea108 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
>> @@ -3448,7 +3448,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool 
>> init_vsi)
>>   
>>   	ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx);
>>   	ret = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx);
>> -	if (ret)
>> +	if (ret && !(test_bit(ICE_SETUP_TC_QDISC, pf->state) && ret == 
>> +-EBUSY))
>>   		dev_err(ice_pf_to_dev(vsi->back), "Failed to remove RDMA scheduler config for VSI %u, err %d\n",
>>   			vsi->vsi_num, ret);
>>   	ice_vsi_free_q_vectors(vsi);
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c 
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>> index a9a7f8b52140..5ff137645f08 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -8706,6 +8706,7 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
>>   	cur_txq = vsi->num_txq;
>>   	cur_rxq = vsi->num_rxq;
>>   
>> +	set_bit(ICE_SETUP_TC_QDISC, pf->state);
>>   	/* proceed with rebuild main VSI using correct number of queues */
>>   	ret = ice_vsi_rebuild(vsi, false);
>>   	if (ret) {
>> @@ -8716,9 +8717,11 @@ static int ice_setup_tc_mqprio_qdisc(struct net_device *netdev, void *type_data)
>>   		clear_bit(ICE_RESET_FAILED, pf->state);
>>   		if (ice_vsi_rebuild(vsi, false)) {
>>   			dev_err(dev, "Rebuild of main VSI failed again\n");
>> +			clear_bit(ICE_SETUP_TC_QDISC, pf->state);
>>   			return ret;
>>   		}
>>   	}
>> +	clear_bit(ICE_SETUP_TC_QDISC, pf->state);
>>   
>>   	vsi->all_numtc = num_tcf;
>>   	vsi->all_enatc = ena_tc_qdisc;
>
>[1]
>https://lore.kernel.org/netdev/MW5PR11MB5811E652D63BC5CC934F256DDD1C9@MW5PR11MB5811.namprd11.prod.outlook.com/
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

end of thread, other threads:[~2023-01-23 15:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-11 14:20 [Intel-wired-lan] [PATCH net v1] ice: Ignore RDMA message on setup tc qdisc Kamil Maziarz
2023-01-12  7:11 ` Michal Swiatkowski
2023-01-12 13:54   ` Rogalski, RafalX
2023-01-13  6:30     ` Michal Swiatkowski
2023-01-18 18:01 ` Tony Nguyen
2023-01-23 11:43   ` Rogalski, RafalX

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.