[PATCH] slip: Use net_device_stats from struct net_device

[PATCH] slip: Use net_device_stats from struct net_device

[Tobias Klauser]

Use net_device->stats for stats instead of private variable copies in
struct slip.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/slip.c |   64 ++++++++++++++++++++++------------------------------
 drivers/net/slip.h |    9 -------
 2 files changed, 27 insertions(+), 46 deletions(-)

diff --git a/drivers/net/slip.c b/drivers/net/slip.c
index fa434fb..721de9d 100644
--- a/drivers/net/slip.c
+++ b/drivers/net/slip.c
@@ -271,7 +271,7 @@ static int sl_realloc_bufs(struct slip *sl, int mtu)
 			memcpy(sl->xbuff, sl->xhead, sl->xleft);
 		} else  {
 			sl->xleft = 0;
-			sl->tx_dropped++;
+			dev->stats.tx_dropped++;
 		}
 	}
 	sl->xhead = sl->xbuff;
@@ -281,7 +281,7 @@ static int sl_realloc_bufs(struct slip *sl, int mtu)
 			memcpy(sl->rbuff, rbuff, sl->rcount);
 		} else  {
 			sl->rcount = 0;
-			sl->rx_over_errors++;
+			dev->stats.rx_over_errors++;
 			set_bit(SLF_ERROR, &sl->flags);
 		}
 	}
@@ -319,6 +319,7 @@ static inline void sl_unlock(struct slip *sl)
 /* Send one completely decapsulated IP datagram to the IP layer. */
 static void sl_bump(struct slip *sl)
 {
+	struct net_device *dev = sl->dev;
 	struct sk_buff *skb;
 	int count;
 
@@ -329,13 +330,13 @@ static void sl_bump(struct slip *sl)
 		if (c & SL_TYPE_COMPRESSED_TCP) {
 			/* ignore compressed packets when CSLIP is off */
 			if (!(sl->mode & SL_MODE_CSLIP)) {
-				printk(KERN_WARNING "%s: compressed packet ignored\n", sl->dev->name);
+				printk(KERN_WARNING "%s: compressed packet ignored\n", dev->name);
 				return;
 			}
 			/* make sure we've reserved enough space for uncompress
 			   to use */
 			if (count + 80 > sl->buffsize) {
-				sl->rx_over_errors++;
+				dev->stats.rx_over_errors++;
 				return;
 			}
 			count = slhc_uncompress(sl->slcomp, sl->rbuff, count);
@@ -346,7 +347,7 @@ static void sl_bump(struct slip *sl)
 				/* turn on header compression */
 				sl->mode |= SL_MODE_CSLIP;
 				sl->mode &= ~SL_MODE_ADAPTIVE;
-				printk(KERN_INFO "%s: header compression turned on\n", sl->dev->name);
+				printk(KERN_INFO "%s: header compression turned on\n", dev->name);
 			}
 			sl->rbuff[0] &= 0x4f;
 			if (slhc_remember(sl->slcomp, sl->rbuff, count) <= 0)
@@ -355,20 +356,20 @@ static void sl_bump(struct slip *sl)
 	}
 #endif  /* SL_INCLUDE_CSLIP */
 
-	sl->rx_bytes += count;
+	dev->stats.rx_bytes += count;
 
 	skb = dev_alloc_skb(count);
 	if (skb == NULL) {
-		printk(KERN_WARNING "%s: memory squeeze, dropping packet.\n", sl->dev->name);
-		sl->rx_dropped++;
+		printk(KERN_WARNING "%s: memory squeeze, dropping packet.\n", dev->name);
+		dev->stats.rx_dropped++;
 		return;
 	}
-	skb->dev = sl->dev;
+	skb->dev = dev;
 	memcpy(skb_put(skb, count), sl->rbuff, count);
 	skb_reset_mac_header(skb);
 	skb->protocol = htons(ETH_P_IP);
 	netif_rx(skb);
-	sl->rx_packets++;
+	dev->stats.rx_packets++;
 }
 
 /* Encapsulate one IP datagram and stuff into a TTY queue. */
@@ -379,7 +380,7 @@ static void sl_encaps(struct slip *sl, unsigned char *icp, int len)
 
 	if (len > sl->mtu) {		/* Sigh, shouldn't occur BUT ... */
 		printk(KERN_WARNING "%s: truncating oversized transmit packet!\n", sl->dev->name);
-		sl->tx_dropped++;
+		sl->dev->stats.tx_dropped++;
 		sl_unlock(sl);
 		return;
 	}
