Message ID | 20240426144408.1353962-1-aleksander.lobakin@intel.com (mailing list archive) |
---|---|
State | Awaiting Upstream |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [iwl] idpf: don't enable NAPI and interrupts prior to allocating Rx buffers | expand |
On Fri, Apr 26, 2024 at 04:44:08PM +0200, Alexander Lobakin wrote: > Currently, idpf enables NAPI and interrupts prior to allocating Rx > buffers. > This may lead to frame loss (there are no buffers to place incoming > frames) and even crashes on quick ifup-ifdown. Interrupts must be > enabled only after all the resources are here and available. > Split interrupt init into two phases: initialization and enabling, > and perform the second only after the queues are fully initialized. > Note that we can't just move interrupt initialization down the init > process, as the queues must have correct a ::q_vector pointer set > and NAPI already added in order to allocate buffers correctly. > Also, during the deinit process, disable HW interrupts first and > only then disable NAPI. Otherwise, there can be a HW event leading > to napi_schedule(), but the NAPI will already be unavailable. > > Fixes: d4d558718266 ("idpf: initialize interrupts and enable vport") > Reported-by: Michal Kubiak <michal.kubiak@intel.com> > Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> > Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com> Reviewed-by: Simon Horman <horms@kernel.org>
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Simon Horman > Sent: Monday, April 29, 2024 5:58 AM > To: Lobakin, Aleksander <aleksander.lobakin@intel.com> > Cc: Drewek, Wojciech <wojciech.drewek@intel.com>; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Eric Dumazet > <edumazet@google.com>; Kubiak, Michal <michal.kubiak@intel.com>; intel- > wired-lan@lists.osuosl.org; NEX SW NCIS OSDT ITP Upstreaming > <nex.sw.ncis.osdt.itp.upstreaming@intel.com>; Jakub Kicinski > <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David S. Miller > <davem@davemloft.net> > Subject: Re: [Intel-wired-lan] [PATCH iwl] idpf: don't enable NAPI and > interrupts prior to allocating Rx buffers > > On Fri, Apr 26, 2024 at 04:44:08PM +0200, Alexander Lobakin wrote: > > Currently, idpf enables NAPI and interrupts prior to allocating Rx > > buffers. > > This may lead to frame loss (there are no buffers to place incoming > > frames) and even crashes on quick ifup-ifdown. Interrupts must be > > enabled only after all the resources are here and available. > > Split interrupt init into two phases: initialization and enabling, > > and perform the second only after the queues are fully initialized. > > Note that we can't just move interrupt initialization down the init > > process, as the queues must have correct a ::q_vector pointer set > > and NAPI already added in order to allocate buffers correctly. > > Also, during the deinit process, disable HW interrupts first and > > only then disable NAPI. Otherwise, there can be a HW event leading > > to napi_schedule(), but the NAPI will already be unavailable. > > > > Fixes: d4d558718266 ("idpf: initialize interrupts and enable vport") > > Reported-by: Michal Kubiak <michal.kubiak@intel.com> > > Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> > > Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com> > > Reviewed-by: Simon Horman <horms@kernel.org> Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h index 3d046b81e507..551391e20464 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h @@ -990,6 +990,7 @@ int idpf_vport_intr_alloc(struct idpf_vport *vport); void idpf_vport_intr_update_itr_ena_irq(struct idpf_q_vector *q_vector); void idpf_vport_intr_deinit(struct idpf_vport *vport); int idpf_vport_intr_init(struct idpf_vport *vport); +void idpf_vport_intr_ena(struct idpf_vport *vport); enum pkt_hash_types idpf_ptype_to_htype(const struct idpf_rx_ptype_decoded *decoded); int idpf_config_rss(struct idpf_vport *vport); int idpf_init_rss(struct idpf_vport *vport); diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c index 5d3532c27d57..ae8a48c48070 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c @@ -1394,6 +1394,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool alloc_res) } idpf_rx_init_buf_tail(vport); + idpf_vport_intr_ena(vport); err = idpf_send_config_queues_msg(vport); if (err) { diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c index 285da2177ee4..b023704bbbda 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c @@ -3746,9 +3746,9 @@ static void idpf_vport_intr_ena_irq_all(struct idpf_vport *vport) */ void idpf_vport_intr_deinit(struct idpf_vport *vport) { + idpf_vport_intr_dis_irq_all(vport); idpf_vport_intr_napi_dis_all(vport); idpf_vport_intr_napi_del_all(vport); - idpf_vport_intr_dis_irq_all(vport); idpf_vport_intr_rel_irq(vport); } @@ -4179,7 +4179,6 @@ int idpf_vport_intr_init(struct idpf_vport *vport) idpf_vport_intr_map_vector_to_qs(vport); idpf_vport_intr_napi_add_all(vport); - idpf_vport_intr_napi_ena_all(vport); err = vport->adapter->dev_ops.reg_ops.intr_reg_init(vport); if (err) @@ -4193,17 +4192,20 @@ int idpf_vport_intr_init(struct idpf_vport *vport) if (err) goto unroll_vectors_alloc; - idpf_vport_intr_ena_irq_all(vport); - return 0; unroll_vectors_alloc: - idpf_vport_intr_napi_dis_all(vport); idpf_vport_intr_napi_del_all(vport); return err; } +void idpf_vport_intr_ena(struct idpf_vport *vport) +{ + idpf_vport_intr_napi_ena_all(vport); + idpf_vport_intr_ena_irq_all(vport); +} + /** * idpf_config_rss - Send virtchnl messages to configure RSS * @vport: virtual port