Bug 92412 - [libexec] [patch] rpc.rstatd reports bogus packets/per/second info
Summary: [libexec] [patch] rpc.rstatd reports bogus packets/per/second info
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-27 09:40 UTC by Bruce Becker
Modified: 2017-12-31 22:27 UTC (History)
0 users

See Also:


Attachments
patch-usr-src-libexec-rpc.rstatd (3.92 KB, text/plain; charset=us-ascii)
2013-04-14 09:30 UTC, G. Paul Ziemba
no flags Details
rstat_proc.c.newer.diff (4.07 KB, patch)
2013-04-14 16:18 UTC, Bruce Becker
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bruce Becker 2006-01-27 09:40:03 UTC
packets/second value reported by rpc.rstatd to remote monitor hovers around
8000 or so with odd downward spikes approx every 90 seconds (it's not at all related to actual interface traffic)

How-To-Repeat: keep displaying rpc.rstatd data from 6.0 system
Comment 1 Kris Kennaway 2006-01-27 10:01:58 UTC
How are you using rpc.rstatd and rup?  I don't see a way to make rup
display "packets/second", it only gives uptime and load average:

# rup
fbsd-amd64.isc.  10:02am  up   4 days,  14:00,  load average: 2.00 2.00 2.00

Kris
Comment 2 hostmaster 2006-01-27 16:59:15 UTC
	Solaris perfmeter, actually.

-- 
Bruce Becker			+1 416 410 0879
GTS Network Administration	Toronto, Ont.
Email:	hostmaster@gts.net
Comment 3 hostmaster 2006-01-27 22:02:33 UTC
Thus saith Kris Kennaway:
| Do you know how I can query this on FreeBSD?


	It's easy to do on any BSD system - in /usr/src/usr.bin/rup,
	make a copy rupx.c with this diff:


*** rup.c       Sat May 21 05:55:07 2005
--- rupx.c      Fri Jan 27 16:17:25 2006
***************
*** 153,158 ****
--- 153,159 ----
                (double)host_stat->avenrun[0]/FSCALE,
                (double)host_stat->avenrun[1]/FSCALE,
                (double)host_stat->avenrun[2]/FSCALE);
+       printf("ipackets: %d opackets %d\n", host_stat->if_ipackets, host_stat->if_opackets);
  
        remember_host(raddrp->sin_addr);
        return(0);


	All other FreeBSD systems seem to return 0 for if_opackets
	(which is wrong), whereas 6.0 returns junk.  NetBSD, Solaris,
	SunOS 4.x, & Irix 6.x all seem to do the right thing.


	I also note that the RPC/XDR code seems to be quite ancient -
	even SunOS 4.1.1 1990 code has declarations for an additional
	version 4 of the rstats protocol: RSTATVERS_VAR, with variable
	args for RSTAT_CPUSTATES & RSTAT_DK_NDRIVE.  All the code
	appears to have version 3 (RSTATVERS_TIME), which rup uses.


Cheers,
Bruce Becker			+1 416 410 0879
GTS Network Administration	Toronto, Ont.
Email:	hostmaster@gts.net
Comment 4 hostmaster 2006-01-29 04:28:25 UTC
Thus saith Kris Kennaway:
| Do you know how I can query this on FreeBSD?

	here's a tester kluge: in /usr/src/usr.bin/rup, make a copy
	rupx.c with this diff:

147a148
> 	printf("ipackets: %d opackets %d\n", host_stat->if_ipackets, host_stat->if_opackets);


	... but I'll do better - it's fixed.  Below are diffs for
	FreeBSD 6.x to fix 2 basic issues:

	1. the code was filling in values to the most restrictive
	   members of the union rather than the least, so values
	   got overlaid;

	2. the last gettimeofday call was clobbering the structure
	   because it has 64-bit values in amd64.


	This fix should work for all 6.x FreeBSD (and probably 5.x too) -
	4.x & 3.x seem to have issues with opackets systcl mib (always
	returns 0), so I made a version for them which uses the kmem
	interface as well as the other fixes shown below....

	BTW, the sysutils/xsysstats package works fine for X users...
	So how does this get tested & into the distro?
	(I also backported these & other fixes to 4.x)