@@ -433,7 +434,7 @@ static void slip_write_wakeup(struct tty_struct *tty)
 	if (sl->xleft <= 0)  {
 		/* Now serial buffer is almost free & we can start
 		 * transmission of another packet */
-		sl->tx_packets++;
+		sl->dev->stats.tx_packets++;
 		clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 		sl_unlock(sl);
 		return;
@@ -496,7 +497,7 @@ sl_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	sl_lock(sl);
-	sl->tx_bytes += skb->len;
+	dev->stats.tx_bytes += skb->len;
 	sl_encaps(sl, skb->data, skb->len);
 	spin_unlock(&sl->lock);
 
@@ -561,36 +562,25 @@ static int sl_change_mtu(struct net_device *dev, int new_mtu)
 static struct net_device_stats *
 sl_get_stats(struct net_device *dev)
 {
-	static struct net_device_stats stats;
+	struct net_device_stats *stats = &dev->stats;
 	struct slip *sl = netdev_priv(dev);
 #ifdef SL_INCLUDE_CSLIP
 	struct slcompress *comp;
 #endif
 
-	memset(&stats, 0, sizeof(struct net_device_stats));
-
-	stats.rx_packets     = sl->rx_packets;
-	stats.tx_packets     = sl->tx_packets;
-	stats.rx_bytes	     = sl->rx_bytes;
-	stats.tx_bytes	     = sl->tx_bytes;
-	stats.rx_dropped     = sl->rx_dropped;
-	stats.tx_dropped     = sl->tx_dropped;
-	stats.tx_errors      = sl->tx_errors;
-	stats.rx_errors      = sl->rx_errors;
-	stats.rx_over_errors = sl->rx_over_errors;
 #ifdef SL_INCLUDE_CSLIP
-	stats.rx_fifo_errors = sl->rx_compressed;
-	stats.tx_fifo_errors = sl->tx_compressed;
-	stats.collisions     = sl->tx_misses;
+	stats->rx_fifo_errors = sl->rx_compressed;
+	stats->tx_fifo_errors = sl->tx_compressed;
+	stats->collisions     = sl->tx_misses;
 	comp = sl->slcomp;
 	if (comp) {
-		stats.rx_fifo_errors += comp->sls_i_compressed;
-		stats.rx_dropped     += comp->sls_i_tossed;
-		stats.tx_fifo_errors += comp->sls_o_compressed;
-		stats.collisions     += comp->sls_o_misses;
+		stats->rx_fifo_errors += comp->sls_i_compressed;
+		stats->rx_dropped     += comp->sls_i_tossed;
+		stats->tx_fifo_errors += comp->sls_o_compressed;
+		stats->collisions     += comp->sls_o_misses;
 	}
-#endif /* CONFIG_INET */
-	return (&stats);
+#endif
+	return stats;
 }
 
 /* Netdevice register callback */
@@ -681,7 +671,7 @@ static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	while (count--) {
 		if (fp && *fp++) {
 			if (!test_and_set_bit(SLF_ERROR, &sl->flags))
-				sl->rx_errors++;
+				sl->dev->stats.rx_errors++;
 			cp++;
 			continue;
 		}
@@ -981,7 +971,7 @@ static void slip_unesc(struct slip *sl, unsigned char s)
 			sl->rbuff[sl->rcount++] = s;
 			return;
 		}
-		sl->rx_over_errors++;
+		sl->dev->stats.rx_over_errors++;
 		set_bit(SLF_ERROR, &sl->flags);
 	}
 }
@@ -1057,7 +1047,7 @@ static void slip_unesc6(struct slip *sl, unsigned char s)
 					sl->rbuff[sl->rcount++] = c;
 					return;
 				}
-				sl->rx_over_errors++;
+				sl->dev->stats.rx_over_errors++;
 				set_bit(SLF_ERROR, &sl->flags);
 			}
 		}
diff --git a/drivers/net/slip.h b/drivers/net/slip.h
index 9ea5c11..914e958 100644
--- a/drivers/net/slip.h
+++ b/drivers/net/slip.h
@@ -67,15 +67,6 @@ struct slip {
   int                   xleft;          /* bytes left in XMIT queue     */
 
   /* SLIP interface statistics. */
-  unsigned long		rx_packets;	/* inbound frames counter	*/
-  unsigned long         tx_packets;     /* outbound frames counter      */
-  unsigned long		rx_bytes;	/* inbound byte counte		*/
-  unsigned long         tx_bytes;       /* outbound byte counter	*/
-  unsigned long         rx_errors;      /* Parity, etc. errors          */
-  unsigned long         tx_errors;      /* Planned stuff                */
-  unsigned long         rx_dropped;     /* No memory for skb            */
-  unsigned long         tx_dropped;     /* When MTU change              */
-  unsigned long         rx_over_errors; /* Frame bigger than SLIP buf.  */
 #ifdef SL_INCLUDE_CSLIP
   unsigned long		tx_compressed;
   unsigned long		rx_compressed;
-- 
1.7.0.4

Re: [PATCH] slip: Use net_device_stats from struct net_device

[Eric Dumazet]

Le jeudi 19 août 2010 à 10:25 +0200, Tobias Klauser a écrit :
> Use net_device->stats for stats instead of private variable copies in
> struct slip.
> 
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
>  drivers/net/slip.c |   64 ++++++++++++++++++++++------------------------------
>  drivers/net/slip.h |    9 -------
>  2 files changed, 27 insertions(+), 46 deletions(-)


> @@ -561,36 +562,25 @@ static int sl_change_mtu(struct net_device *dev, int new_mtu)
>  static struct net_device_stats *
>  sl_get_stats(struct net_device *dev)
>  {
> -	static struct net_device_stats stats;
> +	struct net_device_stats *stats = &dev->stats;
>  	struct slip *sl = netdev_priv(dev);
>  #ifdef SL_INCLUDE_CSLIP
>  	struct slcompress *comp;
>  #endif
>  
> -	memset(&stats, 0, sizeof(struct net_device_stats));
> -
> -	stats.rx_packets     = sl->rx_packets;
> -	stats.tx_packets     = sl->tx_packets;
> -	stats.rx_bytes	     = sl->rx_bytes;
> -	stats.tx_bytes	     = sl->tx_bytes;
> -	stats.rx_dropped     = sl->rx_dropped;
> -	stats.tx_dropped     = sl->tx_dropped;
> -	stats.tx_errors      = sl->tx_errors;
> -	stats.rx_errors      = sl->rx_errors;
> -	stats.rx_over_errors = sl->rx_over_errors;
>  #ifdef SL_INCLUDE_CSLIP
> -	stats.rx_fifo_errors = sl->rx_compressed;
> -	stats.tx_fifo_errors = sl->tx_compressed;
> -	stats.collisions     = sl->tx_misses;
> +	stats->rx_fifo_errors = sl->rx_compressed;
> +	stats->tx_fifo_errors = sl->tx_compressed;
> +	stats->collisions     = sl->tx_misses;
>  	comp = sl->slcomp;
>  	if (comp) {
> -		stats.rx_fifo_errors += comp->sls_i_compressed;
> -		stats.rx_dropped     += comp->sls_i_tossed;
> -		stats.tx_fifo_errors += comp->sls_o_compressed;
> -		stats.collisions     += comp->sls_o_misses;
> +		stats->rx_fifo_errors += comp->sls_i_compressed;
> +		stats->rx_dropped     += comp->sls_i_tossed;
> +		stats->tx_fifo_errors += comp->sls_o_compressed;
> +		stats->collisions     += comp->sls_o_misses;
>  	}
> -#endif /* CONFIG_INET */
> -	return (&stats);
> +#endif
> +	return stats;
>  }
>  

Hmm, this is wrong.

Each time sl_get_stats() is called, you are adding stuff to dev->stats

Quite frankly I dont think its a kernel-janitors@vger.kernel.org patch,
its pretty normal netdev stuff.

Please take a look at prior patch posted yesterday.

http://marc.info/?l=linux-netdev&m=128213719605250&w=2

Because either you should build your patch on top of it, or ask David to
revert mine before ;)

