[PATCH net 0/7] qlge: bug fix

[PATCH net 0/7] qlge: bug fix

[Jitendra Kalsaria]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

Please apply it to net.

Thanks,
Jitendra

[PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[Jitendra Kalsaria]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

qlge driver was acting wrongly when considering TX ring full
as a TX error. TX ring full is expected behavior when NIC is
overwhelmed and is expected to happen, as far as packets are
not lost.

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge_main.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 09d8d33..cdbc860 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -2562,7 +2562,6 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 			   __func__, tx_ring_idx);
 		netif_stop_subqueue(ndev, tx_ring->wq_id);
 		atomic_inc(&tx_ring->queue_stopped);
-		tx_ring->tx_errors++;
 		return NETDEV_TX_BUSY;
 	}
 	tx_ring_desc = &tx_ring->q[tx_ring->prod_idx];
-- 
1.7.1

[PATCH net 2/7] qlge: Stand-up card should not report supporting wol.

[Jitendra Kalsaria]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c |   46 ++++++++++++++---------
 1 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
index 8e2c2a7..98f04d7 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
@@ -388,30 +388,40 @@ static void ql_get_drvinfo(struct net_device *ndev,
 static void ql_get_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
-	/* What we support. */
-	wol->supported = WAKE_MAGIC;
-	/* What we've currently got set. */
-	wol->wolopts = qdev->wol;
+	unsigned short ssys_dev = qdev->pdev->subsystem_device;
+
+	if (ssys_dev == 0x0068 || ssys_dev == 0x0180) {
+		wol->supported = WAKE_MAGIC;
+		wol->wolopts = qdev->wol;
+	}
 }
 
 static int ql_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
-	int status;
+	unsigned short ssys_dev = qdev->pdev->subsystem_device;
 
-	if (wol->wolopts & ~WAKE_MAGIC)
-		return -EINVAL;
-	qdev->wol = wol->wolopts;
-
-	netif_info(qdev, drv, qdev->ndev, "Set wol option 0x%x\n", qdev->wol);
-	if (!qdev->wol) {
-		u32 wol = 0;
-		status = ql_mb_wol_mode(qdev, wol);
-		netif_err(qdev, drv, qdev->ndev, "WOL %s (wol code 0x%x)\n",
-			  status == 0 ? "cleared successfully" : "clear failed",
-			  wol);
-	}
+	if (ssys_dev == 0x0068 || ssys_dev == 0x0180) {
+		if (wol->wolopts & ~WAKE_MAGIC)
+			return -EINVAL;
+		qdev->wol = wol->wolopts;
+
+		netif_info(qdev, drv, qdev->ndev,
+				"Set wol option 0x%x\n", qdev->wol);
+		if (!qdev->wol) {
+			u32 wol = 0;
+			int status = 0;
 
+			status = ql_mb_wol_mode(qdev, wol);
+			netif_err(qdev, drv, qdev->ndev,
+			"WOL %s (wol code 0x%x)\n",
+			status == 0 ? "cleared successfully" : "clear failed",
+			wol);
+		}
+	} else {
+		netif_info(qdev, drv, qdev->ndev,
+				"WOL is not supported on stand-up card\n");
+	}
 	return 0;
 }
 
-- 
1.7.1

[PATCH net 3/7] qlge: Garbage values shown in extra info during selftest.

[Jitendra Kalsaria]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

