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

[Eric Dumazet <eric.dumazet@gmail.com>]

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;