Commit 175c2412 authored by Muhammad Husaini Zulkifli's avatar Muhammad Husaini Zulkifli Committed by Tony Nguyen

igc: Fix TX Hang issue when QBV Gate is closed

If a user schedules a Gate Control List (GCL) to close one of
the QBV gates while also transmitting a packet to that closed gate,
TX Hang will be happen. HW would not drop any packet when the gate
is closed and keep queuing up in HW TX FIFO until the gate is re-opened.
This patch implements the solution to drop the packet for the closed
gate.

This patch will also reset the adapter to perform SW initialization
for each 1st Gate Control List (GCL) to avoid hang.
This is due to the HW design, where changing to TSN transmit mode
requires SW initialization. Intel Discrete I225/6 transmit mode
cannot be changed when in dynamic mode according to Software User
Manual Section 7.5.2.1. Subsequent Gate Control List (GCL) operations
will proceed without a reset, as they already are in TSN Mode.

Step to reproduce:

DUT:
1) Configure GCL List with certain gate close.

BASE=$(date +%s%N)
tc qdisc replace dev $IFACE parent root handle 100 taprio \
    num_tc 4 \
    map 0 1 2 3 3 3 3 3 3 3 3 3 3 3 3 3 \
    queues 1@0 1@1 1@2 1@3 \
    base-time $BASE \
    sched-entry S 0x8 500000 \
    sched-entry S 0x4 500000 \
    flags 0x2

2) Transmit the packet to closed gate. You may use udp_tai
application to transmit UDP packet to any of the closed gate.

./udp_tai -i <interface> -P 100000 -p 90 -c 1 -t <0/1> -u 30004