while running selftest 'ethtool -t' multiple times will get
different values in the 'extra info' section, which was garbage.

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
index 98f04d7..31ee6dc 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
@@ -248,6 +248,9 @@ static char ql_stats_str_arr[][ETH_GSTRING_LEN] = {
 static void ql_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 {
 	switch (stringset) {
+	case ETH_SS_TEST:
+		memcpy(buf, *ql_gstrings_test, QLGE_TEST_LEN * ETH_GSTRING_LEN);
+		break;
 	case ETH_SS_STATS:
 		memcpy(buf, ql_stats_str_arr, sizeof(ql_stats_str_arr));
 		break;
@@ -538,6 +541,8 @@ static void ql_self_test(struct net_device *ndev,
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 
+	memset(data, 0, sizeof(u64) * QLGE_TEST_LEN);
+
 	if (netif_running(ndev)) {
 		set_bit(QL_SELFTEST, &qdev->flags);
 		if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
-- 
1.7.1

[PATCH net 4/7] qlge: Fixed double pci free upon tx_ring->q allocation failure.

[Jitendra Kalsaria]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge_main.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index cdbc860..9ecd15f 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -2701,20 +2701,20 @@ static int ql_alloc_tx_resources(struct ql_adapter *qdev,
 	    pci_alloc_consistent(qdev->pdev, tx_ring->wq_size,
 				 &tx_ring->wq_base_dma);
 
-	if ((tx_ring->wq_base == NULL) ||
-	    tx_ring->wq_base_dma & WQ_ADDR_ALIGN) {
-		netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
-		return -ENOMEM;
-	}
+	if (!tx_ring->wq_base || tx_ring->wq_base_dma & WQ_ADDR_ALIGN)
+		goto err;
+
 	tx_ring->q =
 	    kmalloc(tx_ring->wq_len * sizeof(struct tx_ring_desc), GFP_KERNEL);
-	if (tx_ring->q == NULL)
+	if (!tx_ring->q)
 		goto err;
 
 	return 0;
 err:
 	pci_free_consistent(qdev->pdev, tx_ring->wq_size,
-			    tx_ring->wq_base, tx_ring->wq_base_dma);
+			tx_ring->wq_base, tx_ring->wq_base_dma);
+	tx_ring->wq_base = NULL;
+	netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
 	return -ENOMEM;
 }
 
-- 
1.7.1

[PATCH net 5/7] qlge: Categorize receive frame errors from firmware.

[Jitendra Kalsaria]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge.h         |    8 +++
 drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c |   14 +++++
 drivers/net/ethernet/qlogic/qlge/qlge_main.c    |   63 +++++++++++++----------
 3 files changed, 58 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h b/drivers/net/ethernet/qlogic/qlge/qlge.h
index 5a639df..e81bbb7 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -1535,6 +1535,14 @@ struct nic_stats {
 	u64 rx_1024_to_1518_pkts;
 	u64 rx_1519_to_max_pkts;
 	u64 rx_len_err_pkts;
+	/* Receive Mac Err stats */
+	u64 rx_code_err;
+	u64 rx_oversize_err;
+	u64 rx_undersize_err;
+	u64 rx_preamble_err;
+	u64 rx_frame_len_err;
+	u64 rx_crc_err;
+	u64 rx_err_count;
 	/*
 	 * These stats come from offset 500h to 5C8h
 	 * in the XGMAC register.
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
index 31ee6dc..7163f5d 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
@@ -226,6 +226,13 @@ static char ql_stats_str_arr[][ETH_GSTRING_LEN] = {
 	{"rx_1024_to_1518_pkts"},
 	{"rx_1519_to_max_pkts"},
 	{"rx_len_err_pkts"},
+	{"rx_code_err"},
+	{"rx_oversize_err"},
+	{"rx_undersize_err"},
+	{"rx_preamble_err"},
+	{"rx_frame_len_err"},
+	{"rx_crc_err"},
+	{"rx_err_count"},
 	{"tx_cbfc_pause_frames0"},
 	{"tx_cbfc_pause_frames1"},
 	{"tx_cbfc_pause_frames2"},
@@ -320,6 +327,13 @@ ql_get_ethtool_stats(struct net_device *ndev,
 	*data++ = s->rx_1024_to_1518_pkts;
 	*data++ = s->rx_1519_to_max_pkts;
 	*data++ = s->rx_len_err_pkts;
+	*data++ = s->rx_code_err;
+	*data++ = s->rx_oversize_err;
+	*data++ = s->rx_undersize_err;
+	*data++ = s->rx_preamble_err;
+	*data++ = s->rx_frame_len_err;
+	*data++ = s->rx_crc_err;
+	*data++ = s->rx_err_count;
 	*data++ = s->tx_cbfc_pause_frames0;
 	*data++ = s->tx_cbfc_pause_frames1;
 	*data++ = s->tx_cbfc_pause_frames2;
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 9ecd15f..06dfafe 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -1433,6 +1433,36 @@ map_error:
 	return NETDEV_TX_BUSY;
 }
 
+/* Categorizing receive firmware frame errors */
+static void ql_categorize_rx_err(struct ql_adapter *qdev, u8 rx_err)
+{
+	struct nic_stats *stats = &qdev->nic_stats;
+
+	stats->rx_err_count++;
+
+	switch (rx_err & IB_MAC_IOCB_RSP_ERR_MASK) {
+	case IB_MAC_IOCB_RSP_ERR_CODE_ERR:
+		stats->rx_code_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_OVERSIZE:
+		stats->rx_oversize_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_UNDERSIZE:
+		stats->rx_undersize_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_PREAMBLE:
+		stats->rx_preamble_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_FRAME_LEN:
+		stats->rx_frame_len_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_CRC:
+		stats->rx_crc_err++;
+	default:
+		break;
+	}
+}
+
 /* Process an inbound completion from an rx ring. */
 static void ql_process_mac_rx_gro_page(struct ql_adapter *qdev,
 					struct rx_ring *rx_ring,
@@ -1499,15 +1529,6 @@ static void ql_process_mac_rx_page(struct ql_adapter *qdev,
 	addr = lbq_desc->p.pg_chunk.va;
 	prefetch(addr);
 
-
-	/* Frame error, so drop the packet. */
-	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_info(qdev, drv, qdev->ndev,
-			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
-		rx_ring->rx_errors++;
-		goto err_out;
-	}
-
 	/* The max framesize filter on this chip is set higher than
 	 * MTU since FCoE uses 2k frames.
 	 */
@@ -1593,15 +1614,6 @@ static void ql_process_mac_rx_skb(struct ql_adapter *qdev,
 	memcpy(skb_put(new_skb, length), skb->data, length);
 	skb = new_skb;
 
-	/* Frame error, so drop the packet. */
-	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_info(qdev, drv, qdev->ndev,
-			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
-		dev_kfree_skb_any(skb);
-		rx_ring->rx_errors++;
-		return;
-	}
-
 	/* loopback self test for ethtool */
 	if (test_bit(QL_SELFTEST, &qdev->flags)) {
 		ql_check_lb_frame(qdev, skb);
@@ -1908,15 +1920,6 @@ static void ql_process_mac_split_rx_intr(struct ql_adapter *qdev,
 		return;
 	}
 
-	/* Frame error, so drop the packet. */
-	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_info(qdev, drv, qdev->ndev,
-			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
-		dev_kfree_skb_any(skb);
-		rx_ring->rx_errors++;
-		return;
-	}
-
 	/* The max framesize filter on this chip is set higher than
 	 * MTU since FCoE uses 2k frames.
 	 */
@@ -1999,6 +2002,12 @@ static unsigned long ql_process_mac_rx_intr(struct ql_adapter *qdev,
 
 	QL_DUMP_IB_MAC_RSP(ib_mac_rsp);
 
+	/* Frame error, so drop the packet. */
+	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
+		ql_categorize_rx_err(qdev, ib_mac_rsp->flags2);
+		return (unsigned long)length;
+	}
+
 	if (ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HV) {
 		/* The data and headers are split into
 		 * separate buffers.
-- 
1.7.1

[PATCH net 6/7] qlge: refactoring of ethtool stats.

[Jitendra Kalsaria]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c |  295 ++++++++++++-----------
 1 files changed, 157 insertions(+), 138 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
index 7163f5d..3dcf5c8 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
@@ -35,10 +35,152 @@
 
 #include "qlge.h"
 
+struct ql_stats {
+	char stat_string[ETH_GSTRING_LEN];
+	int sizeof_stat;
+	int stat_offset;
+};
+
+#define QL_SIZEOF(m) FIELD_SIZEOF(struct ql_adapter, m)
+#define QL_OFF(m) offsetof(struct ql_adapter, m)
+
+static const struct ql_stats ql_gstrings_stats[] = {
+	{"tx_pkts", QL_SIZEOF(nic_stats.tx_pkts), QL_OFF(nic_stats.tx_pkts)},
+	{"tx_bytes", QL_SIZEOF(nic_stats.tx_bytes), QL_OFF(nic_stats.tx_bytes)},
+	{"tx_mcast_pkts", QL_SIZEOF(nic_stats.tx_mcast_pkts),
+					QL_OFF(nic_stats.tx_mcast_pkts)},
+	{"tx_bcast_pkts", QL_SIZEOF(nic_stats.tx_bcast_pkts),
+					QL_OFF(nic_stats.tx_bcast_pkts)},
+	{"tx_ucast_pkts", QL_SIZEOF(nic_stats.tx_ucast_pkts),
+					QL_OFF(nic_stats.tx_ucast_pkts)},
+	{"tx_ctl_pkts", QL_SIZEOF(nic_stats.tx_ctl_pkts),
+					QL_OFF(nic_stats.tx_ctl_pkts)},
+	{"tx_pause_pkts", QL_SIZEOF(nic_stats.tx_pause_pkts),
+					QL_OFF(nic_stats.tx_pause_pkts)},
+	{"tx_64_pkts", QL_SIZEOF(nic_stats.tx_64_pkt),
+					QL_OFF(nic_stats.tx_64_pkt)},
+	{"tx_65_to_127_pkts", QL_SIZEOF(nic_stats.tx_65_to_127_pkt),
+					QL_OFF(nic_stats.tx_65_to_127_pkt)},
+	{"tx_128_to_255_pkts", QL_SIZEOF(nic_stats.tx_128_to_255_pkt),
+					QL_OFF(nic_stats.tx_128_to_255_pkt)},
+	{"tx_256_511_pkts", QL_SIZEOF(nic_stats.tx_256_511_pkt),
+					QL_OFF(nic_stats.tx_256_511_pkt)},
+	{"tx_512_to_1023_pkts",	QL_SIZEOF(nic_stats.tx_512_to_1023_pkt),
+					QL_OFF(nic_stats.tx_512_to_1023_pkt)},
+	{"tx_1024_to_1518_pkts", QL_SIZEOF(nic_stats.tx_1024_to_1518_pkt),
+					QL_OFF(nic_stats.tx_1024_to_1518_pkt)},
+	{"tx_1519_to_max_pkts",	QL_SIZEOF(nic_stats.tx_1519_to_max_pkt),
+					QL_OFF(nic_stats.tx_1519_to_max_pkt)},
+	{"tx_undersize_pkts", QL_SIZEOF(nic_stats.tx_undersize_pkt),
+					QL_OFF(nic_stats.tx_undersize_pkt)},
+	{"tx_oversize_pkts", QL_SIZEOF(nic_stats.tx_oversize_pkt),
+					QL_OFF(nic_stats.tx_oversize_pkt)},
+	{"rx_bytes", QL_SIZEOF(nic_stats.rx_bytes), QL_OFF(nic_stats.rx_bytes)},
+	{"rx_bytes_ok",	QL_SIZEOF(nic_stats.rx_bytes_ok),
+					QL_OFF(nic_stats.rx_bytes_ok)},
+	{"rx_pkts", QL_SIZEOF(nic_stats.rx_pkts), QL_OFF(nic_stats.rx_pkts)},
+	{"rx_pkts_ok", QL_SIZEOF(nic_stats.rx_pkts_ok),
+					QL_OFF(nic_stats.rx_pkts_ok)},
+	{"rx_bcast_pkts", QL_SIZEOF(nic_stats.rx_bcast_pkts),
+					QL_OFF(nic_stats.rx_bcast_pkts)},
+	{"rx_mcast_pkts", QL_SIZEOF(nic_stats.rx_mcast_pkts),
+					QL_OFF(nic_stats.rx_mcast_pkts)},
+	{"rx_ucast_pkts", QL_SIZEOF(nic_stats.rx_ucast_pkts),
+					QL_OFF(nic_stats.rx_ucast_pkts)},
+	{"rx_undersize_pkts", QL_SIZEOF(nic_stats.rx_undersize_pkts),
+					QL_OFF(nic_stats.rx_undersize_pkts)},
+	{"rx_oversize_pkts", QL_SIZEOF(nic_stats.rx_oversize_pkts),
+					QL_OFF(nic_stats.rx_oversize_pkts)},
+	{"rx_jabber_pkts", QL_SIZEOF(nic_stats.rx_jabber_pkts),
+					QL_OFF(nic_stats.rx_jabber_pkts)},
+	{"rx_undersize_fcerr_pkts",
+		QL_SIZEOF(nic_stats.rx_undersize_fcerr_pkts),
+				QL_OFF(nic_stats.rx_undersize_fcerr_pkts)},
+	{"rx_drop_events", QL_SIZEOF(nic_stats.rx_drop_events),
+					QL_OFF(nic_stats.rx_drop_events)},
+	{"rx_fcerr_pkts", QL_SIZEOF(nic_stats.rx_fcerr_pkts),
+					QL_OFF(nic_stats.rx_fcerr_pkts)},
+	{"rx_align_err", QL_SIZEOF(nic_stats.rx_align_err),
+					QL_OFF(nic_stats.rx_align_err)},
+	{"rx_symbol_err", QL_SIZEOF(nic_stats.rx_symbol_err),
+					QL_OFF(nic_stats.rx_symbol_err)},
+	{"rx_mac_err", QL_SIZEOF(nic_stats.rx_mac_err),
+					QL_OFF(nic_stats.rx_mac_err)},
+	{"rx_ctl_pkts",	QL_SIZEOF(nic_stats.rx_ctl_pkts),
+					QL_OFF(nic_stats.rx_ctl_pkts)},
+	{"rx_pause_pkts", QL_SIZEOF(nic_stats.rx_pause_pkts),
+					QL_OFF(nic_stats.rx_pause_pkts)},
+	{"rx_64_pkts", QL_SIZEOF(nic_stats.rx_64_pkts),
+					QL_OFF(nic_stats.rx_64_pkts)},
+	{"rx_65_to_127_pkts", QL_SIZEOF(nic_stats.rx_65_to_127_pkts),
+					QL_OFF(nic_stats.rx_65_to_127_pkts)},
+	{"rx_128_255_pkts", QL_SIZEOF(nic_stats.rx_128_255_pkts),
+					QL_OFF(nic_stats.rx_128_255_pkts)},
+	{"rx_256_511_pkts", QL_SIZEOF(nic_stats.rx_256_511_pkts),
+					QL_OFF(nic_stats.rx_256_511_pkts)},
+	{"rx_512_to_1023_pkts",	QL_SIZEOF(nic_stats.rx_512_to_1023_pkts),
+					QL_OFF(nic_stats.rx_512_to_1023_pkts)},
+	{"rx_1024_to_1518_pkts", QL_SIZEOF(nic_stats.rx_1024_to_1518_pkts),
+					QL_OFF(nic_stats.rx_1024_to_1518_pkts)},
+	{"rx_1519_to_max_pkts",	QL_SIZEOF(nic_stats.rx_1519_to_max_pkts),
+					QL_OFF(nic_stats.rx_1519_to_max_pkts)},
+	{"rx_len_err_pkts", QL_SIZEOF(nic_stats.rx_len_err_pkts),
+					QL_OFF(nic_stats.rx_len_err_pkts)},
+	{"rx_code_err",	QL_SIZEOF(nic_stats.rx_code_err),
+					QL_OFF(nic_stats.rx_code_err)},
+	{"rx_oversize_err", QL_SIZEOF(nic_stats.rx_oversize_err),
+					QL_OFF(nic_stats.rx_oversize_err)},
+	{"rx_undersize_err", QL_SIZEOF(nic_stats.rx_undersize_err),
+					QL_OFF(nic_stats.rx_undersize_err)},
+	{"rx_preamble_err", QL_SIZEOF(nic_stats.rx_preamble_err),
+					QL_OFF(nic_stats.rx_preamble_err)},
+	{"rx_frame_len_err", QL_SIZEOF(nic_stats.rx_frame_len_err),
+					QL_OFF(nic_stats.rx_frame_len_err)},
+	{"rx_crc_err", QL_SIZEOF(nic_stats.rx_crc_err),
+					QL_OFF(nic_stats.rx_crc_err)},
+	{"rx_err_count", QL_SIZEOF(nic_stats.rx_err_count),
+					QL_OFF(nic_stats.rx_err_count)},
+	{"tx_cbfc_pause_frames0", QL_SIZEOF(nic_stats.tx_cbfc_pause_frames0),
+				QL_OFF(nic_stats.tx_cbfc_pause_frames0)},
+	{"tx_cbfc_pause_frames1", QL_SIZEOF(nic_stats.tx_cbfc_pause_frames1),
+				QL_OFF(nic_stats.tx_cbfc_pause_frames1)},
+	{"tx_cbfc_pause_frames2", QL_SIZEOF(nic_stats.tx_cbfc_pause_frames2),
+				QL_OFF(nic_stats.tx_cbfc_pause_frames2)},
+	{"tx_cbfc_pause_frames3", QL_SIZEOF(nic_stats.tx_cbfc_pause_frames3),
+				QL_OFF(nic_stats.tx_cbfc_pause_frames3)},
+	{"tx_cbfc_pause_frames4", QL_SIZEOF(nic_stats.tx_cbfc_pause_frames4),
+				QL_OFF(nic_stats.tx_cbfc_pause_frames4)},
+	{"tx_cbfc_pause_frames5", QL_SIZEOF(nic_stats.tx_cbfc_pause_frames5),
+				QL_OFF(nic_stats.tx_cbfc_pause_frames5)},
+	{"tx_cbfc_pause_frames6", QL_SIZEOF(nic_stats.tx_cbfc_pause_frames6),
+				QL_OFF(nic_stats.tx_cbfc_pause_frames6)},
+	{"tx_cbfc_pause_frames7", QL_SIZEOF(nic_stats.tx_cbfc_pause_frames7),
+				QL_OFF(nic_stats.tx_cbfc_pause_frames7)},
+	{"rx_cbfc_pause_frames0", QL_SIZEOF(nic_stats.rx_cbfc_pause_frames0),
+				QL_OFF(nic_stats.rx_cbfc_pause_frames0)},
+	{"rx_cbfc_pause_frames1", QL_SIZEOF(nic_stats.rx_cbfc_pause_frames1),
+				QL_OFF(nic_stats.rx_cbfc_pause_frames1)},
+	{"rx_cbfc_pause_frames2", QL_SIZEOF(nic_stats.rx_cbfc_pause_frames2),
+				QL_OFF(nic_stats.rx_cbfc_pause_frames2)},
+	{"rx_cbfc_pause_frames3", QL_SIZEOF(nic_stats.rx_cbfc_pause_frames3),
+				QL_OFF(nic_stats.rx_cbfc_pause_frames3)},
+	{"rx_cbfc_pause_frames4", QL_SIZEOF(nic_stats.rx_cbfc_pause_frames4),
+				QL_OFF(nic_stats.rx_cbfc_pause_frames4)},
+	{"rx_cbfc_pause_frames5", QL_SIZEOF(nic_stats.rx_cbfc_pause_frames5),
+				QL_OFF(nic_stats.rx_cbfc_pause_frames5)},
+	{"rx_cbfc_pause_frames6", QL_SIZEOF(nic_stats.rx_cbfc_pause_frames6),
+				QL_OFF(nic_stats.rx_cbfc_pause_frames6)},
+	{"rx_cbfc_pause_frames7", QL_SIZEOF(nic_stats.rx_cbfc_pause_frames7),
+				QL_OFF(nic_stats.rx_cbfc_pause_frames7)},
+	{"rx_nic_fifo_drop", QL_SIZEOF(nic_stats.rx_nic_fifo_drop),
+					QL_OFF(nic_stats.rx_nic_fifo_drop)},
+};
+
 static const char ql_gstrings_test[][ETH_GSTRING_LEN] = {
 	"Loopback test  (offline)"
 };
 #define QLGE_TEST_LEN (sizeof(ql_gstrings_test) / ETH_GSTRING_LEN)
+#define QLGE_STATS_LEN ARRAY_SIZE(ql_gstrings_stats)
 
 static int ql_update_ring_coalescing(struct ql_adapter *qdev)
 {
@@ -183,83 +325,19 @@ quit:
 	QL_DUMP_STAT(qdev);
 }
 
-static char ql_stats_str_arr[][ETH_GSTRING_LEN] = {
-	{"tx_pkts"},
-	{"tx_bytes"},
-	{"tx_mcast_pkts"},
-	{"tx_bcast_pkts"},
-	{"tx_ucast_pkts"},
-	{"tx_ctl_pkts"},
-	{"tx_pause_pkts"},
-	{"tx_64_pkts"},
-	{"tx_65_to_127_pkts"},
-	{"tx_128_to_255_pkts"},
-	{"tx_256_511_pkts"},
-	{"tx_512_to_1023_pkts"},
-	{"tx_1024_to_1518_pkts"},
-	{"tx_1519_to_max_pkts"},
-	{"tx_undersize_pkts"},
-	{"tx_oversize_pkts"},
-	{"rx_bytes"},
-	{"rx_bytes_ok"},
-	{"rx_pkts"},
-	{"rx_pkts_ok"},
-	{"rx_bcast_pkts"},
-	{"rx_mcast_pkts"},
-	{"rx_ucast_pkts"},
-	{"rx_undersize_pkts"},
-	{"rx_oversize_pkts"},
-	{"rx_jabber_pkts"},
-	{"rx_undersize_fcerr_pkts"},
-	{"rx_drop_events"},
-	{"rx_fcerr_pkts"},
-	{"rx_align_err"},
-	{"rx_symbol_err"},
-	{"rx_mac_err"},
-	{"rx_ctl_pkts"},
-	{"rx_pause_pkts"},
-	{"rx_64_pkts"},
-	{"rx_65_to_127_pkts"},
-	{"rx_128_255_pkts"},
-	{"rx_256_511_pkts"},
-	{"rx_512_to_1023_pkts"},
-	{"rx_1024_to_1518_pkts"},
-	{"rx_1519_to_max_pkts"},
-	{"rx_len_err_pkts"},
-	{"rx_code_err"},
-	{"rx_oversize_err"},
-	{"rx_undersize_err"},
-	{"rx_preamble_err"},
-	{"rx_frame_len_err"},
-	{"rx_crc_err"},
-	{"rx_err_count"},
-	{"tx_cbfc_pause_frames0"},
-	{"tx_cbfc_pause_frames1"},
-	{"tx_cbfc_pause_frames2"},
-	{"tx_cbfc_pause_frames3"},
-	{"tx_cbfc_pause_frames4"},
-	{"tx_cbfc_pause_frames5"},
-	{"tx_cbfc_pause_frames6"},
-	{"tx_cbfc_pause_frames7"},
-	{"rx_cbfc_pause_frames0"},
-	{"rx_cbfc_pause_frames1"},
-	{"rx_cbfc_pause_frames2"},
-	{"rx_cbfc_pause_frames3"},
-	{"rx_cbfc_pause_frames4"},
-	{"rx_cbfc_pause_frames5"},
-	{"rx_cbfc_pause_frames6"},
-	{"rx_cbfc_pause_frames7"},
-	{"rx_nic_fifo_drop"},
-};
-
 static void ql_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 {
+	int index;
 	switch (stringset) {
 	case ETH_SS_TEST:
 		memcpy(buf, *ql_gstrings_test, QLGE_TEST_LEN * ETH_GSTRING_LEN);
 		break;
 	case ETH_SS_STATS:
-		memcpy(buf, ql_stats_str_arr, sizeof(ql_stats_str_arr));
+		for (index = 0; index < QLGE_STATS_LEN; index++) {
+			memcpy(buf + index * ETH_GSTRING_LEN,
+				ql_gstrings_stats[index].stat_string,
+				ETH_GSTRING_LEN);
+		}
 		break;
 	}
 }
@@ -270,7 +348,7 @@ static int ql_get_sset_count(struct net_device *dev, int sset)
 	case ETH_SS_TEST:
 		return QLGE_TEST_LEN;
 	case ETH_SS_STATS:
-		return ARRAY_SIZE(ql_stats_str_arr);
+		return QLGE_STATS_LEN;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -281,76 +359,17 @@ ql_get_ethtool_stats(struct net_device *ndev,
 		     struct ethtool_stats *stats, u64 *data)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
-	struct nic_stats *s = &qdev->nic_stats;
+	int index, length;
 
+	length = QLGE_STATS_LEN;
 	ql_update_stats(qdev);
 
-	*data++ = s->tx_pkts;
-	*data++ = s->tx_bytes;
-	*data++ = s->tx_mcast_pkts;
-	*data++ = s->tx_bcast_pkts;
-	*data++ = s->tx_ucast_pkts;
-	*data++ = s->tx_ctl_pkts;
-	*data++ = s->tx_pause_pkts;
-	*data++ = s->tx_64_pkt;
-	*data++ = s->tx_65_to_127_pkt;
-	*data++ = s->tx_128_to_255_pkt;
-	*data++ = s->tx_256_511_pkt;
-	*data++ = s->tx_512_to_1023_pkt;
-	*data++ = s->tx_1024_to_1518_pkt;
-	*data++ = s->tx_1519_to_max_pkt;
-	*data++ = s->tx_undersize_pkt;
-	*data++ = s->tx_oversize_pkt;
-	*data++ = s->rx_bytes;
-	*data++ = s->rx_bytes_ok;
-	*data++ = s->rx_pkts;
-	*data++ = s->rx_pkts_ok;
-	*data++ = s->rx_bcast_pkts;
-	*data++ = s->rx_mcast_pkts;
-	*data++ = s->rx_ucast_pkts;
-	*data++ = s->rx_undersize_pkts;
-	*data++ = s->rx_oversize_pkts;
-	*data++ = s->rx_jabber_pkts;
-	*data++ = s->rx_undersize_fcerr_pkts;
-	*data++ = s->rx_drop_events;
-	*data++ = s->rx_fcerr_pkts;
-	*data++ = s->rx_align_err;
-	*data++ = s->rx_symbol_err;
-	*data++ = s->rx_mac_err;
-	*data++ = s->rx_ctl_pkts;
-	*data++ = s->rx_pause_pkts;
-	*data++ = s->rx_64_pkts;
-	*data++ = s->rx_65_to_127_pkts;
-	*data++ = s->rx_128_255_pkts;
-	*data++ = s->rx_256_511_pkts;
-	*data++ = s->rx_512_to_1023_pkts;
-	*data++ = s->rx_1024_to_1518_pkts;
-	*data++ = s->rx_1519_to_max_pkts;
-	*data++ = s->rx_len_err_pkts;
-	*data++ = s->rx_code_err;
-	*data++ = s->rx_oversize_err;
-	*data++ = s->rx_undersize_err;
-	*data++ = s->rx_preamble_err;
-	*data++ = s->rx_frame_len_err;
-	*data++ = s->rx_crc_err;
-	*data++ = s->rx_err_count;
-	*data++ = s->tx_cbfc_pause_frames0;
-	*data++ = s->tx_cbfc_pause_frames1;
-	*data++ = s->tx_cbfc_pause_frames2;
-	*data++ = s->tx_cbfc_pause_frames3;
-	*data++ = s->tx_cbfc_pause_frames4;
-	*data++ = s->tx_cbfc_pause_frames5;
-	*data++ = s->tx_cbfc_pause_frames6;
-	*data++ = s->tx_cbfc_pause_frames7;
-	*data++ = s->rx_cbfc_pause_frames0;
-	*data++ = s->rx_cbfc_pause_frames1;
-	*data++ = s->rx_cbfc_pause_frames2;
-	*data++ = s->rx_cbfc_pause_frames3;
-	*data++ = s->rx_cbfc_pause_frames4;
-	*data++ = s->rx_cbfc_pause_frames5;
-	*data++ = s->rx_cbfc_pause_frames6;
-	*data++ = s->rx_cbfc_pause_frames7;
-	*data++ = s->rx_nic_fifo_drop;
+	for (index = 0; index < length; index++) {
+		char *p = (char *)qdev +
+			ql_gstrings_stats[index].stat_offset;
+		*data++ = (ql_gstrings_stats[index].sizeof_stat ==
+			sizeof(u64)) ? *(u64 *)p : (*(u32 *)p);
+	}
 }
 
 static int ql_get_settings(struct net_device *ndev,
-- 
1.7.1

[PATCH net 7/7] qlge: Bumped driver version to 1.00.00.31

[Jitendra Kalsaria]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h b/drivers/net/ethernet/qlogic/qlge/qlge.h
index e81bbb7..5a8c00c 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -18,7 +18,7 @@
  */
 #define DRV_NAME  	"qlge"
 #define DRV_STRING 	"QLogic 10 Gigabit PCI-E Ethernet Driver "
-#define DRV_VERSION	"v1.00.00.30.00.00-01"
+#define DRV_VERSION	"v1.00.00.31"
 
 #define WQ_ADDR_ALIGN	0x3	/* 4 byte alignment */
 
-- 
1.7.1

Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[David Miller]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Mon,  2 Jul 2012 19:41:48 -0400

> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> qlge driver was acting wrongly when considering TX ring full
> as a TX error. TX ring full is expected behavior when NIC is
> overwhelmed and is expected to happen, as far as packets are
> not lost.
> 
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

If your driver is properly coded, this code path should never trigger,
ever.  So it is an error, and you need to fix whatever bug exists in
your driver which allows this to happen, rather than this change
here which attempts to sweep the issue under the rug.

Re: [PATCH net 4/7] qlge: Fixed double pci free upon tx_ring->q allocation failure.

[Ben Hutchings]

On Mon, 2012-07-02 at 19:41 -0400, Jitendra Kalsaria wrote:
> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> ---
>  drivers/net/ethernet/qlogic/qlge/qlge_main.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index cdbc860..9ecd15f 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -2701,20 +2701,20 @@ static int ql_alloc_tx_resources(struct ql_adapter *qdev,
>  	    pci_alloc_consistent(qdev->pdev, tx_ring->wq_size,
>  				 &tx_ring->wq_base_dma);
>  
> -	if ((tx_ring->wq_base == NULL) ||
> -	    tx_ring->wq_base_dma & WQ_ADDR_ALIGN) {
> -		netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
> -		return -ENOMEM;
> -	}
> +	if (!tx_ring->wq_base || tx_ring->wq_base_dma & WQ_ADDR_ALIGN)
> +		goto err;
> +

So in case pci_alloc_consistent() fails, you now try to free anyway.
Not sure whether that's safe; do you feel lucky?

>  	tx_ring->q =
>  	    kmalloc(tx_ring->wq_len * sizeof(struct tx_ring_desc), GFP_KERNEL);
> -	if (tx_ring->q == NULL)
> +	if (!tx_ring->q)
>  		goto err;

Unrelated change.

>  	return 0;
>  err:
>  	pci_free_consistent(qdev->pdev, tx_ring->wq_size,
> -			    tx_ring->wq_base, tx_ring->wq_base_dma);
> +			tx_ring->wq_base, tx_ring->wq_base_dma);

This was nicely indented before...

> +	tx_ring->wq_base = NULL;
> +	netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
>  	return -ENOMEM;
>  }
>  

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net 6/7] qlge: refactoring of ethtool stats.

[Ben Hutchings]

This is hardly an urgent fix...

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net 6/7] qlge: refactoring of ethtool stats.

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 3 Jul 2012 02:24:08 +0100

> This is hardly an urgent fix...

This entire series is suspect, and largely inappropriate for 'net'
this late in the -rc.

RE: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[Jitendra Kalsaria]

As per your comments, TX ring full is not expected behavior? All I can think of increasing the TX queue to 1024 and clean-up in timer instead of interrupt?

Jiten

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Monday, July 02, 2012 5:19 PM
To: Jitendra Kalsaria
Cc: netdev; Ron Mercer; Dept-NX Linux NIC Driver
Subject: Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Mon,  2 Jul 2012 19:41:48 -0400

> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> qlge driver was acting wrongly when considering TX ring full
> as a TX error. TX ring full is expected behavior when NIC is
> overwhelmed and is expected to happen, as far as packets are
> not lost.
> 
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

If your driver is properly coded, this code path should never trigger,
ever.  So it is an error, and you need to fix whatever bug exists in
your driver which allows this to happen, rather than this change
here which attempts to sweep the issue under the rug.

Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[David Miller]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Mon, 2 Jul 2012 18:30:47 -0700

> As per your comments, TX ring full is not expected behavior? All I
> can think of increasing the TX queue to 1024 and clean-up in timer
> instead of interrupt?

Your transmit function should never be invoked when the queue is
full, logic elsewhere in your driver should have stopped the queue
therefore preventing further invocations of your transmit function
until you wake the queue when space is liberated in the TX ring.

Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 02 Jul 2012 18:38:26 -0700 (PDT)

> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> Date: Mon, 2 Jul 2012 18:30:47 -0700
> 
>> As per your comments, TX ring full is not expected behavior? All I
>> can think of increasing the TX queue to 1024 and clean-up in timer
>> instead of interrupt?
> 
> Your transmit function should never be invoked when the queue is
> full, logic elsewhere in your driver should have stopped the queue
> therefore preventing further invocations of your transmit function
> until you wake the queue when space is liberated in the TX ring.

BTW, did it even occur to you that there is a kernel log message here
in this code path for a reason?

That log message is there because this event is unexpected and a
driver error.

Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[Krishna Kumar2]

> David Miller <davem@davemloft.net> 07/03/2012 07:11 AM
>
> >> As per your comments, TX ring full is not expected behavior? All I
> >> can think of increasing the TX queue to 1024 and clean-up in timer
> >> instead of interrupt?
> >
> > Your transmit function should never be invoked when the queue is
> > full, logic elsewhere in your driver should have stopped the queue
> > therefore preventing further invocations of your transmit function
> > until you wake the queue when space is liberated in the TX ring.
>
> BTW, did it even occur to you that there is a kernel log message here
> in this code path for a reason?
>
> That log message is there because this event is unexpected and a
> driver error.

Correctly coded drivers check for this condition:
"atomic_read(&tx_ring->tx_count) < 2" (without the
unlikely) once again at the end of the xmit and stop
the queue if required. Also, the message should be changed
to show that reaching here is an error.

thanks,
- KK

RE: [PATCH net 4/7] qlge: Fixed double pci free upon tx_ring->q allocation failure.

[Jitendra Kalsaria]

-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com] 
Sent: Monday, July 02, 2012 6:22 PM
To: Jitendra Kalsaria
Cc: David Miller; netdev; Ron Mercer; Dept-NX Linux NIC Driver
Subject: Re: [PATCH net 4/7] qlge: Fixed double pci free upon tx_ring->q allocation failure.

On Mon, 2012-07-02 at 19:41 -0400, Jitendra Kalsaria wrote:
> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> ---
>  drivers/net/ethernet/qlogic/qlge/qlge_main.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index cdbc860..9ecd15f 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -2701,20 +2701,20 @@ static int ql_alloc_tx_resources(struct ql_adapter *qdev,
>  	    pci_alloc_consistent(qdev->pdev, tx_ring->wq_size,
>  				 &tx_ring->wq_base_dma);
>  
> -	if ((tx_ring->wq_base == NULL) ||
> -	    tx_ring->wq_base_dma & WQ_ADDR_ALIGN) {
> -		netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
> -		return -ENOMEM;
> -	}
> +	if (!tx_ring->wq_base || tx_ring->wq_base_dma & WQ_ADDR_ALIGN)
> +		goto err;
> +

So in case pci_alloc_consistent() fails, you now try to free anyway.
Not sure whether that's safe; do you feel lucky?

[JK] Ahh, my apology. 

>  	tx_ring->q =
>  	    kmalloc(tx_ring->wq_len * sizeof(struct tx_ring_desc), GFP_KERNEL);
> -	if (tx_ring->q == NULL)
> +	if (!tx_ring->q)
>  		goto err;

Unrelated change.

>  	return 0;
>  err:
>  	pci_free_consistent(qdev->pdev, tx_ring->wq_size,
> -			    tx_ring->wq_base, tx_ring->wq_base_dma);
> +			tx_ring->wq_base, tx_ring->wq_base_dma);

This was nicely indented before...

> +	tx_ring->wq_base = NULL;
> +	netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
>  	return -ENOMEM;
>  }
>  

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[Jitendra Kalsaria]

-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net] 
>Sent: Monday, July 02, 2012 6:42 PM
>To: Jitendra Kalsaria
>Cc: netdev; Ron Mercer; Dept-NX Linux NIC Driver
>Subject: Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
>
>From: David Miller <davem@davemloft.net>
>Date: Mon, 02 Jul 2012 18:38:26 -0700 (PDT)
>
>> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>> Date: Mon, 2 Jul 2012 18:30:47 -0700
>> 
>>> As per your comments, TX ring full is not expected behavior? All I
>>> can think of increasing the TX queue to 1024 and clean-up in timer
>>> instead of interrupt?
>> 
>> Your transmit function should never be invoked when the queue is
>> full, logic elsewhere in your driver should have stopped the queue
>> therefore preventing further invocations of your transmit function
>> until you wake the queue when space is liberated in the TX ring.
>
>BTW, did it even occur to you that there is a kernel log message here
>in this code path for a reason?
>
>That log message is there because this event is unexpected and a
>driver error.

I think my patch description might have been misleading. We are not fixing a logical problem but rather a statistics reporting problem. Our transmit function is not getting called when queue is full but when we stop the queue it increment tx_error statistic and one of our customers is running a test that deliberately floods the queue causing it to periodically be stopped. The customer has not reported logical problem with the test were driver perform very well but they merely pointed out that we were incorrectly reporting the queue full condition as a tx_error.

This patch was intended to remove the line that increments the tx_error statistic when the queue is correctly stopped.

RE: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[Eric Dumazet]

On Tue, 2012-07-03 at 12:38 -0700, Jitendra Kalsaria wrote:

> I think my patch description might have been misleading. We are not
> fixing a logical problem but rather a statistics reporting problem.
> Our transmit function is not getting called when queue is full but
> when we stop the queue it increment tx_error statistic and one of our
> customers is running a test that deliberately floods the queue causing
> it to periodically be stopped. The customer has not reported logical
> problem with the test were driver perform very well but they merely
> pointed out that we were incorrectly reporting the queue full
> condition as a tx_error.
> 
> This patch was intended to remove the line that increments the
> tx_error statistic when the queue is correctly stopped.

I believe everybody kindly ask you to fix the driver logic instead
of trying to hide to your customers the problems.

In fact, you could just BUG() at this point, and maybe David will accept
such a patch.

testing atomic_read(&tx_ring->tx_count) at the beginning of qlge_send()
is too late. NETDEV_TX_BUSY is deprecated.

Well behaving drivers should perform the test at the end of their
ndo_start_xmit() and stop the queue so that next packet wont come at
all.

Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[Francois Romieu]

Eric Dumazet <eric.dumazet@gmail.com> :
> On Tue, 2012-07-03 at 12:38 -0700, Jitendra Kalsaria wrote:
> 
> > I think my patch description might have been misleading. We are not
> > fixing a logical problem but rather a statistics reporting problem.
> > Our transmit function is not getting called when queue is full but
> > when we stop the queue it increment tx_error statistic and one of our
> > customers is running a test that deliberately floods the queue causing
> > it to periodically be stopped. The customer has not reported logical
> > problem with the test were driver perform very well but they merely
> > pointed out that we were incorrectly reporting the queue full
> > condition as a tx_error.
> > 
> > This patch was intended to remove the line that increments the
> > tx_error statistic when the queue is correctly stopped.
> 
> I believe everybody kindly ask you to fix the driver logic instead
> of trying to hide to your customers the problems.

:o/

Jitendra was speaking about qlge_ethtool_ops.self_test(). It will need
fixing as well.

> In fact, you could just BUG() at this point, and maybe David will accept
> such a patch.

Mildly. It would turn qlge_ethtool_ops.self_test() into a system killer.

[...]
> testing atomic_read(&tx_ring->tx_count) at the beginning of qlge_send()
> is too late. NETDEV_TX_BUSY is deprecated.

Yes.

Returning NETDEV_TX_BUSY when dma mapping fails in ql_map_send isn't nice
either.

-- 
Ueimor

RE: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[Jitendra Kalsaria]

-----Original Message-----
>From: Eric Dumazet [mailto:eric.dumazet@gmail.com] 
>Sent: Tuesday, July 03, 2012 1:27 PM
>To: Jitendra Kalsaria
>Cc: David Miller; netdev; Ron Mercer; Dept-NX Linux NIC Driver
>Subject: RE: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
>
>On Tue, 2012-07-03 at 12:38 -0700, Jitendra Kalsaria wrote:
>
>I believe everybody kindly ask you to fix the driver logic instead
>of trying to hide to your customers the problems.
>
>In fact, you could just BUG() at this point, and maybe David will accept
>such a patch.
>
>testing atomic_read(&tx_ring->tx_count) at the beginning of qlge_send()
>is too late. NETDEV_TX_BUSY is deprecated.
>
>Well behaving drivers should perform the test at the end of their
>ndo_start_xmit() and stop the queue so that next packet wont come at
>all.

Thanks everyone and will definitely change the logic.

Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

[David Miller]

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Tue, 3 Jul 2012 12:38:04 -0700

> This patch was intended to remove the line that increments the
> tx_error statistic when the queue is correctly stopped.

It isn't correctly stopped, for the millionth time this condition is a
BUG, there is a kernel log message there because it is a BUG, are you
blind?
	if (unlikely(atomic_read(&tx_ring->tx_count) < 2)) {
		netif_info(qdev, tx_queued, qdev->ndev,
			   "%s: shutting down tx queue %d du to lack of resources.\n",
			   __func__, tx_ring_idx);
		netif_stop_subqueue(ndev, tx_ring->wq_id);
		atomic_inc(&tx_ring->queue_stopped);
		tx_ring->tx_errors++;
		return NETDEV_TX_BUSY;
	}

THIS CODE BLOCK SHOULD NEVER EXECUTE.  It's a driver bug, it should
never happen.

Even if the driver recovers correctly, it's still an error condition.

It's a bug, and bumping the statistic is not wrong at all.  You should
find out why this happens, because it's a bug, and it should be fixed.

Re: [PATCH net 3/7] qlge: Garbage values shown in extra info during selftest.

[David Miller]

Why are you posting an arbitrary patch from a patch series,
yet not the rest of that series?

This needs to be sent alongside the rest of the series.

RE: [PATCH net 3/7] qlge: Garbage values shown in extra info during selftest.

[Jitendra Kalsaria]

-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net] 
>Sent: Thursday, July 05, 2012 12:24 AM
>To: Jitendra Kalsaria
>Cc: netdev; Ron Mercer; Dept-NX Linux NIC Driver
>Subject: Re: [PATCH net 3/7] qlge: Garbage values shown in extra info during selftest.
>
>
>Why are you posting an arbitrary patch from a patch series,
>yet not the rest of that series?
>
>This needs to be sent alongside the rest of the series.

I haven't sent any arbitrary patch, seems like something wrong with mail server.

Thanks for letting me know about this will get it fixed.