I advise using a ndo_get_stats64() so that you can perform the adds on a
private destination buffer.

Thanks

Re: [PATCH] slip: Use net_device_stats from struct net_device

[Tobias Klauser]

On 2010-08-19 at 10:49:51 +0200, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 19 août 2010 à 10:25 +0200, Tobias Klauser a écrit :
> > Use net_device->stats for stats instead of private variable copies in
> > struct slip.
> > 
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> >  drivers/net/slip.c |   64 ++++++++++++++++++++++------------------------------
> >  drivers/net/slip.h |    9 -------
> >  2 files changed, 27 insertions(+), 46 deletions(-)
> 
> 
> > @@ -561,36 +562,25 @@ static int sl_change_mtu(struct net_device *dev, int new_mtu)
> >  static struct net_device_stats *
> >  sl_get_stats(struct net_device *dev)
> >  {
> > -	static struct net_device_stats stats;
> > +	struct net_device_stats *stats = &dev->stats;
> >  	struct slip *sl = netdev_priv(dev);
> >  #ifdef SL_INCLUDE_CSLIP
> >  	struct slcompress *comp;
> >  #endif
> >  
> > -	memset(&stats, 0, sizeof(struct net_device_stats));
> > -
> > -	stats.rx_packets     = sl->rx_packets;
> > -	stats.tx_packets     = sl->tx_packets;
> > -	stats.rx_bytes	     = sl->rx_bytes;
> > -	stats.tx_bytes	     = sl->tx_bytes;
> > -	stats.rx_dropped     = sl->rx_dropped;
> > -	stats.tx_dropped     = sl->tx_dropped;
> > -	stats.tx_errors      = sl->tx_errors;
> > -	stats.rx_errors      = sl->rx_errors;
> > -	stats.rx_over_errors = sl->rx_over_errors;
> >  #ifdef SL_INCLUDE_CSLIP
> > -	stats.rx_fifo_errors = sl->rx_compressed;
> > -	stats.tx_fifo_errors = sl->tx_compressed;
> > -	stats.collisions     = sl->tx_misses;
> > +	stats->rx_fifo_errors = sl->rx_compressed;
> > +	stats->tx_fifo_errors = sl->tx_compressed;
> > +	stats->collisions     = sl->tx_misses;
> >  	comp = sl->slcomp;
> >  	if (comp) {
> > -		stats.rx_fifo_errors += comp->sls_i_compressed;
> > -		stats.rx_dropped     += comp->sls_i_tossed;
> > -		stats.tx_fifo_errors += comp->sls_o_compressed;
> > -		stats.collisions     += comp->sls_o_misses;
> > +		stats->rx_fifo_errors += comp->sls_i_compressed;
> > +		stats->rx_dropped     += comp->sls_i_tossed;
> > +		stats->tx_fifo_errors += comp->sls_o_compressed;
> > +		stats->collisions     += comp->sls_o_misses;
> >  	}
> > -#endif /* CONFIG_INET */
> > -	return (&stats);
> > +#endif
> > +	return stats;
> >  }
> >  
> 
> Hmm, this is wrong.
> 
> Each time sl_get_stats() is called, you are adding stuff to dev->stats

Hmmm, yeah. Sorry for the mess. That's completely wrong.

> Quite frankly I dont think its a kernel-janitors@vger.kernel.org patch,
> its pretty normal netdev stuff.
> 
> Please take a look at prior patch posted yesterday.
> 
> http://marc.info/?l=linux-netdev&m=128213719605250&w=2
> 
> Because either you should build your patch on top of it, or ask David to
> revert mine before ;)

Oops, I didn't notice that one. I'll rebuild my patch on top of it.

> I advise using a ndo_get_stats64() so that you can perform the adds on a
> private destination buffer.

I'll do that.

Thanks a lot
Tobias

[PATCH v2] slip: Use net_device_stats from struct net_device

[Tobias Klauser]

Use net_device->stats for stats instead of private variable copies in
struct slip.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/slip.c |   47 ++++++++++++++++++++---------------------------
 drivers/net/slip.h |    9 ---------
 2 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/drivers/net/slip.c b/drivers/net/slip.c
index d5a36f5..9512da8 100644
--- a/drivers/net/slip.c
+++ b/drivers/net/slip.c
@@ -271,7 +271,7 @@ static int sl_realloc_bufs(struct slip *sl, int mtu)
 			memcpy(sl->xbuff, sl->xhead, sl->xleft);
 		} else  {
 			sl->xleft = 0;
-			sl->tx_dropped++;
+			dev->stats.tx_dropped++;
 		}
 	}
 	sl->xhead = sl->xbuff;
@@ -281,7 +281,7 @@ static int sl_realloc_bufs(struct slip *sl, int mtu)
 			memcpy(sl->rbuff, rbuff, sl->rcount);
 		} else  {
 			sl->rcount = 0;
-			sl->rx_over_errors++;
+			dev->stats.rx_over_errors++;
 			set_bit(SLF_ERROR, &sl->flags);
 		}
 	}
@@ -319,6 +319,7 @@ static inline void sl_unlock(struct slip *sl)
 /* Send one completely decapsulated IP datagram to the IP layer. */
 static void sl_bump(struct slip *sl)
 {
+	struct net_device *dev = sl->dev;
 	struct sk_buff *skb;
 	int count;
 
@@ -329,13 +330,13 @@ static void sl_bump(struct slip *sl)
 		if (c & SL_TYPE_COMPRESSED_TCP) {
 			/* ignore compressed packets when CSLIP is off */
 			if (!(sl->mode & SL_MODE_CSLIP)) {
-				printk(KERN_WARNING "%s: compressed packet ignored\n", sl->dev->name);
+				printk(KERN_WARNING "%s: compressed packet ignored\n", dev->name);
 				return;
 			}
 			/* make sure we've reserved enough space for uncompress
 			   to use */
 			if (count + 80 > sl->buffsize) {
-				sl->rx_over_errors++;
+				dev->stats.rx_over_errors++;
 				return;
 			}
 			count = slhc_uncompress(sl->slcomp, sl->rbuff, count);
@@ -346,7 +347,7 @@ static void sl_bump(struct slip *sl)
 				/* turn on header compression */
 				sl->mode |= SL_MODE_CSLIP;
 				sl->mode &= ~SL_MODE_ADAPTIVE;
-				printk(KERN_INFO "%s: header compression turned on\n", sl->dev->name);
+				printk(KERN_INFO "%s: header compression turned on\n", dev->name);
 			}
 			sl->rbuff[0] &= 0x4f;
 			if (slhc_remember(sl->slcomp, sl->rbuff, count) <= 0)
@@ -355,20 +356,20 @@ static void sl_bump(struct slip *sl)
 	}
 #endif  /* SL_INCLUDE_CSLIP */
 
-	sl->rx_bytes += count;
+	dev->stats.rx_bytes += count;
 
 	skb = dev_alloc_skb(count);
 	if (skb == NULL) {
-		printk(KERN_WARNING "%s: memory squeeze, dropping packet.\n", sl->dev->name);
-		sl->rx_dropped++;
+		printk(KERN_WARNING "%s: memory squeeze, dropping packet.\n", dev->name);
+		dev->stats.rx_dropped++;
 		return;
 	}
-	skb->dev = sl->dev;
+	skb->dev = dev;
 	memcpy(skb_put(skb, count), sl->rbuff, count);
 	skb_reset_mac_header(skb);
 	skb->protocol = htons(ETH_P_IP);
 	netif_rx(skb);
-	sl->rx_packets++;
+	dev->stats.rx_packets++;
 }
 
 /* Encapsulate one IP datagram and stuff into a TTY queue. */
@@ -379,7 +380,7 @@ static void sl_encaps(struct slip *sl, unsigned char *icp, int len)
 
 	if (len > sl->mtu) {		/* Sigh, shouldn't occur BUT ... */
 		printk(KERN_WARNING "%s: truncating oversized transmit packet!\n", sl->dev->name);
-		sl->tx_dropped++;
+		sl->dev->stats.tx_dropped++;
 		sl_unlock(sl);
 		return;
 	}
@@ -433,7 +434,7 @@ static void slip_write_wakeup(struct tty_struct *tty)
 	if (sl->xleft <= 0)  {
 		/* Now serial buffer is almost free & we can start
 		 * transmission of another packet */
-		sl->tx_packets++;
+		sl->dev->stats.tx_packets++;
 		clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 		sl_unlock(sl);
 		return;
@@ -496,7 +497,7 @@ sl_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	sl_lock(sl);
-	sl->tx_bytes += skb->len;
+	dev->stats.tx_bytes += skb->len;
 	sl_encaps(sl, skb->data, skb->len);
 	spin_unlock(&sl->lock);
 
@@ -562,12 +563,12 @@ static struct net_device_stats *
 sl_get_stats(struct net_device *dev)
 {
 	struct net_device_stats *stats = &dev->stats;
-	struct slip *sl = netdev_priv(dev);
-	unsigned long c_rx_dropped = 0;
 #ifdef SL_INCLUDE_CSLIP
+	unsigned long c_rx_dropped = 0;
 	unsigned long c_rx_fifo_errors = 0;
 	unsigned long c_tx_fifo_errors = 0;
 	unsigned long c_collisions = 0;
+	struct slip *sl = netdev_priv(dev);
 	struct slcompress *comp = sl->slcomp;
 
 	if (comp) {
@@ -579,17 +580,9 @@ sl_get_stats(struct net_device *dev)
 	stats->rx_fifo_errors = sl->rx_compressed + c_rx_fifo_errors;
 	stats->tx_fifo_errors = sl->tx_compressed + c_tx_fifo_errors;
 	stats->collisions     = sl->tx_misses + c_collisions;
+	stats->rx_dropped    += c_rx_dropped;
 #endif
 
-	stats->rx_packets     = sl->rx_packets;
-	stats->tx_packets     = sl->tx_packets;
-	stats->rx_bytes	      = sl->rx_bytes;
-	stats->tx_bytes	      = sl->tx_bytes;
-	stats->rx_dropped     = sl->rx_dropped + c_rx_dropped;
-	stats->tx_dropped     = sl->tx_dropped;
-	stats->tx_errors      = sl->tx_errors;
-	stats->rx_errors      = sl->rx_errors;
-	stats->rx_over_errors = sl->rx_over_errors;
 	return stats;
 }
 
@@ -681,7 +674,7 @@ static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	while (count--) {
 		if (fp && *fp++) {
 			if (!test_and_set_bit(SLF_ERROR, &sl->flags))
-				sl->rx_errors++;
+				sl->dev->stats.rx_errors++;
 			cp++;
 			continue;
 		}
@@ -981,7 +974,7 @@ static void slip_unesc(struct slip *sl, unsigned char s)
 			sl->rbuff[sl->rcount++] = s;
 			return;
 		}
-		sl->rx_over_errors++;
+		sl->dev->stats.rx_over_errors++;
 		set_bit(SLF_ERROR, &sl->flags);
 	}
 }
@@ -1057,7 +1050,7 @@ static void slip_unesc6(struct slip *sl, unsigned char s)
 					sl->rbuff[sl->rcount++] = c;
 					return;
 				}
-				sl->rx_over_errors++;
+				sl->dev->stats.rx_over_errors++;
 				set_bit(SLF_ERROR, &sl->flags);
 			}
 		}
diff --git a/drivers/net/slip.h b/drivers/net/slip.h
index 9ea5c11..914e958 100644
--- a/drivers/net/slip.h
+++ b/drivers/net/slip.h
@@ -67,15 +67,6 @@ struct slip {
   int                   xleft;          /* bytes left in XMIT queue     */
 
   /* SLIP interface statistics. */
-  unsigned long		rx_packets;	/* inbound frames counter	*/
-  unsigned long         tx_packets;     /* outbound frames counter      */
-  unsigned long		rx_bytes;	/* inbound byte counte		*/
-  unsigned long         tx_bytes;       /* outbound byte counter	*/
-  unsigned long         rx_errors;      /* Parity, etc. errors          */
-  unsigned long         tx_errors;      /* Planned stuff                */
-  unsigned long         rx_dropped;     /* No memory for skb            */
-  unsigned long         tx_dropped;     /* When MTU change              */
-  unsigned long         rx_over_errors; /* Frame bigger than SLIP buf.  */
 #ifdef SL_INCLUDE_CSLIP
   unsigned long		tx_compressed;
   unsigned long		rx_compressed;
-- 
1.7.0.4

Re: [PATCH v2] slip: Use net_device_stats from struct net_device

[Eric Dumazet]

Le jeudi 26 août 2010 à 10:28 +0200, Tobias Klauser a écrit :
> Use net_device->stats for stats instead of private variable copies in
> struct slip.
> 
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
>  drivers/net/slip.c |   47 ++++++++++++++++++++---------------------------
>  drivers/net/slip.h |    9 ---------
>  2 files changed, 20 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/slip.c b/drivers/net/slip.c
> index d5a36f5..9512da8 100644
> --- a/drivers/net/slip.c
> +++ b/drivers/net/slip.c
> @@ -271,7 +271,7 @@ static int sl_realloc_bufs(struct slip *sl, int mtu)
>  			memcpy(sl->xbuff, sl->xhead, sl->xleft);
>  		} else  {
>  			sl->xleft = 0;
> -			sl->tx_dropped++;
> +			dev->stats.tx_dropped++;
>  		}
>  	}
>  	sl->xhead = sl->xbuff;
> @@ -281,7 +281,7 @@ static int sl_realloc_bufs(struct slip *sl, int mtu)
>  			memcpy(sl->rbuff, rbuff, sl->rcount);
>  		} else  {
>  			sl->rcount = 0;
> -			sl->rx_over_errors++;
> +			dev->stats.rx_over_errors++;
>  			set_bit(SLF_ERROR, &sl->flags);
>  		}
>  	}
> @@ -319,6 +319,7 @@ static inline void sl_unlock(struct slip *sl)
>  /* Send one completely decapsulated IP datagram to the IP layer. */
>  static void sl_bump(struct slip *sl)
>  {
> +	struct net_device *dev = sl->dev;
>  	struct sk_buff *skb;
>  	int count;
>  
> @@ -329,13 +330,13 @@ static void sl_bump(struct slip *sl)
>  		if (c & SL_TYPE_COMPRESSED_TCP) {
>  			/* ignore compressed packets when CSLIP is off */
>  			if (!(sl->mode & SL_MODE_CSLIP)) {
> -				printk(KERN_WARNING "%s: compressed packet ignored\n", sl->dev->name);
> +				printk(KERN_WARNING "%s: compressed packet ignored\n", dev->name);
>  				return;
>  			}
>  			/* make sure we've reserved enough space for uncompress
>  			   to use */
>  			if (count + 80 > sl->buffsize) {
> -				sl->rx_over_errors++;
> +				dev->stats.rx_over_errors++;
>  				return;
>  			}
>  			count = slhc_uncompress(sl->slcomp, sl->rbuff, count);
> @@ -346,7 +347,7 @@ static void sl_bump(struct slip *sl)
>  				/* turn on header compression */
>  				sl->mode |= SL_MODE_CSLIP;
>  				sl->mode &= ~SL_MODE_ADAPTIVE;
> -				printk(KERN_INFO "%s: header compression turned on\n", sl->dev->name);
> +				printk(KERN_INFO "%s: header compression turned on\n", dev->name);
>  			}
>  			sl->rbuff[0] &= 0x4f;
>  			if (slhc_remember(sl->slcomp, sl->rbuff, count) <= 0)
> @@ -355,20 +356,20 @@ static void sl_bump(struct slip *sl)
>  	}
>  #endif  /* SL_INCLUDE_CSLIP */
>  
> -	sl->rx_bytes += count;
> +	dev->stats.rx_bytes += count;
>  
>  	skb = dev_alloc_skb(count);
>  	if (skb == NULL) {
> -		printk(KERN_WARNING "%s: memory squeeze, dropping packet.\n", sl->dev->name);
> -		sl->rx_dropped++;
> +		printk(KERN_WARNING "%s: memory squeeze, dropping packet.\n", dev->name);
> +		dev->stats.rx_dropped++;
>  		return;
>  	}
> -	skb->dev = sl->dev;
> +	skb->dev = dev;
>  	memcpy(skb_put(skb, count), sl->rbuff, count);
>  	skb_reset_mac_header(skb);
>  	skb->protocol = htons(ETH_P_IP);
>  	netif_rx(skb);
> -	sl->rx_packets++;
> +	dev->stats.rx_packets++;
>  }
>  
>  /* Encapsulate one IP datagram and stuff into a TTY queue. */
> @@ -379,7 +380,7 @@ static void sl_encaps(struct slip *sl, unsigned char *icp, int len)
>  
>  	if (len > sl->mtu) {		/* Sigh, shouldn't occur BUT ... */
>  		printk(KERN_WARNING "%s: truncating oversized transmit packet!\n", sl->dev->name);
> -		sl->tx_dropped++;
> +		sl->dev->stats.tx_dropped++;
>  		sl_unlock(sl);
>  		return;
>  	}
> @@ -433,7 +434,7 @@ static void slip_write_wakeup(struct tty_struct *tty)
>  	if (sl->xleft <= 0)  {
>  		/* Now serial buffer is almost free & we can start
>  		 * transmission of another packet */
> -		sl->tx_packets++;
> +		sl->dev->stats.tx_packets++;
>  		clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
>  		sl_unlock(sl);
>  		return;
> @@ -496,7 +497,7 @@ sl_xmit(struct sk_buff *skb, struct net_device *dev)
>  	}
>  
>  	sl_lock(sl);
> -	sl->tx_bytes += skb->len;
> +	dev->stats.tx_bytes += skb->len;
>  	sl_encaps(sl, skb->data, skb->len);
>  	spin_unlock(&sl->lock);
>  
> @@ -562,12 +563,12 @@ static struct net_device_stats *
>  sl_get_stats(struct net_device *dev)
>  {
>  	struct net_device_stats *stats = &dev->stats;
> -	struct slip *sl = netdev_priv(dev);
> -	unsigned long c_rx_dropped = 0;
>  #ifdef SL_INCLUDE_CSLIP
> +	unsigned long c_rx_dropped = 0;
>  	unsigned long c_rx_fifo_errors = 0;
>  	unsigned long c_tx_fifo_errors = 0;
>  	unsigned long c_collisions = 0;
> +	struct slip *sl = netdev_priv(dev);
>  	struct slcompress *comp = sl->slcomp;
>  
>  	if (comp) {
> @@ -579,17 +580,9 @@ sl_get_stats(struct net_device *dev)
>  	stats->rx_fifo_errors = sl->rx_compressed + c_rx_fifo_errors;
>  	stats->tx_fifo_errors = sl->tx_compressed + c_tx_fifo_errors;
>  	stats->collisions     = sl->tx_misses + c_collisions;
> +	stats->rx_dropped    += c_rx_dropped;

Sorry this bit is wrong.

You cannot do "stats->somefield += somevalue", since its cumulative for
each call to "cat /proc/net/dev"


>  #endif
>  
> -	stats->rx_packets     = sl->rx_packets;
> -	stats->tx_packets     = sl->tx_packets;
> -	stats->rx_bytes	      = sl->rx_bytes;
> -	stats->tx_bytes	      = sl->tx_bytes;
> -	stats->rx_dropped     = sl->rx_dropped + c_rx_dropped;
> -	stats->tx_dropped     = sl->tx_dropped;
> -	stats->tx_errors      = sl->tx_errors;
> -	stats->rx_errors      = sl->rx_errors;
> -	stats->rx_over_errors = sl->rx_over_errors;
>  	return stats;
>  }
>  
> @@ -681,7 +674,7 @@ static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
>  	while (count--) {
>  		if (fp && *fp++) {
>  			if (!test_and_set_bit(SLF_ERROR, &sl->flags))
> -				sl->rx_errors++;
> +				sl->dev->stats.rx_errors++;
>  			cp++;
>  			continue;
>  		}
> @@ -981,7 +974,7 @@ static void slip_unesc(struct slip *sl, unsigned char s)
>  			sl->rbuff[sl->rcount++] = s;
>  			return;
>  		}
> -		sl->rx_over_errors++;
> +		sl->dev->stats.rx_over_errors++;
>  		set_bit(SLF_ERROR, &sl->flags);
>  	}
>  }
> @@ -1057,7 +1050,7 @@ static void slip_unesc6(struct slip *sl, unsigned char s)
>  					sl->rbuff[sl->rcount++] = c;
>  					return;
>  				}
> -				sl->rx_over_errors++;
> +				sl->dev->stats.rx_over_errors++;
>  				set_bit(SLF_ERROR, &sl->flags);
>  			}
>  		}
> diff --git a/drivers/net/slip.h b/drivers/net/slip.h
> index 9ea5c11..914e958 100644
> --- a/drivers/net/slip.h
> +++ b/drivers/net/slip.h
> @@ -67,15 +67,6 @@ struct slip {
>    int                   xleft;          /* bytes left in XMIT queue     */
>  
>    /* SLIP interface statistics. */
> -  unsigned long		rx_packets;	/* inbound frames counter	*/
> -  unsigned long         tx_packets;     /* outbound frames counter      */
> -  unsigned long		rx_bytes;	/* inbound byte counte		*/
> -  unsigned long         tx_bytes;       /* outbound byte counter	*/
> -  unsigned long         rx_errors;      /* Parity, etc. errors          */
> -  unsigned long         tx_errors;      /* Planned stuff                */
> -  unsigned long         rx_dropped;     /* No memory for skb            */
> -  unsigned long         tx_dropped;     /* When MTU change              */
> -  unsigned long         rx_over_errors; /* Frame bigger than SLIP buf.  */
>  #ifdef SL_INCLUDE_CSLIP
>    unsigned long		tx_compressed;
>    unsigned long		rx_compressed;

Re: [PATCH v2] slip: Use net_device_stats from struct net_device

[Tobias Klauser]

On 2010-08-26 at 10:39:32 +0200, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 26 août 2010 à 10:28 +0200, Tobias Klauser a écrit :
> > @@ -562,12 +563,12 @@ static struct net_device_stats *
> >  sl_get_stats(struct net_device *dev)
> >  {
> >  	struct net_device_stats *stats = &dev->stats;
> > -	struct slip *sl = netdev_priv(dev);
> > -	unsigned long c_rx_dropped = 0;
> >  #ifdef SL_INCLUDE_CSLIP
> > +	unsigned long c_rx_dropped = 0;
> >  	unsigned long c_rx_fifo_errors = 0;
> >  	unsigned long c_tx_fifo_errors = 0;
> >  	unsigned long c_collisions = 0;
> > +	struct slip *sl = netdev_priv(dev);
> >  	struct slcompress *comp = sl->slcomp;
> >  
> >  	if (comp) {
> > @@ -579,17 +580,9 @@ sl_get_stats(struct net_device *dev)
> >  	stats->rx_fifo_errors = sl->rx_compressed + c_rx_fifo_errors;
> >  	stats->tx_fifo_errors = sl->tx_compressed + c_tx_fifo_errors;
> >  	stats->collisions     = sl->tx_misses + c_collisions;
> > +	stats->rx_dropped    += c_rx_dropped;
> 
> Sorry this bit is wrong.
> 
> You cannot do "stats->somefield += somevalue", since its cumulative for
> each call to "cat /proc/net/dev"

Sorry for getting it wrong the second time and wasting your time. I
didn't read your answer to the first patch careful enough (I should have
used ndo_get_stats64 as you clearly stated). Hopefully I get it right in
v3 :-)

Sorry again and thanks a lot

[PATCH v3] slip: Use net_device_stats from struct net_device

[Tobias Klauser]

Use net_device->stats for stats instead of private variable copies in
struct slip. Use ndo_get_stat64 so the additions can be performed on a private
destination buffer.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/slip.c |   61 ++++++++++++++++++++++++++-------------------------
 drivers/net/slip.h |    9 -------
 2 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/drivers/net/slip.c b/drivers/net/slip.c
index d5a36f5..38547a8 100644
--- a/drivers/net/slip.c
+++ b/drivers/net/slip.c
@@ -271,7 +271,7 @@ static int sl_realloc_bufs(struct slip *sl, int mtu)
 			memcpy(sl->xbuff, sl->xhead, sl->xleft);
 		} else  {
 			sl->xleft = 0;
-			sl->tx_dropped++;
+			dev->stats.tx_dropped++;
 		}
 	}
 	sl->xhead = sl->xbuff;
@@ -281,7 +281,7 @@ static int sl_realloc_bufs(struct slip *sl, int mtu)
 			memcpy(sl->rbuff, rbuff, sl->rcount);
 		} else  {
 			sl->rcount = 0;
-			sl->rx_over_errors++;
+			dev->stats.rx_over_errors++;
 			set_bit(SLF_ERROR, &sl->flags);
 		}
 	}
@@ -319,6 +319,7 @@ static inline void sl_unlock(struct slip *sl)
 /* Send one completely decapsulated IP datagram to the IP layer. */
 static void sl_bump(struct slip *sl)
 {
+	struct net_device *dev = sl->dev;
 	struct sk_buff *skb;
 	int count;
 
@@ -329,13 +330,13 @@ static void sl_bump(struct slip *sl)
 		if (c & SL_TYPE_COMPRESSED_TCP) {
 			/* ignore compressed packets when CSLIP is off */
 			if (!(sl->mode & SL_MODE_CSLIP)) {
-				printk(KERN_WARNING "%s: compressed packet ignored\n", sl->dev->name);
+				printk(KERN_WARNING "%s: compressed packet ignored\n", dev->name);
 				return;
 			}
 			/* make sure we've reserved enough space for uncompress
 			   to use */
 			if (count + 80 > sl->buffsize) {
-				sl->rx_over_errors++;
+				dev->stats.rx_over_errors++;
 				return;
 			}
 			count = slhc_uncompress(sl->slcomp, sl->rbuff, count);
@@ -346,7 +347,7 @@ static void sl_bump(struct slip *sl)
 				/* turn on header compression */
 				sl->mode |= SL_MODE_CSLIP;
 				sl->mode &= ~SL_MODE_ADAPTIVE;
-				printk(KERN_INFO "%s: header compression turned on\n", sl->dev->name);
+				printk(KERN_INFO "%s: header compression turned on\n", dev->name);
 			}
 			sl->rbuff[0] &= 0x4f;
 			if (slhc_remember(sl->slcomp, sl->rbuff, count) <= 0)
@@ -355,20 +356,20 @@ static void sl_bump(struct slip *sl)
 	}
 #endif  /* SL_INCLUDE_CSLIP */
 
-	sl->rx_bytes += count;
+	dev->stats.rx_bytes += count;
 
 	skb = dev_alloc_skb(count);
 	if (skb == NULL) {
-		printk(KERN_WARNING "%s: memory squeeze, dropping packet.\n", sl->dev->name);
-		sl->rx_dropped++;
+		printk(KERN_WARNING "%s: memory squeeze, dropping packet.\n", dev->name);
+		dev->stats.rx_dropped++;
 		return;
 	}
-	skb->dev = sl->dev;
+	skb->dev = dev;
 	memcpy(skb_put(skb, count), sl->rbuff, count);
 	skb_reset_mac_header(skb);
 	skb->protocol = htons(ETH_P_IP);
 	netif_rx(skb);
-	sl->rx_packets++;
+	dev->stats.rx_packets++;
 }
 
 /* Encapsulate one IP datagram and stuff into a TTY queue. */
@@ -379,7 +380,7 @@ static void sl_encaps(struct slip *sl, unsigned char *icp, int len)
 
 	if (len > sl->mtu) {		/* Sigh, shouldn't occur BUT ... */
 		printk(KERN_WARNING "%s: truncating oversized transmit packet!\n", sl->dev->name);
-		sl->tx_dropped++;
+		sl->dev->stats.tx_dropped++;
 		sl_unlock(sl);
 		return;
 	}
@@ -433,7 +434,7 @@ static void slip_write_wakeup(struct tty_struct *tty)
 	if (sl->xleft <= 0)  {
 		/* Now serial buffer is almost free & we can start
 		 * transmission of another packet */
-		sl->tx_packets++;
+		sl->dev->stats.tx_packets++;
 		clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 		sl_unlock(sl);
 		return;
@@ -496,7 +497,7 @@ sl_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	sl_lock(sl);
-	sl->tx_bytes += skb->len;
+	dev->stats.tx_bytes += skb->len;
 	sl_encaps(sl, skb->data, skb->len);
 	spin_unlock(&sl->lock);
 
@@ -558,16 +559,16 @@ static int sl_change_mtu(struct net_device *dev, int new_mtu)
 
 /* Netdevice get statistics request */
 
-static struct net_device_stats *
-sl_get_stats(struct net_device *dev)
+static struct rtnl_link_stats64 *
+sl_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 {
-	struct net_device_stats *stats = &dev->stats;
-	struct slip *sl = netdev_priv(dev);
+	struct net_device_stats *devstats = &dev->stats;
 	unsigned long c_rx_dropped = 0;
 #ifdef SL_INCLUDE_CSLIP
 	unsigned long c_rx_fifo_errors = 0;
 	unsigned long c_tx_fifo_errors = 0;
 	unsigned long c_collisions = 0;
+	struct slip *sl = netdev_priv(dev);
 	struct slcompress *comp = sl->slcomp;
 
 	if (comp) {
@@ -580,16 +581,16 @@ sl_get_stats(struct net_device *dev)
 	stats->tx_fifo_errors = sl->tx_compressed + c_tx_fifo_errors;
 	stats->collisions     = sl->tx_misses + c_collisions;
 #endif
+	stats->rx_packets     = devstats->rx_packets;
+	stats->tx_packets     = devstats->tx_packets;
+	stats->rx_bytes       = devstats->rx_bytes;
+	stats->tx_bytes       = devstats->tx_bytes;
+	stats->rx_dropped     = devstats->rx_dropped + c_rx_dropped;
+	stats->tx_dropped     = devstats->tx_dropped;
+	stats->tx_errors      = devstats->tx_errors;
+	stats->rx_errors      = devstats->rx_errors;
+	stats->rx_over_errors = devstats->rx_over_errors;
 
-	stats->rx_packets     = sl->rx_packets;
-	stats->tx_packets     = sl->tx_packets;
-	stats->rx_bytes	      = sl->rx_bytes;
-	stats->tx_bytes	      = sl->tx_bytes;
-	stats->rx_dropped     = sl->rx_dropped + c_rx_dropped;
-	stats->tx_dropped     = sl->tx_dropped;
-	stats->tx_errors      = sl->tx_errors;
-	stats->rx_errors      = sl->rx_errors;
-	stats->rx_over_errors = sl->rx_over_errors;
 	return stats;
 }
 
@@ -633,7 +634,7 @@ static const struct net_device_ops sl_netdev_ops = {
 	.ndo_open		= sl_open,
 	.ndo_stop		= sl_close,
 	.ndo_start_xmit		= sl_xmit,
-	.ndo_get_stats	        = sl_get_stats,
+	.ndo_get_stats64        = sl_get_stats64,
 	.ndo_change_mtu		= sl_change_mtu,
 	.ndo_tx_timeout		= sl_tx_timeout,
 #ifdef CONFIG_SLIP_SMART
@@ -681,7 +682,7 @@ static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	while (count--) {
 		if (fp && *fp++) {
 			if (!test_and_set_bit(SLF_ERROR, &sl->flags))
-				sl->rx_errors++;
+				sl->dev->stats.rx_errors++;
 			cp++;
 			continue;
 		}
@@ -981,7 +982,7 @@ static void slip_unesc(struct slip *sl, unsigned char s)
 			sl->rbuff[sl->rcount++] = s;
 			return;
 		}
-		sl->rx_over_errors++;
+		sl->dev->stats.rx_over_errors++;
 		set_bit(SLF_ERROR, &sl->flags);
 	}
 }
@@ -1057,7 +1058,7 @@ static void slip_unesc6(struct slip *sl, unsigned char s)
 					sl->rbuff[sl->rcount++] = c;
 					return;
 				}
-				sl->rx_over_errors++;
+				sl->dev->stats.rx_over_errors++;
 				set_bit(SLF_ERROR, &sl->flags);
 			}
 		}
diff --git a/drivers/net/slip.h b/drivers/net/slip.h
index 9ea5c11..914e958 100644
--- a/drivers/net/slip.h
+++ b/drivers/net/slip.h
@@ -67,15 +67,6 @@ struct slip {
   int                   xleft;          /* bytes left in XMIT queue     */
 
   /* SLIP interface statistics. */
-  unsigned long		rx_packets;	/* inbound frames counter	*/
-  unsigned long         tx_packets;     /* outbound frames counter      */
-  unsigned long		rx_bytes;	/* inbound byte counte		*/
-  unsigned long         tx_bytes;       /* outbound byte counter	*/
-  unsigned long         rx_errors;      /* Parity, etc. errors          */
-  unsigned long         tx_errors;      /* Planned stuff                */
-  unsigned long         rx_dropped;     /* No memory for skb            */
-  unsigned long         tx_dropped;     /* When MTU change              */
-  unsigned long         rx_over_errors; /* Frame bigger than SLIP buf.  */
 #ifdef SL_INCLUDE_CSLIP
   unsigned long		tx_compressed;
   unsigned long		rx_compressed;
-- 
1.7.0.4

Re: [PATCH v3] slip: Use net_device_stats from struct net_device

[Eric Dumazet]

Le vendredi 27 août 2010 à 10:12 +0200, Tobias Klauser a écrit :
> Use net_device->stats for stats instead of private variable copies in
> struct slip. Use ndo_get_stat64 so the additions can be performed on a private
> destination buffer.
> 
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
>  drivers/net/slip.c |   61 ++++++++++++++++++++++++++-------------------------
>  drivers/net/slip.h |    9 -------
>  2 files changed, 31 insertions(+), 39 deletions(-)
> 

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Thanks Tobias !

Re: [PATCH v3] slip: Use net_device_stats from struct net_device

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 27 Aug 2010 10:28:13 +0200

> Le vendredi 27 août 2010 à 10:12 +0200, Tobias Klauser a écrit :
>> Use net_device->stats for stats instead of private variable copies in
>> struct slip. Use ndo_get_stat64 so the additions can be performed on a private
>> destination buffer.
>> 
>> Cc: Eric Dumazet <eric.dumazet@gmail.com>
>> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
>> ---
>>  drivers/net/slip.c |   61 ++++++++++++++++++++++++++-------------------------
>>  drivers/net/slip.h |    9 -------
>>  2 files changed, 31 insertions(+), 39 deletions(-)
>> 
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks everyone.