netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads
@ 2025-04-29 15:52 Maciej Fijalkowski
  2025-04-29 17:11 ` [Intel-wired-lan] " Loktionov, Aleksandr
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Maciej Fijalkowski @ 2025-04-29 15:52 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, michal.kubiak,
	Maciej Fijalkowski, Tobias Böhm, Marcus Wichelmann

Currently ixgbe driver checks periodically in its watchdog subtask if
there is anything to be transmitted (consdidering both Tx and XDP rings)
under state of carrier not being 'ok'. Such event is interpreted as Tx
hang and therefore results in interface reset.

This is currently problematic for ndo_xdp_xmit() as it is allowed to
produce descriptors when interface is going through reset or its carrier
is turned off.

Furthermore, XDP rings should not really be objects of Tx hang
detection. This mechanism is rather a matter of ndo_tx_timeout() being
called from dev_watchdog against Tx rings exposed to networking stack.

Taking into account issues described above, let us have a two fold fix -
do not respect XDP rings in local ixgbe watchdog and do not produce Tx
descriptors in ndo_xdp_xmit callback when there is some problem with
carrier currently. For now, keep the Tx hang checks in clean Tx irq
routine, but adjust it to not execute it for XDP rings.

Cc: Tobias Böhm <tobias.boehm@hetzner-cloud.de>
Reported-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de>
Closes: https://lore.kernel.org/netdev/eca1880f-253a-4955-afe6-732d7c6926ee@hetzner-cloud.de/
Fixes: 6453073987ba ("ixgbe: add initial support for xdp redirect")
Fixes: 33fdc82f0883 ("ixgbe: add support for XDP_TX action")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 34 ++++++-------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 467f81239e12..21bfea8aeb67 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -966,10 +966,6 @@ static void ixgbe_update_xoff_rx_lfc(struct ixgbe_adapter *adapter)
 	for (i = 0; i < adapter->num_tx_queues; i++)
 		clear_bit(__IXGBE_HANG_CHECK_ARMED,
 			  &adapter->tx_ring[i]->state);
-
-	for (i = 0; i < adapter->num_xdp_queues; i++)
-		clear_bit(__IXGBE_HANG_CHECK_ARMED,
-			  &adapter->xdp_ring[i]->state);
 }
 
 static void ixgbe_update_xoff_received(struct ixgbe_adapter *adapter)
@@ -1263,10 +1259,13 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
 				   total_bytes);
 	adapter->tx_ipsec += total_ipsec;
 
+	if (ring_is_xdp(tx_ring))
+		return !!budget;
+
 	if (check_for_tx_hang(tx_ring) && ixgbe_check_tx_hang(tx_ring)) {
 		/* schedule immediate reset if we believe we hung */
 		struct ixgbe_hw *hw = &adapter->hw;
-		e_err(drv, "Detected Tx Unit Hang %s\n"
+		e_err(drv, "Detected Tx Unit Hang\n"
 			"  Tx Queue             <%d>\n"
 			"  TDH, TDT             <%x>, <%x>\n"
 			"  next_to_use          <%x>\n"
@@ -1274,16 +1273,14 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
 			"tx_buffer_info[next_to_clean]\n"
 			"  time_stamp           <%lx>\n"
 			"  jiffies              <%lx>\n",
-			ring_is_xdp(tx_ring) ? "(XDP)" : "",
 			tx_ring->queue_index,
 			IXGBE_READ_REG(hw, IXGBE_TDH(tx_ring->reg_idx)),
 			IXGBE_READ_REG(hw, IXGBE_TDT(tx_ring->reg_idx)),
 			tx_ring->next_to_use, i,
 			tx_ring->tx_buffer_info[i].time_stamp, jiffies);
 
-		if (!ring_is_xdp(tx_ring))
-			netif_stop_subqueue(tx_ring->netdev,
-					    tx_ring->queue_index);
+		netif_stop_subqueue(tx_ring->netdev,
+				    tx_ring->queue_index);
 
 		e_info(probe,
 		       "tx hang %d detected on queue %d, resetting adapter\n",
@@ -1296,9 +1293,6 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
 		return true;
 	}
 
-	if (ring_is_xdp(tx_ring))
-		return !!budget;
-
 #define TX_WAKE_THRESHOLD (DESC_NEEDED * 2)
 	txq = netdev_get_tx_queue(tx_ring->netdev, tx_ring->queue_index);
 	if (!__netif_txq_completed_wake(txq, total_packets, total_bytes,
@@ -7791,12 +7785,9 @@ static void ixgbe_check_hang_subtask(struct ixgbe_adapter *adapter)
 		return;
 
 	/* Force detection of hung controller */
-	if (netif_carrier_ok(adapter->netdev)) {
+	if (netif_carrier_ok(adapter->netdev))
 		for (i = 0; i < adapter->num_tx_queues; i++)
 			set_check_for_tx_hang(adapter->tx_ring[i]);
-		for (i = 0; i < adapter->num_xdp_queues; i++)
-			set_check_for_tx_hang(adapter->xdp_ring[i]);
-	}
 
 	if (!(adapter->flags & IXGBE_FLAG_MSIX_ENABLED)) {
 		/*
@@ -8011,13 +8002,6 @@ static bool ixgbe_ring_tx_pending(struct ixgbe_adapter *adapter)
 			return true;
 	}
 
-	for (i = 0; i < adapter->num_xdp_queues; i++) {
-		struct ixgbe_ring *ring = adapter->xdp_ring[i];
-
-		if (ring->next_to_use != ring->next_to_clean)
-			return true;
-	}
-
 	return false;
 }
 
@@ -10742,6 +10726,10 @@ static int ixgbe_xdp_xmit(struct net_device *dev, int n,
 	if (unlikely(test_bit(__IXGBE_DOWN, &adapter->state)))
 		return -ENETDOWN;
 
+	if (!netif_carrier_ok(adapter->netdev) ||
+	    !netif_running(adapter->netdev))
+		return -ENETDOWN;
+
 	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
 		return -EINVAL;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [Intel-wired-lan] [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads
  2025-04-29 15:52 [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads Maciej Fijalkowski
@ 2025-04-29 17:11 ` Loktionov, Aleksandr
  2025-04-30  8:55 ` Dawid Osuchowski
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Loktionov, Aleksandr @ 2025-04-29 17:11 UTC (permalink / raw)
  To: Fijalkowski, Maciej, intel-wired-lan@lists.osuosl.org
  Cc: netdev@vger.kernel.org, Nguyen, Anthony L, Karlsson, Magnus,
	Kubiak, Michal, Fijalkowski, Maciej, Tobias Böhm,
	Marcus Wichelmann



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Maciej Fijalkowski
> Sent: Tuesday, April 29, 2025 5:52 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Karlsson, Magnus
> <magnus.karlsson@intel.com>; Kubiak, Michal <michal.kubiak@intel.com>;
> Fijalkowski, Maciej <maciej.fijalkowski@intel.com>; Tobias Böhm
> <tobias.boehm@hetzner-cloud.de>; Marcus Wichelmann
> <marcus.wichelmann@hetzner-cloud.de>
> Subject: [Intel-wired-lan] [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads
> 
> Currently ixgbe driver checks periodically in its watchdog subtask if there is
> anything to be transmitted (consdidering both Tx and XDP rings) under state
> of carrier not being 'ok'. Such event is interpreted as Tx hang and therefore
> results in interface reset.
> 
> This is currently problematic for ndo_xdp_xmit() as it is allowed to produce
> descriptors when interface is going through reset or its carrier is turned off.
> 
> Furthermore, XDP rings should not really be objects of Tx hang detection. This
> mechanism is rather a matter of ndo_tx_timeout() being called from
> dev_watchdog against Tx rings exposed to networking stack.
> 
> Taking into account issues described above, let us have a two fold fix - do not
> respect XDP rings in local ixgbe watchdog and do not produce Tx descriptors in
> ndo_xdp_xmit callback when there is some problem with carrier currently. For
> now, keep the Tx hang checks in clean Tx irq routine, but adjust it to not
> execute it for XDP rings.
> 
> Cc: Tobias Böhm <tobias.boehm@hetzner-cloud.de>
> Reported-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de>
> Closes: https://lore.kernel.org/netdev/eca1880f-253a-4955-afe6-
> 732d7c6926ee@hetzner-cloud.de/
> Fixes: 6453073987ba ("ixgbe: add initial support for xdp redirect")
> Fixes: 33fdc82f0883 ("ixgbe: add support for XDP_TX action")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 34 ++++++-------------
>  1 file changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 467f81239e12..21bfea8aeb67 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -966,10 +966,6 @@ static void ixgbe_update_xoff_rx_lfc(struct
> ixgbe_adapter *adapter)
>  	for (i = 0; i < adapter->num_tx_queues; i++)
>  		clear_bit(__IXGBE_HANG_CHECK_ARMED,
>  			  &adapter->tx_ring[i]->state);
> -
> -	for (i = 0; i < adapter->num_xdp_queues; i++)
> -		clear_bit(__IXGBE_HANG_CHECK_ARMED,
> -			  &adapter->xdp_ring[i]->state);
>  }
> 
>  static void ixgbe_update_xoff_received(struct ixgbe_adapter *adapter) @@ -
> 1263,10 +1259,13 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector
> *q_vector,
>  				   total_bytes);
>  	adapter->tx_ipsec += total_ipsec;
> 
> +	if (ring_is_xdp(tx_ring))
> +		return !!budget;
> +
>  	if (check_for_tx_hang(tx_ring) && ixgbe_check_tx_hang(tx_ring)) {
>  		/* schedule immediate reset if we believe we hung */
>  		struct ixgbe_hw *hw = &adapter->hw;
> -		e_err(drv, "Detected Tx Unit Hang %s\n"
> +		e_err(drv, "Detected Tx Unit Hang\n"
>  			"  Tx Queue             <%d>\n"
>  			"  TDH, TDT             <%x>, <%x>\n"
>  			"  next_to_use          <%x>\n"
> @@ -1274,16 +1273,14 @@ static bool ixgbe_clean_tx_irq(struct
> ixgbe_q_vector *q_vector,
>  			"tx_buffer_info[next_to_clean]\n"
>  			"  time_stamp           <%lx>\n"
>  			"  jiffies              <%lx>\n",
> -			ring_is_xdp(tx_ring) ? "(XDP)" : "",
>  			tx_ring->queue_index,
>  			IXGBE_READ_REG(hw, IXGBE_TDH(tx_ring->reg_idx)),
>  			IXGBE_READ_REG(hw, IXGBE_TDT(tx_ring->reg_idx)),
>  			tx_ring->next_to_use, i,
>  			tx_ring->tx_buffer_info[i].time_stamp, jiffies);
> 
> -		if (!ring_is_xdp(tx_ring))
> -			netif_stop_subqueue(tx_ring->netdev,
> -					    tx_ring->queue_index);
> +		netif_stop_subqueue(tx_ring->netdev,
> +				    tx_ring->queue_index);
> 
>  		e_info(probe,
>  		       "tx hang %d detected on queue %d, resetting adapter\n",
> @@ -1296,9 +1293,6 @@ static bool ixgbe_clean_tx_irq(struct
> ixgbe_q_vector *q_vector,
>  		return true;
>  	}
> 
> -	if (ring_is_xdp(tx_ring))
> -		return !!budget;
> -
>  #define TX_WAKE_THRESHOLD (DESC_NEEDED * 2)
>  	txq = netdev_get_tx_queue(tx_ring->netdev, tx_ring->queue_index);
>  	if (!__netif_txq_completed_wake(txq, total_packets, total_bytes, @@
> -7791,12 +7785,9 @@ static void ixgbe_check_hang_subtask(struct
> ixgbe_adapter *adapter)
>  		return;
> 
>  	/* Force detection of hung controller */
> -	if (netif_carrier_ok(adapter->netdev)) {
> +	if (netif_carrier_ok(adapter->netdev))
>  		for (i = 0; i < adapter->num_tx_queues; i++)
>  			set_check_for_tx_hang(adapter->tx_ring[i]);
> -		for (i = 0; i < adapter->num_xdp_queues; i++)
> -			set_check_for_tx_hang(adapter->xdp_ring[i]);
> -	}
> 
>  	if (!(adapter->flags & IXGBE_FLAG_MSIX_ENABLED)) {
>  		/*
> @@ -8011,13 +8002,6 @@ static bool ixgbe_ring_tx_pending(struct
> ixgbe_adapter *adapter)
>  			return true;
>  	}
> 
> -	for (i = 0; i < adapter->num_xdp_queues; i++) {
> -		struct ixgbe_ring *ring = adapter->xdp_ring[i];
> -
> -		if (ring->next_to_use != ring->next_to_clean)
> -			return true;
> -	}
> -
>  	return false;
>  }
> 
> @@ -10742,6 +10726,10 @@ static int ixgbe_xdp_xmit(struct net_device
> *dev, int n,
>  	if (unlikely(test_bit(__IXGBE_DOWN, &adapter->state)))
>  		return -ENETDOWN;
> 
> +	if (!netif_carrier_ok(adapter->netdev) ||
> +	    !netif_running(adapter->netdev))
> +		return -ENETDOWN;
> +
>  	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
>  		return -EINVAL;
> 
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-wired-lan] [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads
  2025-04-29 15:52 [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads Maciej Fijalkowski
  2025-04-29 17:11 ` [Intel-wired-lan] " Loktionov, Aleksandr
