Bug 182828 - [patch] [igb] Revision 247430 broke outgoing interface stats for stable/8
Summary: [patch] [igb] Revision 247430 broke outgoing interface stats for stable/8
Status: Closed Overcome By Events
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 8.4-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Eric Joyner
URL:
Keywords: IntelNetworking
Depends on:
Blocks:
 
Reported: 2013-10-08 13:40 UTC by Eugene Grosbein
Modified: 2015-11-12 17:33 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Eugene Grosbein 2013-10-08 13:40:00 UTC
	There was r241037 commit by glebius@ to head fixing
	"inaccurate overrated if_obytes accounting" in the HEAD:
	http://svnweb.freebsd.org/base?view=revision&revision=241037

	And there was _incomplete_ MFC to stable/8 r247430:
	http://svnweb.freebsd.org/base?view=revision&revision=247430

	r247430 contained some parts of r241037 but missed others vital
	parts, so it introduced "inaccurate overrated if_obytes accounting"
	to stable/8. For example, igb(4) now suffers from same problem.

Fix: 

Make mentioned MFC more complete. At least, merge
	changes to sys/net/if_var.h to delete extra increase of if_obytes
	and to sys/dev/oce/oce_if.c to fix last consumer
	of drbr_stats_update() in stable/8:

--- sys/net/if_var.h	2012/09/28 17:36:00	241036
+++ sys/net/if_var.h	2012/09/28 18:28:27	241037
@@ -590,22 +590,10 @@
 } while (0)
 
 #ifdef _KERNEL
-static __inline void
-drbr_stats_update(struct ifnet *ifp, int len, int mflags)
-{
-#ifndef NO_SLOW_STATS
-	ifp->if_obytes += len;
-	if (mflags & M_MCAST)
-		ifp->if_omcasts++;
-#endif
-}
-
 static __inline int
 drbr_enqueue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m)
 {	
 	int error = 0;
-	int len = m->m_pkthdr.len;
-	int mflags = m->m_flags;
 
 #ifdef ALTQ
 	if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
@@ -613,12 +601,10 @@
 		return (error);
 	}
 #endif
-	if ((error = buf_ring_enqueue_bytes(br, m, len)) == ENOBUFS) {
-		br->br_drops++;
+	error = buf_ring_enqueue(br, m);
+	if (error)
 		m_freem(m);
-	} else
-		drbr_stats_update(ifp, len, mflags);
-	
+
 	return (error);
 }
 
--- sys/dev/oce/oce_if.c	2012/09/28 17:36:00	241036
+++ sys/dev/oce/oce_if.c	2012/09/28 18:28:27	241037
@@ -1183,7 +1183,9 @@
 			}  
 			break;
 		}
-		drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags);
+		ifp->if_obytes += next->m_pkthdr.len;
+		if (next->m_flags & M_MCAST)
+			ifp->if_omcasts++;
 		ETHER_BPF_MTAP(ifp, next);
 		next = drbr_dequeue(ifp, br);
 	}

Eugene Grosbein
How-To-Repeat: 	Run recent 8.4-STABLE with igb(4), make lots of outgoing traffic
	and see that interface statistics is overrated.
Comment 1 Gleb Smirnoff freebsd_committer 2013-10-15 17:51:03 UTC
State Changed
From-To: open->closed

The problem isn't present in supported FreeBSD branches, sorry.
Comment 2 Adrian Chadd freebsd_committer 2013-10-21 07:10:48 UTC
State Changed
From-To: closed->open

Re-open; the issue was due to jfv's merge. 



Comment 3 Adrian Chadd freebsd_committer 2013-10-21 07:10:48 UTC
Responsible Changed
From-To: freebsd-bugs->jfv

Re-open; the issue was due to jfv's merge.
Comment 4 Andrew Kinney 2013-11-07 00:23:17 UTC
I'm also seeing this problem in 8.4-RELEASE installed Oct. 28, 2013. 
Stock kernel. Not custom. 8.4-RELEASE is a supported FreeBSD branch.

[root@redact ~]# uname -a
FreeBSD redact.redact.com 8.4-RELEASE-p4 FreeBSD 8.4-RELEASE-p4 #0: Mon 
Sep  9 23:16:13 UTC 2013 
root@amd64-builder.daemonology.net:/usr/obj/usr/src/sys/GENERIC  amd64

Symptoms visible on a box running bridging and IPFW. Outbound bytes is 
usually about double inbound bytes on the opposite interface. Packet 
counts are about equal. Very visible when there is significant traffic 
graphed via snmpd and cacti.

Sincerely,
Andrew Kinney
Comment 5 Mark Linimon freebsd_committer freebsd_triage 2015-11-12 07:45:53 UTC
Reassign to erj@ for triage.  To submitter: is this issue still relevant?
Comment 6 Eugene Grosbein 2015-11-12 09:04:47 UTC
(In reply to Mark Linimon from comment #5)

FreeBSD 8 reached its EOL since when PR was filled. FreeBSD 9+ does not seem to have the problem. Perhaps, this PR should be closed leaving igb(8) with somewhat broken stats for 8.4-STABLE.
Comment 7 Eric Joyner freebsd_committer 2015-11-12 17:33:20 UTC
It's regrettable, but that sounds like the right thing to do.