| Message ID | 20240201154219.607338-3-maciej.fijalkowski@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | i40e: disable XDP Tx queues on ifdown | expand |
On Thu, Feb 01, 2024 at 04:42:19PM +0100, Maciej Fijalkowski wrote: > Seth reported that on his side XDP traffic can not survive a round of > down/up against i40e interface. Dmesg output was telling us that we were > not able to disable the very first XDP ring. That was due to the fact > that in i40e_vsi_stop_rings() in a pre-work that is done before calling > i40e_vsi_wait_queues_disabled(), XDP Tx queues were not taken into the > account. > > To fix this, let us distinguish between Rx and Tx queue boundaries and > take into the account XDP queues for Tx side. > > Reported-by: Seth Forshee <sforshee@kernel.org> > Closes: https://lore.kernel.org/netdev/ZbkE7Ep1N1Ou17sA@do-x1extreme/ > Fixes: 65662a8dcdd0 ("i40e: Fix logic of disabling queues") > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> This fixes the issue we're seeing. Thanks! Tested-by: Seth Forshee <sforshee@kernel.org> > --- > drivers/net/ethernet/intel/i40e/i40e_main.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c > index 2c46a5e7d222..907be56965f5 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c > @@ -4926,21 +4926,22 @@ int i40e_vsi_start_rings(struct i40e_vsi *vsi) > void i40e_vsi_stop_rings(struct i40e_vsi *vsi) > { > struct i40e_pf *pf = vsi->back; > - int pf_q, q_end; > + u32 pf_q, tx_q_end, rx_q_end; > > /* When port TX is suspended, don't wait */ > if (test_bit(__I40E_PORT_SUSPENDED, vsi->back->state)) > return i40e_vsi_stop_rings_no_wait(vsi); > > - q_end = vsi->base_queue + vsi->num_queue_pairs; > - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) > - i40e_pre_tx_queue_cfg(&pf->hw, (u32)pf_q, false); > + tx_q_end = vsi->alloc_queue_pairs * (i40e_enabled_xdp_vsi(vsi) ? 2 : 1); > + for (pf_q = vsi->base_queue; pf_q < tx_q_end; pf_q++) > + i40e_pre_tx_queue_cfg(&pf->hw, pf_q, false); > > - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) > + rx_q_end = vsi->base_queue + vsi->num_queue_pairs; > + for (pf_q = vsi->base_queue; pf_q < rx_q_end; pf_q++) > i40e_control_rx_q(pf, pf_q, false); > > msleep(I40E_DISABLE_TX_GAP_MSEC); > - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) > + for (pf_q = vsi->base_queue; pf_q < tx_q_end; pf_q++) > wr32(&pf->hw, I40E_QTX_ENA(pf_q), 0); > > i40e_vsi_wait_queues_disabled(vsi); > -- > 2.34.1 >
On Thu, Feb 01, 2024 at 04:42:19PM +0100, Maciej Fijalkowski wrote: > Seth reported that on his side XDP traffic can not survive a round of > down/up against i40e interface. Dmesg output was telling us that we were > not able to disable the very first XDP ring. That was due to the fact > that in i40e_vsi_stop_rings() in a pre-work that is done before calling > i40e_vsi_wait_queues_disabled(), XDP Tx queues were not taken into the > account. > > To fix this, let us distinguish between Rx and Tx queue boundaries and > take into the account XDP queues for Tx side. > > Reported-by: Seth Forshee <sforshee@kernel.org> > Closes: https://lore.kernel.org/netdev/ZbkE7Ep1N1Ou17sA@do-x1extreme/ > Fixes: 65662a8dcdd0 ("i40e: Fix logic of disabling queues") > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Reviewed-by: Simon Horman <horms@kernel.org>
On Thu, Feb 01, 2024 at 12:40:22PM -0600, Seth Forshee wrote: > On Thu, Feb 01, 2024 at 04:42:19PM +0100, Maciej Fijalkowski wrote: > > Seth reported that on his side XDP traffic can not survive a round of > > down/up against i40e interface. Dmesg output was telling us that we were > > not able to disable the very first XDP ring. That was due to the fact > > that in i40e_vsi_stop_rings() in a pre-work that is done before calling > > i40e_vsi_wait_queues_disabled(), XDP Tx queues were not taken into the > > account. > > > > To fix this, let us distinguish between Rx and Tx queue boundaries and > > take into the account XDP queues for Tx side. > > > > Reported-by: Seth Forshee <sforshee@kernel.org> > > Closes: https://lore.kernel.org/netdev/ZbkE7Ep1N1Ou17sA@do-x1extreme/ > > Fixes: 65662a8dcdd0 ("i40e: Fix logic of disabling queues") > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > This fixes the issue we're seeing. Thanks! > > Tested-by: Seth Forshee <sforshee@kernel.org> > > > --- > > drivers/net/ethernet/intel/i40e/i40e_main.c | 13 +++++++------ > > 1 file changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c > > index 2c46a5e7d222..907be56965f5 100644 > > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c > > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c > > @@ -4926,21 +4926,22 @@ int i40e_vsi_start_rings(struct i40e_vsi *vsi) > > void i40e_vsi_stop_rings(struct i40e_vsi *vsi) > > { > > struct i40e_pf *pf = vsi->back; > > - int pf_q, q_end; > > + u32 pf_q, tx_q_end, rx_q_end; > > > > /* When port TX is suspended, don't wait */ > > if (test_bit(__I40E_PORT_SUSPENDED, vsi->back->state)) > > return i40e_vsi_stop_rings_no_wait(vsi); > > > > - q_end = vsi->base_queue + vsi->num_queue_pairs; > > - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) > > - i40e_pre_tx_queue_cfg(&pf->hw, (u32)pf_q, false); > > + tx_q_end = vsi->alloc_queue_pairs * (i40e_enabled_xdp_vsi(vsi) ? 2 : 1); While this fixes one thing, it breaks another ;) vsi->base_queue needs to be involved into tx_q_end, otherwise anything besides PF0 would not go through Tx loops. I noticed that FDIR VSI started to get Tx disable timeouts with this patch. Will send a v2. > > + for (pf_q = vsi->base_queue; pf_q < tx_q_end; pf_q++) > > + i40e_pre_tx_queue_cfg(&pf->hw, pf_q, false); > > > > - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) > > + rx_q_end = vsi->base_queue + vsi->num_queue_pairs; > > + for (pf_q = vsi->base_queue; pf_q < rx_q_end; pf_q++) > > i40e_control_rx_q(pf, pf_q, false); > > > > msleep(I40E_DISABLE_TX_GAP_MSEC); > > - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) > > + for (pf_q = vsi->base_queue; pf_q < tx_q_end; pf_q++) > > wr32(&pf->hw, I40E_QTX_ENA(pf_q), 0); > > > > i40e_vsi_wait_queues_disabled(vsi); > > -- > > 2.34.1 > > >
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 2c46a5e7d222..907be56965f5 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -4926,21 +4926,22 @@ int i40e_vsi_start_rings(struct i40e_vsi *vsi) void i40e_vsi_stop_rings(struct i40e_vsi *vsi) { struct i40e_pf *pf = vsi->back; - int pf_q, q_end; + u32 pf_q, tx_q_end, rx_q_end; /* When port TX is suspended, don't wait */ if (test_bit(__I40E_PORT_SUSPENDED, vsi->back->state)) return i40e_vsi_stop_rings_no_wait(vsi); - q_end = vsi->base_queue + vsi->num_queue_pairs; - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) - i40e_pre_tx_queue_cfg(&pf->hw, (u32)pf_q, false); + tx_q_end = vsi->alloc_queue_pairs * (i40e_enabled_xdp_vsi(vsi) ? 2 : 1); + for (pf_q = vsi->base_queue; pf_q < tx_q_end; pf_q++) + i40e_pre_tx_queue_cfg(&pf->hw, pf_q, false); - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) + rx_q_end = vsi->base_queue + vsi->num_queue_pairs; + for (pf_q = vsi->base_queue; pf_q < rx_q_end; pf_q++) i40e_control_rx_q(pf, pf_q, false); msleep(I40E_DISABLE_TX_GAP_MSEC); - for (pf_q = vsi->base_queue; pf_q < q_end; pf_q++) + for (pf_q = vsi->base_queue; pf_q < tx_q_end; pf_q++) wr32(&pf->hw, I40E_QTX_ENA(pf_q), 0); i40e_vsi_wait_queues_disabled(vsi);
Seth reported that on his side XDP traffic can not survive a round of down/up against i40e interface. Dmesg output was telling us that we were not able to disable the very first XDP ring. That was due to the fact that in i40e_vsi_stop_rings() in a pre-work that is done before calling i40e_vsi_wait_queues_disabled(), XDP Tx queues were not taken into the account. To fix this, let us distinguish between Rx and Tx queue boundaries and take into the account XDP queues for Tx side. Reported-by: Seth Forshee <sforshee@kernel.org> Closes: https://lore.kernel.org/netdev/ZbkE7Ep1N1Ou17sA@do-x1extreme/ Fixes: 65662a8dcdd0 ("i40e: Fix logic of disabling queues") Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> --- drivers/net/ethernet/intel/i40e/i40e_main.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)