| Message ID | 20240604132155.3573752-6-maciej.fijalkowski@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ice: fix AF_XDP ZC timeout and concurrency issues | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/apply | fail | Patch does not apply to net-0 |
On Tue, Jun 04, 2024 at 03:21:52PM +0200, Maciej Fijalkowski wrote: > This so we prevent Tx timeout issues. One of conditions checked on > running in the background dev_watchdog() is netif_carrier_ok(), so let > us turn it off when we disable the queues that belong to a q_vector > where XSK pool is being configured. > > Fixes: 2d4238f55697 ("ice: Add support for AF_XDP") > Reviewed-by: Shannon Nelson <shannon.nelson@amd.com> > Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel) > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_xsk.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c > index 8ca7ff1a7e7f..21df6888961b 100644 > --- a/drivers/net/ethernet/intel/ice/ice_xsk.c > +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c > @@ -180,6 +180,7 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx) > } > > synchronize_net(); > + netif_carrier_off(vsi->netdev); > netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); > > ice_qvec_dis_irq(vsi, rx_ring, q_vector); > @@ -249,6 +250,7 @@ static int ice_qp_ena(struct ice_vsi *vsi, u16 q_idx) > ice_qvec_ena_irq(vsi, q_vector); > > netif_tx_start_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); > + netif_carrier_on(vsi->netdev); I would add checking the physical link state before calling 'netif_carrier_on()'. Please consider the scenario that the link is going physically *down* during the XSk pool configuration. In such a case we can wrongly set "carrier_on" uncoditionally. Please take a look at the followng suggestion: ice_get_link_status(vsi->port_info, &link_up); if (link_up) netif_carrier_on(vsi->netdev); Thanks, Michal
On Wed, Jun 26, 2024 at 02:21:39PM +0200, Michal Kubiak wrote: > On Tue, Jun 04, 2024 at 03:21:52PM +0200, Maciej Fijalkowski wrote: > > This so we prevent Tx timeout issues. One of conditions checked on > > running in the background dev_watchdog() is netif_carrier_ok(), so let > > us turn it off when we disable the queues that belong to a q_vector > > where XSK pool is being configured. > > > > Fixes: 2d4238f55697 ("ice: Add support for AF_XDP") > > Reviewed-by: Shannon Nelson <shannon.nelson@amd.com> > > Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel) > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > --- > > drivers/net/ethernet/intel/ice/ice_xsk.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c > > index 8ca7ff1a7e7f..21df6888961b 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_xsk.c > > +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c > > @@ -180,6 +180,7 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx) > > } > > > > synchronize_net(); > > + netif_carrier_off(vsi->netdev); > > netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); > > > > ice_qvec_dis_irq(vsi, rx_ring, q_vector); > > @@ -249,6 +250,7 @@ static int ice_qp_ena(struct ice_vsi *vsi, u16 q_idx) > > ice_qvec_ena_irq(vsi, q_vector); > > > > netif_tx_start_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); > > + netif_carrier_on(vsi->netdev); > > I would add checking the physical link state before calling > 'netif_carrier_on()'. Please consider the scenario that the link is > going physically *down* during the XSk pool configuration. > In such a case we can wrongly set "carrier_on" uncoditionally. > Please take a look at the followng suggestion: > > ice_get_link_status(vsi->port_info, &link_up); > if (link_up) > netif_carrier_on(vsi->netdev); Will include this in v4. Worth mentioning that it is safe to do so as there is always a service task that will turn carrier on for us. > > Thanks, > Michal
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c index 8ca7ff1a7e7f..21df6888961b 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -180,6 +180,7 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx) } synchronize_net(); + netif_carrier_off(vsi->netdev); netif_tx_stop_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); ice_qvec_dis_irq(vsi, rx_ring, q_vector); @@ -249,6 +250,7 @@ static int ice_qp_ena(struct ice_vsi *vsi, u16 q_idx) ice_qvec_ena_irq(vsi, q_vector); netif_tx_start_queue(netdev_get_tx_queue(vsi->netdev, q_idx)); + netif_carrier_on(vsi->netdev); clear_bit(ICE_CFG_BUSY, vsi->state); return fail;
This so we prevent Tx timeout issues. One of conditions checked on running in the background dev_watchdog() is netif_carrier_ok(), so let us turn it off when we disable the queues that belong to a q_vector where XSK pool is being configured. Fixes: 2d4238f55697 ("ice: Add support for AF_XDP") Reviewed-by: Shannon Nelson <shannon.nelson@amd.com> Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel) Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> --- drivers/net/ethernet/intel/ice/ice_xsk.c | 2 ++ 1 file changed, 2 insertions(+)