@ 2025-04-30  8:55 ` Dawid Osuchowski
  2025-05-23 16:41 ` Marcus Wichelmann
  2025-08-05 13:06 ` Marcus Wichelmann
  3 siblings, 0 replies; 6+ messages in thread
From: Dawid Osuchowski @ 2025-04-30  8:55 UTC (permalink / raw)
  To: Maciej Fijalkowski, intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, michal.kubiak,
	Tobias Böhm, Marcus Wichelmann

On 2025-04-29 5:52 PM, Maciej Fijalkowski wrote:
> Currently ixgbe driver checks periodically in its watchdog subtask if
> there is anything to be transmitted (consdidering both Tx and XDP rings)

typo: s/consdidering/considering

> under state of carrier not being 'ok'. Such event is interpreted as Tx
> hang and therefore results in interface reset.
> 
> This is currently problematic for ndo_xdp_xmit() as it is allowed to
> produce descriptors when interface is going through reset or its carrier
> is turned off.
> 
> Furthermore, XDP rings should not really be objects of Tx hang
> detection. This mechanism is rather a matter of ndo_tx_timeout() being
> called from dev_watchdog against Tx rings exposed to networking stack.
> 
> Taking into account issues described above, let us have a two fold fix -
> do not respect XDP rings in local ixgbe watchdog and do not produce Tx
> descriptors in ndo_xdp_xmit callback when there is some problem with
> carrier currently. For now, keep the Tx hang checks in clean Tx irq
> routine, but adjust it to not execute it for XDP rings.

suggestion: s/adjust it to not execute it/adjust it to not execute

Best regards,
Dawid

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads
  2025-04-29 15:52 [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads Maciej Fijalkowski
  2025-04-29 17:11 ` [Intel-wired-lan] " Loktionov, Aleksandr
  2025-04-30  8:55 ` Dawid Osuchowski
@ 2025-05-23 16:41 ` Marcus Wichelmann
  2025-08-05 13:06 ` Marcus Wichelmann
  3 siblings, 0 replies; 6+ messages in thread
From: Marcus Wichelmann @ 2025-05-23 16:41 UTC (permalink / raw)
  To: Maciej Fijalkowski, intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, michal.kubiak,
	Tobias Böhm

Am 29.04.25 um 17:52 schrieb Maciej Fijalkowski:
> Currently ixgbe driver checks periodically in its watchdog subtask if
> there is anything to be transmitted (consdidering both Tx and XDP rings)
> under state of carrier not being 'ok'. Such event is interpreted as Tx
> hang and therefore results in interface reset.
> 
> This is currently problematic for ndo_xdp_xmit() as it is allowed to
> produce descriptors when interface is going through reset or its carrier
> is turned off.
> 
> Furthermore, XDP rings should not really be objects of Tx hang
> detection. This mechanism is rather a matter of ndo_tx_timeout() being
> called from dev_watchdog against Tx rings exposed to networking stack.
> 
> Taking into account issues described above, let us have a two fold fix -
> do not respect XDP rings in local ixgbe watchdog and do not produce Tx
> descriptors in ndo_xdp_xmit callback when there is some problem with
> carrier currently. For now, keep the Tx hang checks in clean Tx irq
> routine, but adjust it to not execute it for XDP rings.
> 
> Cc: Tobias Böhm <tobias.boehm@hetzner-cloud.de>
> Reported-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de>
> Closes: https://lore.kernel.org/netdev/eca1880f-253a-4955-afe6-732d7c6926ee@hetzner-cloud.de/
> Fixes: 6453073987ba ("ixgbe: add initial support for xdp redirect")
> Fixes: 33fdc82f0883 ("ixgbe: add support for XDP_TX action")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 34 ++++++-------------
>  1 file changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 467f81239e12..21bfea8aeb67 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -966,10 +966,6 @@ static void ixgbe_update_xoff_rx_lfc(struct ixgbe_adapter *adapter)
>  	for (i = 0; i < adapter->num_tx_queues; i++)
>  		clear_bit(__IXGBE_HANG_CHECK_ARMED,
>  			  &adapter->tx_ring[i]->state);
> -
> -	for (i = 0; i < adapter->num_xdp_queues; i++)
> -		clear_bit(__IXGBE_HANG_CHECK_ARMED,
> -			  &adapter->xdp_ring[i]->state);
>  }
>  
>  static void ixgbe_update_xoff_received(struct ixgbe_adapter *adapter)
> @@ -1263,10 +1259,13 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
>  				   total_bytes);
>  	adapter->tx_ipsec += total_ipsec;
>  
> +	if (ring_is_xdp(tx_ring))
> +		return !!budget;
> +
>  	if (check_for_tx_hang(tx_ring) && ixgbe_check_tx_hang(tx_ring)) {
>  		/* schedule immediate reset if we believe we hung */
>  		struct ixgbe_hw *hw = &adapter->hw;
> -		e_err(drv, "Detected Tx Unit Hang %s\n"
> +		e_err(drv, "Detected Tx Unit Hang\n"
>  			"  Tx Queue             <%d>\n"
>  			"  TDH, TDT             <%x>, <%x>\n"
>  			"  next_to_use          <%x>\n"
> @@ -1274,16 +1273,14 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
>  			"tx_buffer_info[next_to_clean]\n"
>  			"  time_stamp           <%lx>\n"
>  			"  jiffies              <%lx>\n",
> -			ring_is_xdp(tx_ring) ? "(XDP)" : "",
>  			tx_ring->queue_index,
>  			IXGBE_READ_REG(hw, IXGBE_TDH(tx_ring->reg_idx)),
>  			IXGBE_READ_REG(hw, IXGBE_TDT(tx_ring->reg_idx)),
>  			tx_ring->next_to_use, i,
>  			tx_ring->tx_buffer_info[i].time_stamp, jiffies);
>  
> -		if (!ring_is_xdp(tx_ring))
> -			netif_stop_subqueue(tx_ring->netdev,
> -					    tx_ring->queue_index);
> +		netif_stop_subqueue(tx_ring->netdev,
> +				    tx_ring->queue_index);
>  
>  		e_info(probe,
>  		       "tx hang %d detected on queue %d, resetting adapter\n",
> @@ -1296,9 +1293,6 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
>  		return true;
>  	}
>  
> -	if (ring_is_xdp(tx_ring))
> -		return !!budget;
> -
>  #define TX_WAKE_THRESHOLD (DESC_NEEDED * 2)
>  	txq = netdev_get_tx_queue(tx_ring->netdev, tx_ring->queue_index);
>  	if (!__netif_txq_completed_wake(txq, total_packets, total_bytes,
> @@ -7791,12 +7785,9 @@ static void ixgbe_check_hang_subtask(struct ixgbe_adapter *adapter)
>  		return;
>  
>  	/* Force detection of hung controller */
> -	if (netif_carrier_ok(adapter->netdev)) {
> +	if (netif_carrier_ok(adapter->netdev))
>  		for (i = 0; i < adapter->num_tx_queues; i++)
>  			set_check_for_tx_hang(adapter->tx_ring[i]);
> -		for (i = 0; i < adapter->num_xdp_queues; i++)
> -			set_check_for_tx_hang(adapter->xdp_ring[i]);
> -	}
>  
>  	if (!(adapter->flags & IXGBE_FLAG_MSIX_ENABLED)) {
>  		/*
> @@ -8011,13 +8002,6 @@ static bool ixgbe_ring_tx_pending(struct ixgbe_adapter *adapter)
>  			return true;
>  	}
>  
> -	for (i = 0; i < adapter->num_xdp_queues; i++) {
> -		struct ixgbe_ring *ring = adapter->xdp_ring[i];
> -
> -		if (ring->next_to_use != ring->next_to_clean)
> -			return true;
> -	}
> -
>  	return false;
>  }
>  
> @@ -10742,6 +10726,10 @@ static int ixgbe_xdp_xmit(struct net_device *dev, int n,
>  	if (unlikely(test_bit(__IXGBE_DOWN, &adapter->state)))
>  		return -ENETDOWN;
>  
> +	if (!netif_carrier_ok(adapter->netdev) ||
> +	    !netif_running(adapter->netdev))
> +		return -ENETDOWN;
> +
>  	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
>  		return -EINVAL;
>  

Hi,

thank you very much for this patch.

We have done more tests now in a production-like environment and I can confirm again
that this solves our issue and no more interface resets occur.

Tested-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de>

Kind regards,
Marcus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads
  2025-04-29 15:52 [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads Maciej Fijalkowski
                   ` (2 preceding siblings ...)
  2025-05-23 16:41 ` Marcus Wichelmann
@ 2025-08-05 13:06 ` Marcus Wichelmann
  2025-08-05 13:56   ` Maciej Fijalkowski
  3 siblings, 1 reply; 6+ messages in thread
From: Marcus Wichelmann @ 2025-08-05 13:06 UTC (permalink / raw)
  To: Maciej Fijalkowski, intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, michal.kubiak, sdn

Am 29.04.25 um 17:52 schrieb Maciej Fijalkowski:
> Currently ixgbe driver checks periodically in its watchdog subtask if
> there is anything to be transmitted (consdidering both Tx and XDP rings)
> under state of carrier not being 'ok'. Such event is interpreted as Tx
> hang and therefore results in interface reset.
> 
> This is currently problematic for ndo_xdp_xmit() as it is allowed to
> produce descriptors when interface is going through reset or its carrier
> is turned off.
> 
> Furthermore, XDP rings should not really be objects of Tx hang
> detection. This mechanism is rather a matter of ndo_tx_timeout() being
> called from dev_watchdog against Tx rings exposed to networking stack.
> 
> Taking into account issues described above, let us have a two fold fix -
> do not respect XDP rings in local ixgbe watchdog and do not produce Tx
> descriptors in ndo_xdp_xmit callback when there is some problem with
> carrier currently. For now, keep the Tx hang checks in clean Tx irq
> routine, but adjust it to not execute it for XDP rings.
> 
> Cc: Tobias Böhm <tobias.boehm@hetzner-cloud.de>
> Reported-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de>
> Closes: https://lore.kernel.org/netdev/eca1880f-253a-4955-afe6-732d7c6926ee@hetzner-cloud.de/
> Fixes: 6453073987ba ("ixgbe: add initial support for xdp redirect")
> Fixes: 33fdc82f0883 ("ixgbe: add support for XDP_TX action")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> [...]

Hi,

could you please consider submitting this patch (or a newer version)
for being merged into mainline?

This would help us not having to build our own kernels with this patch
for forever.

Thanks!

Marcus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads
  2025-08-05 13:06 ` Marcus Wichelmann