Cheers,
Bruce Becker			+1 416 410 0879
GTS Network Administration	Toronto, Ont.
Email:	hostmaster@gts.net

[bugmaster note: older diffs deleted for clarity]

 --------- 8< --------- 8< --------- 8< --------- 8< --------- 8< ---------

--- rstat_proc.c.orig	Sun Jun  1 22:34:36 2003
+++ rstat_proc.c	Sun Jan 29 02:17:10 2006
@@ -138,6 +138,7 @@
     if (! stat_is_init)
         stat_init();
     sincelastreq = 0;
+    stats_all.s2.if_opackets = stats_all.s3.if_opackets;
     return(&stats_all.s2);
 }
 
@@ -147,6 +148,7 @@
     if (! stat_is_init)
         stat_init();
     sincelastreq = 0;
+    stats_all.s1.if_opackets = stats_all.s3.if_opackets;
     return(&stats_all.s1);
 }
 
@@ -219,13 +221,13 @@
 		exit(1);
 	}
 	for(i = 0; i < RSTAT_CPUSTATES ; i++)
-		stats_all.s1.cp_time[i] = bsd_cp_time[cp_time_xlat[i]];
+		stats_all.s3.cp_time[i] = bsd_cp_time[cp_time_xlat[i]];
 
         (void)getloadavg(avrun, sizeof(avrun) / sizeof(avrun[0]));
 
-	stats_all.s2.avenrun[0] = avrun[0] * FSCALE;
-	stats_all.s2.avenrun[1] = avrun[1] * FSCALE;
-	stats_all.s2.avenrun[2] = avrun[2] * FSCALE;
+	stats_all.s3.avenrun[0] = avrun[0] * FSCALE;
+	stats_all.s3.avenrun[1] = avrun[1] * FSCALE;
+	stats_all.s3.avenrun[2] = avrun[2] * FSCALE;
 
 	mib[0] = CTL_KERN;
 	mib[1] = KERN_BOOTTIME;
@@ -234,14 +236,13 @@
 		syslog(LOG_ERR, "sysctl(kern.boottime): %m");
 		exit(1);
 	}
-
-	stats_all.s2.boottime.tv_sec = btm.tv_sec;
-	stats_all.s2.boottime.tv_usec = btm.tv_usec;
+	stats_all.s3.boottime.tv_sec = btm.tv_sec;
+	stats_all.s3.boottime.tv_usec = btm.tv_usec;
 
 
 #ifdef DEBUG
-	fprintf(stderr, "%d %d %d %d\n", stats_all.s1.cp_time[0],
-	    stats_all.s1.cp_time[1], stats_all.s1.cp_time[2], stats_all.s1.cp_time[3]);
+	fprintf(stderr, "%d %d %d %d\n", stats_all.s3.cp_time[0],
+	    stats_all.s3.cp_time[1], stats_all.s3.cp_time[2], stats_all.s3.cp_time[3]);
 #endif
 
 	/* XXX - should use sysctl */
@@ -249,18 +250,18 @@
 		syslog(LOG_ERR, "rstat: can't read cnt from kmem");
 		exit(1);
 	}
-	stats_all.s1.v_pgpgin = cnt.v_vnodepgsin;
-	stats_all.s1.v_pgpgout = cnt.v_vnodepgsout;
-	stats_all.s1.v_pswpin = cnt.v_swappgsin;
-	stats_all.s1.v_pswpout = cnt.v_swappgsout;
-	stats_all.s1.v_intr = cnt.v_intr;
+	stats_all.s3.v_pgpgin = cnt.v_vnodepgsin;
+	stats_all.s3.v_pgpgout = cnt.v_vnodepgsout;
+	stats_all.s3.v_pswpin = cnt.v_swappgsin;
+	stats_all.s3.v_pswpout = cnt.v_swappgsout;
+	stats_all.s3.v_intr = cnt.v_intr;
 	gettimeofday(&tm, (struct timezone *) 0);
