Re: [Intel-wired-lan] [PATCH iwl-net] idpf: avoid mailbox timeout delays during reset
> -Original Message- > From: Intel-wired-lan On Behalf Of > Emil Tantilov > Sent: Thursday, May 8, 2025 11:47 AM > To: [email protected] > Cc: [email protected]; [email protected]; [email protected]; > Nguyen, Anthony L ; [email protected]; > [email protected]; [email protected]; [email protected]; Chittim, > Madhu ; Loktionov, Aleksandr > ; Kitszel, Przemyslaw > ; [email protected]; Hay, Joshua A > ; Zaki, Ahmed > Subject: [Intel-wired-lan] [PATCH iwl-net] idpf: avoid mailbox timeout delays > during reset > > Mailbox operations are not possible while the driver is in reset. > Operations that require MBX exchange with the control plane will result in > long > delays if executed while a reset is in progress: > > ethtool -L combined 8& echo 1 > /sys/class/net//device/reset idpf > :83:00.0: HW reset detected idpf :83:00.0: Device HW Reset > initiated idpf :83:00.0: Transaction timed-out (op:504 cookie:be00 > vc_op:504 salt:be timeout:2000ms) idpf :83:00.0: Transaction timed- > out (op:508 cookie:bf00 vc_op:508 salt:bf timeout:2000ms) idpf > :83:00.0: Transaction timed-out (op:512 cookie:c000 vc_op:512 salt:c0 > timeout:2000ms) idpf :83:00.0: Transaction timed-out (op:510 > cookie:c100 vc_op:510 salt:c1 timeout:2000ms) idpf :83:00.0: > Transaction timed-out (op:509 cookie:c200 vc_op:509 salt:c2 > timeout:6ms) idpf :83:00.0: Transaction timed-out (op:509 > cookie:c300 vc_op:509 salt:c3 timeout:6ms) idpf :83:00.0: > Transaction timed-out (op:505 cookie:c400 vc_op:505 salt:c4 > timeout:6ms) idpf :83:00.0: Failed to configure queues for vport 0, > -62 > > Disable mailbox communication in case of a reset, unless it's done during a > driver load, where the virtchnl operations are needed to configure the device. > > Fixes: 8077c727561aa ("idpf: add controlq init and reset checks") > Co-developed-by: Joshua Hay > Signed-off-by: Joshua Hay > Signed-off-by: Emil Tantilov > Reviewed-by: Ahmed Zaki > --- > 2.17.2 Tested-by: Samuel Salin
Re: [Intel-wired-lan] [PATCH iwl-net] idpf: avoid mailbox timeout delays during reset
On 5/12/2025 4:46 AM, Paul Menzel wrote:
Dear Emil,
Thank you for the patch.
Am 08.05.25 um 20:47 schrieb Emil Tantilov:
Mailbox operations are not possible while the driver is in reset.
Operations that require MBX exchange with the control plane will result
in long delays if executed while a reset is in progress:
ethtool -L combined 8& echo 1 > /sys/class/net//device/reset
idpf :83:00.0: HW reset detected
idpf :83:00.0: Device HW Reset initiated
idpf :83:00.0: Transaction timed-out (op:504 cookie:be00 vc_op:504
salt:be timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:508 cookie:bf00 vc_op:508
salt:bf timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:512 cookie:c000 vc_op:512
salt:c0 timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:510 cookie:c100 vc_op:510
salt:c1 timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:509 cookie:c200 vc_op:509
salt:c2 timeout:6ms)
idpf :83:00.0: Transaction timed-out (op:509 cookie:c300 vc_op:509
salt:c3 timeout:6ms)
idpf :83:00.0: Transaction timed-out (op:505 cookie:c400 vc_op:505
salt:c4 timeout:6ms)
idpf :83:00.0: Failed to configure queues for vport 0, -62
Disable mailbox communication in case of a reset, unless it's done during
a driver load, where the virtchnl operations are needed to configure the
device.
Is the Linux kernel going to log something now, that the mailbox
operation is ignored?
Strictly speaking, the mailbox operations are not ignored, they are
disabled on purpose, because they are not possible during a reset. Only
the timeouts will be absent in the scenario shown above. The error
regarding the queue configuration will still be logged in dmesg.
Thanks,
Emil
Fixes: 8077c727561aa ("idpf: add controlq init and reset checks")
Co-developed-by: Joshua Hay
Signed-off-by: Joshua Hay
Signed-off-by: Emil Tantilov
Reviewed-by: Ahmed Zaki
---
drivers/net/ethernet/intel/idpf/idpf_lib.c | 18 +-
.../net/ethernet/intel/idpf/idpf_virtchnl.c | 2 +-
.../net/ethernet/intel/idpf/idpf_virtchnl.h | 1 +
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/
ethernet/intel/idpf/idpf_lib.c
index 3a033ce19cda..2ed801398971 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1816,11 +1816,19 @@ void idpf_vc_event_task(struct work_struct *work)
if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
return;
- if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags) ||
- test_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
- set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
- idpf_init_hard_reset(adapter);
- }
+ if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags))
+ goto func_reset;
+
+ if (test_bit(IDPF_HR_DRV_LOAD, adapter->flags))
+ goto drv_load;
+
+ return;
+
+func_reset:
+ idpf_vc_xn_shutdown(adapter->vcxn_mngr);
+drv_load:
+ set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
+ idpf_init_hard_reset(adapter);
}
/**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 3d2413b8684f..5d2ca007f682 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -376,7 +376,7 @@ static void idpf_vc_xn_init(struct
idpf_vc_xn_manager *vcxn_mngr)
* All waiting threads will be woken-up and their transaction
aborted. Further
* operations on that object will fail.
*/
-static void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr)
+void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr)
{
int i;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h b/
drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
index 83da5d8da56b..23271cf0a216 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
@@ -66,5 +66,6 @@ int idpf_send_get_stats_msg(struct idpf_vport *vport);
int idpf_send_set_sriov_vfs_msg(struct idpf_adapter *adapter, u16
num_vfs);
int idpf_send_get_set_rss_key_msg(struct idpf_vport *vport, bool get);
int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get);
+void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr);
#endif /* _IDPF_VIRTCHNL_H_ */
Kind regards,
Paul
Re: [Intel-wired-lan] [PATCH iwl-net] idpf: avoid mailbox timeout delays during reset
Dear Emil,
Thank you for the patch.
Am 08.05.25 um 20:47 schrieb Emil Tantilov:
Mailbox operations are not possible while the driver is in reset.
Operations that require MBX exchange with the control plane will result
in long delays if executed while a reset is in progress:
ethtool -L combined 8& echo 1 > /sys/class/net//device/reset
idpf :83:00.0: HW reset detected
idpf :83:00.0: Device HW Reset initiated
idpf :83:00.0: Transaction timed-out (op:504 cookie:be00 vc_op:504 salt:be
timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:508 cookie:bf00 vc_op:508 salt:bf
timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:512 cookie:c000 vc_op:512 salt:c0
timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:510 cookie:c100 vc_op:510 salt:c1
timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:509 cookie:c200 vc_op:509 salt:c2
timeout:6ms)
idpf :83:00.0: Transaction timed-out (op:509 cookie:c300 vc_op:509 salt:c3
timeout:6ms)
idpf :83:00.0: Transaction timed-out (op:505 cookie:c400 vc_op:505 salt:c4
timeout:6ms)
idpf :83:00.0: Failed to configure queues for vport 0, -62
Disable mailbox communication in case of a reset, unless it's done during
a driver load, where the virtchnl operations are needed to configure the
device.
Is the Linux kernel going to log something now, that the mailbox
operation is ignored?
Fixes: 8077c727561aa ("idpf: add controlq init and reset checks")
Co-developed-by: Joshua Hay
Signed-off-by: Joshua Hay
Signed-off-by: Emil Tantilov
Reviewed-by: Ahmed Zaki
---
drivers/net/ethernet/intel/idpf/idpf_lib.c | 18 +-
.../net/ethernet/intel/idpf/idpf_virtchnl.c| 2 +-
.../net/ethernet/intel/idpf/idpf_virtchnl.h| 1 +
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c
b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 3a033ce19cda..2ed801398971 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1816,11 +1816,19 @@ void idpf_vc_event_task(struct work_struct *work)
if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
return;
- if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags) ||
- test_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
- set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
- idpf_init_hard_reset(adapter);
- }
+ if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags))
+ goto func_reset;
+
+ if (test_bit(IDPF_HR_DRV_LOAD, adapter->flags))
+ goto drv_load;
+
+ return;
+
+func_reset:
+ idpf_vc_xn_shutdown(adapter->vcxn_mngr);
+drv_load:
+ set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
+ idpf_init_hard_reset(adapter);
}
/**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 3d2413b8684f..5d2ca007f682 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -376,7 +376,7 @@ static void idpf_vc_xn_init(struct idpf_vc_xn_manager
*vcxn_mngr)
* All waiting threads will be woken-up and their transaction aborted. Further
* operations on that object will fail.
*/
-static void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr)
+void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr)
{
int i;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
index 83da5d8da56b..23271cf0a216 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
@@ -66,5 +66,6 @@ int idpf_send_get_stats_msg(struct idpf_vport *vport);
int idpf_send_set_sriov_vfs_msg(struct idpf_adapter *adapter, u16 num_vfs);
int idpf_send_get_set_rss_key_msg(struct idpf_vport *vport, bool get);
int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get);
+void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr);
#endif /* _IDPF_VIRTCHNL_H_ */
Kind regards,
Paul
Re: [Intel-wired-lan] [PATCH iwl-net] idpf: avoid mailbox timeout delays during reset
On Thu, May 08, 2025 at 11:47:15AM -0700, Emil Tantilov wrote:
> Mailbox operations are not possible while the driver is in reset.
> Operations that require MBX exchange with the control plane will result
> in long delays if executed while a reset is in progress:
>
> ethtool -L combined 8& echo 1 > /sys/class/net//device/reset
> idpf :83:00.0: HW reset detected
> idpf :83:00.0: Device HW Reset initiated
> idpf :83:00.0: Transaction timed-out (op:504 cookie:be00 vc_op:504
> salt:be timeout:2000ms)
> idpf :83:00.0: Transaction timed-out (op:508 cookie:bf00 vc_op:508
> salt:bf timeout:2000ms)
> idpf :83:00.0: Transaction timed-out (op:512 cookie:c000 vc_op:512
> salt:c0 timeout:2000ms)
> idpf :83:00.0: Transaction timed-out (op:510 cookie:c100 vc_op:510
> salt:c1 timeout:2000ms)
> idpf :83:00.0: Transaction timed-out (op:509 cookie:c200 vc_op:509
> salt:c2 timeout:6ms)
> idpf :83:00.0: Transaction timed-out (op:509 cookie:c300 vc_op:509
> salt:c3 timeout:6ms)
> idpf :83:00.0: Transaction timed-out (op:505 cookie:c400 vc_op:505
> salt:c4 timeout:6ms)
> idpf :83:00.0: Failed to configure queues for vport 0, -62
>
> Disable mailbox communication in case of a reset, unless it's done during
> a driver load, where the virtchnl operations are needed to configure the
> device.
>
> Fixes: 8077c727561aa ("idpf: add controlq init and reset checks")
> Co-developed-by: Joshua Hay
> Signed-off-by: Joshua Hay
> Signed-off-by: Emil Tantilov
> Reviewed-by: Ahmed Zaki
Reviewed-by: Simon Horman
Re: [Intel-wired-lan] [PATCH iwl-net] idpf: avoid mailbox timeout delays during reset
> -Original Message- > From: Tantilov, Emil S > Sent: Thursday, May 8, 2025 8:47 PM > To: [email protected] > Cc: [email protected]; [email protected]; [email protected]; > Nguyen, Anthony L ; [email protected]; > [email protected]; [email protected]; [email protected]; Chittim, > Madhu ; Loktionov, Aleksandr > ; Kitszel, Przemyslaw > ; [email protected]; Hay, Joshua A > ; Zaki, Ahmed > Subject: [PATCH iwl-net] idpf: avoid mailbox timeout delays during reset > > Mailbox operations are not possible while the driver is in reset. > Operations that require MBX exchange with the control plane will result in > long > delays if executed while a reset is in progress: > > ethtool -L combined 8& echo 1 > /sys/class/net//device/reset idpf > :83:00.0: HW reset detected idpf :83:00.0: Device HW Reset > initiated idpf :83:00.0: Transaction timed-out (op:504 cookie:be00 > vc_op:504 salt:be timeout:2000ms) idpf :83:00.0: Transaction timed- > out (op:508 cookie:bf00 vc_op:508 salt:bf timeout:2000ms) idpf > :83:00.0: Transaction timed-out (op:512 cookie:c000 vc_op:512 salt:c0 > timeout:2000ms) idpf :83:00.0: Transaction timed-out (op:510 > cookie:c100 vc_op:510 salt:c1 timeout:2000ms) idpf :83:00.0: > Transaction timed-out (op:509 cookie:c200 vc_op:509 salt:c2 > timeout:6ms) idpf :83:00.0: Transaction timed-out (op:509 > cookie:c300 vc_op:509 salt:c3 timeout:6ms) idpf :83:00.0: > Transaction timed-out (op:505 cookie:c400 vc_op:505 salt:c4 > timeout:6ms) idpf :83:00.0: Failed to configure queues for vport 0, > -62 > > Disable mailbox communication in case of a reset, unless it's done during a > driver load, where the virtchnl operations are needed to configure the device. > > Fixes: 8077c727561aa ("idpf: add controlq init and reset checks") > Co-developed-by: Joshua Hay > Signed-off-by: Joshua Hay > Signed-off-by: Emil Tantilov > Reviewed-by: Ahmed Zaki Reviewed-by: Aleksandr Loktionov > --- > drivers/net/ethernet/intel/idpf/idpf_lib.c | 18 +- > .../net/ethernet/intel/idpf/idpf_virtchnl.c| 2 +- > .../net/ethernet/intel/idpf/idpf_virtchnl.h| 1 + > 3 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c > b/drivers/net/ethernet/intel/idpf/idpf_lib.c > index 3a033ce19cda..2ed801398971 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c > +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c > @@ -1816,11 +1816,19 @@ void idpf_vc_event_task(struct work_struct > *work) > if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags)) > return; > > - if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags) || > - test_bit(IDPF_HR_DRV_LOAD, adapter->flags)) { > - set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags); > - idpf_init_hard_reset(adapter); > - } > + if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags)) > + goto func_reset; > + > + if (test_bit(IDPF_HR_DRV_LOAD, adapter->flags)) > + goto drv_load; > + > + return; > + > +func_reset: > + idpf_vc_xn_shutdown(adapter->vcxn_mngr); > +drv_load: > + set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags); > + idpf_init_hard_reset(adapter); > } > > /** > diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > index 3d2413b8684f..5d2ca007f682 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > @@ -376,7 +376,7 @@ static void idpf_vc_xn_init(struct > idpf_vc_xn_manager *vcxn_mngr) > * All waiting threads will be woken-up and their transaction aborted. > Further > * operations on that object will fail. > */ > -static void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr) > +void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr) > { > int i; > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h > b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h > index 83da5d8da56b..23271cf0a216 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h > @@ -66,5 +66,6 @@ int idpf_send_get_stats_msg(struct idpf_vport *vport); > int idpf_send_set_sriov_vfs_msg(struct idpf_adapter *adapter, u16 num_vfs); > int idpf_send_get_set_rss_key_msg(struct idpf_vport *vport, bool get); int > idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get); > +void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr); > > #endif /* _IDPF_VIRTCHNL_H_ */ > -- > 2.17.2