@ 2025-08-05 13:56   ` Maciej Fijalkowski
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej Fijalkowski @ 2025-08-05 13:56 UTC (permalink / raw)
  To: Marcus Wichelmann
  Cc: intel-wired-lan, netdev, anthony.l.nguyen, magnus.karlsson,
	michal.kubiak, sdn

On Tue, Aug 05, 2025 at 03:06:29PM +0200, Marcus Wichelmann wrote:
> Am 29.04.25 um 17:52 schrieb Maciej Fijalkowski:
> > Currently ixgbe driver checks periodically in its watchdog subtask if
> > there is anything to be transmitted (consdidering both Tx and XDP rings)
> > under state of carrier not being 'ok'. Such event is interpreted as Tx
> > hang and therefore results in interface reset.
> > 
> > This is currently problematic for ndo_xdp_xmit() as it is allowed to
> > produce descriptors when interface is going through reset or its carrier
> > is turned off.
> > 
> > Furthermore, XDP rings should not really be objects of Tx hang
> > detection. This mechanism is rather a matter of ndo_tx_timeout() being
> > called from dev_watchdog against Tx rings exposed to networking stack.
> > 
> > Taking into account issues described above, let us have a two fold fix -
> > do not respect XDP rings in local ixgbe watchdog and do not produce Tx
> > descriptors in ndo_xdp_xmit callback when there is some problem with
> > carrier currently. For now, keep the Tx hang checks in clean Tx irq
> > routine, but adjust it to not execute it for XDP rings.
> > 
> > Cc: Tobias Böhm <tobias.boehm@hetzner-cloud.de>
> > Reported-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de>
> > Closes: https://lore.kernel.org/netdev/eca1880f-253a-4955-afe6-732d7c6926ee@hetzner-cloud.de/
> > Fixes: 6453073987ba ("ixgbe: add initial support for xdp redirect")
> > Fixes: 33fdc82f0883 ("ixgbe: add support for XDP_TX action")
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > [...]
> 
> Hi,
> 
> could you please consider submitting this patch (or a newer version)
> for being merged into mainline?
> 
> This would help us not having to build our own kernels with this patch
> for forever.

Somehow I assumed this went through the process and our maintainers took
care of it - apologies for this inconvenience and let me address it.

> 
> Thanks!
> 
> Marcus

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-08-05 13:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-29 15:52 [PATCH iwl-net] ixgbe: fix ndo_xdp_xmit() workloads Maciej Fijalkowski
2025-04-29 17:11 ` [Intel-wired-lan] " Loktionov, Aleksandr
2025-04-30  8:55 ` Dawid Osuchowski
2025-05-23 16:41 ` Marcus Wichelmann
2025-08-05 13:06 ` Marcus Wichelmann
2025-08-05 13:56   ` Maciej Fijalkowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).