Fixes: ec50a9d4 ("igc: Add support for taprio offloading")
Co-developed-by: default avatarTan Tee Min <tee.min.tan@linux.intel.com>
Signed-off-by: default avatarTan Tee Min <tee.min.tan@linux.intel.com>
Tested-by: default avatarChwee Lin Choong <chwee.lin.choong@intel.com>
Signed-off-by: default avatarMuhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Tested-by: default avatarNaama Meir <naamax.meir@linux.intel.com>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent cca28cea
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include <linux/timecounter.h> #include <linux/timecounter.h>
#include <linux/net_tstamp.h> #include <linux/net_tstamp.h>
#include <linux/bitfield.h> #include <linux/bitfield.h>
#include <linux/hrtimer.h>
#include "igc_hw.h" #include "igc_hw.h"
...@@ -101,6 +102,8 @@ struct igc_ring { ...@@ -101,6 +102,8 @@ struct igc_ring {
u32 start_time; u32 start_time;
u32 end_time; u32 end_time;
u32 max_sdu; u32 max_sdu;
bool oper_gate_closed; /* Operating gate. True if the TX Queue is closed */
bool admin_gate_closed; /* Future gate. True if the TX Queue will be closed */
/* CBS parameters */ /* CBS parameters */
bool cbs_enable; /* indicates if CBS is enabled */ bool cbs_enable; /* indicates if CBS is enabled */
...@@ -160,6 +163,7 @@ struct igc_adapter { ...@@ -160,6 +163,7 @@ struct igc_adapter {
struct timer_list watchdog_timer; struct timer_list watchdog_timer;
struct timer_list dma_err_timer; struct timer_list dma_err_timer;
struct timer_list phy_info_timer; struct timer_list phy_info_timer;
struct hrtimer hrtimer;
u32 wol; u32 wol;
u32 en_mng_pt; u32 en_mng_pt;
...@@ -189,6 +193,8 @@ struct igc_adapter { ...@@ -189,6 +193,8 @@ struct igc_adapter {
ktime_t cycle_time; ktime_t cycle_time;
bool qbv_enable; bool qbv_enable;
u32 qbv_config_change_errors; u32 qbv_config_change_errors;
bool qbv_transition;
unsigned int qbv_count;
/* OS defined structs */ /* OS defined structs */
struct pci_dev *pdev; struct pci_dev *pdev;
......
...@@ -1572,6 +1572,9 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb, ...@@ -1572,6 +1572,9 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
first->bytecount = skb->len; first->bytecount = skb->len;
first->gso_segs = 1; first->gso_segs = 1;
if (adapter->qbv_transition || tx_ring->oper_gate_closed)
goto out_drop;
if (tx_ring->max_sdu > 0) { if (tx_ring->max_sdu > 0) {
u32 max_sdu = 0; u32 max_sdu = 0;
...@@ -3011,8 +3014,8 @@ static bool igc_clean_tx_irq(struct igc_q_vector *q_vector, int napi_budget) ...@@ -3011,8 +3014,8 @@ static bool igc_clean_tx_irq(struct igc_q_vector *q_vector, int napi_budget)
time_after(jiffies, tx_buffer->time_stamp + time_after(jiffies, tx_buffer->time_stamp +
(adapter->tx_timeout_factor * HZ)) && (adapter->tx_timeout_factor * HZ)) &&
!(rd32(IGC_STATUS) & IGC_STATUS_TXOFF) && !(rd32(IGC_STATUS) & IGC_STATUS_TXOFF) &&
(rd32(IGC_TDH(tx_ring->reg_idx)) != (rd32(IGC_TDH(tx_ring->reg_idx)) != readl(tx_ring->tail)) &&
readl(tx_ring->tail))) { !tx_ring->oper_gate_closed) {
/* detected Tx unit hang */ /* detected Tx unit hang */
netdev_err(tx_ring->netdev, netdev_err(tx_ring->netdev,
"Detected Tx Unit Hang\n" "Detected Tx Unit Hang\n"
...@@ -6102,6 +6105,8 @@ static int igc_tsn_clear_schedule(struct igc_adapter *adapter) ...@@ -6102,6 +6105,8 @@ static int igc_tsn_clear_schedule(struct igc_adapter *adapter)
adapter->base_time = 0; adapter->base_time = 0;
adapter->cycle_time = NSEC_PER_SEC; adapter->cycle_time = NSEC_PER_SEC;
adapter->qbv_config_change_errors = 0; adapter->qbv_config_change_errors = 0;
adapter->qbv_transition = false;
adapter->qbv_count = 0;
for (i = 0; i < adapter->num_tx_queues; i++) { for (i = 0; i < adapter->num_tx_queues; i++) {
struct igc_ring *ring = adapter->tx_ring[i]; struct igc_ring *ring = adapter->tx_ring[i];
...@@ -6109,6 +6114,8 @@ static int igc_tsn_clear_schedule(struct igc_adapter *adapter) ...@@ -6109,6 +6114,8 @@ static int igc_tsn_clear_schedule(struct igc_adapter *adapter)
ring->start_time = 0; ring->start_time = 0;
ring->end_time = NSEC_PER_SEC; ring->end_time = NSEC_PER_SEC;
ring->max_sdu = 0; ring->max_sdu = 0;
ring->oper_gate_closed = false;
ring->admin_gate_closed = false;
} }
return 0; return 0;
...@@ -6120,6 +6127,7 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter, ...@@ -6120,6 +6127,7 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
bool queue_configured[IGC_MAX_TX_QUEUES] = { }; bool queue_configured[IGC_MAX_TX_QUEUES] = { };
struct igc_hw *hw = &adapter->hw; struct igc_hw *hw = &adapter->hw;
u32 start_time = 0, end_time = 0; u32 start_time = 0, end_time = 0;
struct timespec64 now;
size_t n; size_t n;
int i; int i;
...@@ -6149,6 +6157,8 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter, ...@@ -6149,6 +6157,8 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
adapter->cycle_time = qopt->cycle_time; adapter->cycle_time = qopt->cycle_time;
adapter->base_time = qopt->base_time; adapter->base_time = qopt->base_time;
igc_ptp_read(adapter, &now);
for (n = 0; n < qopt->num_entries; n++) { for (n = 0; n < qopt->num_entries; n++) {
struct tc_taprio_sched_entry *e = &qopt->entries[n]; struct tc_taprio_sched_entry *e = &qopt->entries[n];
...@@ -6183,7 +6193,10 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter, ...@@ -6183,7 +6193,10 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
ring->start_time = start_time; ring->start_time = start_time;
ring->end_time = end_time; ring->end_time = end_time;
queue_configured[i] = true; if (ring->start_time >= adapter->cycle_time)
queue_configured[i] = false;
else
queue_configured[i] = true;
} }
start_time += e->interval; start_time += e->interval;
...@@ -6193,8 +6206,20 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter, ...@@ -6193,8 +6206,20 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
* If not, set the start and end time to be end time. * If not, set the start and end time to be end time.
*/ */
for (i = 0; i < adapter->num_tx_queues; i++) { for (i = 0; i < adapter->num_tx_queues; i++) {
struct igc_ring *ring = adapter->tx_ring[i];
if (!is_base_time_past(qopt->base_time, &now)) {
ring->admin_gate_closed = false;
} else {
ring->oper_gate_closed = false;
ring->admin_gate_closed = false;
}
if (!queue_configured[i]) { if (!queue_configured[i]) {
struct igc_ring *ring = adapter->tx_ring[i]; if (!is_base_time_past(qopt->base_time, &now))
ring->admin_gate_closed = true;
else
ring->oper_gate_closed = true;
ring->start_time = end_time; ring->start_time = end_time;
ring->end_time = end_time; ring->end_time = end_time;
...@@ -6575,6 +6600,27 @@ static const struct xdp_metadata_ops igc_xdp_metadata_ops = { ...@@ -6575,6 +6600,27 @@ static const struct xdp_metadata_ops igc_xdp_metadata_ops = {
.xmo_rx_timestamp = igc_xdp_rx_timestamp, .xmo_rx_timestamp = igc_xdp_rx_timestamp,
}; };
static enum hrtimer_restart igc_qbv_scheduling_timer(struct hrtimer *timer)
{
struct igc_adapter *adapter = container_of(timer, struct igc_adapter,
hrtimer);
unsigned int i;
adapter->qbv_transition = true;
for (i = 0; i < adapter->num_tx_queues; i++) {
struct igc_ring *tx_ring = adapter->tx_ring[i];
if (tx_ring->admin_gate_closed) {
tx_ring->admin_gate_closed = false;
tx_ring->oper_gate_closed = true;
} else {
tx_ring->oper_gate_closed = false;
}
}
adapter->qbv_transition = false;
return HRTIMER_NORESTART;
}
/** /**
* igc_probe - Device Initialization Routine * igc_probe - Device Initialization Routine
* @pdev: PCI device information struct * @pdev: PCI device information struct
...@@ -6753,6 +6799,9 @@ static int igc_probe(struct pci_dev *pdev, ...@@ -6753,6 +6799,9 @@ static int igc_probe(struct pci_dev *pdev,
INIT_WORK(&adapter->reset_task, igc_reset_task); INIT_WORK(&adapter->reset_task, igc_reset_task);
INIT_WORK(&adapter->watchdog_task, igc_watchdog_task); INIT_WORK(&adapter->watchdog_task, igc_watchdog_task);
hrtimer_init(&adapter->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
adapter->hrtimer.function = &igc_qbv_scheduling_timer;
/* Initialize link properties that are user-changeable */ /* Initialize link properties that are user-changeable */
adapter->fc_autoneg = true; adapter->fc_autoneg = true;
hw->mac.autoneg = true; hw->mac.autoneg = true;
...@@ -6856,6 +6905,7 @@ static void igc_remove(struct pci_dev *pdev) ...@@ -6856,6 +6905,7 @@ static void igc_remove(struct pci_dev *pdev)
cancel_work_sync(&adapter->reset_task); cancel_work_sync(&adapter->reset_task);
cancel_work_sync(&adapter->watchdog_task); cancel_work_sync(&adapter->watchdog_task);
hrtimer_cancel(&adapter->hrtimer);
/* Release control of h/w to f/w. If f/w is AMT enabled, this /* Release control of h/w to f/w. If f/w is AMT enabled, this
* would have already happened in close and is redundant. * would have already happened in close and is redundant.
......
...@@ -114,7 +114,6 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter) ...@@ -114,7 +114,6 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter)
static int igc_tsn_enable_offload(struct igc_adapter *adapter) static int igc_tsn_enable_offload(struct igc_adapter *adapter)
{ {
struct igc_hw *hw = &adapter->hw; struct igc_hw *hw = &adapter->hw;
bool tsn_mode_reconfig = false;
u32 tqavctrl, baset_l, baset_h; u32 tqavctrl, baset_l, baset_h;
u32 sec, nsec, cycle; u32 sec, nsec, cycle;
ktime_t base_time, systim; ktime_t base_time, systim;
...@@ -228,11 +227,10 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter) ...@@ -228,11 +227,10 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
tqavctrl = rd32(IGC_TQAVCTRL) & ~IGC_TQAVCTRL_FUTSCDDIS; tqavctrl = rd32(IGC_TQAVCTRL) & ~IGC_TQAVCTRL_FUTSCDDIS;
if (tqavctrl & IGC_TQAVCTRL_TRANSMIT_MODE_TSN)
tsn_mode_reconfig = true;
tqavctrl |= IGC_TQAVCTRL_TRANSMIT_MODE_TSN | IGC_TQAVCTRL_ENHANCED_QAV; tqavctrl |= IGC_TQAVCTRL_TRANSMIT_MODE_TSN | IGC_TQAVCTRL_ENHANCED_QAV;
adapter->qbv_count++;
cycle = adapter->cycle_time; cycle = adapter->cycle_time;
base_time = adapter->base_time; base_time = adapter->base_time;
...@@ -250,17 +248,28 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter) ...@@ -250,17 +248,28 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
*/ */
if ((rd32(IGC_BASET_H) || rd32(IGC_BASET_L)) && if ((rd32(IGC_BASET_H) || rd32(IGC_BASET_L)) &&
(adapter->tc_setup_type == TC_SETUP_QDISC_TAPRIO) && (adapter->tc_setup_type == TC_SETUP_QDISC_TAPRIO) &&
tsn_mode_reconfig) (adapter->qbv_count > 1))
adapter->qbv_config_change_errors++; adapter->qbv_config_change_errors++;
} else { } else {
/* According to datasheet section 7.5.2.9.3.3, FutScdDis bit if (igc_is_device_id_i226(hw)) {
* has to be configured before the cycle time and base time. ktime_t adjust_time, expires_time;
* Tx won't hang if there is a GCL is already running,
* so in this case we don't need to set FutScdDis. /* According to datasheet section 7.5.2.9.3.3, FutScdDis bit
*/ * has to be configured before the cycle time and base time.
if (igc_is_device_id_i226(hw) && * Tx won't hang if a GCL is already running,
!(rd32(IGC_BASET_H) || rd32(IGC_BASET_L))) * so in this case we don't need to set FutScdDis.
tqavctrl |= IGC_TQAVCTRL_FUTSCDDIS; */
if (!(rd32(IGC_BASET_H) || rd32(IGC_BASET_L)))
tqavctrl |= IGC_TQAVCTRL_FUTSCDDIS;
nsec = rd32(IGC_SYSTIML);
sec = rd32(IGC_SYSTIMH);
systim = ktime_set(sec, nsec);
adjust_time = adapter->base_time;
expires_time = ktime_sub_ns(adjust_time, systim);
hrtimer_start(&adapter->hrtimer, expires_time, HRTIMER_MODE_REL);
}
} }
wr32(IGC_TQAVCTRL, tqavctrl); wr32(IGC_TQAVCTRL, tqavctrl);
...@@ -306,7 +315,11 @@ int igc_tsn_offload_apply(struct igc_adapter *adapter) ...@@ -306,7 +315,11 @@ int igc_tsn_offload_apply(struct igc_adapter *adapter)
{ {
struct igc_hw *hw = &adapter->hw; struct igc_hw *hw = &adapter->hw;
if (netif_running(adapter->netdev) && igc_is_device_id_i225(hw)) { /* Per I225/6 HW Design Section 7.5.2.1, transmit mode
* cannot be changed dynamically. Require reset the adapter.
*/
if (netif_running(adapter->netdev) &&
(igc_is_device_id_i225(hw) || !adapter->qbv_count)) {
schedule_work(&adapter->reset_task); schedule_work(&adapter->reset_task);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment