* [net-next 05/11] igb: Move ITR related data into work container within the q_vector
From: Jeff Kirsher @ 2011-10-08 6:47 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1318056461-19562-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change moves information related to interrupt throttle rate
configuration into a separate q_vector sub-structure called a work
container. A similar change has already been made for ixgbe and this work
is based off of that.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/e1000_defines.h | 3 +
drivers/net/ethernet/intel/igb/igb.h | 31 +++--
drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 +-
drivers/net/ethernet/intel/igb/igb_main.c | 203 +++++++++++-------------
4 files changed, 118 insertions(+), 123 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index 7b8ddd8..68558be 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -409,6 +409,9 @@
#define E1000_ICS_DRSTA E1000_ICR_DRSTA /* Device Reset Aserted */
/* Extended Interrupt Cause Set */
+/* E1000_EITR_CNT_IGNR is only for 82576 and newer */
+#define E1000_EITR_CNT_IGNR 0x80000000 /* Don't reset counters on write */
+
/* Transmit Descriptor Control */
/* Enable the counting of descriptors still to be processed. */
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 0df040a..91f90fe 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -42,8 +42,11 @@
struct igb_adapter;
-/* ((1000000000ns / (6000ints/s * 1024ns)) << 2 = 648 */
-#define IGB_START_ITR 648
+/* Interrupt defines */
+#define IGB_START_ITR 648 /* ~6000 ints/sec */
+#define IGB_4K_ITR 980
+#define IGB_20K_ITR 196
+#define IGB_70K_ITR 56
/* TX/RX descriptor defines */
#define IGB_DEFAULT_TXD 256
@@ -175,16 +178,23 @@ struct igb_rx_queue_stats {
u64 alloc_failed;
};
+struct igb_ring_container {
+ struct igb_ring *ring; /* pointer to linked list of rings */
+ unsigned int total_bytes; /* total bytes processed this int */
+ unsigned int total_packets; /* total packets processed this int */
+ u16 work_limit; /* total work allowed per interrupt */
+ u8 count; /* total number of rings in vector */
+ u8 itr; /* current ITR setting for ring */
+};
+
struct igb_q_vector {
- struct igb_adapter *adapter; /* backlink */
- struct igb_ring *rx_ring;
- struct igb_ring *tx_ring;
- struct napi_struct napi;
+ struct igb_adapter *adapter; /* backlink */
+ int cpu; /* CPU for DCA */
+ u32 eims_value; /* EIMS mask value */
- u32 eims_value;
- u16 cpu;
- u16 tx_work_limit;
+ struct igb_ring_container rx, tx;
+ struct napi_struct napi;
int numa_node;
u16 itr_val;
@@ -215,9 +225,6 @@ struct igb_ring {
u16 next_to_clean ____cacheline_aligned_in_smp;
u16 next_to_use;
- unsigned int total_bytes;
- unsigned int total_packets;
-
union {
/* TX */
struct {
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index a893da1..5ebe992 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2013,8 +2013,8 @@ static int igb_set_coalesce(struct net_device *netdev,
for (i = 0; i < adapter->num_q_vectors; i++) {
struct igb_q_vector *q_vector = adapter->q_vector[i];
- q_vector->tx_work_limit = adapter->tx_work_limit;
- if (q_vector->rx_ring)
+ q_vector->tx.work_limit = adapter->tx_work_limit;
+ if (q_vector->rx.ring)
q_vector->itr_val = adapter->rx_itr_setting;
else
q_vector->itr_val = adapter->tx_itr_setting;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index f339de9..8dc04e0 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -764,10 +764,10 @@ static void igb_assign_vector(struct igb_q_vector *q_vector, int msix_vector)
int rx_queue = IGB_N0_QUEUE;
int tx_queue = IGB_N0_QUEUE;
- if (q_vector->rx_ring)
- rx_queue = q_vector->rx_ring->reg_idx;
- if (q_vector->tx_ring)
- tx_queue = q_vector->tx_ring->reg_idx;
+ if (q_vector->rx.ring)
+ rx_queue = q_vector->rx.ring->reg_idx;
+ if (q_vector->tx.ring)
+ tx_queue = q_vector->tx.ring->reg_idx;
switch (hw->mac.type) {
case e1000_82575:
@@ -950,15 +950,15 @@ static int igb_request_msix(struct igb_adapter *adapter)
q_vector->itr_register = hw->hw_addr + E1000_EITR(vector);
- if (q_vector->rx_ring && q_vector->tx_ring)
+ if (q_vector->rx.ring && q_vector->tx.ring)
sprintf(q_vector->name, "%s-TxRx-%u", netdev->name,
- q_vector->rx_ring->queue_index);
- else if (q_vector->tx_ring)
+ q_vector->rx.ring->queue_index);
+ else if (q_vector->tx.ring)
sprintf(q_vector->name, "%s-tx-%u", netdev->name,
- q_vector->tx_ring->queue_index);
- else if (q_vector->rx_ring)
+ q_vector->tx.ring->queue_index);
+ else if (q_vector->rx.ring)
sprintf(q_vector->name, "%s-rx-%u", netdev->name,
- q_vector->rx_ring->queue_index);
+ q_vector->rx.ring->queue_index);
else
sprintf(q_vector->name, "%s-unused", netdev->name);
@@ -1157,8 +1157,9 @@ static void igb_map_rx_ring_to_vector(struct igb_adapter *adapter,
{
struct igb_q_vector *q_vector = adapter->q_vector[v_idx];
- q_vector->rx_ring = adapter->rx_ring[ring_idx];
- q_vector->rx_ring->q_vector = q_vector;
+ q_vector->rx.ring = adapter->rx_ring[ring_idx];
+ q_vector->rx.ring->q_vector = q_vector;
+ q_vector->rx.count++;
q_vector->itr_val = adapter->rx_itr_setting;
if (q_vector->itr_val && q_vector->itr_val <= 3)
q_vector->itr_val = IGB_START_ITR;
@@ -1169,10 +1170,11 @@ static void igb_map_tx_ring_to_vector(struct igb_adapter *adapter,
{
struct igb_q_vector *q_vector = adapter->q_vector[v_idx];
- q_vector->tx_ring = adapter->tx_ring[ring_idx];
- q_vector->tx_ring->q_vector = q_vector;
+ q_vector->tx.ring = adapter->tx_ring[ring_idx];
+ q_vector->tx.ring->q_vector = q_vector;
+ q_vector->tx.count++;
q_vector->itr_val = adapter->tx_itr_setting;
- q_vector->tx_work_limit = adapter->tx_work_limit;
+ q_vector->tx.work_limit = adapter->tx_work_limit;
if (q_vector->itr_val && q_vector->itr_val <= 3)
q_vector->itr_val = IGB_START_ITR;
}
@@ -3826,33 +3828,24 @@ static void igb_update_ring_itr(struct igb_q_vector *q_vector)
int new_val = q_vector->itr_val;
int avg_wire_size = 0;
struct igb_adapter *adapter = q_vector->adapter;
- struct igb_ring *ring;
unsigned int packets;
/* For non-gigabit speeds, just fix the interrupt rate at 4000
* ints/sec - ITR timer value of 120 ticks.
*/
if (adapter->link_speed != SPEED_1000) {
- new_val = 976;
+ new_val = IGB_4K_ITR;
goto set_itr_val;
}
- ring = q_vector->rx_ring;
- if (ring) {
- packets = ACCESS_ONCE(ring->total_packets);
-
- if (packets)
- avg_wire_size = ring->total_bytes / packets;
- }
+ packets = q_vector->rx.total_packets;
+ if (packets)
+ avg_wire_size = q_vector->rx.total_bytes / packets;
- ring = q_vector->tx_ring;
- if (ring) {
- packets = ACCESS_ONCE(ring->total_packets);
-
- if (packets)
- avg_wire_size = max_t(u32, avg_wire_size,
- ring->total_bytes / packets);
- }
+ packets = q_vector->tx.total_packets;
+ if (packets)
+ avg_wire_size = max_t(u32, avg_wire_size,
+ q_vector->tx.total_bytes / packets);
/* if avg_wire_size isn't set no work was done */
if (!avg_wire_size)
@@ -3870,9 +3863,11 @@ static void igb_update_ring_itr(struct igb_q_vector *q_vector)
else
new_val = avg_wire_size / 2;
- /* when in itr mode 3 do not exceed 20K ints/sec */
- if (adapter->rx_itr_setting == 3 && new_val < 196)
- new_val = 196;
+ /* conservative mode (itr 3) eliminates the lowest_latency setting */
+ if (new_val < IGB_20K_ITR &&
+ ((q_vector->rx.ring && adapter->rx_itr_setting == 3) ||
+ (!q_vector->rx.ring && adapter->tx_itr_setting == 3)))
+ new_val = IGB_20K_ITR;
set_itr_val:
if (new_val != q_vector->itr_val) {
@@ -3880,14 +3875,10 @@ set_itr_val:
q_vector->set_itr = 1;
}
clear_counts:
- if (q_vector->rx_ring) {
- q_vector->rx_ring->total_bytes = 0;
- q_vector->rx_ring->total_packets = 0;
- }
- if (q_vector->tx_ring) {
- q_vector->tx_ring->total_bytes = 0;
- q_vector->tx_ring->total_packets = 0;
- }
+ q_vector->rx.total_bytes = 0;
+ q_vector->rx.total_packets = 0;
+ q_vector->tx.total_bytes = 0;
+ q_vector->tx.total_packets = 0;
}
/**
@@ -3903,106 +3894,102 @@ clear_counts:
* parameter (see igb_param.c)
* NOTE: These calculations are only valid when operating in a single-
* queue environment.
- * @adapter: pointer to adapter
- * @itr_setting: current q_vector->itr_val
- * @packets: the number of packets during this measurement interval
- * @bytes: the number of bytes during this measurement interval
+ * @q_vector: pointer to q_vector
+ * @ring_container: ring info to update the itr for
**/
-static unsigned int igb_update_itr(struct igb_adapter *adapter, u16 itr_setting,
- int packets, int bytes)
+static void igb_update_itr(struct igb_q_vector *q_vector,
+ struct igb_ring_container *ring_container)
{
- unsigned int retval = itr_setting;
+ unsigned int packets = ring_container->total_packets;
+ unsigned int bytes = ring_container->total_bytes;
+ u8 itrval = ring_container->itr;
+ /* no packets, exit with status unchanged */
if (packets == 0)
- goto update_itr_done;
+ return;
- switch (itr_setting) {
+ switch (itrval) {
case lowest_latency:
/* handle TSO and jumbo frames */
if (bytes/packets > 8000)
- retval = bulk_latency;
+ itrval = bulk_latency;
else if ((packets < 5) && (bytes > 512))
- retval = low_latency;
+ itrval = low_latency;
break;
case low_latency: /* 50 usec aka 20000 ints/s */
if (bytes > 10000) {
/* this if handles the TSO accounting */
if (bytes/packets > 8000) {
- retval = bulk_latency;
+ itrval = bulk_latency;
} else if ((packets < 10) || ((bytes/packets) > 1200)) {
- retval = bulk_latency;
+ itrval = bulk_latency;
} else if ((packets > 35)) {
- retval = lowest_latency;
+ itrval = lowest_latency;
}
} else if (bytes/packets > 2000) {
- retval = bulk_latency;
+ itrval = bulk_latency;
} else if (packets <= 2 && bytes < 512) {
- retval = lowest_latency;
+ itrval = lowest_latency;
}
break;
case bulk_latency: /* 250 usec aka 4000 ints/s */
if (bytes > 25000) {
if (packets > 35)
- retval = low_latency;
+ itrval = low_latency;
} else if (bytes < 1500) {
- retval = low_latency;
+ itrval = low_latency;
}
break;
}
-update_itr_done:
- return retval;
+ /* clear work counters since we have the values we need */
+ ring_container->total_bytes = 0;
+ ring_container->total_packets = 0;
+
+ /* write updated itr to ring container */
+ ring_container->itr = itrval;
}
-static void igb_set_itr(struct igb_adapter *adapter)
+static void igb_set_itr(struct igb_q_vector *q_vector)
{
- struct igb_q_vector *q_vector = adapter->q_vector[0];
- u16 current_itr;
+ struct igb_adapter *adapter = q_vector->adapter;
u32 new_itr = q_vector->itr_val;
+ u8 current_itr = 0;
/* for non-gigabit speeds, just fix the interrupt rate at 4000 */
if (adapter->link_speed != SPEED_1000) {
current_itr = 0;
- new_itr = 4000;
+ new_itr = IGB_4K_ITR;
goto set_itr_now;
}
- adapter->rx_itr = igb_update_itr(adapter,
- adapter->rx_itr,
- q_vector->rx_ring->total_packets,
- q_vector->rx_ring->total_bytes);
+ igb_update_itr(q_vector, &q_vector->tx);
+ igb_update_itr(q_vector, &q_vector->rx);
- adapter->tx_itr = igb_update_itr(adapter,
- adapter->tx_itr,
- q_vector->tx_ring->total_packets,
- q_vector->tx_ring->total_bytes);
- current_itr = max(adapter->rx_itr, adapter->tx_itr);
+ current_itr = max(q_vector->rx.itr, q_vector->tx.itr);
/* conservative mode (itr 3) eliminates the lowest_latency setting */
- if (adapter->rx_itr_setting == 3 && current_itr == lowest_latency)
+ if (current_itr == lowest_latency &&
+ ((q_vector->rx.ring && adapter->rx_itr_setting == 3) ||
+ (!q_vector->rx.ring && adapter->tx_itr_setting == 3)))
current_itr = low_latency;
switch (current_itr) {
/* counts and packets in update_itr are dependent on these numbers */
case lowest_latency:
- new_itr = 56; /* aka 70,000 ints/sec */
+ new_itr = IGB_70K_ITR; /* 70,000 ints/sec */
break;
case low_latency:
- new_itr = 196; /* aka 20,000 ints/sec */
+ new_itr = IGB_20K_ITR; /* 20,000 ints/sec */
break;
case bulk_latency:
- new_itr = 980; /* aka 4,000 ints/sec */
+ new_itr = IGB_4K_ITR; /* 4,000 ints/sec */
break;
default:
break;
}
set_itr_now:
- q_vector->rx_ring->total_bytes = 0;
- q_vector->rx_ring->total_packets = 0;
- q_vector->tx_ring->total_bytes = 0;
- q_vector->tx_ring->total_packets = 0;
-
if (new_itr != q_vector->itr_val) {
/* this attempts to bias the interrupt rate towards Bulk
* by adding intermediate steps when interrupt rate is
@@ -4010,7 +3997,7 @@ set_itr_now:
new_itr = new_itr > q_vector->itr_val ?
max((new_itr * q_vector->itr_val) /
(new_itr + (q_vector->itr_val >> 2)),
- new_itr) :
+ new_itr) :
new_itr;
/* Don't write the value here; it resets the adapter's
* internal timer, and causes us to delay far longer than
@@ -4830,7 +4817,7 @@ static void igb_write_itr(struct igb_q_vector *q_vector)
if (adapter->hw.mac.type == e1000_82575)
itr_val |= itr_val << 16;
else
- itr_val |= 0x8000000;
+ itr_val |= E1000_EITR_CNT_IGNR;
writel(itr_val, q_vector->itr_register);
q_vector->set_itr = 0;
@@ -4858,8 +4845,8 @@ static void igb_update_dca(struct igb_q_vector *q_vector)
if (q_vector->cpu == cpu)
goto out_no_update;
- if (q_vector->tx_ring) {
- int q = q_vector->tx_ring->reg_idx;
+ if (q_vector->tx.ring) {
+ int q = q_vector->tx.ring->reg_idx;
u32 dca_txctrl = rd32(E1000_DCA_TXCTRL(q));
if (hw->mac.type == e1000_82575) {
dca_txctrl &= ~E1000_DCA_TXCTRL_CPUID_MASK;
@@ -4872,8 +4859,8 @@ static void igb_update_dca(struct igb_q_vector *q_vector)
dca_txctrl |= E1000_DCA_TXCTRL_DESC_DCA_EN;
wr32(E1000_DCA_TXCTRL(q), dca_txctrl);
}
- if (q_vector->rx_ring) {
- int q = q_vector->rx_ring->reg_idx;
+ if (q_vector->rx.ring) {
+ int q = q_vector->rx.ring->reg_idx;
u32 dca_rxctrl = rd32(E1000_DCA_RXCTRL(q));
if (hw->mac.type == e1000_82575) {
dca_rxctrl &= ~E1000_DCA_RXCTRL_CPUID_MASK;
@@ -5517,16 +5504,14 @@ static irqreturn_t igb_intr(int irq, void *data)
/* Interrupt Auto-Mask...upon reading ICR, interrupts are masked. No
* need for the IMC write */
u32 icr = rd32(E1000_ICR);
- if (!icr)
- return IRQ_NONE; /* Not our interrupt */
-
- igb_write_itr(q_vector);
/* IMS will not auto-mask if INT_ASSERTED is not set, and if it is
* not set, then the adapter didn't send an interrupt */
if (!(icr & E1000_ICR_INT_ASSERTED))
return IRQ_NONE;
+ igb_write_itr(q_vector);
+
if (icr & E1000_ICR_DRSTA)
schedule_work(&adapter->reset_task);
@@ -5547,15 +5532,15 @@ static irqreturn_t igb_intr(int irq, void *data)
return IRQ_HANDLED;
}
-static inline void igb_ring_irq_enable(struct igb_q_vector *q_vector)
+void igb_ring_irq_enable(struct igb_q_vector *q_vector)
{
struct igb_adapter *adapter = q_vector->adapter;
struct e1000_hw *hw = &adapter->hw;
- if ((q_vector->rx_ring && (adapter->rx_itr_setting & 3)) ||
- (!q_vector->rx_ring && (adapter->tx_itr_setting & 3))) {
- if (!adapter->msix_entries)
- igb_set_itr(adapter);
+ if ((q_vector->rx.ring && (adapter->rx_itr_setting & 3)) ||
+ (!q_vector->rx.ring && (adapter->tx_itr_setting & 3))) {
+ if ((adapter->num_q_vectors == 1) && !adapter->vf_data)
+ igb_set_itr(q_vector);
else
igb_update_ring_itr(q_vector);
}
@@ -5584,10 +5569,10 @@ static int igb_poll(struct napi_struct *napi, int budget)
if (q_vector->adapter->flags & IGB_FLAG_DCA_ENABLED)
igb_update_dca(q_vector);
#endif
- if (q_vector->tx_ring)
+ if (q_vector->tx.ring)
clean_complete = igb_clean_tx_irq(q_vector);
- if (q_vector->rx_ring)
+ if (q_vector->rx.ring)
clean_complete &= igb_clean_rx_irq(q_vector, budget);
/* If all work not completed, return budget and keep polling */
@@ -5667,11 +5652,11 @@ static void igb_tx_hwtstamp(struct igb_q_vector *q_vector,
static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
{
struct igb_adapter *adapter = q_vector->adapter;
- struct igb_ring *tx_ring = q_vector->tx_ring;
+ struct igb_ring *tx_ring = q_vector->tx.ring;
struct igb_tx_buffer *tx_buffer;
union e1000_adv_tx_desc *tx_desc, *eop_desc;
unsigned int total_bytes = 0, total_packets = 0;
- unsigned int budget = q_vector->tx_work_limit;
+ unsigned int budget = q_vector->tx.work_limit;
unsigned int i = tx_ring->next_to_clean;
if (test_bit(__IGB_DOWN, &adapter->state))
@@ -5757,8 +5742,8 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
tx_ring->tx_stats.bytes += total_bytes;
tx_ring->tx_stats.packets += total_packets;
u64_stats_update_end(&tx_ring->tx_syncp);
- tx_ring->total_bytes += total_bytes;
- tx_ring->total_packets += total_packets;
+ q_vector->tx.total_bytes += total_bytes;
+ q_vector->tx.total_packets += total_packets;
if (tx_ring->detect_tx_hung) {
struct e1000_hw *hw = &adapter->hw;
@@ -5907,7 +5892,7 @@ static inline u16 igb_get_hlen(union e1000_adv_rx_desc *rx_desc)
static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
{
- struct igb_ring *rx_ring = q_vector->rx_ring;
+ struct igb_ring *rx_ring = q_vector->rx.ring;
union e1000_adv_rx_desc *rx_desc;
const int current_node = numa_node_id();
unsigned int total_bytes = 0, total_packets = 0;
@@ -6024,8 +6009,8 @@ next_desc:
rx_ring->rx_stats.packets += total_packets;
rx_ring->rx_stats.bytes += total_bytes;
u64_stats_update_end(&rx_ring->rx_syncp);
- rx_ring->total_packets += total_packets;
- rx_ring->total_bytes += total_bytes;
+ q_vector->rx.total_packets += total_packets;
+ q_vector->rx.total_bytes += total_bytes;
if (cleaned_count)
igb_alloc_rx_buffers(rx_ring, cleaned_count);
--
1.7.6.4
^ permalink raw reply related
* [net-next 07/11] igb: retire the RX_CSUM flag and use the netdev flag instead
From: Jeff Kirsher @ 2011-10-08 6:47 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1318056461-19562-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
Since the netdev now has its' own checksum flag to indicate if Rx checksum
is enabled we might as well use that instead of using the ring flag.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 1 -
drivers/net/ethernet/intel/igb/igb_main.c | 22 ++++++----------------
2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 91f90fe..fde381a 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -245,7 +245,6 @@ struct igb_ring {
};
enum e1000_ring_flags_t {
- IGB_RING_FLAG_RX_CSUM,
IGB_RING_FLAG_RX_SCTP_CSUM,
IGB_RING_FLAG_TX_CTX_IDX,
IGB_RING_FLAG_TX_DETECT_HANG
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index ec715f4..cae4abb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -732,8 +732,6 @@ static int igb_alloc_queues(struct igb_adapter *adapter)
ring->dev = &adapter->pdev->dev;
ring->netdev = adapter->netdev;
ring->numa_node = adapter->node;
- /* enable rx checksum */
- set_bit(IGB_RING_FLAG_RX_CSUM, &ring->flags);
/* set flag indicating ring supports SCTP checksum offload */
if (adapter->hw.mac.type >= e1000_82576)
set_bit(IGB_RING_FLAG_RX_SCTP_CSUM, &ring->flags);
@@ -1811,19 +1809,8 @@ static u32 igb_fix_features(struct net_device *netdev, u32 features)
static int igb_set_features(struct net_device *netdev, u32 features)
{
- struct igb_adapter *adapter = netdev_priv(netdev);
- int i;
u32 changed = netdev->features ^ features;
- for (i = 0; i < adapter->num_rx_queues; i++) {
- if (features & NETIF_F_RXCSUM)
- set_bit(IGB_RING_FLAG_RX_CSUM,
- &adapter->rx_ring[i]->flags);
- else
- clear_bit(IGB_RING_FLAG_RX_CSUM,
- &adapter->rx_ring[i]->flags);
- }
-
if (changed & NETIF_F_HW_VLAN_RX)
igb_vlan_mode(netdev, features);
@@ -5807,9 +5794,12 @@ static inline void igb_rx_checksum(struct igb_ring *ring,
{
skb_checksum_none_assert(skb);
- /* Ignore Checksum bit is set or checksum is disabled through ethtool */
- if (!test_bit(IGB_RING_FLAG_RX_CSUM, &ring->flags) ||
- (status_err & E1000_RXD_STAT_IXSM))
+ /* Ignore Checksum bit is set */
+ if (status_err & E1000_RXD_STAT_IXSM)
+ return;
+
+ /* Rx checksum disabled via ethtool */
+ if (!(ring->netdev->features & NETIF_F_RXCSUM))
return;
/* TCP/UDP checksum error bit is set */
--
1.7.6.4
^ permalink raw reply related
* [net-next 06/11] igb: cleanup IVAR configuration
From: Jeff Kirsher @ 2011-10-08 6:47 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1318056461-19562-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change is meant to cleanup some of the IVAR register configuration.
igb_assign_vector had become pretty large with multiple copies of the same
general code for setting the IVAR. This change consolidates most of that
code by adding the igb_write_ivar function which allows us just to compute
the index and offset and then use that information to setup the IVAR.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 120 +++++++++++++---------------
1 files changed, 56 insertions(+), 64 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 8dc04e0..ec715f4 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -754,15 +754,40 @@ err:
return -ENOMEM;
}
+/**
+ * igb_write_ivar - configure ivar for given MSI-X vector
+ * @hw: pointer to the HW structure
+ * @msix_vector: vector number we are allocating to a given ring
+ * @index: row index of IVAR register to write within IVAR table
+ * @offset: column offset of in IVAR, should be multiple of 8
+ *
+ * This function is intended to handle the writing of the IVAR register
+ * for adapters 82576 and newer. The IVAR table consists of 2 columns,
+ * each containing an cause allocation for an Rx and Tx ring, and a
+ * variable number of rows depending on the number of queues supported.
+ **/
+static void igb_write_ivar(struct e1000_hw *hw, int msix_vector,
+ int index, int offset)
+{
+ u32 ivar = array_rd32(E1000_IVAR0, index);
+
+ /* clear any bits that are currently set */
+ ivar &= ~((u32)0xFF << offset);
+
+ /* write vector and valid bit */
+ ivar |= (msix_vector | E1000_IVAR_VALID) << offset;
+
+ array_wr32(E1000_IVAR0, index, ivar);
+}
+
#define IGB_N0_QUEUE -1
static void igb_assign_vector(struct igb_q_vector *q_vector, int msix_vector)
{
- u32 msixbm = 0;
struct igb_adapter *adapter = q_vector->adapter;
struct e1000_hw *hw = &adapter->hw;
- u32 ivar, index;
int rx_queue = IGB_N0_QUEUE;
int tx_queue = IGB_N0_QUEUE;
+ u32 msixbm = 0;
if (q_vector->rx.ring)
rx_queue = q_vector->rx.ring->reg_idx;
@@ -785,72 +810,39 @@ static void igb_assign_vector(struct igb_q_vector *q_vector, int msix_vector)
q_vector->eims_value = msixbm;
break;
case e1000_82576:
- /* 82576 uses a table-based method for assigning vectors.
- Each queue has a single entry in the table to which we write
- a vector number along with a "valid" bit. Sadly, the layout
- of the table is somewhat counterintuitive. */
- if (rx_queue > IGB_N0_QUEUE) {
- index = (rx_queue & 0x7);
- ivar = array_rd32(E1000_IVAR0, index);
- if (rx_queue < 8) {
- /* vector goes into low byte of register */
- ivar = ivar & 0xFFFFFF00;
- ivar |= msix_vector | E1000_IVAR_VALID;
- } else {
- /* vector goes into third byte of register */
- ivar = ivar & 0xFF00FFFF;
- ivar |= (msix_vector | E1000_IVAR_VALID) << 16;
- }
- array_wr32(E1000_IVAR0, index, ivar);
- }
- if (tx_queue > IGB_N0_QUEUE) {
- index = (tx_queue & 0x7);
- ivar = array_rd32(E1000_IVAR0, index);
- if (tx_queue < 8) {
- /* vector goes into second byte of register */
- ivar = ivar & 0xFFFF00FF;
- ivar |= (msix_vector | E1000_IVAR_VALID) << 8;
- } else {
- /* vector goes into high byte of register */
- ivar = ivar & 0x00FFFFFF;
- ivar |= (msix_vector | E1000_IVAR_VALID) << 24;
- }
- array_wr32(E1000_IVAR0, index, ivar);
- }
+ /*
+ * 82576 uses a table that essentially consists of 2 columns
+ * with 8 rows. The ordering is column-major so we use the
+ * lower 3 bits as the row index, and the 4th bit as the
+ * column offset.
+ */
+ if (rx_queue > IGB_N0_QUEUE)
+ igb_write_ivar(hw, msix_vector,
+ rx_queue & 0x7,
+ (rx_queue & 0x8) << 1);
+ if (tx_queue > IGB_N0_QUEUE)
+ igb_write_ivar(hw, msix_vector,
+ tx_queue & 0x7,
+ ((tx_queue & 0x8) << 1) + 8);
q_vector->eims_value = 1 << msix_vector;
break;
case e1000_82580:
case e1000_i350:
- /* 82580 uses the same table-based approach as 82576 but has fewer
- entries as a result we carry over for queues greater than 4. */
- if (rx_queue > IGB_N0_QUEUE) {
- index = (rx_queue >> 1);
- ivar = array_rd32(E1000_IVAR0, index);
- if (rx_queue & 0x1) {
- /* vector goes into third byte of register */
- ivar = ivar & 0xFF00FFFF;
- ivar |= (msix_vector | E1000_IVAR_VALID) << 16;
- } else {
- /* vector goes into low byte of register */
- ivar = ivar & 0xFFFFFF00;
- ivar |= msix_vector | E1000_IVAR_VALID;
- }
- array_wr32(E1000_IVAR0, index, ivar);
- }
- if (tx_queue > IGB_N0_QUEUE) {
- index = (tx_queue >> 1);
- ivar = array_rd32(E1000_IVAR0, index);
- if (tx_queue & 0x1) {
- /* vector goes into high byte of register */
- ivar = ivar & 0x00FFFFFF;
- ivar |= (msix_vector | E1000_IVAR_VALID) << 24;
- } else {
- /* vector goes into second byte of register */
- ivar = ivar & 0xFFFF00FF;
- ivar |= (msix_vector | E1000_IVAR_VALID) << 8;
- }
- array_wr32(E1000_IVAR0, index, ivar);
- }
+ /*
+ * On 82580 and newer adapters the scheme is similar to 82576
+ * however instead of ordering column-major we have things
+ * ordered row-major. So we traverse the table by using
+ * bit 0 as the column offset, and the remaining bits as the
+ * row index.
+ */
+ if (rx_queue > IGB_N0_QUEUE)
+ igb_write_ivar(hw, msix_vector,
+ rx_queue >> 1,
+ (rx_queue & 0x1) << 4);
+ if (tx_queue > IGB_N0_QUEUE)
+ igb_write_ivar(hw, msix_vector,
+ tx_queue >> 1,
+ ((tx_queue & 0x1) << 4) + 8);
q_vector->eims_value = 1 << msix_vector;
break;
default:
--
1.7.6.4
^ permalink raw reply related
* [net-next 08/11] igb: leave staterr in place and instead us a helper function to check bits
From: Jeff Kirsher @ 2011-10-08 6:47 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1318056461-19562-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
Instead of doing a byte swap on the staterr bits in the Rx descriptor we can
save ourselves a bit of space and some CPU time by instead just testing for
the various bits out of the Rx descriptor directly.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 7 +++
drivers/net/ethernet/intel/igb/igb_ethtool.c | 5 +--
drivers/net/ethernet/intel/igb/igb_main.c | 55 ++++++++++++++-----------
3 files changed, 39 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index fde381a..11d17f1 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -259,6 +259,13 @@ enum e1000_ring_flags_t {
#define IGB_TX_CTXTDESC(R, i) \
(&(((struct e1000_adv_tx_context_desc *)((R)->desc))[i]))
+/* igb_test_staterr - tests bits within Rx descriptor status and error fields */
+static inline __le32 igb_test_staterr(union e1000_adv_rx_desc *rx_desc,
+ const u32 stat_err_bits)
+{
+ return rx_desc->wb.upper.status_error & cpu_to_le32(stat_err_bits);
+}
+
/* igb_desc_unused - calculate if we have unused descriptors */
static inline int igb_desc_unused(struct igb_ring *ring)
{
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 5ebe992..bc198ea 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -1581,16 +1581,14 @@ static int igb_clean_test_rings(struct igb_ring *rx_ring,
union e1000_adv_rx_desc *rx_desc;
struct igb_rx_buffer *rx_buffer_info;
struct igb_tx_buffer *tx_buffer_info;
- u32 staterr;
u16 rx_ntc, tx_ntc, count = 0;
/* initialize next to clean and descriptor values */
rx_ntc = rx_ring->next_to_clean;
tx_ntc = tx_ring->next_to_clean;
rx_desc = IGB_RX_DESC(rx_ring, rx_ntc);
- staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
- while (staterr & E1000_RXD_STAT_DD) {
+ while (igb_test_staterr(rx_desc, E1000_RXD_STAT_DD)) {
/* check rx buffer */
rx_buffer_info = &rx_ring->rx_buffer_info[rx_ntc];
@@ -1619,7 +1617,6 @@ static int igb_clean_test_rings(struct igb_ring *rx_ring,
/* fetch next descriptor */
rx_desc = IGB_RX_DESC(rx_ring, rx_ntc);
- staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
}
/* re-map buffers to ring, store next to clean values */
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index cae4abb..1419ae8 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5790,12 +5790,13 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
}
static inline void igb_rx_checksum(struct igb_ring *ring,
- u32 status_err, struct sk_buff *skb)
+ union e1000_adv_rx_desc *rx_desc,
+ struct sk_buff *skb)
{
skb_checksum_none_assert(skb);
/* Ignore Checksum bit is set */
- if (status_err & E1000_RXD_STAT_IXSM)
+ if (igb_test_staterr(rx_desc, E1000_RXD_STAT_IXSM))
return;
/* Rx checksum disabled via ethtool */
@@ -5803,8 +5804,9 @@ static inline void igb_rx_checksum(struct igb_ring *ring,
return;
/* TCP/UDP checksum error bit is set */
- if (status_err &
- (E1000_RXDEXT_STATERR_TCPE | E1000_RXDEXT_STATERR_IPE)) {
+ if (igb_test_staterr(rx_desc,
+ E1000_RXDEXT_STATERR_TCPE |
+ E1000_RXDEXT_STATERR_IPE)) {
/*
* work around errata with sctp packets where the TCPE aka
* L4E bit is set incorrectly on 64 byte (60 byte w/o crc)
@@ -5820,19 +5822,26 @@ static inline void igb_rx_checksum(struct igb_ring *ring,
return;
}
/* It must be a TCP or UDP packet with a valid checksum */
- if (status_err & (E1000_RXD_STAT_TCPCS | E1000_RXD_STAT_UDPCS))
+ if (igb_test_staterr(rx_desc, E1000_RXD_STAT_TCPCS |
+ E1000_RXD_STAT_UDPCS))
skb->ip_summed = CHECKSUM_UNNECESSARY;
- dev_dbg(ring->dev, "cksum success: bits %08X\n", status_err);
+ dev_dbg(ring->dev, "cksum success: bits %08X\n",
+ le32_to_cpu(rx_desc->wb.upper.status_error));
}
-static void igb_rx_hwtstamp(struct igb_q_vector *q_vector, u32 staterr,
- struct sk_buff *skb)
+static void igb_rx_hwtstamp(struct igb_q_vector *q_vector,
+ union e1000_adv_rx_desc *rx_desc,
+ struct sk_buff *skb)
{
struct igb_adapter *adapter = q_vector->adapter;
struct e1000_hw *hw = &adapter->hw;
u64 regval;
+ if (!igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP |
+ E1000_RXDADV_STAT_TS))
+ return;
+
/*
* If this bit is set, then the RX registers contain the time stamp. No
* other packet will be time stamped until we read these registers, so
@@ -5844,7 +5853,7 @@ static void igb_rx_hwtstamp(struct igb_q_vector *q_vector, u32 staterr,
* If nothing went wrong, then it should have a shared tx_flags that we
* can turn into a skb_shared_hwtstamps.
*/
- if (staterr & E1000_RXDADV_STAT_TSIP) {
+ if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) {
u32 *stamp = (u32 *)skb->data;
regval = le32_to_cpu(*(stamp + 2));
regval |= (u64)le32_to_cpu(*(stamp + 3)) << 32;
@@ -5878,14 +5887,12 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
union e1000_adv_rx_desc *rx_desc;
const int current_node = numa_node_id();
unsigned int total_bytes = 0, total_packets = 0;
- u32 staterr;
u16 cleaned_count = igb_desc_unused(rx_ring);
u16 i = rx_ring->next_to_clean;
rx_desc = IGB_RX_DESC(rx_ring, i);
- staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
- while (staterr & E1000_RXD_STAT_DD) {
+ while (igb_test_staterr(rx_desc, E1000_RXD_STAT_DD)) {
struct igb_rx_buffer *buffer_info = &rx_ring->rx_buffer_info[i];
struct sk_buff *skb = buffer_info->skb;
union e1000_adv_rx_desc *next_rxd;
@@ -5938,7 +5945,7 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
buffer_info->page_dma = 0;
}
- if (!(staterr & E1000_RXD_STAT_EOP)) {
+ if (!igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP)) {
struct igb_rx_buffer *next_buffer;
next_buffer = &rx_ring->rx_buffer_info[i];
buffer_info->skb = next_buffer->skb;
@@ -5948,25 +5955,26 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
goto next_desc;
}
- if (staterr & E1000_RXDEXT_ERR_FRAME_ERR_MASK) {
+ if (igb_test_staterr(rx_desc,
+ E1000_RXDEXT_ERR_FRAME_ERR_MASK)) {
dev_kfree_skb_any(skb);
goto next_desc;
}
- if (staterr & (E1000_RXDADV_STAT_TSIP | E1000_RXDADV_STAT_TS))
- igb_rx_hwtstamp(q_vector, staterr, skb);
- total_bytes += skb->len;
- total_packets++;
-
- igb_rx_checksum(rx_ring, staterr, skb);
-
- skb->protocol = eth_type_trans(skb, rx_ring->netdev);
+ igb_rx_hwtstamp(q_vector, rx_desc, skb);
+ igb_rx_checksum(rx_ring, rx_desc, skb);
- if (staterr & E1000_RXD_STAT_VP) {
+ if (igb_test_staterr(rx_desc, E1000_RXD_STAT_VP)) {
u16 vid = le16_to_cpu(rx_desc->wb.upper.vlan);
__vlan_hwaccel_put_tag(skb, vid);
}
+
+ total_bytes += skb->len;
+ total_packets++;
+
+ skb->protocol = eth_type_trans(skb, rx_ring->netdev);
+
napi_gro_receive(&q_vector->napi, skb);
budget--;
@@ -5983,7 +5991,6 @@ next_desc:
/* use prefetched values */
rx_desc = next_rxd;
- staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
}
rx_ring->next_to_clean = i;
--
1.7.6.4
^ permalink raw reply related
* [net-next 09/11] igb: fix recent VLAN changes that would leave VLANs disabled after reset
From: Jeff Kirsher @ 2011-10-08 6:47 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1318056461-19562-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This patch cleans up several issues with VLANs on igb after the recent
changes that were meant to leave the VLANs enabled/disable via the
netdev->features flags.
Specifically the Rx VLAN settings were being dropped after reset due to the
fact that they were not being restored correctly. In addition I removed
the IRQ disable/enable since those were in place to protect the setting of
vlgrp.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 18 ++++--------------
1 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 1419ae8..971aea9 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2112,8 +2112,6 @@ static int __devinit igb_probe(struct pci_dev *pdev,
if (err)
goto err_register;
- igb_vlan_mode(netdev, netdev->features);
-
/* carrier off reporting is important to ethtool even BEFORE open */
netif_carrier_off(netdev);
@@ -5120,7 +5118,6 @@ static s32 igb_vlvf_set(struct igb_adapter *adapter, u32 vid, bool add, u32 vf)
}
adapter->vf_data[vf].vlans_enabled++;
- return 0;
}
} else {
if (i < E1000_VLVF_ARRAY_SIZE) {
@@ -6385,10 +6382,9 @@ static void igb_vlan_mode(struct net_device *netdev, u32 features)
struct igb_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
u32 ctrl, rctl;
+ bool enable = !!(features & NETIF_F_HW_VLAN_RX);
- igb_irq_disable(adapter);
-
- if (features & NETIF_F_HW_VLAN_RX) {
+ if (enable) {
/* enable VLAN tag insert/strip */
ctrl = rd32(E1000_CTRL);
ctrl |= E1000_CTRL_VME;
@@ -6406,9 +6402,6 @@ static void igb_vlan_mode(struct net_device *netdev, u32 features)
}
igb_rlpml_set(adapter);
-
- if (!test_bit(__IGB_DOWN, &adapter->state))
- igb_irq_enable(adapter);
}
static void igb_vlan_rx_add_vid(struct net_device *netdev, u16 vid)
@@ -6433,11 +6426,6 @@ static void igb_vlan_rx_kill_vid(struct net_device *netdev, u16 vid)
int pf_id = adapter->vfs_allocated_count;
s32 err;
- igb_irq_disable(adapter);
-
- if (!test_bit(__IGB_DOWN, &adapter->state))
- igb_irq_enable(adapter);
-
/* remove vlan from VLVF table array */
err = igb_vlvf_set(adapter, vid, false, pf_id);
@@ -6452,6 +6440,8 @@ static void igb_restore_vlan(struct igb_adapter *adapter)
{
u16 vid;
+ igb_vlan_mode(adapter->netdev, adapter->netdev->features);
+
for_each_set_bit(vid, adapter->active_vlans, VLAN_N_VID)
igb_vlan_rx_add_vid(adapter->netdev, vid);
}
--
1.7.6.4
^ permalink raw reply related
* [net-next 10/11] igb: move TX hang check flag into ring->flags
From: Jeff Kirsher @ 2011-10-08 6:47 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1318056461-19562-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change moves the Tx hang check into the ring flags.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 1 -
drivers/net/ethernet/intel/igb/igb_main.c | 6 +++---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 11d17f1..4e665a9 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -231,7 +231,6 @@ struct igb_ring {
struct igb_tx_queue_stats tx_stats;
struct u64_stats_sync tx_syncp;
struct u64_stats_sync tx_syncp2;
- bool detect_tx_hung;
};
/* RX */
struct {
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 971aea9..77ade67 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3754,7 +3754,7 @@ static void igb_watchdog_task(struct work_struct *work)
}
/* Force detection of hung controller every watchdog period */
- tx_ring->detect_tx_hung = true;
+ set_bit(IGB_RING_FLAG_TX_DETECT_HANG, &tx_ring->flags);
}
/* Cause software interrupt to ensure rx ring is cleaned */
@@ -5721,14 +5721,14 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
q_vector->tx.total_bytes += total_bytes;
q_vector->tx.total_packets += total_packets;
- if (tx_ring->detect_tx_hung) {
+ if (test_bit(IGB_RING_FLAG_TX_DETECT_HANG, &tx_ring->flags)) {
struct e1000_hw *hw = &adapter->hw;
eop_desc = tx_buffer->next_to_watch;
/* Detect a transmit hang in hardware, this serializes the
* check with the clearing of time_stamp and movement of i */
- tx_ring->detect_tx_hung = false;
+ clear_bit(IGB_RING_FLAG_TX_DETECT_HANG, &tx_ring->flags);
if (eop_desc &&
time_after(jiffies, tx_buffer->time_stamp +
(adapter->tx_timeout_factor * HZ)) &&
--
1.7.6.4
^ permalink raw reply related
* [net-next 11/11] igb: add support for NETIF_F_RXHASH
From: Jeff Kirsher @ 2011-10-08 6:47 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1318056461-19562-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This patch adds support for Rx hashing.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 52 +++++++++++++++++++---------
1 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 77ade67..10670f9 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1978,23 +1978,32 @@ static int __devinit igb_probe(struct pci_dev *pdev,
dev_info(&pdev->dev,
"PHY reset is blocked due to SOL/IDER session.\n");
- netdev->hw_features = NETIF_F_SG |
- NETIF_F_IP_CSUM |
- NETIF_F_IPV6_CSUM |
- NETIF_F_TSO |
- NETIF_F_TSO6 |
- NETIF_F_RXCSUM |
- NETIF_F_HW_VLAN_RX;
-
- netdev->features = netdev->hw_features |
- NETIF_F_HW_VLAN_TX |
- NETIF_F_HW_VLAN_FILTER;
-
- netdev->vlan_features |= NETIF_F_TSO;
- netdev->vlan_features |= NETIF_F_TSO6;
- netdev->vlan_features |= NETIF_F_IP_CSUM;
- netdev->vlan_features |= NETIF_F_IPV6_CSUM;
- netdev->vlan_features |= NETIF_F_SG;
+ /*
+ * features is initialized to 0 in allocation, it might have bits
+ * set by igb_sw_init so we should use an or instead of an
+ * assignment.
+ */
+ netdev->features |= NETIF_F_SG |
+ NETIF_F_IP_CSUM |
+ NETIF_F_IPV6_CSUM |
+ NETIF_F_TSO |
+ NETIF_F_TSO6 |
+ NETIF_F_RXHASH |
+ NETIF_F_RXCSUM |
+ NETIF_F_HW_VLAN_RX |
+ NETIF_F_HW_VLAN_TX;
+
+ /* copy netdev features into list of user selectable features */
+ netdev->hw_features |= netdev->features;
+
+ /* set this bit last since it cannot be part of hw_features */
+ netdev->features |= NETIF_F_HW_VLAN_FILTER;
+
+ netdev->vlan_features |= NETIF_F_TSO |
+ NETIF_F_TSO6 |
+ NETIF_F_IP_CSUM |
+ NETIF_F_IPV6_CSUM |
+ NETIF_F_SG;
if (pci_using_dac) {
netdev->features |= NETIF_F_HIGHDMA;
@@ -5827,6 +5836,14 @@ static inline void igb_rx_checksum(struct igb_ring *ring,
le32_to_cpu(rx_desc->wb.upper.status_error));
}
+static inline void igb_rx_hash(struct igb_ring *ring,
+ union e1000_adv_rx_desc *rx_desc,
+ struct sk_buff *skb)
+{
+ if (ring->netdev->features & NETIF_F_RXHASH)
+ skb->rxhash = le32_to_cpu(rx_desc->wb.lower.hi_dword.rss);
+}
+
static void igb_rx_hwtstamp(struct igb_q_vector *q_vector,
union e1000_adv_rx_desc *rx_desc,
struct sk_buff *skb)
@@ -5959,6 +5976,7 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
}
igb_rx_hwtstamp(q_vector, rx_desc, skb);
+ igb_rx_hash(rx_ring, rx_desc, skb);
igb_rx_checksum(rx_ring, rx_desc, skb);
if (igb_test_staterr(rx_desc, E1000_RXD_STAT_VP)) {
--
1.7.6.4
^ permalink raw reply related
* Re: [net-next 00/11][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2011-10-08 6:52 UTC (permalink / raw)
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <1318056461-19562-1-git-send-email-jeffrey.t.kirsher@intel.com>
[-- Attachment #1: Type: text/plain, Size: 715 bytes --]
On Fri, 2011-10-07 at 23:47 -0700, Kirsher, Jeffrey T wrote:
> The following series contains updates to igb only. They are a
> continuation of the cleanups and refactoring that Alex has done.
> After this series there are 4-5 more patches to complete the work
> that Alex has done on igb.
>
> The following are changes since commit
> 1d0861acfb24d0ca0661ff5a156b992b2c589458:
> Add ethtool -g support to 8139cp
> and are available in the git repository at
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next.git
> or
> git://github.com/Jkirsher/net-next.git
Even though I have my kernel.org tree back up and running, I will keep
the github tree's updated (at least for now).
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: moving GIT back to kernel.org...
From: Yinglin Sun @ 2011-10-08 6:55 UTC (permalink / raw)
To: David Miller
Cc: netdev, sfr, linux-wireless, netfilter-devel, sparclinux,
linux-ide
In-Reply-To: <20111007.155902.916470368193047875.davem@davemloft.net>
On Fri, Oct 7, 2011 at 12:59 PM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Fri, 07 Oct 2011 14:57:03 -0400 (EDT)
>
>>
>> I'm about to setup my GIT trees on kernel.org, once that is complete
>> I will be solely updating those trees again.
>>
>> I will notify everyone when this is ready to go.
>>
>> Just a heads up for everyone...
>
> Ok, they are now online, please update your URLs.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide.git
>
I'm still new to net kernel development, so a little confused about these trees.
So we should submit patches based on these trees, instead of Linus'?
About net and net-next, How to decide which one to use?
Thanks!
Yinglin
^ permalink raw reply
* Re: moving GIT back to kernel.org...
From: Jeff Kirsher @ 2011-10-08 7:01 UTC (permalink / raw)
To: Yinglin Sun
Cc: David Miller, netdev, sfr, linux-wireless, netfilter-devel,
sparclinux, linux-ide
In-Reply-To: <CAN17JHUGnAbHJmwybjJVdxq4JOgHfDxChw1P7q1yrkqH6Ci05w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]
On 10/07/2011 11:55 PM, Yinglin Sun wrote:
> On Fri, Oct 7, 2011 at 12:59 PM, David Miller <davem@davemloft.net> wrote:
>> From: David Miller <davem@davemloft.net>
>> Date: Fri, 07 Oct 2011 14:57:03 -0400 (EDT)
>>
>>> I'm about to setup my GIT trees on kernel.org, once that is complete
>>> I will be solely updating those trees again.
>>>
>>> I will notify everyone when this is ready to go.
>>>
>>> Just a heads up for everyone...
>> Ok, they are now online, please update your URLs.
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git
>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide.git
>>
> I'm still new to net kernel development, so a little confused about these trees.
> So we should submit patches based on these trees, instead of Linus'?
> About net and net-next, How to decide which one to use?
>
> Thanks!
>
> Yinglin
>
*If* you have change against the network core/drivers then, you would
want to use David Miller's net or net-next trees.
As a general rule:
- net tree is only for fixes/critical fixes
- net-next tree is for everything else
There are always exceptions, but if you stick to the above general rule,
you will be fine.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]
^ permalink raw reply
* Re: moving GIT back to kernel.org...
From: Yinglin Sun @ 2011-10-08 7:09 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: David Miller, netdev, sfr, linux-wireless, netfilter-devel,
sparclinux, linux-ide
In-Reply-To: <4E8FF54F.1080306@gmail.com>
On Sat, Oct 8, 2011 at 12:01 AM, Jeff Kirsher <tarbal@gmail.com> wrote:
> On 10/07/2011 11:55 PM, Yinglin Sun wrote:
>> On Fri, Oct 7, 2011 at 12:59 PM, David Miller <davem@davemloft.net> wrote:
>>> From: David Miller <davem@davemloft.net>
>>> Date: Fri, 07 Oct 2011 14:57:03 -0400 (EDT)
>>>
>>>> I'm about to setup my GIT trees on kernel.org, once that is complete
>>>> I will be solely updating those trees again.
>>>>
>>>> I will notify everyone when this is ready to go.
>>>>
>>>> Just a heads up for everyone...
>>> Ok, they are now online, please update your URLs.
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
>>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git
>>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide.git
>>>
>> I'm still new to net kernel development, so a little confused about these trees.
>> So we should submit patches based on these trees, instead of Linus'?
>> About net and net-next, How to decide which one to use?
>>
>> Thanks!
>>
>> Yinglin
>>
>
> *If* you have change against the network core/drivers then, you would
> want to use David Miller's net or net-next trees.
>
> As a general rule:
> - net tree is only for fixes/critical fixes
> - net-next tree is for everything else
>
> There are always exceptions, but if you stick to the above general rule,
> you will be fine.
>
>
Got it. Thanks Jeff!
Yinglin
^ permalink raw reply
* Re: [PATCH] dev: use name hash for dev_seq_ops.
From: Mihai Maruseac @ 2011-10-08 7:22 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, eric.dumazet, mirq-linux, therbert, jpirko, netdev,
linux-kernel, dbaluta, Mihai Maruseac
In-Reply-To: <20111007092445.4f097ed9@nehalam.linuxnetplumber.net>
On Fri, Oct 7, 2011 at 7:24 PM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> On Fri, 7 Oct 2011 18:20:49 +0300
> Mihai Maruseac <mihai.maruseac@gmail.com> wrote:
>
>> Instead of using the dev->next chain and trying to resync at each call to
>> dev_seq_start, use this hash and store bucket number and bucket offset in
>> seq->private field.
>>
>> Tests revealed the following results for ifconfig > /dev/null
>> * 1000 interfaces:
>> * 0.114s without patch
>> * 0.020s with patch
>> * 3000 interfaces:
>> * 0.489s without patch
>> * 0.048s with patch
>> * 5000 interfaces:
>> * 1.363s without patch
>> * 0.131s with patch
>>
>> As one can notice the improvement is of 1 order of magnitude.
>
> Good idea,
> This will change the ordering of entries in /proc which may upset
> some program, not a critical flaw but worth noting.
>
> Rather than recording the bucket and offset of last entry, another
> alternative would be to just record the ifindex.
>
> Also ifconfig is considered deprecated and replaced by ip commands
> for general use.
>
Thanks,
This is a patch from a series of improvements to both the ifconfig and
ip commands. The ip part will come later, after being properly
implemented and tested.
--
Mihai
^ permalink raw reply
* Re: [RFC] net: remove erroneous sk null assignment in timestamping
From: Richard Cochran @ 2011-10-08 7:57 UTC (permalink / raw)
To: David Miller; +Cc: johannes, netdev
In-Reply-To: <20111007.133356.489094996618032061.davem@davemloft.net>
On Fri, Oct 07, 2011 at 01:33:56PM -0400, David Miller wrote:
> It looks like skb_clone_tx_timestamp() sets clone->sk without any
> proper refcounting, so I bet this NULL'ing it out is working
> around that bug.
I don't remember why I put it that way, but I took a look at the
problem, and I am not sure how to solve it. The other callers of
sock_queue_err_skb all create or clone the error skb immediately
before queueing it:
net/core/skbuff.c: skb_tstamp_tx
net/ipv4/ip_sockglue.c: ip_icmp_error, ip_local_error
net/ipv6/datagram.c: ipv6_icmp_error, ipv6_local_error
So I need to prevent the socket from disappearing between
skb_clone_tx_timestamp and skb_complete_tx_timestamp:
skb_clone_tx_timestamp
clone = skb_clone(skb, GFP_ATOMIC);
sock_hold
skb_complete_tx_timestamp
sock_queue_err_skb(sk, skb);
sock_put
What do you think?
BTW, while looking for a good pattern to follow, I found that the can
driver also sets skb->sk after clone with no special treatment, like
so:
drivers/net/can/dev.c:285
can_put_echo_skb
struct sock *srcsk = skb->sk;
skb = skb_clone(old_skb, GFP_ATOMIC);
skb->sk = srcsk;
> The TX side of this infrastructure seems very poorly tested.
In fact, we do have the phyter driver used in an extensive automated
test farm, but the applications just don't do the kinds of things
suggested to trigger the problem. The normal pattern is, send event
packet, get tx timestamp, and so we haven't seen the bug at all.
Thanks,
Richard
^ permalink raw reply
* Re: [RFC] net: remove erroneous sk null assignment in timestamping
From: Richard Cochran @ 2011-10-08 7:59 UTC (permalink / raw)
To: Johannes Berg; +Cc: David Miller, netdev
In-Reply-To: <1318012932.3974.7.camel@jlt3.sipsolutions.net>
On Fri, Oct 07, 2011 at 08:42:12PM +0200, Johannes Berg wrote:
> Maybe that's how you can trigger it: have one thread turn on and off
> timestamping all the time, and another thread send frames all the time,
> then eventually you'll probably run into the kfree_skb() case there. If
> you ever manage to run into that case, it'll crash either when freeing
> this skb or when freeing the original.
Thats one weird app, but I get the point, and thanks for your
attention to my code.
Richard
^ permalink raw reply
* [net-next v2] cs89x0: Move the driver into the Cirrus dir
From: Jeff Kirsher @ 2011-10-08 8:07 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann, Russell Nelson,
Andrew Morton
The cs89x0 driver was initial placed in the apple/ when it
should have been placed in the cirrus/. This resolves the
issue by moving the dirver and fixing up the respective
Kconfig(s) and Makefile(s).
Thanks to Sascha for reporting the issue.
-v2 Fix a config error that was introduced with v1 by removing
the dependency on MACE for NET_VENDOR_APPLE.
CC: Russell Nelson <nelson@crynwr.com>
CC: Andrew Morton <akpm@linux-foundation.org>
Reported-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/apple/Kconfig | 22 +---------------------
drivers/net/ethernet/apple/Makefile | 1 -
drivers/net/ethernet/cirrus/Kconfig | 22 +++++++++++++++++++++-
drivers/net/ethernet/cirrus/Makefile | 1 +
drivers/net/ethernet/{apple => cirrus}/cs89x0.c | 0
drivers/net/ethernet/{apple => cirrus}/cs89x0.h | 0
6 files changed, 23 insertions(+), 23 deletions(-)
rename drivers/net/ethernet/{apple => cirrus}/cs89x0.c (100%)
rename drivers/net/ethernet/{apple => cirrus}/cs89x0.h (100%)
diff --git a/drivers/net/ethernet/apple/Kconfig b/drivers/net/ethernet/apple/Kconfig
index 59d5c26..a759d54 100644
--- a/drivers/net/ethernet/apple/Kconfig
+++ b/drivers/net/ethernet/apple/Kconfig
@@ -5,8 +5,7 @@
config NET_VENDOR_APPLE
bool "Apple devices"
default y
- depends on (PPC_PMAC && PPC32) || MAC || ISA || EISA || MACH_IXDP2351 \
- || ARCH_IXDP2X01 || MACH_MX31ADS || MACH_QQ2440
+ depends on (PPC_PMAC && PPC32) || MAC
---help---
If you have a network (Ethernet) card belonging to this class, say Y
and read the Ethernet-HOWTO, available from
@@ -75,23 +74,4 @@ config MACMACE
say Y and read the Ethernet-HOWTO, available from
<http://www.tldp.org/docs.html#howto>.
-config CS89x0
- tristate "CS89x0 support"
- depends on (ISA || EISA || MACH_IXDP2351 \
- || ARCH_IXDP2X01 || MACH_MX31ADS || MACH_QQ2440)
- ---help---
- Support for CS89x0 chipset based Ethernet cards. If you have a
- network (Ethernet) card of this type, say Y and read the
- Ethernet-HOWTO, available from
- <http://www.tldp.org/docs.html#howto> as well as
- <file:Documentation/networking/cs89x0.txt>.
-
- To compile this driver as a module, choose M here. The module
- will be called cs89x0.
-
-config CS89x0_NONISA_IRQ
- def_bool y
- depends on CS89x0 != n
- depends on MACH_IXDP2351 || ARCH_IXDP2X01 || MACH_MX31ADS || MACH_QQ2440
-
endif # NET_VENDOR_APPLE
diff --git a/drivers/net/ethernet/apple/Makefile b/drivers/net/ethernet/apple/Makefile
index 9d30086..0d3a591 100644
--- a/drivers/net/ethernet/apple/Makefile
+++ b/drivers/net/ethernet/apple/Makefile
@@ -5,5 +5,4 @@
obj-$(CONFIG_MACE) += mace.o
obj-$(CONFIG_BMAC) += bmac.o
obj-$(CONFIG_MAC89x0) += mac89x0.o
-obj-$(CONFIG_CS89x0) += cs89x0.o
obj-$(CONFIG_MACMACE) += macmace.o
diff --git a/drivers/net/ethernet/cirrus/Kconfig b/drivers/net/ethernet/cirrus/Kconfig
index e9386ef..6cbb81c 100644
--- a/drivers/net/ethernet/cirrus/Kconfig
+++ b/drivers/net/ethernet/cirrus/Kconfig
@@ -5,7 +5,8 @@
config NET_VENDOR_CIRRUS
bool "Cirrus devices"
default y
- depends on ARM && ARCH_EP93XX
+ depends on ISA || EISA || MACH_IXDP2351 || ARCH_IXDP2X01 \
+ || MACH_MX31ADS || MACH_QQ2440 || (ARM && ARCH_EP93XX)
---help---
If you have a network (Ethernet) card belonging to this class, say Y
and read the Ethernet-HOWTO, available from
@@ -18,6 +19,25 @@ config NET_VENDOR_CIRRUS
if NET_VENDOR_CIRRUS
+config CS89x0
+ tristate "CS89x0 support"
+ depends on (ISA || EISA || MACH_IXDP2351 \
+ || ARCH_IXDP2X01 || MACH_MX31ADS || MACH_QQ2440)
+ ---help---
+ Support for CS89x0 chipset based Ethernet cards. If you have a
+ network (Ethernet) card of this type, say Y and read the
+ Ethernet-HOWTO, available from
+ <http://www.tldp.org/docs.html#howto> as well as
+ <file:Documentation/networking/cs89x0.txt>.
+
+ To compile this driver as a module, choose M here. The module
+ will be called cs89x0.
+
+config CS89x0_NONISA_IRQ
+ def_bool y
+ depends on CS89x0 != n
+ depends on MACH_IXDP2351 || ARCH_IXDP2X01 || MACH_MX31ADS || MACH_QQ2440
+
config EP93XX_ETH
tristate "EP93xx Ethernet support"
depends on ARM && ARCH_EP93XX
diff --git a/drivers/net/ethernet/cirrus/Makefile b/drivers/net/ethernet/cirrus/Makefile
index 9905ea2..14bd77e 100644
--- a/drivers/net/ethernet/cirrus/Makefile
+++ b/drivers/net/ethernet/cirrus/Makefile
@@ -2,4 +2,5 @@
# Makefile for the Cirrus network device drivers.
#
+obj-$(CONFIG_CS89x0) += cs89x0.o
obj-$(CONFIG_EP93XX_ETH) += ep93xx_eth.o
diff --git a/drivers/net/ethernet/apple/cs89x0.c b/drivers/net/ethernet/cirrus/cs89x0.c
similarity index 100%
rename from drivers/net/ethernet/apple/cs89x0.c
rename to drivers/net/ethernet/cirrus/cs89x0.c
diff --git a/drivers/net/ethernet/apple/cs89x0.h b/drivers/net/ethernet/cirrus/cs89x0.h
similarity index 100%
rename from drivers/net/ethernet/apple/cs89x0.h
rename to drivers/net/ethernet/cirrus/cs89x0.h
--
1.7.6.4
^ permalink raw reply related
* Re: [RFC] net: remove erroneous sk null assignment in timestamping
From: Johannes Berg @ 2011-10-08 8:16 UTC (permalink / raw)
To: Richard Cochran; +Cc: David Miller, netdev
In-Reply-To: <20111008075719.GA2284@netboy.at.omicron.at>
On Sat, 2011-10-08 at 09:57 +0200, Richard Cochran wrote:
> I don't remember why I put it that way, but I took a look at the
> problem, and I am not sure how to solve it. The other callers of
> sock_queue_err_skb all create or clone the error skb immediately
> before queueing it:
>
> net/core/skbuff.c: skb_tstamp_tx
> net/ipv4/ip_sockglue.c: ip_icmp_error, ip_local_error
> net/ipv6/datagram.c: ipv6_icmp_error, ipv6_local_error
Yeah, I noticed that too. That's also the reason they pass the socket
externally I believe, since it's not a properly refcounted socket (the
reference they use is still from the original skb).
The thing that makes it work is that
a) they don't release the original SKB before sock_queue_err_skb() and
b) skb->sk is NULL for them
Since this is just a single function, they can guarantee that -- in the
case we found here it's scattered across the code and won't always be
guaranteed -- e.g. the kfree_skb() case in the PHY driver potentially
violates b).
> So I need to prevent the socket from disappearing between
> skb_clone_tx_timestamp and skb_complete_tx_timestamp:
>
> skb_clone_tx_timestamp
> clone = skb_clone(skb, GFP_ATOMIC);
> sock_hold
> skb_complete_tx_timestamp
> sock_queue_err_skb(sk, skb);
> sock_put
>
> What do you think?
I'm not terribly familiar with struct sock. Looking at it, I'm a bit
confused by skb_orphan() -- it doesn't put the sock reference. So are
sockets not refcounted for skbs in this way? They seem to use
sock_wfree() which does a bit more than this it seems, and I don't see
it using sk_refcnt anywhere so I'm a bit confused now.
> BTW, while looking for a good pattern to follow, I found that the can
> driver also sets skb->sk after clone with no special treatment, like
> so:
>
> drivers/net/can/dev.c:285
> can_put_echo_skb
> struct sock *srcsk = skb->sk;
> skb = skb_clone(old_skb, GFP_ATOMIC);
> skb->sk = srcsk;
Yeah that looks fishy too. But to me it looks a bit like it should
charge to the socket instead of refcounting it -- though of course
that's not really the correct thing to do from a socket buffer point of
view, but it seems the sk_refcnt and sk_wmem_alloc are two separate
mechanisms of refcounting the socket -- I just haven't figured out yet
how they interact.
> > The TX side of this infrastructure seems very poorly tested.
>
> In fact, we do have the phyter driver used in an extensive automated
> test farm, but the applications just don't do the kinds of things
> suggested to trigger the problem. The normal pattern is, send event
> packet, get tx timestamp, and so we haven't seen the bug at all.
Makes sense, you never wrote an application trying to crash it :-)
> > Maybe that's how you can trigger it: have one thread turn on and off
> > timestamping all the time, and another thread send frames all the time,
> > then eventually you'll probably run into the kfree_skb() case there. If
> > you ever manage to run into that case, it'll crash either when freeing
> > this skb or when freeing the original.
>
> Thats one weird app, but I get the point, and thanks for your
> attention to my code.
Agree, it's obviously a specifically devised app to try to make it
crash. It serves no other practical purpose.
johannes
^ permalink raw reply
* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
From: Sasha Levin @ 2011-10-08 8:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <20111003190516.GD22427@redhat.com>
On Mon, 2011-10-03 at 21:05 +0200, Michael S. Tsirkin wrote:
> I thought about this some more. If length was too large host is
> right now writing into pages that we have freed.
> That is very bad, and I don't know what do do with it,
> really not worth prettifying that IMO, NULL pointer is the least of our
> worries.
>
> But, with mergeable buffers at least, and that is the main mode anyway,
> there is always a single page per buf, right?
> So I think we should change the code,
> and for mergeable buffers we shall only verify that
> length <= PAGE_SIZE.
In receive_mergeable() we already verify both 'page' being non-null and
length:
if (len > PAGE_SIZE)
len = PAGE_SIZE;
>
> IOW copy a bit of code from page_to_skb(vi, page, len)
> to receive_mergeable, maybe add a common function
> if we need to avoid duplication.
--
Sasha.
^ permalink raw reply
* Re: [RFC] net: remove erroneous sk null assignment in timestamping
From: Eric Dumazet @ 2011-10-08 8:57 UTC (permalink / raw)
To: Johannes Berg; +Cc: Richard Cochran, David Miller, netdev
In-Reply-To: <1318061808.3991.12.camel@jlt3.sipsolutions.net>
Le samedi 08 octobre 2011 à 10:16 +0200, Johannes Berg a écrit :
> I'm not terribly familiar with struct sock. Looking at it, I'm a bit
> confused by skb_orphan() -- it doesn't put the sock reference. So are
> sockets not refcounted for skbs in this way? They seem to use
> sock_wfree() which does a bit more than this it seems, and I don't see
> it using sk_refcnt anywhere so I'm a bit confused now.
Check following commit changelog to get some information on this.
commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu Jun 11 02:55:43 2009 -0700
net: No more expensive sock_hold()/sock_put() on each tx
One of the problem with sock memory accounting is it uses
a pair of sock_hold()/sock_put() for each transmitted packet.
This slows down bidirectional flows because the receive path
also needs to take a refcount on socket and might use a different
cpu than transmit path or transmit completion path. So these
two atomic operations also trigger cache line bounces.
We can see this in tx or tx/rx workloads (media gateways for example),
where sock_wfree() can be in top five functions in profiles.
We use this sock_hold()/sock_put() so that sock freeing
is delayed until all tx packets are completed.
As we also update sk_wmem_alloc, we could offset sk_wmem_alloc
by one unit at init time, until sk_free() is called.
Once sk_free() is called, we atomic_dec_and_test(sk_wmem_alloc)
to decrement initial offset and atomicaly check if any packets
are in flight.
skb_set_owner_w() doesnt call sock_hold() anymore
sock_wfree() doesnt call sock_put() anymore, but check if sk_wmem_alloc
reached 0 to perform the final freeing.
Drawback is that a skb->truesize error could lead to unfreeable sockets, or
even worse, prematurely calling __sk_free() on a live socket.
Nice speedups on SMP. tbench for example, going from 2691 MB/s to 2711 MB/s
on my 8 cpu dev machine, even if tbench was not really hitting sk_refcnt
contention point. 5 % speedup on a UDP transmit workload (depends
on number of flows), lowering TX completion cpu usage.
^ permalink raw reply
* Wir bieten Darlehen
From: Franck.DEBOS @ 2011-10-08 9:19 UTC (permalink / raw)
Dies ist zu informieren, dass CAPITAL GLEICHHEIT
bietet derzeit einen Kredit mit 3% Zinsen
Rate.
Wir bieten eine Vielzahl von Krediten an unsere
Kunden. Was auch immer Ihre Bedürfnisse sind Darlehen,
große oder kleine, persönliche oder Hypotheken, sind wir
bereit, mit Ihnen darüber, wie wir uns treffen können sprechen
Ihre Bedürfnisse. Anmeldeformular unter
CAPITAL GLEICHHEIT
71 Queen Victoria Street
London, United Kingdom, EC4V 4DE
Anmeldeformular:
1) Vollständiger Name :.........
2) Land :..............
3) Adresse: ...
4) Sex :..............
5) Familienstand :......................
6) Beruf :..............
7) Telefon :......
8) Monatliches Einkommen :.....................
9) Darlehensbetrag Benötigte :...............
10) Dauer der Ausleihe :..................
11) Zweck des Darlehens :.................
12) Alter: ..............
Rosmarie Neely
Loan Officer
E-mail: cequality24hoursloans@gmail.com
Tell: +447035958575
Fax: +448447742385
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
^ permalink raw reply
* e100 + VLANs?
From: Michael Tokarev @ 2011-10-08 10:08 UTC (permalink / raw)
To: netdev
Yesterday I tried to use 802.1Q VLAN tagging with an (oldish)
e100-driven network card, identified by lspci like this:
00:12.0 Ethernet controller: Intel Corporation 82557/8/9/0/1 Ethernet Pro 100 (rev 02)
Just to discover that it does not quite work: packets of
size 1497+ bytes gets lost.
This appears to be a classical problems in this case -
something forgot to allocate extra 4 bytes for the
packets.
There's at least one bugreport from 2008 (!) about this
very issue: http://bugs.centos.org/view.php?id=2719
which is still open.
The kernel I tried this on was 2.6.32, I checked git log
for drivers/net/e100.c - there was no changes up to
current version which may be related to this issue.
The question: is this a driver problem or hardware? If
it's the driver, can it be fixed? And if it's hardware,
can the driver notify the user somehow - like, by refusing
to enable VLAN (sub)devices maybe?
Yesterday it was actually a bit more complicated for me,
since the card in question was used to connect to our
ISP, and they use fixed MAC address per port, so I had
to find another NIC which is a) able to work with VLAN
tags properly, and b) is able to change its mac address.
Lucky I had a VIA RhineIII which does both :)
Thanks,
/mjt
^ permalink raw reply
* [PATCH net] vlan:make mtu of vlan equal to physical dev
From: Weiping Pan @ 2011-10-08 10:12 UTC (permalink / raw)
To: netdev; +Cc: Weiping Pan
Default mtu of vlan device is the same with mtu of physical device,
for example 1500, but when change physics mtu to 1600,
VLAN device's mtu is still 1500.
Certainly, you can change vlan device's mtu to 1600 manually,
but I think when you change physics device's mtu, VLAN's mtu should be changed
automatically instead of by manually.
Steps to Reproduce:
1.vconfig add eth4 3
2.ifconfig eth4 mtu 1600
3.check mtu on eth4.3
And what's worse is that if you decrease mtu of pyhsical device,
and when you want to increase it, the mtu of vlan device won't be changed.
Steps to Reproduce:
1.vconfig add eth4 3
2.ifconfig eth4 mtu 100
3.ifconfig eth4 mtu 1500
4.the mtu of eth4.3 is still 100
This bug is reported by Liang Zheng(lzheng@redhat.com).
Signed-off-by: Weiping Pan <panweiping3@gmail.com>
---
net/8021q/vlan.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 8970ba1..f6072b4 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -417,9 +417,6 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
if (!vlandev)
continue;
- if (vlandev->mtu <= dev->mtu)
- continue;
-
dev_set_mtu(vlandev, dev->mtu);
}
break;
--
1.7.4.4
^ permalink raw reply related
* Re: [RFC] net: remove erroneous sk null assignment in timestamping
From: Johannes Berg @ 2011-10-08 10:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Richard Cochran, David Miller, netdev
In-Reply-To: <1318064238.5276.2.camel@edumazet-laptop>
On Sat, 2011-10-08 at 10:57 +0200, Eric Dumazet wrote:
> Le samedi 08 octobre 2011 à 10:16 +0200, Johannes Berg a écrit :
>
> > I'm not terribly familiar with struct sock. Looking at it, I'm a bit
> > confused by skb_orphan() -- it doesn't put the sock reference. So are
> > sockets not refcounted for skbs in this way? They seem to use
> > sock_wfree() which does a bit more than this it seems, and I don't see
> > it using sk_refcnt anywhere so I'm a bit confused now.
>
> Check following commit changelog to get some information on this.
>
> commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
> Author: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu Jun 11 02:55:43 2009 -0700
>
> net: No more expensive sock_hold()/sock_put() on each tx
Aha, thanks for the pointer!
> As we also update sk_wmem_alloc, we could offset sk_wmem_alloc
> by one unit at init time, until sk_free() is called.
This is the trick! Neat. I see it now, now sk_free() makes sense to
me :-)
There's one thing I still miss though: It seems to me that if you have a
reference to a socket that has been sk_free()'ed (which is possible
since it might still have sk_wmem_alloc > 0) you can't sock_hold() that
socket. That feels a bit unexpected -- and might happen in the code
Richard just suggested.
Basically, while you can bump a reference you own via sock_hold() by
sk_wmem_alloc, as soon as sk_refcnt reaches 0 it must never go > 0 again
because that will have released the sk_wmem_alloc offset.
That can be fixed, but I'm not exactly sure what would be an efficient
way of doing it. Maybe by adding some flag that says whether or not the
sk_wmem_alloc offset is present, but that flag would have to be atomic
since I guess even this could race.
> Drawback is that a skb->truesize error could lead to unfreeable sockets, or
> even worse, prematurely calling __sk_free() on a live socket.
I was thinking about this yesterday as well. What we could do is wrap
all the skb truesize operations in inlines -- the regular ones like
skb_truesize_set()/add()/sub() would (depending on a debug Kconfig)
check that the skb isn't charged to a socket, and common sequences like
changing truesize and updating the sock could be wrapped into another
set of inlines that do both. Or something like that.
I was actually thinking about this for other reasons but then realised
that with the truesize bug check gone (which was really checking
something else) I didn't really need to worry any more.
johannes
^ permalink raw reply
* Re: [RFC] net: remove erroneous sk null assignment in timestamping
From: Richard Cochran @ 2011-10-08 10:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Johannes Berg, David Miller, netdev
In-Reply-To: <1318064238.5276.2.camel@edumazet-laptop>
On Sat, Oct 08, 2011 at 10:57:18AM +0200, Eric Dumazet wrote:
> Le samedi 08 octobre 2011 à 10:16 +0200, Johannes Berg a écrit :
>
> > I'm not terribly familiar with struct sock. Looking at it, I'm a bit
> > confused by skb_orphan() -- it doesn't put the sock reference. So are
> > sockets not refcounted for skbs in this way? They seem to use
> > sock_wfree() which does a bit more than this it seems, and I don't see
> > it using sk_refcnt anywhere so I'm a bit confused now.
Me, too.
> Check following commit changelog to get some information on this.
Thanks, Eric, that does help explain.
> We use this sock_hold()/sock_put() so that sock freeing
> is delayed until all tx packets are completed.
>
> As we also update sk_wmem_alloc, we could offset sk_wmem_alloc
> by one unit at init time, until sk_free() is called.
> Once sk_free() is called, we atomic_dec_and_test(sk_wmem_alloc)
> to decrement initial offset and atomicaly check if any packets
> are in flight.
>
> skb_set_owner_w() doesnt call sock_hold() anymore
So, if I understand, then I can solve my particular problem by doing:
* skb_clone_tx_timestamp
clone = skb_clone(skb, GFP_ATOMIC);
skb_set_owner_w(clone, sk)
// instead of
// clone->sk = sk;
phydev->drv->txtstamp(phydev, clone, type);
* skb_complete_tx_timestamp
serr->ee.ee_errno = ENOMSG;
serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
// just remove this:
// skb->sk = NULL;
err = sock_queue_err_skb(sk, skb);
The only problem(?) I see is that it violates the rules from sock.h,
quoted below. The cloned tx skb destined for the error queue would be
budgeted to sk_wmem_alloc while wait for the time stamp. But maybe we
can allow this?
from sock.h:
/*
* Socket reference counting postulates.
*
* * Each user of socket SHOULD hold a reference count.
* * Each access point to socket (an hash table bucket, reference from a list,
* running timer, skb in flight MUST hold a reference count.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/
BTW, no longer true.
* * Packets, delivered from outside (from network or from another process)
* and enqueued on receive/error queues SHOULD NOT grab reference count,
* when they sit in queue. ~~~~~~~~~~~~~~~~~~~~~~~
Want to break/bend this rule.
Thanks,
Richard
^ permalink raw reply
* SOMAXCONN = 128, but max defaults to 2048
From: Olaf van der Spek @ 2011-10-08 11:51 UTC (permalink / raw)
To: netdev
Hi,
SOMAXCONN is defined as 128, but the max
("/proc/sys/net/ipv4/tcp_max_syn_backlog") appears to be 2048 by
default.
Is the name of SOMAXCONN wrong and is it actually defining the default?
Would there be a disadvantage to defining SOMAXCONN as INT_MAX and
letting the kernel control the actual max?
Greetings,
Olaf
^ permalink raw reply
* Re: SOMAXCONN = 128, but max defaults to 2048
From: Eric Dumazet @ 2011-10-08 12:35 UTC (permalink / raw)
To: Olaf van der Spek; +Cc: netdev
In-Reply-To: <CAGVGHmt1OVOj0NSEo3qHCh1CR10eZTNjxuUh_mB6sXxcx5gKmA@mail.gmail.com>
Le samedi 08 octobre 2011 à 13:51 +0200, Olaf van der Spek a écrit :
> Hi,
>
> SOMAXCONN is defined as 128, but the max
> ("/proc/sys/net/ipv4/tcp_max_syn_backlog") appears to be 2048 by
> default.
It depends on memory size and TCP hash table size :
dmesg | grep "TCP established hash table entries"
-> TCP established hash table entries: 524288 (order: 11, 8388608 bytes)
sysctl_max_syn_backlog = max(128, cnt / 256);
cnt = 524288
cnt/256 -> 2048
> Is the name of SOMAXCONN wrong and is it actually defining the default?
> Would there be a disadvantage to defining SOMAXCONN as INT_MAX and
> letting the kernel control the actual max?
>
You mean : remove somaxconn tunable ?
accept()/listen() is not bound to TCP only.
But yes, 128 default is a bit old today, given that Apache uses a
listen(fd, 511) default value itself...
I routinely set net.core.somaxconn to 1024 on my servers.
We had a recent discussion on the matter lately, but Hagen Paul Pfeifer
did not polish his patches enough :
http://lists.openwall.net/netdev/2011/03/20/3
FreeBSD has a separate mechanism for not yet established sockets, called
syncache, that allows a low somaxconn per listener.
The socket queue holds only fully established (but not yet accept()ed)
sockets, while syncache holds all the SYN_RECV ones.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox