These patches keep getsockopt(PACKET_STATISTICS) from resetting the packet
stats to zero and it creates PACKET_RESET_STATISTICS, which is used with
setsockopt(), to zero the packet stats.
Signed-off by: Kris Katterjohn <[email protected]>
---
Before I start: This is a diff from 2.6.14 and I am not subscribed so please CC
me on any replies.
I doubt I'm the only one who's a little annoyed that we have to keep track of the
packet count in userland after getsockopt(PACKET_STATISTICS). I did a little
searching but didn't find a patch similar to this so I'm not sure if I'm the only
one who thinks this is a generally Good Thing or not.
The ways these affect programs:
1) Programs can set the packet count to zero at anytime without having to make a
struct tpacket_stats just to reset it with getsockopt()
2) Programs don't need extra variables to hold packet counts (think libpcap),
they just call getsockopt(PACKET_STATISTICS) multiple times for updated stats.
NOTE:
The second patch adds PACKET_RESET_STATISTICS to include/linux/if_packet.h, but I
don't know if it's okay to #define it as "7" since that was PACKET_COPY_THRESH's
value. I did it so that the two PACKET*STATISTICS macros would be next to each
other. I checked the glibc header netpacket/packet.h and it doesn't even define
PACKET_COPY_THRESH so it shouldn't cause any problems (that I can see).
Thanks!
---
--- x/net/packet/af_packet.c 2005-10-27 19:02:08.000000000 -0500
+++ y/net/packet/af_packet.c 2005-11-28 21:49:37.000000000 -0600
@@ -41,6 +41,9 @@
* will simply extend the hardware address
* byte arrays at the end of sockaddr_ll
* and packet_mreq.
+ * Kris Katterjohn : Added PACKET_RESET_STATISTICS setsockopt
+ * option. PACKET_STATISTICS no longer sets
+ * stats to zero. 2005-11-28
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -1352,6 +1355,17 @@ packet_setsockopt(struct socket *sock, i
return ret;
}
#endif
+
+ case PACKET_RESET_STATISTICS:
+ {
+ struct packet_sock *po = pkt_sk(sk);
+
+ spin_lock_bh(&sk->sk_receive_queue.lock);
+ memset(&po->stats, 0, sizeof(po->stats));
+ spin_unlock_bh(&sk->sk_receive_queue.lock);
+ return 0;
+ }
+
#ifdef CONFIG_PACKET_MMAP
case PACKET_RX_RING:
{
@@ -1406,7 +1420,6 @@ static int packet_getsockopt(struct sock
len = sizeof(struct tpacket_stats);
spin_lock_bh(&sk->sk_receive_queue.lock);
st = po->stats;
- memset(&po->stats, 0, sizeof(st));
spin_unlock_bh(&sk->sk_receive_queue.lock);
st.tp_packets += st.tp_drops;
--- x/include/linux/if_packet.h 2005-10-27 19:02:08.000000000 -0500
+++ y/include/linux/if_packet.h 2005-11-28 21:49:37.000000000 -0600
@@ -38,7 +38,8 @@ struct sockaddr_ll
/* Value 4 is still used by obsolete turbo-packet. */
#define PACKET_RX_RING 5
#define PACKET_STATISTICS 6
-#define PACKET_COPY_THRESH 7
+#define PACKET_RESET_STATISTICS 7
+#define PACKET_COPY_THRESH 8
struct tpacket_stats
{
From: "Kris Katterjohn" <[email protected]>
Date: Mon, 28 Nov 2005 19:55:52 -0800
> These patches keep getsockopt(PACKET_STATISTICS) from resetting the packet
> stats to zero and it creates PACKET_RESET_STATISTICS, which is used with
> setsockopt(), to zero the packet stats.
>
> Signed-off by: Kris Katterjohn <[email protected]>
You can't change existing behavior in order to get the new behavior
you want. Some applications might be depending upon what happens
currently.
From: David S. Miller
Sent: 11/28/2005 10:12:38 PM
> You can't change existing behavior in order to get the new behavior
> you want. Some applications might be depending upon what happens
> currently.
Is there a way to work around this? It seems to me this is a better way of doing it
because it doesn't require extra variables on the user's end and the stats are only
changed if you explicitly ask for it. Do you agree with that?
Kris
From: "Kris Katterjohn" <[email protected]>
Date: Mon, 28 Nov 2005 22:28:18 -0800
> From: David S. Miller
> Sent: 11/28/2005 10:12:38 PM
> > You can't change existing behavior in order to get the new behavior
> > you want. Some applications might be depending upon what happens
> > currently.
>
> Is there a way to work around this?
Why not create a new call that doesn't reset the statistic,
and change your application to invoke it instead of the call
which exists?
That's one backwards compatible way to fix things like this.
Or, you can create a new call that says "from now on, do not
reset statistics on calls using this socket." That's another
clean, and backwards compatible way, to deal with this
kind of issue.
From: David S. Miller
Sent: 11/28/2005 11:02:24 PM
> Or, you can create a new call that says "from now on, do not
> reset statistics on calls using this socket." That's another
> clean, and backwards compatible way, to deal with this
> kind of issue.
I like that one. Here are the updated patches and descriptions:
PACKET_AUTO_STATISTICS (default) lets the kernel reset the stats with each call
of PACKET_STATISTICS and PACKET_MANUAL_STATISTICS won't reset them unless
PACKET_RESET_STATISTICS is called. AUTO and MANUAL can be called multiple times
to change the "state" if necessary.
I didn't check for auto_reset_stats under PACKET_RESET_STATISTICS so it can still
be used to reset the stats while in "auto mode".
Also, just to be sure, is changing PACKET_COPY_THRESH's value in if_packet.h
okay?
Thanks a lot!
--- x/net/packet/af_packet.c 2005-10-27 19:02:08.000000000 -0500
+++ y/net/packet/af_packet.c 2005-11-29 03:02:19.000000000 -0600
@@ -41,6 +41,12 @@
* will simply extend the hardware address
* byte arrays at the end of sockaddr_ll
* and packet_mreq.
+ * Kris Katterjohn : Added setsockopt options:
+ * PACKET_AUTO_STATISTICS,
+ * PACKET_MANUAL_STATISTICS, and
+ * PACKET_RESET_STATISTICS to handle the
+ * zero-ing of packet stats when using
+ * PACKET_STATISTICS. 2005-11-29.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -189,6 +195,7 @@ struct packet_sock {
/* struct sock has to be the first member of packet_sock */
struct sock sk;
struct tpacket_stats stats;
+ int auto_reset_stats;
#ifdef CONFIG_PACKET_MMAP
char * *pg_vec;
unsigned int head;
@@ -1020,6 +1027,7 @@ static int packet_create(struct socket *
po = pkt_sk(sk);
sk->sk_family = PF_PACKET;
po->num = protocol;
+ po->auto_reset_stats = 1;
sk->sk_destruct = packet_sock_destruct;
atomic_inc(&packet_socks_nr);
@@ -1324,6 +1332,7 @@ static int
packet_setsockopt(struct socket *sock, int level, int optname, char __user *optval, int optlen)
{
struct sock *sk = sock->sk;
+ struct packet_sock *po = pkt_sk(sk);
int ret;
if (level != SOL_PACKET)
@@ -1352,6 +1361,21 @@ packet_setsockopt(struct socket *sock, i
return ret;
}
#endif
+
+ case PACKET_AUTO_STATISTICS:
+ po->auto_reset_stats = 1;
+ return 0;
+
+ case PACKET_MANUAL_STATISTICS:
+ po->auto_reset_stats = 0;
+ return 0;
+
+ case PACKET_RESET_STATISTICS:
+ spin_lock_bh(&sk->sk_receive_queue.lock);
+ memset(&po->stats, 0, sizeof(po->stats));
+ spin_unlock_bh(&sk->sk_receive_queue.lock);
+ return 0;
+
#ifdef CONFIG_PACKET_MMAP
case PACKET_RX_RING:
{
@@ -1406,7 +1430,8 @@ static int packet_getsockopt(struct sock
len = sizeof(struct tpacket_stats);
spin_lock_bh(&sk->sk_receive_queue.lock);
st = po->stats;
- memset(&po->stats, 0, sizeof(st));
+ if (po->auto_reset_stats)
+ memset(&po->stats, 0, sizeof(po->stats));
spin_unlock_bh(&sk->sk_receive_queue.lock);
st.tp_packets += st.tp_drops;
--- x/include/linux/if_packet.h 2005-10-27 19:02:08.000000000 -0500
+++ y/include/linux/if_packet.h 2005-11-29 03:02:19.000000000 -0600
@@ -38,7 +38,10 @@ struct sockaddr_ll
/* Value 4 is still used by obsolete turbo-packet. */
#define PACKET_RX_RING 5
#define PACKET_STATISTICS 6
-#define PACKET_COPY_THRESH 7
+#define PACKET_AUTO_STATISTICS 7
+#define PACKET_MANUAL_STATISTICS 8
+#define PACKET_RESET_STATISTICS 9
+#define PACKET_COPY_THRESH 10
struct tpacket_stats
{