-	stats_all.s1.v_intr -= hz*(tm.tv_sec - btm.tv_sec) +
+	stats_all.s3.v_intr -= hz*(tm.tv_sec - btm.tv_sec) +
 	    hz*(tm.tv_usec - btm.tv_usec)/1000000;
-	stats_all.s2.v_swtch = cnt.v_swtch;
+	stats_all.s3.v_swtch = cnt.v_swtch;
 
 	/* update disk transfers */
-	updatexfers(RSTAT_DK_NDRIVE, stats_all.s1.dk_xfer);
+	updatexfers(RSTAT_DK_NDRIVE, stats_all.s3.dk_xfer);
 
 	mib[0] = CTL_NET;
 	mib[1] = PF_LINK;
@@ -272,12 +273,11 @@
 		syslog(LOG_ERR, "sysctl(net.link.generic.system.ifcount): %m");
 		exit(1);
 	}
-
-	stats_all.s1.if_ipackets = 0;
-	stats_all.s1.if_opackets = 0;
-	stats_all.s1.if_ierrors = 0;
-	stats_all.s1.if_oerrors = 0;
-	stats_all.s1.if_collisions = 0;
+	stats_all.s3.if_ipackets = 0;
+	stats_all.s3.if_opackets = 0;
+	stats_all.s3.if_ierrors = 0;
+	stats_all.s3.if_oerrors = 0;
+	stats_all.s3.if_collisions = 0;
 	for (i = 1; i <= ifcount; i++) {
 		len = sizeof ifmd;
 		mib[3] = IFMIB_IFDATA;
@@ -287,19 +287,19 @@
 			if (errno == ENOENT)
 				continue;
 
-			syslog(LOG_ERR, "sysctl(net.link.ifdata.%d.general)"
-			       ": %m", i);
+			syslog(LOG_ERR, "sysctl(net.link.ifdata.%d.general): %m", i);
 			exit(1);
 		}
-
-		stats_all.s1.if_ipackets += ifmd.ifmd_data.ifi_ipackets;
-		stats_all.s1.if_opackets += ifmd.ifmd_data.ifi_opackets;
-		stats_all.s1.if_ierrors += ifmd.ifmd_data.ifi_ierrors;
-		stats_all.s1.if_oerrors += ifmd.ifmd_data.ifi_oerrors;
-		stats_all.s1.if_collisions += ifmd.ifmd_data.ifi_collisions;
+		stats_all.s3.if_ipackets += ifmd.ifmd_data.ifi_ipackets;
+		stats_all.s3.if_opackets += ifmd.ifmd_data.ifi_opackets;
+		stats_all.s3.if_ierrors += ifmd.ifmd_data.ifi_ierrors;
+		stats_all.s3.if_oerrors += ifmd.ifmd_data.ifi_oerrors;
+		stats_all.s3.if_collisions += ifmd.ifmd_data.ifi_collisions;
 	}
-	gettimeofday((struct timeval *)&stats_all.s3.curtime,
-		(struct timezone *) 0);
+
+	gettimeofday(&tm, (struct timezone *) 0);
+	stats_all.s3.curtime.tv_sec = tm.tv_sec;
+	stats_all.s3.curtime.tv_usec = tm.tv_usec;
 	alarm(1);
 }
 
