* [Intel-wired-lan] [PATCH iwl-net 0/2] fix reset issues @ 2024-10-22 17:35 ` Pavan Kumar Linga 0 siblings, 0 replies; 10+ messages in thread From: Pavan Kumar Linga @ 2024-10-22 17:35 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, Pavan Kumar Linga This series fixes reset related issues, especially the case where the idpf is running on the host and the platform running the device control plane is rebooted. The first patch fixes the link_ksettings and the second patch fixes the error path in idpf_vc_core_init function. Pavan Kumar Linga (2): idpf: avoid vport access in idpf_get_link_ksettings idpf: fix idpf_vc_core_init error path drivers/net/ethernet/intel/idpf/idpf.h | 4 ++-- drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 11 +++-------- drivers/net/ethernet/intel/idpf/idpf_lib.c | 5 +++-- drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 9 +-------- 4 files changed, 9 insertions(+), 20 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH iwl-net 0/2] fix reset issues @ 2024-10-22 17:35 ` Pavan Kumar Linga 0 siblings, 0 replies; 10+ messages in thread From: Pavan Kumar Linga @ 2024-10-22 17:35 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, Pavan Kumar Linga This series fixes reset related issues, especially the case where the idpf is running on the host and the platform running the device control plane is rebooted. The first patch fixes the link_ksettings and the second patch fixes the error path in idpf_vc_core_init function. Pavan Kumar Linga (2): idpf: avoid vport access in idpf_get_link_ksettings idpf: fix idpf_vc_core_init error path drivers/net/ethernet/intel/idpf/idpf.h | 4 ++-- drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 11 +++-------- drivers/net/ethernet/intel/idpf/idpf_lib.c | 5 +++-- drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 9 +-------- 4 files changed, 9 insertions(+), 20 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH iwl-net 1/2] idpf: avoid vport access in idpf_get_link_ksettings 2024-10-22 17:35 ` Pavan Kumar Linga @ 2024-10-22 17:35 ` Pavan Kumar Linga -1 siblings, 0 replies; 10+ messages in thread From: Pavan Kumar Linga @ 2024-10-22 17:35 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, Pavan Kumar Linga, stable, Tarun K Singh When the device control plane is removed or the platform running device control plane is rebooted, a reset is detected on the driver. On driver reset, it releases the resources and waits for the reset to complete. If the reset fails, it takes the error path and releases the vport lock. At this time if the monitoring tools tries to access link settings, it call traces for accessing released vport pointer. To avoid it, move link_speed_mbps to netdev_priv structure which removes the dependency on vport pointer and the vport lock in idpf_get_link_ksettings. Also use netif_carrier_ok() to check the link status and adjust the offsetof to use link_up instead of link_speed_mbps. Fixes: 02cbfba1add5 ("idpf: add ethtool callbacks") Cc: stable@vger.kernel.org # 6.7+ Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> --- drivers/net/ethernet/intel/idpf/idpf.h | 4 ++-- drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 11 +++-------- drivers/net/ethernet/intel/idpf/idpf_lib.c | 4 ++-- drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 2 +- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h index 2c31ad87587a..66544faab710 100644 --- a/drivers/net/ethernet/intel/idpf/idpf.h +++ b/drivers/net/ethernet/intel/idpf/idpf.h @@ -141,6 +141,7 @@ enum idpf_vport_state { * @adapter: Adapter back pointer * @vport: Vport back pointer * @vport_id: Vport identifier + * @link_speed_mbps: Link speed in mbps * @vport_idx: Relative vport index * @state: See enum idpf_vport_state * @netstats: Packet and byte stats @@ -150,6 +151,7 @@ struct idpf_netdev_priv { struct idpf_adapter *adapter; struct idpf_vport *vport; u32 vport_id; + u32 link_speed_mbps; u16 vport_idx; enum idpf_vport_state state; struct rtnl_link_stats64 netstats; @@ -287,7 +289,6 @@ struct idpf_port_stats { * @tx_itr_profile: TX profiles for Dynamic Interrupt Moderation * @port_stats: per port csum, header split, and other offload stats * @link_up: True if link is up - * @link_speed_mbps: Link speed in mbps * @sw_marker_wq: workqueue for marker packets */ struct idpf_vport { @@ -331,7 +332,6 @@ struct idpf_vport { struct idpf_port_stats port_stats; bool link_up; - u32 link_speed_mbps; wait_queue_head_t sw_marker_wq; }; diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c index 3806ddd3ce4a..59b1a1a09996 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c @@ -1296,24 +1296,19 @@ static void idpf_set_msglevel(struct net_device *netdev, u32 data) static int idpf_get_link_ksettings(struct net_device *netdev, struct ethtool_link_ksettings *cmd) { - struct idpf_vport *vport; - - idpf_vport_ctrl_lock(netdev); - vport = idpf_netdev_to_vport(netdev); + struct idpf_netdev_priv *np = netdev_priv(netdev); ethtool_link_ksettings_zero_link_mode(cmd, supported); cmd->base.autoneg = AUTONEG_DISABLE; cmd->base.port = PORT_NONE; - if (vport->link_up) { + if (netif_carrier_ok(netdev)) { cmd->base.duplex = DUPLEX_FULL; - cmd->base.speed = vport->link_speed_mbps; + cmd->base.speed = np->link_speed_mbps; } else { cmd->base.duplex = DUPLEX_UNKNOWN; cmd->base.speed = SPEED_UNKNOWN; } - idpf_vport_ctrl_unlock(netdev); - return 0; } diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c index 4f20343e49a9..c3848e10e7db 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c @@ -1860,7 +1860,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport, * mess with. Nothing below should use those variables from new_vport * and should instead always refer to them in vport if they need to. */ - memcpy(new_vport, vport, offsetof(struct idpf_vport, link_speed_mbps)); + memcpy(new_vport, vport, offsetof(struct idpf_vport, link_up)); /* Adjust resource parameters prior to reallocating resources */ switch (reset_cause) { @@ -1906,7 +1906,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport, /* Same comment as above regarding avoiding copying the wait_queues and * mutexes applies here. We do not want to mess with those if possible. */ - memcpy(vport, new_vport, offsetof(struct idpf_vport, link_speed_mbps)); + memcpy(vport, new_vport, offsetof(struct idpf_vport, link_up)); if (reset_cause == IDPF_SR_Q_CHANGE) idpf_vport_alloc_vec_indexes(vport); diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c index 70986e12da28..3be883726b87 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c @@ -141,7 +141,7 @@ static void idpf_handle_event_link(struct idpf_adapter *adapter, } np = netdev_priv(vport->netdev); - vport->link_speed_mbps = le32_to_cpu(v2e->link_speed); + np->link_speed_mbps = le32_to_cpu(v2e->link_speed); if (vport->link_up == v2e->link_status) return; -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH iwl-net 1/2] idpf: avoid vport access in idpf_get_link_ksettings @ 2024-10-22 17:35 ` Pavan Kumar Linga 0 siblings, 0 replies; 10+ messages in thread From: Pavan Kumar Linga @ 2024-10-22 17:35 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, Pavan Kumar Linga, stable, Tarun K Singh When the device control plane is removed or the platform running device control plane is rebooted, a reset is detected on the driver. On driver reset, it releases the resources and waits for the reset to complete. If the reset fails, it takes the error path and releases the vport lock. At this time if the monitoring tools tries to access link settings, it call traces for accessing released vport pointer. To avoid it, move link_speed_mbps to netdev_priv structure which removes the dependency on vport pointer and the vport lock in idpf_get_link_ksettings. Also use netif_carrier_ok() to check the link status and adjust the offsetof to use link_up instead of link_speed_mbps. Fixes: 02cbfba1add5 ("idpf: add ethtool callbacks") Cc: stable@vger.kernel.org # 6.7+ Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> --- drivers/net/ethernet/intel/idpf/idpf.h | 4 ++-- drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 11 +++-------- drivers/net/ethernet/intel/idpf/idpf_lib.c | 4 ++-- drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 2 +- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h index 2c31ad87587a..66544faab710 100644 --- a/drivers/net/ethernet/intel/idpf/idpf.h +++ b/drivers/net/ethernet/intel/idpf/idpf.h @@ -141,6 +141,7 @@ enum idpf_vport_state { * @adapter: Adapter back pointer * @vport: Vport back pointer * @vport_id: Vport identifier + * @link_speed_mbps: Link speed in mbps * @vport_idx: Relative vport index * @state: See enum idpf_vport_state * @netstats: Packet and byte stats @@ -150,6 +151,7 @@ struct idpf_netdev_priv { struct idpf_adapter *adapter; struct idpf_vport *vport; u32 vport_id; + u32 link_speed_mbps; u16 vport_idx; enum idpf_vport_state state; struct rtnl_link_stats64 netstats; @@ -287,7 +289,6 @@ struct idpf_port_stats { * @tx_itr_profile: TX profiles for Dynamic Interrupt Moderation * @port_stats: per port csum, header split, and other offload stats * @link_up: True if link is up - * @link_speed_mbps: Link speed in mbps * @sw_marker_wq: workqueue for marker packets */ struct idpf_vport { @@ -331,7 +332,6 @@ struct idpf_vport { struct idpf_port_stats port_stats; bool link_up; - u32 link_speed_mbps; wait_queue_head_t sw_marker_wq; }; diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c index 3806ddd3ce4a..59b1a1a09996 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c @@ -1296,24 +1296,19 @@ static void idpf_set_msglevel(struct net_device *netdev, u32 data) static int idpf_get_link_ksettings(struct net_device *netdev, struct ethtool_link_ksettings *cmd) { - struct idpf_vport *vport; - - idpf_vport_ctrl_lock(netdev); - vport = idpf_netdev_to_vport(netdev); + struct idpf_netdev_priv *np = netdev_priv(netdev); ethtool_link_ksettings_zero_link_mode(cmd, supported); cmd->base.autoneg = AUTONEG_DISABLE; cmd->base.port = PORT_NONE; - if (vport->link_up) { + if (netif_carrier_ok(netdev)) { cmd->base.duplex = DUPLEX_FULL; - cmd->base.speed = vport->link_speed_mbps; + cmd->base.speed = np->link_speed_mbps; } else { cmd->base.duplex = DUPLEX_UNKNOWN; cmd->base.speed = SPEED_UNKNOWN; } - idpf_vport_ctrl_unlock(netdev); - return 0; } diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c index 4f20343e49a9..c3848e10e7db 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c @@ -1860,7 +1860,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport, * mess with. Nothing below should use those variables from new_vport * and should instead always refer to them in vport if they need to. */ - memcpy(new_vport, vport, offsetof(struct idpf_vport, link_speed_mbps)); + memcpy(new_vport, vport, offsetof(struct idpf_vport, link_up)); /* Adjust resource parameters prior to reallocating resources */ switch (reset_cause) { @@ -1906,7 +1906,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport, /* Same comment as above regarding avoiding copying the wait_queues and * mutexes applies here. We do not want to mess with those if possible. */ - memcpy(vport, new_vport, offsetof(struct idpf_vport, link_speed_mbps)); + memcpy(vport, new_vport, offsetof(struct idpf_vport, link_up)); if (reset_cause == IDPF_SR_Q_CHANGE) idpf_vport_alloc_vec_indexes(vport); diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c index 70986e12da28..3be883726b87 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c @@ -141,7 +141,7 @@ static void idpf_handle_event_link(struct idpf_adapter *adapter, } np = netdev_priv(vport->netdev); - vport->link_speed_mbps = le32_to_cpu(v2e->link_speed); + np->link_speed_mbps = le32_to_cpu(v2e->link_speed); if (vport->link_up == v2e->link_status) return; -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH iwl-net 2/2] idpf: fix idpf_vc_core_init error path 2024-10-22 17:35 ` Pavan Kumar Linga @ 2024-10-22 17:35 ` Pavan Kumar Linga -1 siblings, 0 replies; 10+ messages in thread From: Pavan Kumar Linga @ 2024-10-22 17:35 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, Pavan Kumar Linga, stable, Tarun K Singh In an event where the platform running the device control plane is rebooted, reset is detected on the driver. It releases all the resources and waits for the reset to complete. Once the reset is done, it tries to build the resources back. At this time if the device control plane is not yet started, then the driver timeouts on the virtchnl message and retries to establish the mailbox again. In the retry flow, mailbox is deinitialized but the mailbox workqueue is still alive and polling for the mailbox message. This results in accessing the released control queue leading to null-ptr-deref. Fix it by unrolling the work queue cancellation and mailbox deinitialization in the order which they got initialized. Also remove the redundant scheduling of the mailbox task in idpf_vc_core_init. Fixes: 4930fbf419a7 ("idpf: add core init and interrupt request") Fixes: 34c21fa894a1 ("idpf: implement virtchnl transaction manager") Cc: stable@vger.kernel.org # 6.9+ Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> --- drivers/net/ethernet/intel/idpf/idpf_lib.c | 1 + drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 7 ------- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c index c3848e10e7db..b4fbb99bfad2 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c @@ -1786,6 +1786,7 @@ static int idpf_init_hard_reset(struct idpf_adapter *adapter) */ err = idpf_vc_core_init(adapter); if (err) { + cancel_delayed_work_sync(&adapter->mbx_task); idpf_deinit_dflt_mbx(adapter); goto unlock_mutex; } diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c index 3be883726b87..d77d6c3805e2 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c @@ -3017,11 +3017,6 @@ int idpf_vc_core_init(struct idpf_adapter *adapter) goto err_netdev_alloc; } - /* Start the mailbox task before requesting vectors. This will ensure - * vector information response from mailbox is handled - */ - queue_delayed_work(adapter->mbx_wq, &adapter->mbx_task, 0); - queue_delayed_work(adapter->serv_wq, &adapter->serv_task, msecs_to_jiffies(5 * (adapter->pdev->devfn & 0x07))); @@ -3046,7 +3041,6 @@ int idpf_vc_core_init(struct idpf_adapter *adapter) err_intr_req: cancel_delayed_work_sync(&adapter->serv_task); - cancel_delayed_work_sync(&adapter->mbx_task); idpf_vport_params_buf_rel(adapter); err_netdev_alloc: kfree(adapter->vports); @@ -3070,7 +3064,6 @@ int idpf_vc_core_init(struct idpf_adapter *adapter) adapter->state = __IDPF_VER_CHECK; if (adapter->vcxn_mngr) idpf_vc_xn_shutdown(adapter->vcxn_mngr); - idpf_deinit_dflt_mbx(adapter); set_bit(IDPF_HR_DRV_LOAD, adapter->flags); queue_delayed_work(adapter->vc_event_wq, &adapter->vc_event_task, msecs_to_jiffies(task_delay)); -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH iwl-net 2/2] idpf: fix idpf_vc_core_init error path @ 2024-10-22 17:35 ` Pavan Kumar Linga 0 siblings, 0 replies; 10+ messages in thread From: Pavan Kumar Linga @ 2024-10-22 17:35 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, Pavan Kumar Linga, stable, Tarun K Singh In an event where the platform running the device control plane is rebooted, reset is detected on the driver. It releases all the resources and waits for the reset to complete. Once the reset is done, it tries to build the resources back. At this time if the device control plane is not yet started, then the driver timeouts on the virtchnl message and retries to establish the mailbox again. In the retry flow, mailbox is deinitialized but the mailbox workqueue is still alive and polling for the mailbox message. This results in accessing the released control queue leading to null-ptr-deref. Fix it by unrolling the work queue cancellation and mailbox deinitialization in the order which they got initialized. Also remove the redundant scheduling of the mailbox task in idpf_vc_core_init. Fixes: 4930fbf419a7 ("idpf: add core init and interrupt request") Fixes: 34c21fa894a1 ("idpf: implement virtchnl transaction manager") Cc: stable@vger.kernel.org # 6.9+ Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> --- drivers/net/ethernet/intel/idpf/idpf_lib.c | 1 + drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 7 ------- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c index c3848e10e7db..b4fbb99bfad2 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c @@ -1786,6 +1786,7 @@ static int idpf_init_hard_reset(struct idpf_adapter *adapter) */ err = idpf_vc_core_init(adapter); if (err) { + cancel_delayed_work_sync(&adapter->mbx_task); idpf_deinit_dflt_mbx(adapter); goto unlock_mutex; } diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c index 3be883726b87..d77d6c3805e2 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c @@ -3017,11 +3017,6 @@ int idpf_vc_core_init(struct idpf_adapter *adapter) goto err_netdev_alloc; } - /* Start the mailbox task before requesting vectors. This will ensure - * vector information response from mailbox is handled - */ - queue_delayed_work(adapter->mbx_wq, &adapter->mbx_task, 0); - queue_delayed_work(adapter->serv_wq, &adapter->serv_task, msecs_to_jiffies(5 * (adapter->pdev->devfn & 0x07))); @@ -3046,7 +3041,6 @@ int idpf_vc_core_init(struct idpf_adapter *adapter) err_intr_req: cancel_delayed_work_sync(&adapter->serv_task); - cancel_delayed_work_sync(&adapter->mbx_task); idpf_vport_params_buf_rel(adapter); err_netdev_alloc: kfree(adapter->vports); @@ -3070,7 +3064,6 @@ int idpf_vc_core_init(struct idpf_adapter *adapter) adapter->state = __IDPF_VER_CHECK; if (adapter->vcxn_mngr) idpf_vc_xn_shutdown(adapter->vcxn_mngr); - idpf_deinit_dflt_mbx(adapter); set_bit(IDPF_HR_DRV_LOAD, adapter->flags); queue_delayed_work(adapter->vc_event_wq, &adapter->vc_event_task, msecs_to_jiffies(task_delay)); -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net 2/2] idpf: fix idpf_vc_core_init error path 2024-10-22 17:35 ` Pavan Kumar Linga @ 2024-10-24 14:39 ` Simon Horman -1 siblings, 0 replies; 10+ messages in thread From: Simon Horman @ 2024-10-24 14:39 UTC (permalink / raw) To: Pavan Kumar Linga; +Cc: intel-wired-lan, netdev, stable, Tarun K Singh On Tue, Oct 22, 2024 at 10:35:27AM -0700, Pavan Kumar Linga wrote: > In an event where the platform running the device control plane > is rebooted, reset is detected on the driver. It releases > all the resources and waits for the reset to complete. Once the > reset is done, it tries to build the resources back. At this > time if the device control plane is not yet started, then > the driver timeouts on the virtchnl message and retries to > establish the mailbox again. > > In the retry flow, mailbox is deinitialized but the mailbox > workqueue is still alive and polling for the mailbox message. > This results in accessing the released control queue leading to > null-ptr-deref. Fix it by unrolling the work queue cancellation > and mailbox deinitialization in the order which they got > initialized. > > Also remove the redundant scheduling of the mailbox task in > idpf_vc_core_init. I think it might be better to move this last change into a separate patch targeted at iwl rather than iwl-net. It isn't a fix, right? > > Fixes: 4930fbf419a7 ("idpf: add core init and interrupt request") > Fixes: 34c21fa894a1 ("idpf: implement virtchnl transaction manager") > Cc: stable@vger.kernel.org # 6.9+ > Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com> > Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> ... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iwl-net 2/2] idpf: fix idpf_vc_core_init error path @ 2024-10-24 14:39 ` Simon Horman 0 siblings, 0 replies; 10+ messages in thread From: Simon Horman @ 2024-10-24 14:39 UTC (permalink / raw) To: Pavan Kumar Linga; +Cc: intel-wired-lan, netdev, stable, Tarun K Singh On Tue, Oct 22, 2024 at 10:35:27AM -0700, Pavan Kumar Linga wrote: > In an event where the platform running the device control plane > is rebooted, reset is detected on the driver. It releases > all the resources and waits for the reset to complete. Once the > reset is done, it tries to build the resources back. At this > time if the device control plane is not yet started, then > the driver timeouts on the virtchnl message and retries to > establish the mailbox again. > > In the retry flow, mailbox is deinitialized but the mailbox > workqueue is still alive and polling for the mailbox message. > This results in accessing the released control queue leading to > null-ptr-deref. Fix it by unrolling the work queue cancellation > and mailbox deinitialization in the order which they got > initialized. > > Also remove the redundant scheduling of the mailbox task in > idpf_vc_core_init. I think it might be better to move this last change into a separate patch targeted at iwl rather than iwl-net. It isn't a fix, right? > > Fixes: 4930fbf419a7 ("idpf: add core init and interrupt request") > Fixes: 34c21fa894a1 ("idpf: implement virtchnl transaction manager") > Cc: stable@vger.kernel.org # 6.9+ > Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com> > Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> ... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net 2/2] idpf: fix idpf_vc_core_init error path 2024-10-24 14:39 ` Simon Horman @ 2024-10-25 17:11 ` Linga, Pavan Kumar -1 siblings, 0 replies; 10+ messages in thread From: Linga, Pavan Kumar @ 2024-10-25 17:11 UTC (permalink / raw) To: Simon Horman; +Cc: intel-wired-lan, netdev, stable, Tarun K Singh On 10/24/2024 7:39 AM, Simon Horman wrote: > On Tue, Oct 22, 2024 at 10:35:27AM -0700, Pavan Kumar Linga wrote: >> In an event where the platform running the device control plane >> is rebooted, reset is detected on the driver. It releases >> all the resources and waits for the reset to complete. Once the >> reset is done, it tries to build the resources back. At this >> time if the device control plane is not yet started, then >> the driver timeouts on the virtchnl message and retries to >> establish the mailbox again. >> >> In the retry flow, mailbox is deinitialized but the mailbox >> workqueue is still alive and polling for the mailbox message. >> This results in accessing the released control queue leading to >> null-ptr-deref. Fix it by unrolling the work queue cancellation >> and mailbox deinitialization in the order which they got >> initialized. >> >> Also remove the redundant scheduling of the mailbox task in >> idpf_vc_core_init. > > I think it might be better to move this last change into a separate patch > targeted at iwl rather than iwl-net. It isn't a fix, right? > If we do not make that change in this patch, it results in calling cancel_delayed_work_sync twice in case of an error in idpf_vc_core_init (err_intr_req). Looks like there aren't any side effects of calling cancel_delayed_work_sync twice. I will remove the said changes from this patch. Thanks, Pavan >> >> Fixes: 4930fbf419a7 ("idpf: add core init and interrupt request") >> Fixes: 34c21fa894a1 ("idpf: implement virtchnl transaction manager") >> Cc: stable@vger.kernel.org # 6.9+ >> Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com> >> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> > > ... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iwl-net 2/2] idpf: fix idpf_vc_core_init error path @ 2024-10-25 17:11 ` Linga, Pavan Kumar 0 siblings, 0 replies; 10+ messages in thread From: Linga, Pavan Kumar @ 2024-10-25 17:11 UTC (permalink / raw) To: Simon Horman; +Cc: intel-wired-lan, netdev, stable, Tarun K Singh On 10/24/2024 7:39 AM, Simon Horman wrote: > On Tue, Oct 22, 2024 at 10:35:27AM -0700, Pavan Kumar Linga wrote: >> In an event where the platform running the device control plane >> is rebooted, reset is detected on the driver. It releases >> all the resources and waits for the reset to complete. Once the >> reset is done, it tries to build the resources back. At this >> time if the device control plane is not yet started, then >> the driver timeouts on the virtchnl message and retries to >> establish the mailbox again. >> >> In the retry flow, mailbox is deinitialized but the mailbox >> workqueue is still alive and polling for the mailbox message. >> This results in accessing the released control queue leading to >> null-ptr-deref. Fix it by unrolling the work queue cancellation >> and mailbox deinitialization in the order which they got >> initialized. >> >> Also remove the redundant scheduling of the mailbox task in >> idpf_vc_core_init. > > I think it might be better to move this last change into a separate patch > targeted at iwl rather than iwl-net. It isn't a fix, right? > If we do not make that change in this patch, it results in calling cancel_delayed_work_sync twice in case of an error in idpf_vc_core_init (err_intr_req). Looks like there aren't any side effects of calling cancel_delayed_work_sync twice. I will remove the said changes from this patch. Thanks, Pavan >> >> Fixes: 4930fbf419a7 ("idpf: add core init and interrupt request") >> Fixes: 34c21fa894a1 ("idpf: implement virtchnl transaction manager") >> Cc: stable@vger.kernel.org # 6.9+ >> Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com> >> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> > > ... ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-10-25 17:11 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-22 17:35 [Intel-wired-lan] [PATCH iwl-net 0/2] fix reset issues Pavan Kumar Linga 2024-10-22 17:35 ` Pavan Kumar Linga 2024-10-22 17:35 ` [Intel-wired-lan] [PATCH iwl-net 1/2] idpf: avoid vport access in idpf_get_link_ksettings Pavan Kumar Linga 2024-10-22 17:35 ` Pavan Kumar Linga 2024-10-22 17:35 ` [Intel-wired-lan] [PATCH iwl-net 2/2] idpf: fix idpf_vc_core_init error path Pavan Kumar Linga 2024-10-22 17:35 ` Pavan Kumar Linga 2024-10-24 14:39 ` [Intel-wired-lan] " Simon Horman 2024-10-24 14:39 ` Simon Horman 2024-10-25 17:11 ` [Intel-wired-lan] " Linga, Pavan Kumar 2024-10-25 17:11 ` Linga, Pavan Kumar
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.