From 77d5dfca41ef00dbe4368ba05297adc32d5e5741 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 11 May 2012 08:32:19 +0000 Subject: ixgbevf: Drop all dead or unnecessary code There is a large amount of code present in this driver to support features that either do no exist or are not supported such ask packet split, DCA, or RSC. This patch strips out almost all of that code and in the case of conditionals based on unused flags I am flatting the code out to just the path that would have been selected. Signed-off-by: Alexander Duyck Signed-off-by: Greg Rose Tested-by: Sibai Li Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/ixgbevf/ethtool.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'drivers/net/ethernet/intel/ixgbevf/ethtool.c') diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c index e8dddf572d38..af9c6570f4b5 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c +++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c @@ -43,7 +43,6 @@ #define IXGBE_ALL_RAR_ENTRIES 16 -#ifdef ETHTOOL_GSTATS struct ixgbe_stats { char stat_string[ETH_GSTRING_LEN]; int sizeof_stat; @@ -75,21 +74,17 @@ static const struct ixgbe_stats ixgbe_gstrings_stats[] = { zero_base)}, {"tx_csum_offload_ctxt", IXGBEVF_STAT(hw_csum_tx_good, zero_base, zero_base)}, - {"rx_header_split", IXGBEVF_STAT(rx_hdr_split, zero_base, zero_base)}, }; #define IXGBE_QUEUE_STATS_LEN 0 #define IXGBE_GLOBAL_STATS_LEN ARRAY_SIZE(ixgbe_gstrings_stats) #define IXGBEVF_STATS_LEN (IXGBE_GLOBAL_STATS_LEN + IXGBE_QUEUE_STATS_LEN) -#endif /* ETHTOOL_GSTATS */ -#ifdef ETHTOOL_TEST static const char ixgbe_gstrings_test[][ETH_GSTRING_LEN] = { "Register test (offline)", "Link test (on/offline)" }; #define IXGBE_TEST_LEN (sizeof(ixgbe_gstrings_test) / ETH_GSTRING_LEN) -#endif /* ETHTOOL_TEST */ static int ixgbevf_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd) @@ -674,10 +669,8 @@ static int ixgbevf_nway_reset(struct net_device *netdev) { struct ixgbevf_adapter *adapter = netdev_priv(netdev); - if (netif_running(netdev)) { - if (!adapter->dev_closed) - ixgbevf_reinit_locked(adapter); - } + if (netif_running(netdev)) + ixgbevf_reinit_locked(adapter); return 0; } -- cgit v1.2.3 From fa71ae270a9af0ee3a1bd605d008f750371cfc1f Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 11 May 2012 08:32:50 +0000 Subject: ixgbevf: Move Tx clean-up into NAPI context Currently the VF driver is processing all of the transmits in interrupt context. This can be messy since the Rx is all handled in NAPI and this may result in interrupts being disabled. In order to resolve this move all of the Tx packet processing into NAPI and combine all of the interrupt and polling routines into just a pair of functions. Signed-off-by: Alexander Duyck Signed-off-by: Greg Rose Tested-by: Sibai Li Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/ixgbevf/ethtool.c | 2 - drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 12 +- drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 257 +++++++--------------- 3 files changed, 89 insertions(+), 182 deletions(-) (limited to 'drivers/net/ethernet/intel/ixgbevf/ethtool.c') diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c index af9c6570f4b5..15947c9122e1 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c +++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c @@ -364,7 +364,6 @@ static int ixgbevf_set_ringparam(struct net_device *netdev, } goto err_tx_ring_setup; } - tx_ring[i].v_idx = adapter->tx_ring[i].v_idx; } memcpy(rx_ring, adapter->rx_ring, @@ -380,7 +379,6 @@ static int ixgbevf_set_ringparam(struct net_device *netdev, } goto err_rx_ring_setup; } - rx_ring[i].v_idx = adapter->rx_ring[i].v_idx; } /* diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h index 8cae4ff95315..8bedd0fef0b7 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h @@ -81,11 +81,6 @@ struct ixgbevf_ring { * offset associated with this ring, which is different * for DCB and RSS modes */ - u64 v_idx; /* maps directly to the index for this ring in the hardware - * vector array, can also be used for finding the bit in EICR - * and friends that represents the vector for this ring */ - - u16 work_limit; /* max work per interrupt */ u16 rx_buf_len; }; @@ -140,6 +135,7 @@ struct ixgbevf_q_vector { struct ixgbevf_ring_container rx, tx; u32 eitr; int v_idx; /* vector index in list */ + char name[IFNAMSIZ + 9]; }; /* Helper macros to switch between ints/sec and what the register uses. @@ -167,9 +163,8 @@ struct ixgbevf_q_vector { #define NON_Q_VECTORS (OTHER_VECTOR) #define MAX_MSIX_Q_VECTORS 2 -#define MAX_MSIX_COUNT 2 -#define MIN_MSIX_Q_VECTORS 2 +#define MIN_MSIX_Q_VECTORS 1 #define MIN_MSIX_COUNT (MIN_MSIX_Q_VECTORS + NON_Q_VECTORS) /* board specific private data structure */ @@ -179,7 +174,6 @@ struct ixgbevf_adapter { u16 bd_number; struct work_struct reset_task; struct ixgbevf_q_vector *q_vector[MAX_MSIX_Q_VECTORS]; - char name[MAX_MSIX_COUNT][IFNAMSIZ + 9]; /* Interrupt Throttle Rate */ u32 itr_setting; @@ -187,6 +181,7 @@ struct ixgbevf_adapter { /* TX */ struct ixgbevf_ring *tx_ring; /* One per active queue */ int num_tx_queues; + u16 tx_itr_setting; u64 restart_queue; u64 hw_csum_tx_good; u64 lsc_int; @@ -197,6 +192,7 @@ struct ixgbevf_adapter { /* RX */ struct ixgbevf_ring *rx_ring; /* One per active queue */ int num_rx_queues; + u16 rx_itr_setting; u64 hw_csum_rx_error; u64 hw_rx_no_dma_resources; u64 hw_csum_rx_good; diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c index 744a02697ef6..628643b7f286 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c @@ -97,7 +97,7 @@ module_param(debug, int, 0); MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); /* forward decls */ -static void ixgbevf_set_itr_msix(struct ixgbevf_q_vector *q_vector); +static void ixgbevf_set_itr(struct ixgbevf_q_vector *q_vector); static void ixgbevf_write_eitr(struct ixgbevf_adapter *adapter, int v_idx, u32 itr_reg); @@ -182,14 +182,14 @@ static void ixgbevf_tx_timeout(struct net_device *netdev); /** * ixgbevf_clean_tx_irq - Reclaim resources after transmit completes - * @adapter: board private structure + * @q_vector: board private structure * @tx_ring: tx ring to clean **/ -static bool ixgbevf_clean_tx_irq(struct ixgbevf_adapter *adapter, +static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector, struct ixgbevf_ring *tx_ring) { + struct ixgbevf_adapter *adapter = q_vector->adapter; struct net_device *netdev = adapter->netdev; - struct ixgbe_hw *hw = &adapter->hw; union ixgbe_adv_tx_desc *tx_desc, *eop_desc; struct ixgbevf_tx_buffer *tx_buffer_info; unsigned int i, eop, count = 0; @@ -200,7 +200,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_adapter *adapter, eop_desc = IXGBE_TX_DESC_ADV(*tx_ring, eop); while ((eop_desc->wb.status & cpu_to_le32(IXGBE_TXD_STAT_DD)) && - (count < tx_ring->work_limit)) { + (count < tx_ring->count)) { bool cleaned = false; rmb(); /* read buffer_info after eop_desc */ /* eop could change between read and DD-check */ @@ -256,18 +256,12 @@ cont_loop: } } - /* re-arm the interrupt */ - if ((count >= tx_ring->work_limit) && - (!test_bit(__IXGBEVF_DOWN, &adapter->state))) { - IXGBE_WRITE_REG(hw, IXGBE_VTEICS, tx_ring->v_idx); - } - u64_stats_update_begin(&tx_ring->syncp); tx_ring->total_bytes += total_bytes; tx_ring->total_packets += total_packets; u64_stats_update_end(&tx_ring->syncp); - return count < tx_ring->work_limit; + return count < tx_ring->count; } /** @@ -402,7 +396,7 @@ static inline void ixgbevf_irq_enable_queues(struct ixgbevf_adapter *adapter, static bool ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector, struct ixgbevf_ring *rx_ring, - int *work_done, int work_to_do) + int budget) { struct ixgbevf_adapter *adapter = q_vector->adapter; struct pci_dev *pdev = adapter->pdev; @@ -411,7 +405,6 @@ static bool ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector, struct sk_buff *skb; unsigned int i; u32 len, staterr; - bool cleaned = false; int cleaned_count = 0; unsigned int total_rx_bytes = 0, total_rx_packets = 0; @@ -421,13 +414,12 @@ static bool ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector, rx_buffer_info = &rx_ring->rx_buffer_info[i]; while (staterr & IXGBE_RXD_STAT_DD) { - if (*work_done >= work_to_do) + if (!budget) break; - (*work_done)++; + budget--; rmb(); /* read descriptor and rx_buffer_info after status DD */ len = le16_to_cpu(rx_desc->wb.upper.length); - cleaned = true; skb = rx_buffer_info->skb; prefetch(skb->data - NET_IP_ALIGN); rx_buffer_info->skb = NULL; @@ -510,74 +502,52 @@ next_desc: rx_ring->total_bytes += total_rx_bytes; u64_stats_update_end(&rx_ring->syncp); - return cleaned; -} - -/** - * ixgbevf_clean_rxonly - msix (aka one shot) rx clean routine - * @napi: napi struct with our devices info in it - * @budget: amount of work driver is allowed to do this pass, in packets - * - * This function is optimized for cleaning one queue only on a single - * q_vector!!! - **/ -static int ixgbevf_clean_rxonly(struct napi_struct *napi, int budget) -{ - struct ixgbevf_q_vector *q_vector = - container_of(napi, struct ixgbevf_q_vector, napi); - struct ixgbevf_adapter *adapter = q_vector->adapter; - int work_done = 0; - - ixgbevf_clean_rx_irq(q_vector, q_vector->rx.ring, &work_done, budget); - - /* If all Rx work done, exit the polling mode */ - if (work_done < budget) { - napi_complete(napi); - if (adapter->itr_setting & 1) - ixgbevf_set_itr_msix(q_vector); - if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) - ixgbevf_irq_enable_queues(adapter, - 1 << q_vector->v_idx); - } - - return work_done; + return !!budget; } /** - * ixgbevf_clean_rxonly_many - msix (aka one shot) rx clean routine + * ixgbevf_poll - NAPI polling calback * @napi: napi struct with our devices info in it * @budget: amount of work driver is allowed to do this pass, in packets * - * This function will clean more than one rx queue associated with a + * This function will clean more than one or more rings associated with a * q_vector. **/ -static int ixgbevf_clean_rxonly_many(struct napi_struct *napi, int budget) +static int ixgbevf_poll(struct napi_struct *napi, int budget) { struct ixgbevf_q_vector *q_vector = container_of(napi, struct ixgbevf_q_vector, napi); struct ixgbevf_adapter *adapter = q_vector->adapter; - struct ixgbevf_ring *rx_ring; - int work_done = 0; + struct ixgbevf_ring *ring; + int per_ring_budget; + bool clean_complete = true; + + ixgbevf_for_each_ring(ring, q_vector->tx) + clean_complete &= ixgbevf_clean_tx_irq(q_vector, ring); /* attempt to distribute budget to each queue fairly, but don't allow * the budget to go below 1 because we'll exit polling */ - budget /= (q_vector->rx.count ?: 1); - budget = max(budget, 1); - - ixgbevf_for_each_ring(rx_ring, q_vector->rx) - ixgbevf_clean_rx_irq(q_vector, rx_ring, &work_done, budget); - - /* If all Rx work done, exit the polling mode */ - if (work_done < budget) { - napi_complete(napi); - if (adapter->itr_setting & 1) - ixgbevf_set_itr_msix(q_vector); - if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) - ixgbevf_irq_enable_queues(adapter, - 1 << q_vector->v_idx); - } + if (q_vector->rx.count > 1) + per_ring_budget = max(budget/q_vector->rx.count, 1); + else + per_ring_budget = budget; + + ixgbevf_for_each_ring(ring, q_vector->rx) + clean_complete &= ixgbevf_clean_rx_irq(q_vector, ring, + per_ring_budget); + + /* If all work not completed, return budget and keep polling */ + if (!clean_complete) + return budget; + /* all work done, exit the polling mode */ + napi_complete(napi); + if (adapter->rx_itr_setting & 1) + ixgbevf_set_itr(q_vector); + if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) + ixgbevf_irq_enable_queues(adapter, + 1 << q_vector->v_idx); - return work_done; + return 0; } @@ -720,7 +690,7 @@ static void ixgbevf_write_eitr(struct ixgbevf_adapter *adapter, int v_idx, IXGBE_WRITE_REG(hw, IXGBE_VTEITR(v_idx), itr_reg); } -static void ixgbevf_set_itr_msix(struct ixgbevf_q_vector *q_vector) +static void ixgbevf_set_itr(struct ixgbevf_q_vector *q_vector) { struct ixgbevf_adapter *adapter = q_vector->adapter; u32 new_itr; @@ -780,8 +750,7 @@ static void ixgbevf_set_itr_msix(struct ixgbevf_q_vector *q_vector) static irqreturn_t ixgbevf_msix_mbx(int irq, void *data) { - struct net_device *netdev = data; - struct ixgbevf_adapter *adapter = netdev_priv(netdev); + struct ixgbevf_adapter *adapter = data; struct ixgbe_hw *hw = &adapter->hw; u32 eicr; u32 msg; @@ -821,59 +790,22 @@ static irqreturn_t ixgbevf_msix_mbx(int irq, void *data) return IRQ_HANDLED; } -static irqreturn_t ixgbevf_msix_clean_tx(int irq, void *data) -{ - struct ixgbevf_q_vector *q_vector = data; - struct ixgbevf_adapter *adapter = q_vector->adapter; - struct ixgbevf_ring *tx_ring; - - if (!q_vector->tx.ring) - return IRQ_HANDLED; - - ixgbevf_for_each_ring(tx_ring, q_vector->tx) { - tx_ring->total_bytes = 0; - tx_ring->total_packets = 0; - ixgbevf_clean_tx_irq(adapter, tx_ring); - } - - if (adapter->itr_setting & 1) - ixgbevf_set_itr_msix(q_vector); - - return IRQ_HANDLED; -} /** - * ixgbevf_msix_clean_rx - single unshared vector rx clean (all queues) + * ixgbevf_msix_clean_rings - single unshared vector rx clean (all queues) * @irq: unused * @data: pointer to our q_vector struct for this interrupt vector **/ -static irqreturn_t ixgbevf_msix_clean_rx(int irq, void *data) +static irqreturn_t ixgbevf_msix_clean_rings(int irq, void *data) { struct ixgbevf_q_vector *q_vector = data; struct ixgbevf_adapter *adapter = q_vector->adapter; struct ixgbe_hw *hw = &adapter->hw; - struct ixgbevf_ring *rx_ring; - - ixgbevf_for_each_ring(rx_ring, q_vector->rx) { - rx_ring->total_bytes = 0; - rx_ring->total_packets = 0; - } - - if (!q_vector->rx.ring) - return IRQ_HANDLED; /* disable interrupts on this vector only */ IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, 1 << q_vector->v_idx); - napi_schedule(&q_vector->napi); - - - return IRQ_HANDLED; -} - -static irqreturn_t ixgbevf_msix_clean_many(int irq, void *data) -{ - ixgbevf_msix_clean_rx(irq, data); - ixgbevf_msix_clean_tx(irq, data); + if (q_vector->rx.ring || q_vector->tx.ring) + napi_schedule(&q_vector->napi); return IRQ_HANDLED; } @@ -886,7 +818,6 @@ static inline void map_vector_to_rxq(struct ixgbevf_adapter *a, int v_idx, a->rx_ring[r_idx].next = q_vector->rx.ring; q_vector->rx.ring = &a->rx_ring[r_idx]; q_vector->rx.count++; - a->rx_ring[r_idx].v_idx = 1 << v_idx; } static inline void map_vector_to_txq(struct ixgbevf_adapter *a, int v_idx, @@ -897,7 +828,6 @@ static inline void map_vector_to_txq(struct ixgbevf_adapter *a, int v_idx, a->tx_ring[t_idx].next = q_vector->tx.ring; q_vector->tx.ring = &a->tx_ring[t_idx]; q_vector->tx.count++; - a->tx_ring[t_idx].v_idx = 1 << v_idx; } /** @@ -973,37 +903,30 @@ out: static int ixgbevf_request_msix_irqs(struct ixgbevf_adapter *adapter) { struct net_device *netdev = adapter->netdev; - irqreturn_t (*handler)(int, void *); - int i, vector, q_vectors, err; + int q_vectors = adapter->num_msix_vectors - NON_Q_VECTORS; + int vector, err; int ri = 0, ti = 0; - /* Decrement for Other and TCP Timer vectors */ - q_vectors = adapter->num_msix_vectors - NON_Q_VECTORS; - -#define SET_HANDLER(_v) (((_v)->rx.ring && (_v)->tx.ring) \ - ? &ixgbevf_msix_clean_many : \ - (_v)->rx.ring ? &ixgbevf_msix_clean_rx : \ - (_v)->tx.ring ? &ixgbevf_msix_clean_tx : \ - NULL) for (vector = 0; vector < q_vectors; vector++) { - handler = SET_HANDLER(adapter->q_vector[vector]); - - if (handler == &ixgbevf_msix_clean_rx) { - sprintf(adapter->name[vector], "%s-%s-%d", - netdev->name, "rx", ri++); - } else if (handler == &ixgbevf_msix_clean_tx) { - sprintf(adapter->name[vector], "%s-%s-%d", - netdev->name, "tx", ti++); - } else if (handler == &ixgbevf_msix_clean_many) { - sprintf(adapter->name[vector], "%s-%s-%d", - netdev->name, "TxRx", vector); + struct ixgbevf_q_vector *q_vector = adapter->q_vector[vector]; + struct msix_entry *entry = &adapter->msix_entries[vector]; + + if (q_vector->tx.ring && q_vector->rx.ring) { + snprintf(q_vector->name, sizeof(q_vector->name) - 1, + "%s-%s-%d", netdev->name, "TxRx", ri++); + ti++; + } else if (q_vector->rx.ring) { + snprintf(q_vector->name, sizeof(q_vector->name) - 1, + "%s-%s-%d", netdev->name, "rx", ri++); + } else if (q_vector->tx.ring) { + snprintf(q_vector->name, sizeof(q_vector->name) - 1, + "%s-%s-%d", netdev->name, "tx", ti++); } else { /* skip this unused q_vector */ continue; } - err = request_irq(adapter->msix_entries[vector].vector, - handler, 0, adapter->name[vector], - adapter->q_vector[vector]); + err = request_irq(entry->vector, &ixgbevf_msix_clean_rings, 0, + q_vector->name, q_vector); if (err) { hw_dbg(&adapter->hw, "request_irq failed for MSIX interrupt " @@ -1012,9 +935,8 @@ static int ixgbevf_request_msix_irqs(struct ixgbevf_adapter *adapter) } } - sprintf(adapter->name[vector], "%s:mbx", netdev->name); err = request_irq(adapter->msix_entries[vector].vector, - &ixgbevf_msix_mbx, 0, adapter->name[vector], netdev); + &ixgbevf_msix_mbx, 0, netdev->name, adapter); if (err) { hw_dbg(&adapter->hw, "request_irq for msix_mbx failed: %d\n", err); @@ -1024,9 +946,11 @@ static int ixgbevf_request_msix_irqs(struct ixgbevf_adapter *adapter) return 0; free_queue_irqs: - for (i = vector - 1; i >= 0; i--) - free_irq(adapter->msix_entries[--vector].vector, - &(adapter->q_vector[i])); + while (vector) { + vector--; + free_irq(adapter->msix_entries[vector].vector, + adapter->q_vector[vector]); + } pci_disable_msix(adapter->pdev); kfree(adapter->msix_entries); adapter->msix_entries = NULL; @@ -1069,17 +993,20 @@ static int ixgbevf_request_irq(struct ixgbevf_adapter *adapter) static void ixgbevf_free_irq(struct ixgbevf_adapter *adapter) { - struct net_device *netdev = adapter->netdev; int i, q_vectors; q_vectors = adapter->num_msix_vectors; - i = q_vectors - 1; - free_irq(adapter->msix_entries[i].vector, netdev); + free_irq(adapter->msix_entries[i].vector, adapter); i--; for (; i >= 0; i--) { + /* free only the irqs that were actually requested */ + if (!adapter->q_vector[i]->rx.ring && + !adapter->q_vector[i]->tx.ring) + continue; + free_irq(adapter->msix_entries[i].vector, adapter->q_vector[i]); } @@ -1317,15 +1244,8 @@ static void ixgbevf_napi_enable_all(struct ixgbevf_adapter *adapter) int q_vectors = adapter->num_msix_vectors - NON_Q_VECTORS; for (q_idx = 0; q_idx < q_vectors; q_idx++) { - struct napi_struct *napi; q_vector = adapter->q_vector[q_idx]; - if (!q_vector->rx.ring) - continue; - napi = &q_vector->napi; - if (q_vector->rx.count > 1) - napi->poll = &ixgbevf_clean_rxonly_many; - - napi_enable(napi); + napi_enable(&q_vector->napi); } } @@ -1337,8 +1257,6 @@ static void ixgbevf_napi_disable_all(struct ixgbevf_adapter *adapter) for (q_idx = 0; q_idx < q_vectors; q_idx++) { q_vector = adapter->q_vector[q_idx]; - if (!q_vector->rx.ring) - continue; napi_disable(&q_vector->napi); } } @@ -1703,10 +1621,9 @@ static void ixgbevf_acquire_msix_vectors(struct ixgbevf_adapter *adapter, { int err, vector_threshold; - /* We'll want at least 3 (vector_threshold): - * 1) TxQ[0] Cleanup - * 2) RxQ[0] Cleanup - * 3) Other (Link Status Change, etc.) + /* We'll want at least 2 (vector_threshold): + * 1) TxQ[0] + RxQ[0] handler + * 2) Other (Link Status Change, etc.) */ vector_threshold = MIN_MSIX_COUNT; @@ -1821,10 +1738,12 @@ static int ixgbevf_set_interrupt_capability(struct ixgbevf_adapter *adapter) * It's easy to be greedy for MSI-X vectors, but it really * doesn't do us much good if we have a lot more vectors * than CPU's. So let's be conservative and only ask for - * (roughly) twice the number of vectors as there are CPU's. + * (roughly) the same number of vectors as there are CPU's. + * The default is to use pairs of vectors. */ - v_budget = min(adapter->num_rx_queues + adapter->num_tx_queues, - (int)(num_online_cpus() * 2)) + NON_Q_VECTORS; + v_budget = max(adapter->num_rx_queues, adapter->num_tx_queues); + v_budget = min_t(int, v_budget, num_online_cpus()); + v_budget += NON_Q_VECTORS; /* A failure in MSI-X entry allocation isn't fatal, but it does * mean we disable MSI-X capabilities of the adapter. */ @@ -1855,12 +1774,8 @@ static int ixgbevf_alloc_q_vectors(struct ixgbevf_adapter *adapter) { int q_idx, num_q_vectors; struct ixgbevf_q_vector *q_vector; - int napi_vectors; - int (*poll)(struct napi_struct *, int); num_q_vectors = adapter->num_msix_vectors - NON_Q_VECTORS; - napi_vectors = adapter->num_rx_queues; - poll = &ixgbevf_clean_rxonly; for (q_idx = 0; q_idx < num_q_vectors; q_idx++) { q_vector = kzalloc(sizeof(struct ixgbevf_q_vector), GFP_KERNEL); @@ -1869,9 +1784,8 @@ static int ixgbevf_alloc_q_vectors(struct ixgbevf_adapter *adapter) q_vector->adapter = adapter; q_vector->v_idx = q_idx; q_vector->eitr = adapter->eitr_param; - if (q_idx < napi_vectors) - netif_napi_add(adapter->netdev, &q_vector->napi, - (*poll), 64); + netif_napi_add(adapter->netdev, &q_vector->napi, + ixgbevf_poll, 64); adapter->q_vector[q_idx] = q_vector; } @@ -2272,7 +2186,6 @@ int ixgbevf_setup_tx_resources(struct ixgbevf_adapter *adapter, tx_ring->next_to_use = 0; tx_ring->next_to_clean = 0; - tx_ring->work_limit = tx_ring->count; return 0; err: -- cgit v1.2.3 From fb40195cc975b14c5d4e44863ea996f999ba5aee Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 11 May 2012 08:33:16 +0000 Subject: ixgbevf: Add netdev to ring structure This change adds the netdev to the ring structure. This allows for a quicker transition from ring to netdev without having to go from ring to adapter to netdev. Signed-off-by: Alexander Duyck Signed-off-by: Greg Rose Tested-by: Sibai Li Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/ixgbevf/ethtool.c | 6 +-- drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 + drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 54 ++++++++++------------- 3 files changed, 28 insertions(+), 34 deletions(-) (limited to 'drivers/net/ethernet/intel/ixgbevf/ethtool.c') diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c index 15947c9122e1..2c3b20edd84d 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c +++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c @@ -359,8 +359,7 @@ static int ixgbevf_set_ringparam(struct net_device *netdev, if (err) { while (i) { i--; - ixgbevf_free_tx_resources(adapter, - &tx_ring[i]); + ixgbevf_free_tx_resources(adapter, &tx_ring[i]); } goto err_tx_ring_setup; } @@ -374,8 +373,7 @@ static int ixgbevf_set_ringparam(struct net_device *netdev, if (err) { while (i) { i--; - ixgbevf_free_rx_resources(adapter, - &rx_ring[i]); + ixgbevf_free_rx_resources(adapter, &rx_ring[i]); } goto err_rx_ring_setup; } diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h index 1f1376515f70..e167d1bb6dea 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h @@ -56,6 +56,8 @@ struct ixgbevf_rx_buffer { struct ixgbevf_ring { struct ixgbevf_ring *next; + struct net_device *netdev; + struct device *dev; struct ixgbevf_adapter *adapter; /* backlink */ void *desc; /* descriptor ring memory */ dma_addr_t dma; /* phys. address of descriptor ring */ diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c index c27ce447e04e..1c53e13b466d 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c @@ -187,7 +187,6 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector, struct ixgbevf_ring *tx_ring) { struct ixgbevf_adapter *adapter = q_vector->adapter; - struct net_device *netdev = adapter->netdev; union ixgbe_adv_tx_desc *tx_desc, *eop_desc; struct ixgbevf_tx_buffer *tx_buffer_info; unsigned int i, eop, count = 0; @@ -241,15 +240,17 @@ cont_loop: tx_ring->next_to_clean = i; #define TX_WAKE_THRESHOLD (DESC_NEEDED * 2) - if (unlikely(count && netif_carrier_ok(netdev) && + if (unlikely(count && netif_carrier_ok(tx_ring->netdev) && (IXGBE_DESC_UNUSED(tx_ring) >= TX_WAKE_THRESHOLD))) { /* Make sure that anybody stopping the queue after this * sees the new next_to_clean. */ smp_mb(); - if (__netif_subqueue_stopped(netdev, tx_ring->queue_index) && + if (__netif_subqueue_stopped(tx_ring->netdev, + tx_ring->queue_index) && !test_bit(__IXGBEVF_DOWN, &adapter->state)) { - netif_wake_subqueue(netdev, tx_ring->queue_index); + netif_wake_subqueue(tx_ring->netdev, + tx_ring->queue_index); ++adapter->restart_queue; } } @@ -292,12 +293,13 @@ static void ixgbevf_receive_skb(struct ixgbevf_q_vector *q_vector, * @skb: skb currently being received and modified **/ static inline void ixgbevf_rx_checksum(struct ixgbevf_adapter *adapter, + struct ixgbevf_ring *ring, u32 status_err, struct sk_buff *skb) { skb_checksum_none_assert(skb); /* Rx csum disabled */ - if (!(adapter->netdev->features & NETIF_F_RXCSUM)) + if (!(ring->netdev->features & NETIF_F_RXCSUM)) return; /* if IP and error */ @@ -332,31 +334,21 @@ static void ixgbevf_alloc_rx_buffers(struct ixgbevf_adapter *adapter, union ixgbe_adv_rx_desc *rx_desc; struct ixgbevf_rx_buffer *bi; struct sk_buff *skb; - unsigned int i; - unsigned int bufsz = rx_ring->rx_buf_len + NET_IP_ALIGN; + unsigned int i = rx_ring->next_to_use; - i = rx_ring->next_to_use; bi = &rx_ring->rx_buffer_info[i]; while (cleaned_count--) { rx_desc = IXGBEVF_RX_DESC(rx_ring, i); skb = bi->skb; if (!skb) { - skb = netdev_alloc_skb(adapter->netdev, - bufsz); - + skb = netdev_alloc_skb_ip_align(rx_ring->netdev, + rx_ring->rx_buf_len); if (!skb) { adapter->alloc_rx_buff_failed++; goto no_buffers; } - /* - * Make buffer alignment 2 beyond a 16 byte boundary - * this will result in a 16 byte aligned IP header after - * the 14 byte MAC header is removed - */ - skb_reserve(skb, NET_IP_ALIGN); - bi->skb = skb; } if (!bi->dma) { @@ -449,7 +441,7 @@ static bool ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector, goto next_desc; } - ixgbevf_rx_checksum(adapter, staterr, skb); + ixgbevf_rx_checksum(adapter, rx_ring, staterr, skb); /* probably a little skewed due to removing CRC */ total_rx_bytes += skb->len; @@ -464,7 +456,7 @@ static bool ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector, if (header_fixup_len < 14) skb_push(skb, header_fixup_len); } - skb->protocol = eth_type_trans(skb, adapter->netdev); + skb->protocol = eth_type_trans(skb, rx_ring->netdev); ixgbevf_receive_skb(q_vector, skb, staterr, rx_ring, rx_desc); @@ -1669,12 +1661,16 @@ static int ixgbevf_alloc_queues(struct ixgbevf_adapter *adapter) adapter->tx_ring[i].count = adapter->tx_ring_count; adapter->tx_ring[i].queue_index = i; adapter->tx_ring[i].reg_idx = i; + adapter->tx_ring[i].dev = &adapter->pdev->dev; + adapter->tx_ring[i].netdev = adapter->netdev; } for (i = 0; i < adapter->num_rx_queues; i++) { adapter->rx_ring[i].count = adapter->rx_ring_count; adapter->rx_ring[i].queue_index = i; adapter->rx_ring[i].reg_idx = i; + adapter->rx_ring[i].dev = &adapter->pdev->dev; + adapter->rx_ring[i].netdev = adapter->netdev; } return 0; @@ -2721,12 +2717,11 @@ static void ixgbevf_tx_queue(struct ixgbevf_adapter *adapter, writel(i, adapter->hw.hw_addr + tx_ring->tail); } -static int __ixgbevf_maybe_stop_tx(struct net_device *netdev, - struct ixgbevf_ring *tx_ring, int size) +static int __ixgbevf_maybe_stop_tx(struct ixgbevf_ring *tx_ring, int size) { - struct ixgbevf_adapter *adapter = netdev_priv(netdev); + struct ixgbevf_adapter *adapter = netdev_priv(tx_ring->netdev); - netif_stop_subqueue(netdev, tx_ring->queue_index); + netif_stop_subqueue(tx_ring->netdev, tx_ring->queue_index); /* Herbert's original patch had: * smp_mb__after_netif_stop_queue(); * but since that doesn't exist yet, just open code it. */ @@ -2738,17 +2733,16 @@ static int __ixgbevf_maybe_stop_tx(struct net_device *netdev, return -EBUSY; /* A reprieve! - use start_queue because it doesn't call schedule */ - netif_start_subqueue(netdev, tx_ring->queue_index); + netif_start_subqueue(tx_ring->netdev, tx_ring->queue_index); ++adapter->restart_queue; return 0; } -static int ixgbevf_maybe_stop_tx(struct net_device *netdev, - struct ixgbevf_ring *tx_ring, int size) +static int ixgbevf_maybe_stop_tx(struct ixgbevf_ring *tx_ring, int size) { if (likely(IXGBE_DESC_UNUSED(tx_ring) >= size)) return 0; - return __ixgbevf_maybe_stop_tx(netdev, tx_ring, size); + return __ixgbevf_maybe_stop_tx(tx_ring, size); } static int ixgbevf_xmit_frame(struct sk_buff *skb, struct net_device *netdev) @@ -2779,7 +2773,7 @@ static int ixgbevf_xmit_frame(struct sk_buff *skb, struct net_device *netdev) #else count += skb_shinfo(skb)->nr_frags; #endif - if (ixgbevf_maybe_stop_tx(netdev, tx_ring, count + 3)) { + if (ixgbevf_maybe_stop_tx(tx_ring, count + 3)) { adapter->tx_busy++; return NETDEV_TX_BUSY; } @@ -2810,7 +2804,7 @@ static int ixgbevf_xmit_frame(struct sk_buff *skb, struct net_device *netdev) ixgbevf_tx_map(adapter, tx_ring, skb, tx_flags, first), skb->len, hdr_len); - ixgbevf_maybe_stop_tx(netdev, tx_ring, DESC_NEEDED); + ixgbevf_maybe_stop_tx(tx_ring, DESC_NEEDED); return NETDEV_TX_OK; } -- cgit v1.2.3 From eb022d058f5ad48c45f1f31c36865d2c676aedc9 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Thu, 19 Jul 2012 00:18:05 +0000 Subject: ixgbevf: Fix multiple issues in ixgbevf_get/set_ringparam In ixgbevf_get_ringparam we could run into a NULL pointer dereference if the rings were not allocated when we attempted the call. To prevent that we can just access the tx/rx_ring_count values instead of attempting to access the rings to get the count. This change corrects a memory leak and memory corruption in ixgbevf_set_ringparam. The memory leak was due to us not freeing the resources from the ring before overwriting them. This change corrects the memory leak by making certain to call ixgbe_free_tx/rx_resources on the rings prior to freeing them. The memory corruption was because we were replacing the rings but not updating the q_vectors. It addresses the memory corruption by leaving the rings in place and instead just copying the contents of the new rings into the existing rings. Signed-off-by: Alexander Duyck Acked-by: Greg Rose Tested-by: Sibai Li Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/ixgbevf/ethtool.c | 153 +++++++++++++++------------ 1 file changed, 83 insertions(+), 70 deletions(-) (limited to 'drivers/net/ethernet/intel/ixgbevf/ethtool.c') diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c index 2c3b20edd84d..8f2070439b59 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c +++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c @@ -284,13 +284,11 @@ static void ixgbevf_get_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring) { struct ixgbevf_adapter *adapter = netdev_priv(netdev); - struct ixgbevf_ring *tx_ring = adapter->tx_ring; - struct ixgbevf_ring *rx_ring = adapter->rx_ring; ring->rx_max_pending = IXGBEVF_MAX_RXD; ring->tx_max_pending = IXGBEVF_MAX_TXD; - ring->rx_pending = rx_ring->count; - ring->tx_pending = tx_ring->count; + ring->rx_pending = adapter->rx_ring_count; + ring->tx_pending = adapter->tx_ring_count; } static int ixgbevf_set_ringparam(struct net_device *netdev, @@ -298,33 +296,28 @@ static int ixgbevf_set_ringparam(struct net_device *netdev, { struct ixgbevf_adapter *adapter = netdev_priv(netdev); struct ixgbevf_ring *tx_ring = NULL, *rx_ring = NULL; - int i, err = 0; u32 new_rx_count, new_tx_count; + int i, err = 0; if ((ring->rx_mini_pending) || (ring->rx_jumbo_pending)) return -EINVAL; - new_rx_count = max(ring->rx_pending, (u32)IXGBEVF_MIN_RXD); - new_rx_count = min(new_rx_count, (u32)IXGBEVF_MAX_RXD); - new_rx_count = ALIGN(new_rx_count, IXGBE_REQ_RX_DESCRIPTOR_MULTIPLE); - - new_tx_count = max(ring->tx_pending, (u32)IXGBEVF_MIN_TXD); - new_tx_count = min(new_tx_count, (u32)IXGBEVF_MAX_TXD); + new_tx_count = max_t(u32, ring->tx_pending, IXGBEVF_MIN_TXD); + new_tx_count = min_t(u32, new_tx_count, IXGBEVF_MAX_TXD); new_tx_count = ALIGN(new_tx_count, IXGBE_REQ_TX_DESCRIPTOR_MULTIPLE); - if ((new_tx_count == adapter->tx_ring->count) && - (new_rx_count == adapter->rx_ring->count)) { - /* nothing to do */ + new_rx_count = max_t(u32, ring->rx_pending, IXGBEVF_MIN_RXD); + new_rx_count = min_t(u32, new_rx_count, IXGBEVF_MAX_RXD); + new_rx_count = ALIGN(new_rx_count, IXGBE_REQ_RX_DESCRIPTOR_MULTIPLE); + + /* if nothing to do return success */ + if ((new_tx_count == adapter->tx_ring_count) && + (new_rx_count == adapter->rx_ring_count)) return 0; - } while (test_and_set_bit(__IXGBEVF_RESETTING, &adapter->state)) - msleep(1); + usleep_range(1000, 2000); - /* - * If the adapter isn't up and running then just set the - * new parameters and scurry for the exits. - */ if (!netif_running(adapter->netdev)) { for (i = 0; i < adapter->num_tx_queues; i++) adapter->tx_ring[i].count = new_tx_count; @@ -335,78 +328,98 @@ static int ixgbevf_set_ringparam(struct net_device *netdev, goto clear_reset; } - tx_ring = kcalloc(adapter->num_tx_queues, - sizeof(struct ixgbevf_ring), GFP_KERNEL); - if (!tx_ring) { - err = -ENOMEM; - goto clear_reset; - } - - rx_ring = kcalloc(adapter->num_rx_queues, - sizeof(struct ixgbevf_ring), GFP_KERNEL); - if (!rx_ring) { - err = -ENOMEM; - goto err_rx_setup; - } - - ixgbevf_down(adapter); + if (new_tx_count != adapter->tx_ring_count) { + tx_ring = vmalloc(adapter->num_tx_queues * sizeof(*tx_ring)); + if (!tx_ring) { + err = -ENOMEM; + goto clear_reset; + } - memcpy(tx_ring, adapter->tx_ring, - adapter->num_tx_queues * sizeof(struct ixgbevf_ring)); - for (i = 0; i < adapter->num_tx_queues; i++) { - tx_ring[i].count = new_tx_count; - err = ixgbevf_setup_tx_resources(adapter, &tx_ring[i]); - if (err) { + for (i = 0; i < adapter->num_tx_queues; i++) { + /* clone ring and setup updated count */ + tx_ring[i] = adapter->tx_ring[i]; + tx_ring[i].count = new_tx_count; + err = ixgbevf_setup_tx_resources(adapter, &tx_ring[i]); + if (!err) + continue; while (i) { i--; ixgbevf_free_tx_resources(adapter, &tx_ring[i]); } - goto err_tx_ring_setup; + + vfree(tx_ring); + tx_ring = NULL; + + goto clear_reset; } } - memcpy(rx_ring, adapter->rx_ring, - adapter->num_rx_queues * sizeof(struct ixgbevf_ring)); - for (i = 0; i < adapter->num_rx_queues; i++) { - rx_ring[i].count = new_rx_count; - err = ixgbevf_setup_rx_resources(adapter, &rx_ring[i]); - if (err) { + if (new_rx_count != adapter->rx_ring_count) { + rx_ring = vmalloc(adapter->num_rx_queues * sizeof(*rx_ring)); + if (!rx_ring) { + err = -ENOMEM; + goto clear_reset; + } + + for (i = 0; i < adapter->num_rx_queues; i++) { + /* clone ring and setup updated count */ + rx_ring[i] = adapter->rx_ring[i]; + rx_ring[i].count = new_rx_count; + err = ixgbevf_setup_rx_resources(adapter, &rx_ring[i]); + if (!err) + continue; while (i) { i--; ixgbevf_free_rx_resources(adapter, &rx_ring[i]); } - goto err_rx_ring_setup; + + vfree(rx_ring); + rx_ring = NULL; + + goto clear_reset; } } - /* - * Only switch to new rings if all the prior allocations - * and ring setups have succeeded. - */ - kfree(adapter->tx_ring); - adapter->tx_ring = tx_ring; - adapter->tx_ring_count = new_tx_count; - - kfree(adapter->rx_ring); - adapter->rx_ring = rx_ring; - adapter->rx_ring_count = new_rx_count; + /* bring interface down to prepare for update */ + ixgbevf_down(adapter); - /* success! */ - ixgbevf_up(adapter); + /* Tx */ + if (tx_ring) { + for (i = 0; i < adapter->num_tx_queues; i++) { + ixgbevf_free_tx_resources(adapter, + &adapter->tx_ring[i]); + adapter->tx_ring[i] = tx_ring[i]; + } + adapter->tx_ring_count = new_tx_count; - goto clear_reset; + vfree(tx_ring); + tx_ring = NULL; + } -err_rx_ring_setup: - for(i = 0; i < adapter->num_tx_queues; i++) - ixgbevf_free_tx_resources(adapter, &tx_ring[i]); + /* Rx */ + if (rx_ring) { + for (i = 0; i < adapter->num_rx_queues; i++) { + ixgbevf_free_rx_resources(adapter, + &adapter->rx_ring[i]); + adapter->rx_ring[i] = rx_ring[i]; + } + adapter->rx_ring_count = new_rx_count; -err_tx_ring_setup: - kfree(rx_ring); + vfree(rx_ring); + rx_ring = NULL; + } -err_rx_setup: - kfree(tx_ring); + /* restore interface using new values */ + ixgbevf_up(adapter); clear_reset: + /* free Tx resources if Rx error is encountered */ + if (tx_ring) { + for (i = 0; i < adapter->num_tx_queues; i++) + ixgbevf_free_tx_resources(adapter, &tx_ring[i]); + vfree(tx_ring); + } + clear_bit(__IXGBEVF_RESETTING, &adapter->state); return err; } -- cgit v1.2.3