@@ -307,12 +307,21 @@
 setup()
 {
 	char errbuf[_POSIX2_LINE_MAX];
-
 	int en;
+	static int is_kd_setup = 0;
 
-	if ((kd = kvm_openfiles(NULL, NULL, NULL, O_RDONLY, errbuf)) == NULL) {
-		syslog(LOG_ERR, "rpc.rstatd, %s", errbuf);
-		exit(1);
+	/*  setup() is called after each dormant->active
+	 *  transition.  Since we never close the kvm files
+	 *  (there's no reason), make sure we don't open them
+	 *  each time, as that can lead to exhaustion of all open
+	 *  files!  */
+	if (!is_kd_setup) {
+		kd = kvm_openfiles(NULL, NULL, NULL, O_RDONLY, errbuf);
+		if (kd == NULL) {
+			syslog(LOG_ERR, "%s", errbuf);
+			exit (1);
+		}
+		is_kd_setup = 1;
 	}
 
 	if ((en = kvm_nlist(kd, nl)) != 0) {
Comment 5 hostmaster 2006-01-30 19:02:14 UTC
	rup is also broken, sigh - patches below to account for
	8-btye timevals


-- 
Bruce Becker			+1 416 410 0879
GTS Network Administration	Toronto, Ont.
Email:	hostmaster@gts.net

 --------- 8< --------- 8< --------- 8< --------- 8< --------- 8< ---------

--- rup.c.orig	Sat May 21 05:55:07 2005
+++ rup.c	Mon Jan 30 13:55:34 2006
@@ -93,6 +93,7 @@
 static bool_t
 rstat_reply(caddr_t replyp, struct sockaddr_in *raddrp)
 {
+	long longtime;
 	struct tm *tmp_time;
 	struct tm host_time;
 	struct tm host_uptime;
@@ -118,12 +119,14 @@
 
 	printf("%-*s\t", HOST_WIDTH, host);
 
-	tmp_time = localtime((time_t *)&host_stat->curtime.tv_sec);
+	longtime = host_stat->curtime.tv_sec;
+	tmp_time = localtime((time_t *)&longtime);
 	host_time = *tmp_time;
 
 	host_stat->curtime.tv_sec -= host_stat->boottime.tv_sec;
 
-	tmp_time = gmtime((time_t *)&host_stat->curtime.tv_sec);
+	longtime = host_stat->curtime.tv_sec;
+	tmp_time = gmtime((time_t *)&longtime);
 	host_uptime = *tmp_time;
 
 	#define updays (host_stat->curtime.tv_sec  / 86400)
Comment 6 Sean McNeil 2006-01-30 19:07:55 UTC
On Jan 30, 2006, at 11:02 AM, hotlips Internet admin wrote:

> --- rup.c.orig	Sat May 21 05:55:07 2005
> +++ rup.c	Mon Jan 30 13:55:34 2006
> @@ -93,6 +93,7 @@
>  static bool_t
>  rstat_reply(caddr_t replyp, struct sockaddr_in *raddrp)
>  {
> +	long longtime;
>  	struct tm *tmp_time;
>  	struct tm host_time;
>  	struct tm host_uptime;
> @@ -118,12 +119,14 @@
>
>  	printf("%-*s\t", HOST_WIDTH, host);
>
> -	tmp_time = localtime((time_t *)&host_stat->curtime.tv_sec);
> +	longtime = host_stat->curtime.tv_sec;
> +	tmp_time = localtime((time_t *)&longtime);
>  	host_time = *tmp_time;
>

Just curious, but why not declare a variable as time_t instead of  
long?  Seems like that would be more correct.

>  	host_stat->curtime.tv_sec -= host_stat->boottime.tv_sec;
>
> -	tmp_time = gmtime((time_t *)&host_stat->curtime.tv_sec);
> +	longtime = host_stat->curtime.tv_sec;
> +	tmp_time = gmtime((time_t *)&longtime);
>  	host_uptime = *tmp_time;
>
>  	#define updays (host_stat->curtime.tv_sec  / 86400)
>
Comment 7 hostmaster 2006-01-30 19:22:52 UTC
	it didn't seem to matter because of the "u_int tv_sec"
	declaration in rstat_timeval in /usr/include/rpcsvc/rstat.h,
	and the surrounding "(time_t *)&" - it should be as you suggest
	however

	in fact the problem mentioned seems to have been noticed
	recently and is fixed somewhat more cleanly in current ...

Cheers,
Bruce Becker			+1 416 410 0879
GTS Network Administration	Toronto, Ont.
Email:	hostmaster@gts.net
Comment 8 Mark Linimon freebsd_committer freebsd_triage 2006-02-21 23:52:43 UTC
Responsible Changed
From-To: freebsd-amd64->freebsd-bugs

This does not sound amd64-specific.
Comment 9 hostmaster 2008-07-08 17:33:34 UTC
Recently kern/122875 bug report provoked a change to libexec/rpc.rstatd
(rather than a still-needed fix to sys/kern/kern_clock.c for RELENG_7 & HEAD) -

Below is an updated patch to re-establish the required fixes
so that it works properly...


Bruce Becker			+1 416 410 0879
GTS Network Administration	Toronto, Ont.
Email:	hostmaster@GTS.Infra-service.CA

  --------- 8< --------- 8< --------- 8< --------- 8< --------- 8< ---------

--- rstat_proc.c.2008062600	2008-06-26 22:20:37.000000000 -0400
+++ rstat_proc.c.2008070600	2008-07-06 15:36:25.000000000 -0400
@@ -125,6 +125,7 @@
      if (! stat_is_init)
          stat_init();
      sincelastreq = 0;
+    stats_all.s2.if_opackets = stats_all.s3.if_opackets;
      return(&stats_all.s2);
  }

@@ -134,6 +135,7 @@
      if (! stat_is_init)
          stat_init();
      sincelastreq = 0;
+    stats_all.s1.if_opackets = stats_all.s3.if_opackets;
      return(&stats_all.s1);
  }

@@ -205,13 +207,13 @@
  		exit(1);
  	}
  	for(i = 0; i < RSTAT_CPUSTATES ; i++)
-		stats_all.s1.cp_time[i] = bsd_cp_time[cp_time_xlat[i]];
+		stats_all.s3.cp_time[i] = bsd_cp_time[cp_time_xlat[i]];

          (void)getloadavg(avrun, sizeof(avrun) / sizeof(avrun[0]));

-	stats_all.s2.avenrun[0] = avrun[0] * FSCALE;
-	stats_all.s2.avenrun[1] = avrun[1] * FSCALE;
-	stats_all.s2.avenrun[2] = avrun[2] * FSCALE;
+	stats_all.s3.avenrun[0] = avrun[0] * FSCALE;
+	stats_all.s3.avenrun[1] = avrun[1] * FSCALE;
+	stats_all.s3.avenrun[2] = avrun[2] * FSCALE;

  	mib[0] = CTL_KERN;
  	mib[1] = KERN_BOOTTIME;
@@ -221,13 +223,13 @@
  		exit(1);
  	}

-	stats_all.s2.boottime.tv_sec = btm.tv_sec;
-	stats_all.s2.boottime.tv_usec = btm.tv_usec;
+	stats_all.s3.boottime.tv_sec = btm.tv_sec;
+	stats_all.s3.boottime.tv_usec = btm.tv_usec;


  #ifdef DEBUG
-	fprintf(stderr, "%d %d %d %d\n", stats_all.s1.cp_time[0],
-	    stats_all.s1.cp_time[1], stats_all.s1.cp_time[2], stats_all.s1.cp_time[3]);
+	fprintf(stderr, "%d %d %d %d\n", stats_all.s3.cp_time[0],
+	    stats_all.s3.cp_time[1], stats_all.s3.cp_time[2], stats_all.s3.cp_time[3]);
  #endif

  #define	FETCH_CNT(stat, cnt) do {					\
@@ -238,12 +240,12 @@
  	}								\
  } while (0)

