Patchwork [net-next] drivers/net/ethernet/micrel/ks8851_mll: Implement basic statistics

login
register
mail settings
Submitter Choi, David
Date Jan. 23, 2013, 7:36 p.m.
Message ID <FD9AD8C5375B924CABC56D982DB3A802079D340B@EXMB1.micrel.com>
Download mbox | patch
Permalink /patch/215027/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Choi, David - Jan. 23, 2013, 7:36 p.m.
From: David J. Choi <david.choi@micrel.com>
 
Summary of changes:
  add codes to collect basic statistical information about Ethernet packets.
 
Signed-off-by: David J. Choi <david.choi@micrel.com>

---


---

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches - Jan. 29, 2013, 1:34 a.m.
On Wed, 2013-01-23 at 19:36 +0000, Choi, David wrote:
> From: David J. Choi <david.choi@micrel.com>
> --- net-next/drivers/net/ethernet/micrel/ks8851_mll.c.orig	2013-01-22 17:25:59.000000000 -0800
> +		if (unlikely(!skb)) {
> +			/* discard the packet from the device */
> +			ks_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr | RXQCR_RRXEF);
> +			netdev->stats.rx_dropped++;
> +		} else if (likely((frame_hdr->sts & RXFSHR_RXFV) &&
>  			(frame_hdr->len < RX_BUF_SIZE) && frame_hdr->len)) {

Just one small nit.

This would be better indented as:

		} else if (likely((frame_hdr->sts & RXFSHR_RXFV) &&
				  frame_hdr->len > 0 &&
				  frame_hdr->len < RX_BUF_SIZE)) {

I do wonder why and when frame_hdr->len would be 0
and why the test is < RX_BUF_SIZE and not <=


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- net-next/drivers/net/ethernet/micrel/ks8851_mll.c.orig	2013-01-22 17:25:59.000000000 -0800
+++ net-next/drivers/net/ethernet/micrel/ks8851_mll.c	2013-01-23 11:28:45.000000000 -0800
@@ -793,19 +793,34 @@  static void ks_rcv(struct ks_net *ks, st
 	frame_hdr = ks->frame_head_info;
 	while (ks->frame_cnt--) {
 		skb = netdev_alloc_skb(netdev, frame_hdr->len + 16);
-		if (likely(skb && (frame_hdr->sts & RXFSHR_RXFV) &&
+		if (unlikely(!skb)) {
+			/* discard the packet from the device */
+			ks_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr | RXQCR_RRXEF);
+			netdev->stats.rx_dropped++;
+		} else if (likely((frame_hdr->sts & RXFSHR_RXFV) &&
 			(frame_hdr->len < RX_BUF_SIZE) && frame_hdr->len)) {
 			skb_reserve(skb, 2);
 			/* read data block including CRC 4 bytes */
 			ks_read_qmu(ks, (u16 *)skb->data, frame_hdr->len);
-			skb_put(skb, frame_hdr->len);
+
+			/* exclude the size of CRC */
+			skb_put(skb, frame_hdr->len - 4);
 			skb->protocol = eth_type_trans(skb, netdev);
 			netif_rx(skb);
+			netdev->stats.rx_packets++;
+
+			/* crc field */
+			netdev->stats.rx_bytes += frame_hdr->len - 4;
 		} else {
-			pr_err("%s: err:skb alloc\n", __func__);
-			ks_wrreg16(ks, KS_RXQCR, (ks->rc_rxqcr | RXQCR_RRXEF));
-			if (skb)
-				dev_kfree_skb_irq(skb);
+			/* discard the packet from the device */
+			ks_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr | RXQCR_RRXEF);
+			netdev->stats.rx_dropped++;
+			if (frame_hdr->len >= RX_BUF_SIZE || !frame_hdr->len)
+				netdev->stats.rx_length_errors++;
+			if (!(frame_hdr->sts & RXFSHR_RXFV))
+				netdev->stats.rx_frame_errors++;
+
+			dev_kfree_skb_irq(skb);
 		}
 		frame_hdr++;
 	}
@@ -876,6 +891,8 @@  static irqreturn_t ks_irq(int irq, void 
 		pmecr &= ~PMECR_WKEVT_MASK;
 		ks_wrreg16(ks, KS_PMECR, pmecr | PMECR_WKEVT_LINK);
 	}
+	if (unlikely(status & IRQ_RXOI))
+		ks->netdev->stats.rx_over_errors++;
 
 	/* this should be the last in IRQ handler*/
 	ks_restore_cmd_reg(ks);
@@ -1015,6 +1032,8 @@  static int ks_start_xmit(struct sk_buff 
 
 	if (likely(ks_tx_fifo_space(ks) >= skb->len + 12)) {
 		ks_write_qmu(ks, skb->data, skb->len);
+		netdev->stats.tx_bytes += skb->len;
+		netdev->stats.tx_packets++;
 		dev_kfree_skb(skb);
 	} else
 		retv = NETDEV_TX_BUSY;