[PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[Rasesh Mody]

From: Rasesh Mody <rmody@brocade.com>

This is patch 0/6 which contains the summary of changes we've
done in this particular submission for Brocade's BR1010/BR1020
10Gb CEE capable ethernet adapter. Source is based against
net-next-2.6.

The changes are as follows:

- Removed unused variables and misc cleanup

We wish these patches to be considered for inclusion in net-next-2.6.

Signed-off-by: Debashis Dutt <ddutt@brocade.com>
Signed-off-by: Rasesh Mody <rmody@brocade.com>
---
 0 files changed

Re: [PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[David Miller]

From: Rasesh Mody <rmody@brocade.com>
Date: Mon, 23 Aug 2010 14:23:24 -0700

> From: Rasesh Mody <rmody@brocade.com>
> 
> This is patch 0/6 which contains the summary of changes we've
> done in this particular submission for Brocade's BR1010/BR1020
> 10Gb CEE capable ethernet adapter. Source is based against
> net-next-2.6.
> 
> The changes are as follows:
> 
> - Removed unused variables and misc cleanup
> 
> We wish these patches to be considered for inclusion in net-next-2.6.
> 
> Signed-off-by: Debashis Dutt <ddutt@brocade.com>
> Signed-off-by: Rasesh Mody <rmody@brocade.com>

Applied to net-next-2.6, but I had to apply the following bug
fix.  Do you even look at the warnings generated by the build
of the code you submit?

--------------------
bna: Delete get_flags and set_flags ethtool methods.

This driver doesn't support LRO, NTUPLE, or the RXHASH
features.  So it should not set these ethtool operations.

This also fixes the warning:

drivers/net/bna/bnad_ethtool.c:1272: warning: initialization from incompatible pointer type

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/net/bna/bnad_ethtool.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bna/bnad_ethtool.c b/drivers/net/bna/bnad_ethtool.c
index e982785..7e630f5 100644
--- a/drivers/net/bna/bnad_ethtool.c
+++ b/drivers/net/bna/bnad_ethtool.c
@@ -1268,8 +1268,6 @@ static struct ethtool_ops bnad_ethtool_ops = {
 	.set_sg = ethtool_op_set_sg,
 	.get_tso = ethtool_op_get_tso,
 	.set_tso = bnad_set_tso,
-	.get_flags = ethtool_op_get_flags,
-	.set_flags = ethtool_op_set_flags,
 	.get_strings = bnad_get_strings,
 	.get_ethtool_stats = bnad_get_ethtool_stats,
 	.get_sset_count = bnad_get_sset_count
-- 
1.7.2.2

[PATCH net-next-2.6] bna: fix stats handling

[Eric Dumazet]

Brocade guys, could you please test the following patch on your
hardware ?

Thanks

[PATCH net-next-2.6] bna: fix stats handling

get_stats() method incorrectly clears a global array before folding
various stats. This can break SNMP applications.

Switch to 64 bit flavor to work on a user supplied buffer, and provide
64bit counters even on 32bit arches.

Fix a bug in bnad_netdev_hwstats_fill(), for rx_fifo_errors, missing a
folding (only the last counter was taken into account)

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/bna/bnad.c         |   52 ++++++++++++++-----------------
 drivers/net/bna/bnad.h         |    5 +-
 drivers/net/bna/bnad_ethtool.c |   17 ++++------
 3 files changed, 33 insertions(+), 41 deletions(-)

diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index 491d148..a935b96 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c
@@ -1964,25 +1964,24 @@ bnad_enable_default_bcast(struct bnad *bnad)
 
 /* Statistics utilities */
 void
-bnad_netdev_qstats_fill(struct bnad *bnad)
+bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
 {
-	struct net_device_stats *net_stats = &bnad->net_stats;
 	int i, j;
 
 	for (i = 0; i < bnad->num_rx; i++) {
 		for (j = 0; j < bnad->num_rxp_per_rx; j++) {
 			if (bnad->rx_info[i].rx_ctrl[j].ccb) {
-				net_stats->rx_packets += bnad->rx_info[i].
+				stats->rx_packets += bnad->rx_info[i].
 				rx_ctrl[j].ccb->rcb[0]->rxq->rx_packets;
-				net_stats->rx_bytes += bnad->rx_info[i].
+				stats->rx_bytes += bnad->rx_info[i].
 					rx_ctrl[j].ccb->rcb[0]->rxq->rx_bytes;
 				if (bnad->rx_info[i].rx_ctrl[j].ccb->rcb[1] &&
 					bnad->rx_info[i].rx_ctrl[j].ccb->
 					rcb[1]->rxq) {
-					net_stats->rx_packets +=
+					stats->rx_packets +=
 						bnad->rx_info[i].rx_ctrl[j].
 						ccb->rcb[1]->rxq->rx_packets;
-					net_stats->rx_bytes +=
+					stats->rx_bytes +=
 						bnad->rx_info[i].rx_ctrl[j].
 						ccb->rcb[1]->rxq->rx_bytes;
 				}
@@ -1992,9 +1991,9 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
 	for (i = 0; i < bnad->num_tx; i++) {
 		for (j = 0; j < bnad->num_txq_per_tx; j++) {
 			if (bnad->tx_info[i].tcb[j]) {
-				net_stats->tx_packets +=
+				stats->tx_packets +=
 				bnad->tx_info[i].tcb[j]->txq->tx_packets;
-				net_stats->tx_bytes +=
+				stats->tx_bytes +=
 					bnad->tx_info[i].tcb[j]->txq->tx_bytes;
 			}
 		}
@@ -2005,37 +2004,36 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
  * Must be called with the bna_lock held.
  */
 void
-bnad_netdev_hwstats_fill(struct bnad *bnad)
+bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
 {
 	struct bfi_ll_stats_mac *mac_stats;
-	struct net_device_stats *net_stats = &bnad->net_stats;
 	u64 bmap;
 	int i;
 
 	mac_stats = &bnad->stats.bna_stats->hw_stats->mac_stats;
-	net_stats->rx_errors =
+	stats->rx_errors =
 		mac_stats->rx_fcs_error + mac_stats->rx_alignment_error +
 		mac_stats->rx_frame_length_error + mac_stats->rx_code_error +
 		mac_stats->rx_undersize;
-	net_stats->tx_errors = mac_stats->tx_fcs_error +
+	stats->tx_errors = mac_stats->tx_fcs_error +
 					mac_stats->tx_undersize;
-	net_stats->rx_dropped = mac_stats->rx_drop;
-	net_stats->tx_dropped = mac_stats->tx_drop;
-	net_stats->multicast = mac_stats->rx_multicast;
-	net_stats->collisions = mac_stats->tx_total_collision;
+	stats->rx_dropped = mac_stats->rx_drop;
+	stats->tx_dropped = mac_stats->tx_drop;
+	stats->multicast = mac_stats->rx_multicast;
+	stats->collisions = mac_stats->tx_total_collision;
 
-	net_stats->rx_length_errors = mac_stats->rx_frame_length_error;
+	stats->rx_length_errors = mac_stats->rx_frame_length_error;
 
 	/* receive ring buffer overflow  ?? */
 
-	net_stats->rx_crc_errors = mac_stats->rx_fcs_error;
-	net_stats->rx_frame_errors = mac_stats->rx_alignment_error;
+	stats->rx_crc_errors = mac_stats->rx_fcs_error;
+	stats->rx_frame_errors = mac_stats->rx_alignment_error;
 	/* recv'r fifo overrun */
 	bmap = (u64)bnad->stats.bna_stats->rxf_bmap[0] |
 		((u64)bnad->stats.bna_stats->rxf_bmap[1] << 32);
 	for (i = 0; bmap && (i < BFI_LL_RXF_ID_MAX); i++) {
 		if (bmap & 1) {
-			net_stats->rx_fifo_errors =
+			stats->rx_fifo_errors +=
 				bnad->stats.bna_stats->
 					hw_stats->rxf_stats[i].frame_drops;
 			break;
@@ -2638,22 +2636,20 @@ bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
  * Used spin_lock to synchronize reading of stats structures, which
  * is written by BNA under the same lock.
  */
-static struct net_device_stats *
-bnad_get_netdev_stats(struct net_device *netdev)
+static struct rtnl_link_stats64 *
+bnad_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *stats)
 {
 	struct bnad *bnad = netdev_priv(netdev);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 
-	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
-
-	bnad_netdev_qstats_fill(bnad);
-	bnad_netdev_hwstats_fill(bnad);
+	bnad_netdev_qstats_fill(bnad, stats);
+	bnad_netdev_hwstats_fill(bnad, stats);
 
 	spin_unlock_irqrestore(&bnad->bna_lock, flags);
 
-	return &bnad->net_stats;
+	return stats;
 }
 
 static void
@@ -2858,7 +2854,7 @@ static const struct net_device_ops bnad_netdev_ops = {
 	.ndo_open		= bnad_open,
 	.ndo_stop		= bnad_stop,
 	.ndo_start_xmit		= bnad_start_xmit,
-	.ndo_get_stats		= bnad_get_netdev_stats,
+	.ndo_get_stats64	= bnad_get_stats64,
 	.ndo_set_rx_mode	= bnad_set_rx_mode,
 	.ndo_set_multicast_list = bnad_set_rx_mode,
 	.ndo_validate_addr      = eth_validate_addr,
diff --git a/drivers/net/bna/bnad.h b/drivers/net/bna/bnad.h
index 3261401..ee37788 100644
--- a/drivers/net/bna/bnad.h
+++ b/drivers/net/bna/bnad.h
@@ -273,7 +273,6 @@ struct bnad {
 
 	/* Statistics */
 	struct bnad_stats stats;
-	struct net_device_stats net_stats;
 
 	struct bnad_diag *diag;
 
@@ -308,8 +307,8 @@ extern void bnad_cleanup_rx(struct bnad *bnad, uint rx_id);
 extern void bnad_dim_timer_start(struct bnad *bnad);
 
 /* Statistics */
-extern void bnad_netdev_qstats_fill(struct bnad *bnad);
-extern void bnad_netdev_hwstats_fill(struct bnad *bnad);
+extern void bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
+extern void bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
 
 /**
  * MACROS
diff --git a/drivers/net/bna/bnad_ethtool.c b/drivers/net/bna/bnad_ethtool.c
index 7e630f5..1f0c124 100644
--- a/drivers/net/bna/bnad_ethtool.c
+++ b/drivers/net/bna/bnad_ethtool.c
@@ -34,7 +34,7 @@
 #define BNAD_NUM_TXQ_COUNTERS 5
 
 #define BNAD_ETHTOOL_STATS_NUM						\
-	(sizeof(struct net_device_stats) / sizeof(unsigned long) +	\
+	(sizeof(struct rtnl_link_stats64) / sizeof(u64) +	\
 	sizeof(struct bnad_drv_stats) / sizeof(u64) +		\
 	offsetof(struct bfi_ll_stats, rxf_stats[0]) / sizeof(u64))
 
@@ -1159,7 +1159,8 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
 {
 	struct bnad *bnad = netdev_priv(netdev);
 	int i, j, bi;
-	unsigned long *net_stats, flags;
+	unsigned long flags;
+	struct rtnl_link_stats64 *net_stats64;
 	u64 *stats64;
 	u64 bmap;
 
@@ -1176,16 +1177,12 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 	bi = 0;
 	memset(buf, 0, stats->n_stats * sizeof(u64));
-	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
 
-	bnad_netdev_qstats_fill(bnad);
-	bnad_netdev_hwstats_fill(bnad);
+	net_stats64 = (struct rtnl_link_stats64 *)buf;
+	bnad_netdev_qstats_fill(bnad, net_stats64);
+	bnad_netdev_hwstats_fill(bnad, net_stats64);
 
-	/* Fill net_stats into ethtool buffers */
-	net_stats = (unsigned long *)&bnad->net_stats;
-	for (i = 0; i < sizeof(struct net_device_stats) / sizeof(unsigned long);
-	     i++)
-		buf[bi++] = net_stats[i];
+	bi = sizeof(*net_stats64) / sizeof(u64);
 
 	/* Fill driver stats into ethtool buffers */
 	stats64 = (u64 *)&bnad->stats.drv_stats;

RE: [PATCH net-next-2.6] bna: fix stats handling

[Debashis Dutt]

Eric, 

Thanks for the patch. We are working on verifying it on our h/w. 
We will get back to you as soon as we are done.

Thanks
--Debashis

-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com] 
Sent: Monday, August 23, 2010 10:32 PM
To: David Miller
Cc: Rasesh Mody; netdev@vger.kernel.org; Akshay Mathur; Debashis Dutt; Jing Huang
Subject: [PATCH net-next-2.6] bna: fix stats handling

Brocade guys, could you please test the following patch on your
hardware ?

Thanks

[PATCH net-next-2.6] bna: fix stats handling

get_stats() method incorrectly clears a global array before folding
various stats. This can break SNMP applications.

Switch to 64 bit flavor to work on a user supplied buffer, and provide
64bit counters even on 32bit arches.

Fix a bug in bnad_netdev_hwstats_fill(), for rx_fifo_errors, missing a
folding (only the last counter was taken into account)

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/bna/bnad.c         |   52 ++++++++++++++-----------------
 drivers/net/bna/bnad.h         |    5 +-
 drivers/net/bna/bnad_ethtool.c |   17 ++++------
 3 files changed, 33 insertions(+), 41 deletions(-)

diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index 491d148..a935b96 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c
@@ -1964,25 +1964,24 @@ bnad_enable_default_bcast(struct bnad *bnad)
 
 /* Statistics utilities */
 void
-bnad_netdev_qstats_fill(struct bnad *bnad)
+bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
 {
-	struct net_device_stats *net_stats = &bnad->net_stats;
 	int i, j;
 
 	for (i = 0; i < bnad->num_rx; i++) {
 		for (j = 0; j < bnad->num_rxp_per_rx; j++) {
 			if (bnad->rx_info[i].rx_ctrl[j].ccb) {
-				net_stats->rx_packets += bnad->rx_info[i].
+				stats->rx_packets += bnad->rx_info[i].
 				rx_ctrl[j].ccb->rcb[0]->rxq->rx_packets;
-				net_stats->rx_bytes += bnad->rx_info[i].
+				stats->rx_bytes += bnad->rx_info[i].
 					rx_ctrl[j].ccb->rcb[0]->rxq->rx_bytes;
 				if (bnad->rx_info[i].rx_ctrl[j].ccb->rcb[1] &&
 					bnad->rx_info[i].rx_ctrl[j].ccb->
 					rcb[1]->rxq) {
-					net_stats->rx_packets +=
+					stats->rx_packets +=
 						bnad->rx_info[i].rx_ctrl[j].
 						ccb->rcb[1]->rxq->rx_packets;
-					net_stats->rx_bytes +=
+					stats->rx_bytes +=
 						bnad->rx_info[i].rx_ctrl[j].
 						ccb->rcb[1]->rxq->rx_bytes;
 				}
@@ -1992,9 +1991,9 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
 	for (i = 0; i < bnad->num_tx; i++) {
 		for (j = 0; j < bnad->num_txq_per_tx; j++) {
 			if (bnad->tx_info[i].tcb[j]) {
-				net_stats->tx_packets +=
+				stats->tx_packets +=
 				bnad->tx_info[i].tcb[j]->txq->tx_packets;
-				net_stats->tx_bytes +=
+				stats->tx_bytes +=
 					bnad->tx_info[i].tcb[j]->txq->tx_bytes;
 			}
 		}
@@ -2005,37 +2004,36 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
  * Must be called with the bna_lock held.
  */
 void
-bnad_netdev_hwstats_fill(struct bnad *bnad)
+bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
 {
 	struct bfi_ll_stats_mac *mac_stats;
-	struct net_device_stats *net_stats = &bnad->net_stats;
 	u64 bmap;
 	int i;
 
 	mac_stats = &bnad->stats.bna_stats->hw_stats->mac_stats;
-	net_stats->rx_errors =
+	stats->rx_errors =
 		mac_stats->rx_fcs_error + mac_stats->rx_alignment_error +
 		mac_stats->rx_frame_length_error + mac_stats->rx_code_error +
 		mac_stats->rx_undersize;
-	net_stats->tx_errors = mac_stats->tx_fcs_error +
+	stats->tx_errors = mac_stats->tx_fcs_error +
 					mac_stats->tx_undersize;
-	net_stats->rx_dropped = mac_stats->rx_drop;
-	net_stats->tx_dropped = mac_stats->tx_drop;
-	net_stats->multicast = mac_stats->rx_multicast;
-	net_stats->collisions = mac_stats->tx_total_collision;
+	stats->rx_dropped = mac_stats->rx_drop;
+	stats->tx_dropped = mac_stats->tx_drop;
+	stats->multicast = mac_stats->rx_multicast;
+	stats->collisions = mac_stats->tx_total_collision;
 
-	net_stats->rx_length_errors = mac_stats->rx_frame_length_error;
+	stats->rx_length_errors = mac_stats->rx_frame_length_error;
 
 	/* receive ring buffer overflow  ?? */
 
-	net_stats->rx_crc_errors = mac_stats->rx_fcs_error;
-	net_stats->rx_frame_errors = mac_stats->rx_alignment_error;
+	stats->rx_crc_errors = mac_stats->rx_fcs_error;
+	stats->rx_frame_errors = mac_stats->rx_alignment_error;
 	/* recv'r fifo overrun */
 	bmap = (u64)bnad->stats.bna_stats->rxf_bmap[0] |
 		((u64)bnad->stats.bna_stats->rxf_bmap[1] << 32);
 	for (i = 0; bmap && (i < BFI_LL_RXF_ID_MAX); i++) {
 		if (bmap & 1) {
-			net_stats->rx_fifo_errors =
+			stats->rx_fifo_errors +=
 				bnad->stats.bna_stats->
 					hw_stats->rxf_stats[i].frame_drops;
 			break;
@@ -2638,22 +2636,20 @@ bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
  * Used spin_lock to synchronize reading of stats structures, which
  * is written by BNA under the same lock.
  */
-static struct net_device_stats *
-bnad_get_netdev_stats(struct net_device *netdev)
+static struct rtnl_link_stats64 *
+bnad_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *stats)
 {
 	struct bnad *bnad = netdev_priv(netdev);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 
-	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
-
-	bnad_netdev_qstats_fill(bnad);
-	bnad_netdev_hwstats_fill(bnad);
+	bnad_netdev_qstats_fill(bnad, stats);
+	bnad_netdev_hwstats_fill(bnad, stats);
 
 	spin_unlock_irqrestore(&bnad->bna_lock, flags);
 
-	return &bnad->net_stats;
+	return stats;
 }
 
 static void
@@ -2858,7 +2854,7 @@ static const struct net_device_ops bnad_netdev_ops = {
 	.ndo_open		= bnad_open,
 	.ndo_stop		= bnad_stop,
 	.ndo_start_xmit		= bnad_start_xmit,
-	.ndo_get_stats		= bnad_get_netdev_stats,
+	.ndo_get_stats64	= bnad_get_stats64,
 	.ndo_set_rx_mode	= bnad_set_rx_mode,
 	.ndo_set_multicast_list = bnad_set_rx_mode,
 	.ndo_validate_addr      = eth_validate_addr,
diff --git a/drivers/net/bna/bnad.h b/drivers/net/bna/bnad.h
index 3261401..ee37788 100644
--- a/drivers/net/bna/bnad.h
+++ b/drivers/net/bna/bnad.h
@@ -273,7 +273,6 @@ struct bnad {
 
 	/* Statistics */
 	struct bnad_stats stats;
-	struct net_device_stats net_stats;
 
 	struct bnad_diag *diag;
 
@@ -308,8 +307,8 @@ extern void bnad_cleanup_rx(struct bnad *bnad, uint rx_id);
 extern void bnad_dim_timer_start(struct bnad *bnad);
 
 /* Statistics */
-extern void bnad_netdev_qstats_fill(struct bnad *bnad);
-extern void bnad_netdev_hwstats_fill(struct bnad *bnad);
+extern void bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
+extern void bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
 
 /**
  * MACROS
diff --git a/drivers/net/bna/bnad_ethtool.c b/drivers/net/bna/bnad_ethtool.c
index 7e630f5..1f0c124 100644
--- a/drivers/net/bna/bnad_ethtool.c
+++ b/drivers/net/bna/bnad_ethtool.c
@@ -34,7 +34,7 @@
 #define BNAD_NUM_TXQ_COUNTERS 5
 
 #define BNAD_ETHTOOL_STATS_NUM						\
-	(sizeof(struct net_device_stats) / sizeof(unsigned long) +	\
+	(sizeof(struct rtnl_link_stats64) / sizeof(u64) +	\
 	sizeof(struct bnad_drv_stats) / sizeof(u64) +		\
 	offsetof(struct bfi_ll_stats, rxf_stats[0]) / sizeof(u64))
 
@@ -1159,7 +1159,8 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
 {
 	struct bnad *bnad = netdev_priv(netdev);
 	int i, j, bi;
-	unsigned long *net_stats, flags;
+	unsigned long flags;
+	struct rtnl_link_stats64 *net_stats64;
 	u64 *stats64;
 	u64 bmap;
 
@@ -1176,16 +1177,12 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 	bi = 0;
 	memset(buf, 0, stats->n_stats * sizeof(u64));
-	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
 
-	bnad_netdev_qstats_fill(bnad);
-	bnad_netdev_hwstats_fill(bnad);
+	net_stats64 = (struct rtnl_link_stats64 *)buf;
+	bnad_netdev_qstats_fill(bnad, net_stats64);
+	bnad_netdev_hwstats_fill(bnad, net_stats64);
 
-	/* Fill net_stats into ethtool buffers */
-	net_stats = (unsigned long *)&bnad->net_stats;
-	for (i = 0; i < sizeof(struct net_device_stats) / sizeof(unsigned long);
-	     i++)
-		buf[bi++] = net_stats[i];
+	bi = sizeof(*net_stats64) / sizeof(u64);
 
 	/* Fill driver stats into ethtool buffers */
 	stats64 = (u64 *)&bnad->stats.drv_stats;

RE: [PATCH net-next-2.6] bna: fix stats handling

[Rasesh Mody]

Eric,

We tried applying the patch to the source, it looks corrupted. Can you please resubmit the patch?

Thanks,
Rasesh

-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com] 
Sent: Monday, August 23, 2010 10:32 PM
To: David Miller
Cc: Rasesh Mody; netdev@vger.kernel.org; Akshay Mathur; Debashis Dutt; Jing Huang
Subject: [PATCH net-next-2.6] bna: fix stats handling

Brocade guys, could you please test the following patch on your
hardware ?

Thanks

[PATCH net-next-2.6] bna: fix stats handling

get_stats() method incorrectly clears a global array before folding
various stats. This can break SNMP applications.

Switch to 64 bit flavor to work on a user supplied buffer, and provide
64bit counters even on 32bit arches.

Fix a bug in bnad_netdev_hwstats_fill(), for rx_fifo_errors, missing a
folding (only the last counter was taken into account)

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/bna/bnad.c         |   52 ++++++++++++++-----------------
 drivers/net/bna/bnad.h         |    5 +-
 drivers/net/bna/bnad_ethtool.c |   17 ++++------
 3 files changed, 33 insertions(+), 41 deletions(-)

diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index 491d148..a935b96 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c
@@ -1964,25 +1964,24 @@ bnad_enable_default_bcast(struct bnad *bnad)
 
 /* Statistics utilities */
 void
-bnad_netdev_qstats_fill(struct bnad *bnad)
+bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
 {
-	struct net_device_stats *net_stats = &bnad->net_stats;
 	int i, j;
 
 	for (i = 0; i < bnad->num_rx; i++) {
 		for (j = 0; j < bnad->num_rxp_per_rx; j++) {
 			if (bnad->rx_info[i].rx_ctrl[j].ccb) {
-				net_stats->rx_packets += bnad->rx_info[i].
+				stats->rx_packets += bnad->rx_info[i].
 				rx_ctrl[j].ccb->rcb[0]->rxq->rx_packets;
-				net_stats->rx_bytes += bnad->rx_info[i].
+				stats->rx_bytes += bnad->rx_info[i].
 					rx_ctrl[j].ccb->rcb[0]->rxq->rx_bytes;
 				if (bnad->rx_info[i].rx_ctrl[j].ccb->rcb[1] &&
 					bnad->rx_info[i].rx_ctrl[j].ccb->
 					rcb[1]->rxq) {
-					net_stats->rx_packets +=
+					stats->rx_packets +=
 						bnad->rx_info[i].rx_ctrl[j].
 						ccb->rcb[1]->rxq->rx_packets;
-					net_stats->rx_bytes +=
+					stats->rx_bytes +=
 						bnad->rx_info[i].rx_ctrl[j].
 						ccb->rcb[1]->rxq->rx_bytes;
 				}
@@ -1992,9 +1991,9 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
 	for (i = 0; i < bnad->num_tx; i++) {
 		for (j = 0; j < bnad->num_txq_per_tx; j++) {
 			if (bnad->tx_info[i].tcb[j]) {
-				net_stats->tx_packets +=
+				stats->tx_packets +=
 				bnad->tx_info[i].tcb[j]->txq->tx_packets;
-				net_stats->tx_bytes +=
+				stats->tx_bytes +=
 					bnad->tx_info[i].tcb[j]->txq->tx_bytes;
 			}
 		}
@@ -2005,37 +2004,36 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
  * Must be called with the bna_lock held.
  */
 void
-bnad_netdev_hwstats_fill(struct bnad *bnad)
+bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
 {
 	struct bfi_ll_stats_mac *mac_stats;
-	struct net_device_stats *net_stats = &bnad->net_stats;
 	u64 bmap;
 	int i;
 
 	mac_stats = &bnad->stats.bna_stats->hw_stats->mac_stats;
-	net_stats->rx_errors =
+	stats->rx_errors =
 		mac_stats->rx_fcs_error + mac_stats->rx_alignment_error +
 		mac_stats->rx_frame_length_error + mac_stats->rx_code_error +
 		mac_stats->rx_undersize;
-	net_stats->tx_errors = mac_stats->tx_fcs_error +
+	stats->tx_errors = mac_stats->tx_fcs_error +
 					mac_stats->tx_undersize;
-	net_stats->rx_dropped = mac_stats->rx_drop;
-	net_stats->tx_dropped = mac_stats->tx_drop;
-	net_stats->multicast = mac_stats->rx_multicast;
-	net_stats->collisions = mac_stats->tx_total_collision;
+	stats->rx_dropped = mac_stats->rx_drop;
+	stats->tx_dropped = mac_stats->tx_drop;
+	stats->multicast = mac_stats->rx_multicast;
+	stats->collisions = mac_stats->tx_total_collision;
 
-	net_stats->rx_length_errors = mac_stats->rx_frame_length_error;
+	stats->rx_length_errors = mac_stats->rx_frame_length_error;
 
 	/* receive ring buffer overflow  ?? */
 
-	net_stats->rx_crc_errors = mac_stats->rx_fcs_error;
-	net_stats->rx_frame_errors = mac_stats->rx_alignment_error;
+	stats->rx_crc_errors = mac_stats->rx_fcs_error;
+	stats->rx_frame_errors = mac_stats->rx_alignment_error;
 	/* recv'r fifo overrun */
 	bmap = (u64)bnad->stats.bna_stats->rxf_bmap[0] |
 		((u64)bnad->stats.bna_stats->rxf_bmap[1] << 32);
 	for (i = 0; bmap && (i < BFI_LL_RXF_ID_MAX); i++) {
 		if (bmap & 1) {
-			net_stats->rx_fifo_errors =
+			stats->rx_fifo_errors +=
 				bnad->stats.bna_stats->
 					hw_stats->rxf_stats[i].frame_drops;
 			break;
@@ -2638,22 +2636,20 @@ bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
  * Used spin_lock to synchronize reading of stats structures, which
  * is written by BNA under the same lock.
  */
-static struct net_device_stats *
-bnad_get_netdev_stats(struct net_device *netdev)
+static struct rtnl_link_stats64 *
+bnad_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *stats)
 {
 	struct bnad *bnad = netdev_priv(netdev);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 
-	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
-
-	bnad_netdev_qstats_fill(bnad);
-	bnad_netdev_hwstats_fill(bnad);
+	bnad_netdev_qstats_fill(bnad, stats);
+	bnad_netdev_hwstats_fill(bnad, stats);
 
 	spin_unlock_irqrestore(&bnad->bna_lock, flags);
 
-	return &bnad->net_stats;
+	return stats;
 }
 
 static void
@@ -2858,7 +2854,7 @@ static const struct net_device_ops bnad_netdev_ops = {
 	.ndo_open		= bnad_open,
 	.ndo_stop		= bnad_stop,
 	.ndo_start_xmit		= bnad_start_xmit,
-	.ndo_get_stats		= bnad_get_netdev_stats,
+	.ndo_get_stats64	= bnad_get_stats64,
 	.ndo_set_rx_mode	= bnad_set_rx_mode,
 	.ndo_set_multicast_list = bnad_set_rx_mode,
 	.ndo_validate_addr      = eth_validate_addr,
diff --git a/drivers/net/bna/bnad.h b/drivers/net/bna/bnad.h
index 3261401..ee37788 100644
--- a/drivers/net/bna/bnad.h
+++ b/drivers/net/bna/bnad.h
@@ -273,7 +273,6 @@ struct bnad {
 
 	/* Statistics */
 	struct bnad_stats stats;
-	struct net_device_stats net_stats;
 
 	struct bnad_diag *diag;
 
@@ -308,8 +307,8 @@ extern void bnad_cleanup_rx(struct bnad *bnad, uint rx_id);
 extern void bnad_dim_timer_start(struct bnad *bnad);
 
 /* Statistics */
-extern void bnad_netdev_qstats_fill(struct bnad *bnad);
-extern void bnad_netdev_hwstats_fill(struct bnad *bnad);
+extern void bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
+extern void bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
 
 /**
  * MACROS
diff --git a/drivers/net/bna/bnad_ethtool.c b/drivers/net/bna/bnad_ethtool.c
index 7e630f5..1f0c124 100644
--- a/drivers/net/bna/bnad_ethtool.c
+++ b/drivers/net/bna/bnad_ethtool.c
@@ -34,7 +34,7 @@
 #define BNAD_NUM_TXQ_COUNTERS 5
 
 #define BNAD_ETHTOOL_STATS_NUM						\
-	(sizeof(struct net_device_stats) / sizeof(unsigned long) +	\
+	(sizeof(struct rtnl_link_stats64) / sizeof(u64) +	\
 	sizeof(struct bnad_drv_stats) / sizeof(u64) +		\
 	offsetof(struct bfi_ll_stats, rxf_stats[0]) / sizeof(u64))
 
@@ -1159,7 +1159,8 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
 {
 	struct bnad *bnad = netdev_priv(netdev);
 	int i, j, bi;
-	unsigned long *net_stats, flags;
+	unsigned long flags;
+	struct rtnl_link_stats64 *net_stats64;
 	u64 *stats64;
 	u64 bmap;
 
@@ -1176,16 +1177,12 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 	bi = 0;
 	memset(buf, 0, stats->n_stats * sizeof(u64));
-	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
 
-	bnad_netdev_qstats_fill(bnad);
-	bnad_netdev_hwstats_fill(bnad);
+	net_stats64 = (struct rtnl_link_stats64 *)buf;
+	bnad_netdev_qstats_fill(bnad, net_stats64);
+	bnad_netdev_hwstats_fill(bnad, net_stats64);
 
-	/* Fill net_stats into ethtool buffers */
-	net_stats = (unsigned long *)&bnad->net_stats;
-	for (i = 0; i < sizeof(struct net_device_stats) / sizeof(unsigned long);
-	     i++)
-		buf[bi++] = net_stats[i];
+	bi = sizeof(*net_stats64) / sizeof(u64);
 
 	/* Fill driver stats into ethtool buffers */
 	stats64 = (u64 *)&bnad->stats.drv_stats;

RE: [PATCH net-next-2.6] bna: fix stats handling

[Eric Dumazet]

Le mardi 24 août 2010 à 11:49 -0700, Rasesh Mody a écrit :
> Eric,
> 
> We tried applying the patch to the source, it looks corrupted. Can you please resubmit the patch?

Hmm... could you tell me how it is corrupted ?

Re: [PATCH net-next-2.6] bna: fix stats handling

[David Miller]

From: Rasesh Mody <rmody@brocade.com>
Date: Tue, 24 Aug 2010 11:49:45 -0700

> We tried applying the patch to the source, it looks corrupted. Can
> you please resubmit the patch?

His patch applies absolutely cleanly to net-next-2.6

I had to make fixes to your driver, as I mentioned when I checked
in your patch yesterday, so you absolutely must test against
what I added to net-next-2.6 and not what you have locally in
your trees.

RE: [PATCH net-next-2.6] bna: fix stats handling

[Rasesh Mody]

Hi Eric, David,

It was a mistake, I think it was a false alarm due to an email client issue. The patch was applied to net-next-2.6 and it went through cleanly. We are testing the patch and the fixes. We will let you know the results soon.

>diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c index 491d148..a935b96 100644
>--- a/drivers/net/bna/bnad.c
>+++ b/drivers/net/bna/bnad.c
>@@ -1964,25 +1964,24 @@ bnad_enable_default_bcast(struct bnad *bnad)
> 
> /* Statistics utilities */
> void
>-bnad_netdev_qstats_fill(struct bnad *bnad)
>+bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 
>+*stats)

It looks like the email client split the line and also added a + sign, giving an illusion of insertion which was not actually there.

Thanks,
Rasesh

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Tuesday, August 24, 2010 12:37 PM
To: Rasesh Mody
Cc: eric.dumazet@gmail.com; netdev@vger.kernel.org; Akshay Mathur; Debashis Dutt; Jing Huang
Subject: Re: [PATCH net-next-2.6] bna: fix stats handling

From: Rasesh Mody <rmody@brocade.com>
Date: Tue, 24 Aug 2010 11:49:45 -0700

> We tried applying the patch to the source, it looks corrupted. Can
> you please resubmit the patch?

His patch applies absolutely cleanly to net-next-2.6

I had to make fixes to your driver, as I mentioned when I checked
in your patch yesterday, so you absolutely must test against
what I added to net-next-2.6 and not what you have locally in
your trees.

RE: [PATCH net-next-2.6] bna: fix stats handling

[Rasesh Mody]

On 8/23/2010 10:31 PM, Eric Dumazet wrote:
> Brocade guys, could you please test the following patch on your
> hardware ?
> 
> Thanks
> 
> [PATCH net-next-2.6] bna: fix stats handling
> 
> get_stats() method incorrectly clears a global array before folding
> various stats. This can break SNMP applications.
> 
> Switch to 64 bit flavor to work on a user supplied buffer, and provide
> 64bit counters even on 32bit arches.
> 
> Fix a bug in bnad_netdev_hwstats_fill(), for rx_fifo_errors, missing a
> folding (only the last counter was taken into account)
> 

Thanks Eric! 
We tested the patch with our hardware and the patch works fine.
The patch applies cleanly to the net-next-2.6.

> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
Acked-by: Rasesh Mody <rmody@brocade.com>

> ---
>   drivers/net/bna/bnad.c         |   52 ++++++++++++++-----------------
>   drivers/net/bna/bnad.h         |    5 +-
>   drivers/net/bna/bnad_ethtool.c |   17 ++++------
>   3 files changed, 33 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
> index 491d148..a935b96 100644
> --- a/drivers/net/bna/bnad.c
> +++ b/drivers/net/bna/bnad.c
> @@ -1964,25 +1964,24 @@ bnad_enable_default_bcast(struct bnad *bnad)
> 
>   /* Statistics utilities */
>   void
> -bnad_netdev_qstats_fill(struct bnad *bnad)
> +bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
>   {
> -	struct net_device_stats *net_stats =&bnad->net_stats;
>   	int i, j;
> 
>   	for (i = 0; i<  bnad->num_rx; i++) {
>   		for (j = 0; j<  bnad->num_rxp_per_rx; j++) {
>   			if (bnad->rx_info[i].rx_ctrl[j].ccb) {
> -				net_stats->rx_packets += bnad->rx_info[i].
> +				stats->rx_packets += bnad->rx_info[i].
>   				rx_ctrl[j].ccb->rcb[0]->rxq->rx_packets;
> -				net_stats->rx_bytes += bnad->rx_info[i].
> +				stats->rx_bytes += bnad->rx_info[i].
>   					rx_ctrl[j].ccb->rcb[0]->rxq->rx_bytes;
>   				if (bnad->rx_info[i].rx_ctrl[j].ccb->rcb[1]&&
>   					bnad->rx_info[i].rx_ctrl[j].ccb->
>   					rcb[1]->rxq) {
> -					net_stats->rx_packets +=
> +					stats->rx_packets +=
>   						bnad->rx_info[i].rx_ctrl[j].
>   						ccb->rcb[1]->rxq->rx_packets;
> -					net_stats->rx_bytes +=
> +					stats->rx_bytes +=
>   						bnad->rx_info[i].rx_ctrl[j].
>   						ccb->rcb[1]->rxq->rx_bytes;
>   				}
> @@ -1992,9 +1991,9 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
>   	for (i = 0; i<  bnad->num_tx; i++) {
>   		for (j = 0; j<  bnad->num_txq_per_tx; j++) {
>   			if (bnad->tx_info[i].tcb[j]) {
> -				net_stats->tx_packets +=
> +				stats->tx_packets +=
>   				bnad->tx_info[i].tcb[j]->txq->tx_packets;
> -				net_stats->tx_bytes +=
> +				stats->tx_bytes +=
>   					bnad->tx_info[i].tcb[j]->txq->tx_bytes;
>   			}
>   		}
> @@ -2005,37 +2004,36 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
>    * Must be called with the bna_lock held.
>    */
>   void
> -bnad_netdev_hwstats_fill(struct bnad *bnad)
> +bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
>   {
>   	struct bfi_ll_stats_mac *mac_stats;
> -	struct net_device_stats *net_stats =&bnad->net_stats;
>   	u64 bmap;
>   	int i;
> 
>   	mac_stats =&bnad->stats.bna_stats->hw_stats->mac_stats;
> -	net_stats->rx_errors =
> +	stats->rx_errors =
>   		mac_stats->rx_fcs_error + mac_stats->rx_alignment_error +
>   		mac_stats->rx_frame_length_error + mac_stats->rx_code_error +
>   		mac_stats->rx_undersize;
> -	net_stats->tx_errors = mac_stats->tx_fcs_error +
> +	stats->tx_errors = mac_stats->tx_fcs_error +
>   					mac_stats->tx_undersize;
> -	net_stats->rx_dropped = mac_stats->rx_drop;
> -	net_stats->tx_dropped = mac_stats->tx_drop;
> -	net_stats->multicast = mac_stats->rx_multicast;
> -	net_stats->collisions = mac_stats->tx_total_collision;
> +	stats->rx_dropped = mac_stats->rx_drop;
> +	stats->tx_dropped = mac_stats->tx_drop;
> +	stats->multicast = mac_stats->rx_multicast;
> +	stats->collisions = mac_stats->tx_total_collision;
> 
> -	net_stats->rx_length_errors = mac_stats->rx_frame_length_error;
> +	stats->rx_length_errors = mac_stats->rx_frame_length_error;
> 
>   	/* receive ring buffer overflow  ?? */
> 
> -	net_stats->rx_crc_errors = mac_stats->rx_fcs_error;
> -	net_stats->rx_frame_errors = mac_stats->rx_alignment_error;
> +	stats->rx_crc_errors = mac_stats->rx_fcs_error;
> +	stats->rx_frame_errors = mac_stats->rx_alignment_error;
>   	/* recv'r fifo overrun */
>   	bmap = (u64)bnad->stats.bna_stats->rxf_bmap[0] |
>   		((u64)bnad->stats.bna_stats->rxf_bmap[1]<<  32);
>   	for (i = 0; bmap&&  (i<  BFI_LL_RXF_ID_MAX); i++) {
>   		if (bmap&  1) {
> -			net_stats->rx_fifo_errors =
> +			stats->rx_fifo_errors +=
>   				bnad->stats.bna_stats->
>   					hw_stats->rxf_stats[i].frame_drops;
>   			break;
> @@ -2638,22 +2636,20 @@ bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
>    * Used spin_lock to synchronize reading of stats structures, which
>    * is written by BNA under the same lock.
>    */
> -static struct net_device_stats *
> -bnad_get_netdev_stats(struct net_device *netdev)
> +static struct rtnl_link_stats64 *
> +bnad_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *stats)
>   {
>   	struct bnad *bnad = netdev_priv(netdev);
>   	unsigned long flags;
> 
>   	spin_lock_irqsave(&bnad->bna_lock, flags);
> 
> -	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
> -
> -	bnad_netdev_qstats_fill(bnad);
> -	bnad_netdev_hwstats_fill(bnad);
> +	bnad_netdev_qstats_fill(bnad, stats);
> +	bnad_netdev_hwstats_fill(bnad, stats);
> 
>   	spin_unlock_irqrestore(&bnad->bna_lock, flags);
> 
> -	return&bnad->net_stats;
> +	return stats;
>   }
> 
>   static void
> @@ -2858,7 +2854,7 @@ static const struct net_device_ops bnad_netdev_ops = {
>   	.ndo_open		= bnad_open,
>   	.ndo_stop		= bnad_stop,
>   	.ndo_start_xmit		= bnad_start_xmit,
> -	.ndo_get_stats		= bnad_get_netdev_stats,
> +	.ndo_get_stats64	= bnad_get_stats64,
>   	.ndo_set_rx_mode	= bnad_set_rx_mode,
>   	.ndo_set_multicast_list = bnad_set_rx_mode,
>   	.ndo_validate_addr      = eth_validate_addr,
> diff --git a/drivers/net/bna/bnad.h b/drivers/net/bna/bnad.h
> index 3261401..ee37788 100644
> --- a/drivers/net/bna/bnad.h
> +++ b/drivers/net/bna/bnad.h
> @@ -273,7 +273,6 @@ struct bnad {
> 
>   	/* Statistics */
>   	struct bnad_stats stats;
> -	struct net_device_stats net_stats;
> 
>   	struct bnad_diag *diag;
> 
> @@ -308,8 +307,8 @@ extern void bnad_cleanup_rx(struct bnad *bnad, uint rx_id);
>   extern void bnad_dim_timer_start(struct bnad *bnad);
> 
>   /* Statistics */
> -extern void bnad_netdev_qstats_fill(struct bnad *bnad);
> -extern void bnad_netdev_hwstats_fill(struct bnad *bnad);
> +extern void bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
> +extern void bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
> 
>   /**
>    * MACROS
> diff --git a/drivers/net/bna/bnad_ethtool.c b/drivers/net/bna/bnad_ethtool.c
> index 7e630f5..1f0c124 100644
> --- a/drivers/net/bna/bnad_ethtool.c
> +++ b/drivers/net/bna/bnad_ethtool.c
> @@ -34,7 +34,7 @@
>   #define BNAD_NUM_TXQ_COUNTERS 5
> 
>   #define BNAD_ETHTOOL_STATS_NUM						\
> -	(sizeof(struct net_device_stats) / sizeof(unsigned long) +	\
> +	(sizeof(struct rtnl_link_stats64) / sizeof(u64) +	\
>   	sizeof(struct bnad_drv_stats) / sizeof(u64) +		\
>   	offsetof(struct bfi_ll_stats, rxf_stats[0]) / sizeof(u64))
> 
> @@ -1159,7 +1159,8 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
>   {
>   	struct bnad *bnad = netdev_priv(netdev);
>   	int i, j, bi;
> -	unsigned long *net_stats, flags;
> +	unsigned long flags;
> +	struct rtnl_link_stats64 *net_stats64;
>   	u64 *stats64;
>   	u64 bmap;
> 
> @@ -1176,16 +1177,12 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
>   	spin_lock_irqsave(&bnad->bna_lock, flags);
>   	bi = 0;
>   	memset(buf, 0, stats->n_stats * sizeof(u64));
> -	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
> 
> -	bnad_netdev_qstats_fill(bnad);
> -	bnad_netdev_hwstats_fill(bnad);
> +	net_stats64 = (struct rtnl_link_stats64 *)buf;
> +	bnad_netdev_qstats_fill(bnad, net_stats64);
> +	bnad_netdev_hwstats_fill(bnad, net_stats64);
> 
> -	/* Fill net_stats into ethtool buffers */
> -	net_stats = (unsigned long *)&bnad->net_stats;
> -	for (i = 0; i<  sizeof(struct net_device_stats) / sizeof(unsigned long);
> -	     i++)
> -		buf[bi++] = net_stats[i];
> +	bi = sizeof(*net_stats64) / sizeof(u64);
> 
>   	/* Fill driver stats into ethtool buffers */
>   	stats64 = (u64 *)&bnad->stats.drv_stats;
> 
> 

[PATCH net-next-2.6] bna: fix stats handling

[Eric Dumazet]

Here is the bna patch again just in case it was lost ;)

Rasesh Mody tested it and Acked it.

Thanks !

[PATCH net-next-2.6] bna: fix stats handling

get_stats() method incorrectly clears a global array before folding
various stats. This can break SNMP applications.

Switch to 64 bit flavor to work on a user supplied buffer, and provide
64bit counters even on 32bit arches.

Fix a bug in bnad_netdev_hwstats_fill(), for rx_fifo_errors, missing a
folding (only the last counter was taken into account)

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Rasesh Mody <rmody@brocade.com>
---
 drivers/net/bna/bnad.c         |   52 ++++++++++++++-----------------
 drivers/net/bna/bnad.h         |    5 +-
 drivers/net/bna/bnad_ethtool.c |   17 ++++------
 3 files changed, 33 insertions(+), 41 deletions(-)

diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index 491d148..a935b96 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c
@@ -1964,25 +1964,24 @@ bnad_enable_default_bcast(struct bnad *bnad)
 
 /* Statistics utilities */
 void
-bnad_netdev_qstats_fill(struct bnad *bnad)
+bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
 {
-	struct net_device_stats *net_stats = &bnad->net_stats;
 	int i, j;
 
 	for (i = 0; i < bnad->num_rx; i++) {
 		for (j = 0; j < bnad->num_rxp_per_rx; j++) {
 			if (bnad->rx_info[i].rx_ctrl[j].ccb) {
-				net_stats->rx_packets += bnad->rx_info[i].
+				stats->rx_packets += bnad->rx_info[i].
 				rx_ctrl[j].ccb->rcb[0]->rxq->rx_packets;
-				net_stats->rx_bytes += bnad->rx_info[i].
+				stats->rx_bytes += bnad->rx_info[i].
 					rx_ctrl[j].ccb->rcb[0]->rxq->rx_bytes;
 				if (bnad->rx_info[i].rx_ctrl[j].ccb->rcb[1] &&
 					bnad->rx_info[i].rx_ctrl[j].ccb->
 					rcb[1]->rxq) {
-					net_stats->rx_packets +=
+					stats->rx_packets +=
 						bnad->rx_info[i].rx_ctrl[j].
 						ccb->rcb[1]->rxq->rx_packets;
-					net_stats->rx_bytes +=
+					stats->rx_bytes +=
 						bnad->rx_info[i].rx_ctrl[j].
 						ccb->rcb[1]->rxq->rx_bytes;
 				}
@@ -1992,9 +1991,9 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
 	for (i = 0; i < bnad->num_tx; i++) {
 		for (j = 0; j < bnad->num_txq_per_tx; j++) {
 			if (bnad->tx_info[i].tcb[j]) {
-				net_stats->tx_packets +=
+				stats->tx_packets +=
 				bnad->tx_info[i].tcb[j]->txq->tx_packets;
-				net_stats->tx_bytes +=
+				stats->tx_bytes +=
 					bnad->tx_info[i].tcb[j]->txq->tx_bytes;
 			}
 		}
@@ -2005,37 +2004,36 @@ bnad_netdev_qstats_fill(struct bnad *bnad)
  * Must be called with the bna_lock held.
  */
 void
-bnad_netdev_hwstats_fill(struct bnad *bnad)
+bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats)
 {
 	struct bfi_ll_stats_mac *mac_stats;
-	struct net_device_stats *net_stats = &bnad->net_stats;
 	u64 bmap;
 	int i;
 
 	mac_stats = &bnad->stats.bna_stats->hw_stats->mac_stats;
-	net_stats->rx_errors =
+	stats->rx_errors =
 		mac_stats->rx_fcs_error + mac_stats->rx_alignment_error +
 		mac_stats->rx_frame_length_error + mac_stats->rx_code_error +
 		mac_stats->rx_undersize;
-	net_stats->tx_errors = mac_stats->tx_fcs_error +
+	stats->tx_errors = mac_stats->tx_fcs_error +
 					mac_stats->tx_undersize;
-	net_stats->rx_dropped = mac_stats->rx_drop;
-	net_stats->tx_dropped = mac_stats->tx_drop;
-	net_stats->multicast = mac_stats->rx_multicast;
-	net_stats->collisions = mac_stats->tx_total_collision;
+	stats->rx_dropped = mac_stats->rx_drop;
+	stats->tx_dropped = mac_stats->tx_drop;
+	stats->multicast = mac_stats->rx_multicast;
+	stats->collisions = mac_stats->tx_total_collision;
 
-	net_stats->rx_length_errors = mac_stats->rx_frame_length_error;
+	stats->rx_length_errors = mac_stats->rx_frame_length_error;
 
 	/* receive ring buffer overflow  ?? */
 
-	net_stats->rx_crc_errors = mac_stats->rx_fcs_error;
-	net_stats->rx_frame_errors = mac_stats->rx_alignment_error;
+	stats->rx_crc_errors = mac_stats->rx_fcs_error;
+	stats->rx_frame_errors = mac_stats->rx_alignment_error;
 	/* recv'r fifo overrun */
 	bmap = (u64)bnad->stats.bna_stats->rxf_bmap[0] |
 		((u64)bnad->stats.bna_stats->rxf_bmap[1] << 32);
 	for (i = 0; bmap && (i < BFI_LL_RXF_ID_MAX); i++) {
 		if (bmap & 1) {
-			net_stats->rx_fifo_errors =
+			stats->rx_fifo_errors +=
 				bnad->stats.bna_stats->
 					hw_stats->rxf_stats[i].frame_drops;
 			break;
@@ -2638,22 +2636,20 @@ bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
  * Used spin_lock to synchronize reading of stats structures, which
  * is written by BNA under the same lock.
  */
-static struct net_device_stats *
-bnad_get_netdev_stats(struct net_device *netdev)
+static struct rtnl_link_stats64 *
+bnad_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *stats)
 {
 	struct bnad *bnad = netdev_priv(netdev);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 
-	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
-
-	bnad_netdev_qstats_fill(bnad);
-	bnad_netdev_hwstats_fill(bnad);
+	bnad_netdev_qstats_fill(bnad, stats);
+	bnad_netdev_hwstats_fill(bnad, stats);
 
 	spin_unlock_irqrestore(&bnad->bna_lock, flags);
 
-	return &bnad->net_stats;
+	return stats;
 }
 
 static void
@@ -2858,7 +2854,7 @@ static const struct net_device_ops bnad_netdev_ops = {
 	.ndo_open		= bnad_open,
 	.ndo_stop		= bnad_stop,
 	.ndo_start_xmit		= bnad_start_xmit,
-	.ndo_get_stats		= bnad_get_netdev_stats,
+	.ndo_get_stats64		= bnad_get_stats64,
 	.ndo_set_rx_mode	= bnad_set_rx_mode,
 	.ndo_set_multicast_list = bnad_set_rx_mode,
 	.ndo_validate_addr      = eth_validate_addr,
diff --git a/drivers/net/bna/bnad.h b/drivers/net/bna/bnad.h
index 3261401..ee37788 100644
--- a/drivers/net/bna/bnad.h
+++ b/drivers/net/bna/bnad.h
@@ -273,7 +273,6 @@ struct bnad {
 
 	/* Statistics */
 	struct bnad_stats stats;
-	struct net_device_stats net_stats;
 
 	struct bnad_diag *diag;
 
@@ -308,8 +307,8 @@ extern void bnad_cleanup_rx(struct bnad *bnad, uint rx_id);
 extern void bnad_dim_timer_start(struct bnad *bnad);
 
 /* Statistics */
-extern void bnad_netdev_qstats_fill(struct bnad *bnad);
-extern void bnad_netdev_hwstats_fill(struct bnad *bnad);
+extern void bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
+extern void bnad_netdev_hwstats_fill(struct bnad *bnad, struct rtnl_link_stats64 *stats);
 
 /**
  * MACROS
diff --git a/drivers/net/bna/bnad_ethtool.c b/drivers/net/bna/bnad_ethtool.c
index 7e630f5..1f0c124 100644
--- a/drivers/net/bna/bnad_ethtool.c
+++ b/drivers/net/bna/bnad_ethtool.c
@@ -34,7 +34,7 @@
 #define BNAD_NUM_TXQ_COUNTERS 5
 
 #define BNAD_ETHTOOL_STATS_NUM						\
-	(sizeof(struct net_device_stats) / sizeof(unsigned long) +	\
+	(sizeof(struct rtnl_link_stats64) / sizeof(u64) +	\
 	sizeof(struct bnad_drv_stats) / sizeof(u64) +		\
 	offsetof(struct bfi_ll_stats, rxf_stats[0]) / sizeof(u64))
 
@@ -1159,7 +1159,8 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
 {
 	struct bnad *bnad = netdev_priv(netdev);
 	int i, j, bi;
-	unsigned long *net_stats, flags;
+	unsigned long flags;
+	struct rtnl_link_stats64 *net_stats64;
 	u64 *stats64;
 	u64 bmap;
 
@@ -1176,16 +1177,12 @@ bnad_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats,
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 	bi = 0;
 	memset(buf, 0, stats->n_stats * sizeof(u64));
-	memset(&bnad->net_stats, 0, sizeof(struct net_device_stats));
 
-	bnad_netdev_qstats_fill(bnad);
-	bnad_netdev_hwstats_fill(bnad);
+	net_stats64 = (struct rtnl_link_stats64 *)buf;
+	bnad_netdev_qstats_fill(bnad, net_stats64);
+	bnad_netdev_hwstats_fill(bnad, net_stats64);
 
-	/* Fill net_stats into ethtool buffers */
-	net_stats = (unsigned long *)&bnad->net_stats;
-	for (i = 0; i < sizeof(struct net_device_stats) / sizeof(unsigned long);
-	     i++)
-		buf[bi++] = net_stats[i];
+	bi = sizeof(*net_stats64) / sizeof(u64);
 
 	/* Fill driver stats into ethtool buffers */
 	stats64 = (u64 *)&bnad->stats.drv_stats;

Re: [PATCH net-next-2.6] bna: fix stats handling

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 02 Sep 2010 21:27:03 +0200

> [PATCH net-next-2.6] bna: fix stats handling
> 
> get_stats() method incorrectly clears a global array before folding
> various stats. This can break SNMP applications.
> 
> Switch to 64 bit flavor to work on a user supplied buffer, and provide
> 64bit counters even on 32bit arches.
> 
> Fix a bug in bnad_netdev_hwstats_fill(), for rx_fifo_errors, missing a
> folding (only the last counter was taken into account)
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Acked-by: Rasesh Mody <rmody@brocade.com>

Applied.

RE: [PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[Debashis Dutt]

David, 

Thanks for accepting the driver patch into net-next-2.6. 

--Debashis

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Monday, August 23, 2010 8:36 PM
To: Rasesh Mody
Cc: netdev@vger.kernel.org; Akshay Mathur; Debashis Dutt; Jing Huang
Subject: Re: [PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

From: Rasesh Mody <rmody@brocade.com>
Date: Mon, 23 Aug 2010 14:23:24 -0700

> From: Rasesh Mody <rmody@brocade.com>
> 
> This is patch 0/6 which contains the summary of changes we've
> done in this particular submission for Brocade's BR1010/BR1020
> 10Gb CEE capable ethernet adapter. Source is based against
> net-next-2.6.
> 
> The changes are as follows:
> 
> - Removed unused variables and misc cleanup
> 
> We wish these patches to be considered for inclusion in net-next-2.6.
> 
> Signed-off-by: Debashis Dutt <ddutt@brocade.com>
> Signed-off-by: Rasesh Mody <rmody@brocade.com>

Applied to net-next-2.6, but I had to apply the following bug
fix.  Do you even look at the warnings generated by the build
of the code you submit?

--------------------
bna: Delete get_flags and set_flags ethtool methods.

This driver doesn't support LRO, NTUPLE, or the RXHASH
features.  So it should not set these ethtool operations.

This also fixes the warning:

drivers/net/bna/bnad_ethtool.c:1272: warning: initialization from incompatible pointer type

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/net/bna/bnad_ethtool.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bna/bnad_ethtool.c b/drivers/net/bna/bnad_ethtool.c
index e982785..7e630f5 100644
--- a/drivers/net/bna/bnad_ethtool.c
+++ b/drivers/net/bna/bnad_ethtool.c
@@ -1268,8 +1268,6 @@ static struct ethtool_ops bnad_ethtool_ops = {
 	.set_sg = ethtool_op_set_sg,
 	.get_tso = ethtool_op_get_tso,
 	.set_tso = bnad_set_tso,
-	.get_flags = ethtool_op_get_flags,
-	.set_flags = ethtool_op_set_flags,
 	.get_strings = bnad_get_strings,
 	.get_ethtool_stats = bnad_get_ethtool_stats,
 	.get_sset_count = bnad_get_sset_count
-- 
1.7.2.2

RE: [PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[Rasesh Mody]

David,

Thanks for reviewing the patches and applying it to net-next-2.6.

Regards,
Rasesh

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Monday, August 23, 2010 8:36 PM
To: Rasesh Mody
Cc: netdev@vger.kernel.org; Akshay Mathur; Debashis Dutt; Jing Huang
Subject: Re: [PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

From: Rasesh Mody <rmody@brocade.com>
Date: Mon, 23 Aug 2010 14:23:24 -0700

> From: Rasesh Mody <rmody@brocade.com>
> 
> This is patch 0/6 which contains the summary of changes we've
> done in this particular submission for Brocade's BR1010/BR1020
> 10Gb CEE capable ethernet adapter. Source is based against
> net-next-2.6.
> 
> The changes are as follows:
> 
> - Removed unused variables and misc cleanup
> 
> We wish these patches to be considered for inclusion in net-next-2.6.
> 
> Signed-off-by: Debashis Dutt <ddutt@brocade.com>
> Signed-off-by: Rasesh Mody <rmody@brocade.com>

Applied to net-next-2.6, but I had to apply the following bug
fix.  Do you even look at the warnings generated by the build
of the code you submit?

--------------------
bna: Delete get_flags and set_flags ethtool methods.

This driver doesn't support LRO, NTUPLE, or the RXHASH
features.  So it should not set these ethtool operations.

This also fixes the warning:

drivers/net/bna/bnad_ethtool.c:1272: warning: initialization from incompatible pointer type

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/net/bna/bnad_ethtool.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bna/bnad_ethtool.c b/drivers/net/bna/bnad_ethtool.c
index e982785..7e630f5 100644
--- a/drivers/net/bna/bnad_ethtool.c
+++ b/drivers/net/bna/bnad_ethtool.c
@@ -1268,8 +1268,6 @@ static struct ethtool_ops bnad_ethtool_ops = {
 	.set_sg = ethtool_op_set_sg,
 	.get_tso = ethtool_op_get_tso,
 	.set_tso = bnad_set_tso,
-	.get_flags = ethtool_op_get_flags,
-	.set_flags = ethtool_op_set_flags,
 	.get_strings = bnad_get_strings,
 	.get_ethtool_stats = bnad_get_ethtool_stats,
 	.get_sset_count = bnad_get_sset_count
-- 
1.7.2.2

[PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[Rasesh Mody]

From: Rasesh Mody <rmody@brocade.com>

This is patch 0/6 which contains the summary of changes we've
done in this particular submission for Brocade's BR1010/BR1020
10Gb CEE capable ethernet adapter. Source is based against
net-next-2.6.

The changes are as follows:

- Minimized the scope of some APIs as per the comments

- Converted spin_lock _irq()/spin_unlock_irq() to safer spin_lock_irqsave()/spin_lock_irqrestore() respectively

We wish these patches to be considered for inclusion in net-next-2.6.

Signed-off-by: Rasesh Mody <rmody@brocade.com>
---
 0 files changed

Re: [PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[Simon Horman]

Hi Rasesh,

it would be helpful if all six patches in your series
didn't feature the same subject and each had unique
description that described them in a little more detail.

Thanks

RE: [PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[Debashis Dutt]

Hi Simon, 

Thanks for your feedback. 
The only reason that we did not put a separate subject line
for each of the patches is because this is a whole new driver
submission, waiting for acceptance. So these are not really logically
separate entities (unlike other patches adding features / bug fixes).

If you still think that we should try to logically separate the patches
please let me know.

Thanks
--Debashis
Brocade Linux Driver Team.



-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: Wednesday, August 18, 2010 6:03 AM
To: Rasesh Mody
Cc: netdev@vger.kernel.org; Adapter Linux Open SRC Team
Subject: Re: [PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

Hi Rasesh,

it would be helpful if all six patches in your series
didn't feature the same subject and each had unique
description that described them in a little more detail.

Thanks

[PATCH 0/6] bna: Brocade 10Gb Ethernet device driver

[Rasesh Mody]

From: Rasesh Mody <rmody@brocade.com>

This is patch 0/6 which contains the summary of changes we've
done in this particular submission for Brocade's BR1010/BR1020
10Gb CEE capable ethernet adapter. Source is based against
net-next-2.6.

The changes are as follows:

1) Addressed comments received so far.

- Added GRO support and removed LRO completely

- Used OS primitives in place of abstractions wherever possible

- Removed duplication of PCI_DEVICE_IDs

- Corrected bnad_netpoll implementation

- Eliminated firmware image copy

- Reduced module parameters to only 2

- Return type of bnad_xmit_start() changed to netdev_tx_t

- Removed unnecessary comments in the code

- Flattened the directory structure and reduced the number of files (form ~45 to 25)

2) Added support for new hardware (e.g. IBM/Dell Mezzanine cards)

We wish this patch to be considered for inclusion in net-next-2.6.

Signed-off-by: Rasesh Mody <rmody@brocade.com>
---
 0 files changed