diff mbox

[net] net: rename and move busy poll mib counter

Message ID 20130806095207.5887.2196.stgit@ladj378.jer.intel.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Eliezer Tamir Aug. 6, 2013, 9:52 a.m. UTC
Move the low latency mib counter to the ip section.
Rename it from low latency to busy poll.

Reported-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
Signed-off-by: Eliezer Tamir <eliezer.tamir@linux.intel.com>
---

 include/net/busy_poll.h   |    4 ++--
 include/uapi/linux/snmp.h |    2 +-
 net/ipv4/proc.c           |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)


--
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

Comments

Eric Dumazet Aug. 6, 2013, 10:14 a.m. UTC | #1
On Tue, 2013-08-06 at 12:52 +0300, Eliezer Tamir wrote:
> Move the low latency mib counter to the ip section.
> Rename it from low latency to busy poll.
> 
> Reported-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
> Signed-off-by: Eliezer Tamir <eliezer.tamir@linux.intel.com>
> ---

Well, it should not be part of IP mib, but a socket one (not existing so
far)

Linux MIB already contains few non TCP counters :

LINUX_MIB_ARPFILTER
LINUX_MIB_IPRPFILTER

Its mostly populated by TCP counters, sure.


--
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
Eliezer Tamir Aug. 6, 2013, 10:23 a.m. UTC | #2
On 06/08/2013 13:14, Eric Dumazet wrote:
> On Tue, 2013-08-06 at 12:52 +0300, Eliezer Tamir wrote:
>> Move the low latency mib counter to the ip section.
>> Rename it from low latency to busy poll.
>>
>> Reported-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
>> Signed-off-by: Eliezer Tamir <eliezer.tamir@linux.intel.com>
>> ---
> 
> Well, it should not be part of IP mib, but a socket one (not existing so
> far)
> 
> Linux MIB already contains few non TCP counters :
> 
> LINUX_MIB_ARPFILTER
> LINUX_MIB_IPRPFILTER
> 
> Its mostly populated by TCP counters, sure.

So, just rename it to busy poll?
Or maybe just drop the whole patch?
--
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
Eric Dumazet Aug. 6, 2013, 10:33 a.m. UTC | #3
On Tue, 2013-08-06 at 13:23 +0300, Eliezer Tamir wrote:

> So, just rename it to busy poll?
> Or maybe just drop the whole patch?

I guess a rename would be fine.


--
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
Shawn Bohrer Aug. 6, 2013, 6:18 p.m. UTC | #4
On Tue, Aug 06, 2013 at 03:14:48AM -0700, Eric Dumazet wrote:
> On Tue, 2013-08-06 at 12:52 +0300, Eliezer Tamir wrote:
> > Move the low latency mib counter to the ip section.
> > Rename it from low latency to busy poll.
> > 
> > Reported-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
> > Signed-off-by: Eliezer Tamir <eliezer.tamir@linux.intel.com>
> > ---
> 
> Well, it should not be part of IP mib, but a socket one (not existing so
> far)
> 
> Linux MIB already contains few non TCP counters :
> 
> LINUX_MIB_ARPFILTER
> LINUX_MIB_IPRPFILTER

Doesn't mean they are in the correct place either, but perhaps it's too
late for them.

> Its mostly populated by TCP counters, sure.

See, on the kernel side these are called "LINUX_MIB*" which seems
perfectly sane and I wouldn't even think the statistic is out of
place.  On the user-mode side these are all reported in
/proc/net/netstat as TcpExt statistics.  I can tell you that I don't
look at TCP statistics when I'm debugging/testing UDP issues
(apparently I should).

--
Shawn
diff mbox

Patch

diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index 8e2dfc1..1b48741 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -121,8 +121,8 @@  static inline bool sk_busy_loop(struct sock *sk, int nonblock)
 
 		if (rc > 0)
 			/* local bh are disabled so it is ok to use _BH */
-			NET_ADD_STATS_BH(sock_net(sk),
-					 LINUX_MIB_LOWLATENCYRXPACKETS, rc);
+			IP_ADD_STATS_BH(dev_net(napi->dev),
+					IPSTATS_MIB_BUSYPOLLRXPACKETS, rc);
 
 	} while (!nonblock && skb_queue_empty(&sk->sk_receive_queue) &&
 		 !need_resched() && !busy_loop_timeout(end_time));
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index af0a674..5575d9b 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -51,6 +51,7 @@  enum
 	IPSTATS_MIB_INBCASTOCTETS,		/* InBcastOctets */
 	IPSTATS_MIB_OUTBCASTOCTETS,		/* OutBcastOctets */
 	IPSTATS_MIB_CSUMERRORS,			/* InCsumErrors */
+	IPSTATS_MIB_BUSYPOLLRXPACKETS,		/* BusyPollRxPackets */
 	__IPSTATS_MIB_MAX
 };
 
@@ -253,7 +254,6 @@  enum
 	LINUX_MIB_TCPFASTOPENLISTENOVERFLOW,	/* TCPFastOpenListenOverflow */
 	LINUX_MIB_TCPFASTOPENCOOKIEREQD,	/* TCPFastOpenCookieReqd */
 	LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES, /* TCPSpuriousRtxHostQueues */
-	LINUX_MIB_LOWLATENCYRXPACKETS,		/* LowLatencyRxPackets */
 	__LINUX_MIB_MAX
 };
 
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 6577a11..3c54a63 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -108,6 +108,7 @@  static const struct snmp_mib snmp4_ipstats_list[] = {
 	SNMP_MIB_ITEM("FragOKs", IPSTATS_MIB_FRAGOKS),
 	SNMP_MIB_ITEM("FragFails", IPSTATS_MIB_FRAGFAILS),
 	SNMP_MIB_ITEM("FragCreates", IPSTATS_MIB_FRAGCREATES),
+	SNMP_MIB_ITEM("BusyPollRxPackets", IPSTATS_MIB_BUSYPOLLRXPACKETS),
 	SNMP_MIB_SENTINEL
 };
 
@@ -273,7 +274,6 @@  static const struct snmp_mib snmp4_net_list[] = {
 	SNMP_MIB_ITEM("TCPFastOpenListenOverflow", LINUX_MIB_TCPFASTOPENLISTENOVERFLOW),
 	SNMP_MIB_ITEM("TCPFastOpenCookieReqd", LINUX_MIB_TCPFASTOPENCOOKIEREQD),
 	SNMP_MIB_ITEM("TCPSpuriousRtxHostQueues", LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES),
-	SNMP_MIB_ITEM("LowLatencyRxPackets", LINUX_MIB_LOWLATENCYRXPACKETS),
 	SNMP_MIB_SENTINEL
 };