-	FETCH_CNT(stats_all.s1.v_pgpgin, vm.v_vnodepgsin);
-	FETCH_CNT(stats_all.s1.v_pgpgout, vm.v_vnodepgsout);
-	FETCH_CNT(stats_all.s1.v_pswpin, vm.v_swappgsin);
-	FETCH_CNT(stats_all.s1.v_pswpout, vm.v_swappgsout);
-	FETCH_CNT(stats_all.s1.v_intr, sys.v_intr);
-	FETCH_CNT(stats_all.s2.v_swtch, sys.v_swtch);
+	FETCH_CNT(stats_all.s3.v_pgpgin, vm.v_vnodepgsin);
+	FETCH_CNT(stats_all.s3.v_pgpgout, vm.v_vnodepgsout);
+	FETCH_CNT(stats_all.s3.v_pswpin, vm.v_swappgsin);
+	FETCH_CNT(stats_all.s3.v_pswpout, vm.v_swappgsout);
+	FETCH_CNT(stats_all.s3.v_intr, sys.v_intr);
+	FETCH_CNT(stats_all.s3.v_swtch, sys.v_swtch);
  	gettimeofday(&tm, (struct timezone *) 0);
  	stats_all.s1.v_intr -= hz*(tm.tv_sec - btm.tv_sec) +
  	    hz*(tm.tv_usec - btm.tv_usec)/1000000;
@@ -262,11 +264,11 @@
  		exit(1);
  	}

-	stats_all.s1.if_ipackets = 0;
-	stats_all.s1.if_opackets = 0;
-	stats_all.s1.if_ierrors = 0;
-	stats_all.s1.if_oerrors = 0;
-	stats_all.s1.if_collisions = 0;
+	stats_all.s3.if_ipackets = 0;
+	stats_all.s3.if_opackets = 0;
+	stats_all.s3.if_ierrors = 0;
+	stats_all.s3.if_oerrors = 0;
+	stats_all.s3.if_collisions = 0;
  	for (i = 1; i <= ifcount; i++) {
  		len = sizeof ifmd;
  		mib[3] = IFMIB_IFDATA;
@@ -281,11 +283,11 @@
  			exit(1);
  		}

-		stats_all.s1.if_ipackets += ifmd.ifmd_data.ifi_ipackets;
-		stats_all.s1.if_opackets += ifmd.ifmd_data.ifi_opackets;
-		stats_all.s1.if_ierrors += ifmd.ifmd_data.ifi_ierrors;
-		stats_all.s1.if_oerrors += ifmd.ifmd_data.ifi_oerrors;
-		stats_all.s1.if_collisions += ifmd.ifmd_data.ifi_collisions;
+		stats_all.s3.if_ipackets += ifmd.ifmd_data.ifi_ipackets;
+		stats_all.s3.if_opackets += ifmd.ifmd_data.ifi_opackets;
+		stats_all.s3.if_ierrors += ifmd.ifmd_data.ifi_ierrors;
+		stats_all.s3.if_oerrors += ifmd.ifmd_data.ifi_oerrors;
+		stats_all.s3.if_collisions += ifmd.ifmd_data.ifi_collisions;
  	}
  	gettimeofday((struct timeval *)&stats_all.s3.curtime,
  		(struct timezone *) 0);
Comment 10 G. Paul Ziemba 2013-04-14 09:30:06 UTC
Attached is an updated patch for 64-bit systems. I hope someone
can apply it to HEAD - fix has been waiting for some years. Please?

-- 
G. Paul Ziemba
FreeBSD unix:
 1:21AM  up  1:41, 1 user, load averages: 0.76, 0.89, 0.90
Comment 11 Bruce Becker 2013-04-14 16:18:41 UTC
On Sun, 14 Apr 2013, G. Paul Ziemba wrote:

|Attached is an updated patch for 64-bit systems. I hope someone
|can apply it to HEAD - fix has been waiting for some years. Please?


	here's what i use, which saves a call to
	gettimeofday() and gives slightly more
	accurate valuse for v_intr


Bruce Becker			+1 416 410 0879
GTS Network Administration	Toronto, Ont.
Email:	hostmaster@GTS.Infra-service.CA
Comment 12 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 07:59:34 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped