diff mbox series

[net,v1] ice: Ignore RDMA message on setup tc qdisc

Message ID 20230111142029.318092-1-kamil.maziarz@intel.com
State Changes Requested
Headers show
Series [net,v1] ice: Ignore RDMA message on setup tc qdisc | expand

Commit Message

Kamil Maziarz Jan. 11, 2023, 2:20 p.m. UTC
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(-)

Comments

Michal Swiatkowski Jan. 12, 2023, 7:11 a.m. UTC | #1
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
Rogalski, RafalX Jan. 12, 2023, 1:54 p.m. UTC | #2
>-----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.
Michal Swiatkowski Jan. 13, 2023, 6:30 a.m. UTC | #3
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
Tony Nguyen Jan. 18, 2023, 6:01 p.m. UTC | #4
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/
Rogalski, RafalX Jan. 23, 2023, 11:43 a.m. UTC | #5
>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/
>
diff mbox series

Patch

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;