IPV4: add raw drops counter

PATCH In review Wang Chen 3 participants 11 messages Nov 13, 2007 – Nov 14, 2007

Subsystems (3)

Networking Network drivers Sockets

lore

Share & tools

lore

Wang Chen

author

2007-11-13 09:59 UTC #0 lore raw

PATCH 1/1 IPV4: add raw drops counter

Add raw drops counter for IPv4 in /proc/net/raw .

Signed-off-by Wang Chen <[email protected]>

---
 include/net/sock.h |    2 ++
 net/core/sock.c    |    1 +
 net/ipv4/raw.c     |   16 +++++++++-------
 3 files changed, 12 insertions(+), 7 deletions(-)

include/net/sock.h +2 -0

diff -Nurp linux-2.6.24-rc2.org/include/net/sock.h linux-2.6.24-rc2/include/net/sock.h
@@ -156,6 +156,7 @@ struct sock_common {
   *	@sk_prot_creator: sk_prot of original sock creator (see ipv6_setsockopt, IPV6_ADDRFORM for instance)
   *	@sk_err: last error
   *	@sk_err_soft: errors that don't cause failure but are the cause of a persistent failure not just 'timed out'
+  *	@sk_drops: raw drops counter
   *	@sk_ack_backlog: current listen backlog
   *	@sk_max_ack_backlog: listen backlog set in listen()
   *	@sk_priority: %SO_PRIORITY setting

@@ -239,6 +240,7 @@ struct sock {
 	rwlock_t		sk_callback_lock;
 	int			sk_err,
 				sk_err_soft;
+	atomic_t		sk_drops;
 	unsigned short		sk_ack_backlog;
 	unsigned short		sk_max_ack_backlog;
 	__u32			sk_priority;

net/core/sock.c +1 -0

diff -Nurp linux-2.6.24-rc2.org/net/core/sock.c linux-2.6.24-rc2/net/core/sock.c
@@ -1611,6 +1611,7 @@ void sock_init_data(struct socket *sock,
 	sk->sk_stamp = ktime_set(-1L, -1L);
 
 	atomic_set(&sk->sk_refcnt, 1);
+	atomic_set(&sk->sk_drops, 0);
 }
 
 void fastcall lock_sock_nested(struct sock *sk, int subclass)

net/ipv4/raw.c +9 -7

diff -Nurp linux-2.6.24-rc2.org/net/ipv4/raw.c linux-2.6.24-rc2/net/ipv4/raw.c
@@ -241,7 +241,7 @@ static int raw_rcv_skb(struct sock * sk,
 	/* Charge it to the socket. */
 
 	if (sock_queue_rcv_skb(sk, skb) < 0) {
-		/* FIXME: increment a raw drops counter here */
+		atomic_inc(&sk->sk_drops);	
 		kfree_skb(skb);
 		return NET_RX_DROP;
 	}

@@ -866,28 +866,30 @@ static __inline__ char *get_raw_sock(str
 	      srcp  = inet->num;
 
 	sprintf(tmpbuf, "%4d: %08X:%04X %08X:%04X"
-		" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p",
+		" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d",
 		i, src, srcp, dest, destp, sp->sk_state,
 		atomic_read(&sp->sk_wmem_alloc),
 		atomic_read(&sp->sk_rmem_alloc),
 		0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
-		atomic_read(&sp->sk_refcnt), sp);
+		atomic_read(&sp->sk_refcnt), sp, atomic_read(&sp->sk_drops));
 	return tmpbuf;
 }
 
+#define TMPSZ 128
+
 static int raw_seq_show(struct seq_file *seq, void *v)
 {
-	char tmpbuf[129];
+	char tmpbuf[TMPSZ+1];
 
 	if (v == SEQ_START_TOKEN)
-		seq_printf(seq, "%-127s\n",
+		seq_printf(seq, "%-*s\n", TMPSZ-1,
 			       "  sl  local_address rem_address   st tx_queue "
 			       "rx_queue tr tm->when retrnsmt   uid  timeout "
-			       "inode");
+			       "inode  drops");
 	else {
 		struct raw_iter_state *state = raw_seq_private(seq);
 
-		seq_printf(seq, "%-127s\n",
+		seq_printf(seq, "%-*s\n", TMPSZ-1,
 			   get_raw_sock(v, tmpbuf, state->bucket));
 	}
 	return 0;

Eric Dumazet

2007-11-13 11:11 UTC #1 lore raw

Quoted 21 lines · Wang Chen

Add raw drops counter for IPv4 in /proc/net/raw .

Signed-off-by Wang Chen <[email protected]>

---
 include/net/sock.h |    2 ++
 net/core/sock.c    |    1 +
 net/ipv4/raw.c     |   16 +++++++++-------
 3 files changed, 12 insertions(+), 7 deletions(-)

diff -Nurp linux-2.6.24-rc2.org/include/net/sock.h linux-2.6.24-rc2/include/net/sock.h

include/net/sock.h +1 -0

@@ -156,6 +156,7 @@ struct sock_common {
   *	@sk_prot_creator: sk_prot of original sock creator (see ipv6_setsockopt, IPV6_ADDRFORM for instance)
   *	@sk_err: last error
   *	@sk_err_soft: errors that don't cause failure but are the cause of a persistent failure not just 'timed out'
+  *	@sk_drops: raw drops counter
   *	@sk_ack_backlog: current listen backlog
   *	@sk_max_ack_backlog: listen backlog set in listen()
   *	@sk_priority: %SO_PRIORITY setting

include/net/sock.h struct sock { · line 244

Quoted 5 lines

include/net/sock.h +1 -0

@@ -239,6 +240,7 @@ struct sock {
 	rwlock_t		sk_callback_lock;
 	int			sk_err,
 				sk_err_soft;
+	atomic_t		sk_drops;

This doesnt need an atomic_t , just an 'unsigned int' is OK, since sock_queue_rcv_skb() is called on a locked socket.

Also, I suggest doing the sk_drops increment in sock_queue_rcv_skb() so that it can be used for other sockets as well ?

I like having this counter per socket, but only if an application can retrieve its value with a getsockopt() call, dont you think ?

getsockopt(sock, SOL_SOCKET, SO_DROPSCNT, &val, &vallen);

Wang Chen

author

2007-11-14 01:29 UTC #2 lore raw

Eric Dumazet said the following on 2007-11-13 19:11:

Quoted 8 lines

Quoted 4 lines · Wang Chen
Add raw drops counter for IPv4 in /proc/net/raw .
+ atomic_t sk_drops;
This doesnt need an atomic_t , just an 'unsigned int' is OK, since sock_queue_rcv_skb() is called on a locked socket.

Yes, sock_queue_rcv_skb() is called on a locked socket. But sk_drops will not only used with sock_queue_rcv_skb(), but also with xfrm4_policy_check(), skb_checksum_complete(), skb_kill_datagram(),etc. So, atomic_t ensure sk_drops will be atomic increment.

Quoted 3 lines

Also, I suggest doing the sk_drops increment in sock_queue_rcv_skb() so that it can be used for other sockets as well ?

As I described before, sk_drops will be used on different conditions, on which the raw drop happens.
So doing sk_drops increment in upper caller is better than in sock_queue_rcv_skb().

Thank you for your suggestion, I will make a new patch to add sk_drops increment in other places.

Quoted 6 lines

I like having this counter per socket, but only if an application can retrieve its value with a getsockopt() call, dont you think ?
getsockopt(sock, SOL_SOCKET, SO_DROPSCNT, &val, &vallen);

Yes. I agree.
How do other people think about it?

Eric Dumazet

2007-11-14 06:01 UTC #3 lore raw

Quoted 18 lines · Wang Chen

Eric Dumazet said the following on 2007-11-13 19:11:
Quoted 8 lines
Quoted 4 lines · Wang Chen
Add raw drops counter for IPv4 in /proc/net/raw .
+ atomic_t sk_drops;
This doesnt need an atomic_t , just an 'unsigned int' is OK, since sock_queue_rcv_skb() is called on a locked socket.
Yes, sock_queue_rcv_skb() is called on a locked socket. But sk_drops
will not only used with sock_queue_rcv_skb(), but also with xfrm4_policy_check(), skb_checksum_complete(), skb_kill_datagram(),etc. So, atomic_t ensure sk_drops will be atomic increment.
Quoted 3 lines
Also, I suggest doing the sk_drops increment in sock_queue_rcv_skb() so that it can be used for other sockets as well ?

Quoted 7 lines

As I described before, sk_drops will be used on different conditions, on which the raw drop happens.
So doing sk_drops increment in upper caller is better than in sock_queue_rcv_skb().
Thank you for your suggestion, I will make a new patch to add sk_drops increment in other places.

Thank you for clarifications, I was not aware upcoming patches were planed.

Wang Chen

author

2007-11-14 03:14 UTC #4 lore raw

PATCH 1/2 SOCK: add raw drops counter

In reply to Wang Chen

Add raw drops counter for IPv4 in /proc/net/raw .

Signed-off-by Wang Chen <[email protected]>

---
 include/net/sock.h |   11 ++++++++---
 net/core/sock.c    |    1 +
 net/ipv4/raw.c     |   17 ++++++++++-------
 3 files changed, 19 insertions(+), 10 deletions(-)

include/net/sock.h +8 -3

diff -Nurp linux-2.6.24-rc2.org/include/net/sock.h linux-2.6.24-rc2/include/net/sock.h
@@ -145,7 +145,8 @@ struct sock_common {
   *	@sk_forward_alloc: space allocated forward
   *	@sk_allocation: allocation mode
   *	@sk_sndbuf: size of send buffer in bytes
-  *	@sk_flags: %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE, %SO_OOBINLINE settings
+  *	@sk_flags: %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE, 
+  *		   %SO_OOBINLINE settings
   *	@sk_no_check: %SO_NO_CHECK setting, wether or not checkup packets
   *	@sk_route_caps: route capabilities (e.g. %NETIF_F_TSO)
   *	@sk_gso_type: GSO type (e.g. %SKB_GSO_TCPV4)

@@ -153,9 +154,12 @@ struct sock_common {
   *	@sk_backlog: always used with the per-socket spinlock held
   *	@sk_callback_lock: used with the callbacks in the end of this struct
   *	@sk_error_queue: rarely used
-  *	@sk_prot_creator: sk_prot of original sock creator (see ipv6_setsockopt, IPV6_ADDRFORM for instance)
+  *	@sk_prot_creator: sk_prot of original sock creator (see ipv6_setsockopt, 
+  *			  IPV6_ADDRFORM for instance)
   *	@sk_err: last error
-  *	@sk_err_soft: errors that don't cause failure but are the cause of a persistent failure not just 'timed out'
+  *	@sk_err_soft: errors that don't cause failure but are the cause of a 
+  *		      persistent failure not just 'timed out'
+  *	@sk_drops: raw drops counter
   *	@sk_ack_backlog: current listen backlog
   *	@sk_max_ack_backlog: listen backlog set in listen()
   *	@sk_priority: %SO_PRIORITY setting

@@ -239,6 +243,7 @@ struct sock {
 	rwlock_t		sk_callback_lock;
 	int			sk_err,
 				sk_err_soft;
+	atomic_t		sk_drops;
 	unsigned short		sk_ack_backlog;
 	unsigned short		sk_max_ack_backlog;
 	__u32			sk_priority;

net/core/sock.c +1 -0

diff -Nurp linux-2.6.24-rc2.org/net/core/sock.c linux-2.6.24-rc2/net/core/sock.c
@@ -1611,6 +1611,7 @@ void sock_init_data(struct socket *sock,
 	sk->sk_stamp = ktime_set(-1L, -1L);
 
 	atomic_set(&sk->sk_refcnt, 1);
+	atomic_set(&sk->sk_drops, 0);
 }
 
 void fastcall lock_sock_nested(struct sock *sk, int subclass)

net/ipv4/raw.c +10 -7

diff -Nurp linux-2.6.24-rc2.org/net/ipv4/raw.c linux-2.6.24-rc2/net/ipv4/raw.c
@@ -241,7 +241,7 @@ static int raw_rcv_skb(struct sock * sk,
 	/* Charge it to the socket. */
 
 	if (sock_queue_rcv_skb(sk, skb) < 0) {
-		/* FIXME: increment a raw drops counter here */
+		atomic_inc(&sk->sk_drops);	
 		kfree_skb(skb);
 		return NET_RX_DROP;
 	}

@@ -252,6 +252,7 @@ static int raw_rcv_skb(struct sock * sk,
 int raw_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb)) {
+		atomic_inc(&sk->sk_drops);
 		kfree_skb(skb);
 		return NET_RX_DROP;
 	}

@@ -866,28 +867,30 @@ static __inline__ char *get_raw_sock(str
 	      srcp  = inet->num;
 
 	sprintf(tmpbuf, "%4d: %08X:%04X %08X:%04X"
-		" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p",
+		" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d",
 		i, src, srcp, dest, destp, sp->sk_state,
 		atomic_read(&sp->sk_wmem_alloc),
 		atomic_read(&sp->sk_rmem_alloc),
 		0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
-		atomic_read(&sp->sk_refcnt), sp);
+		atomic_read(&sp->sk_refcnt), sp, atomic_read(&sp->sk_drops));
 	return tmpbuf;
 }
 
+#define TMPSZ 128
+
 static int raw_seq_show(struct seq_file *seq, void *v)
 {
-	char tmpbuf[129];
+	char tmpbuf[TMPSZ+1];
 
 	if (v == SEQ_START_TOKEN)
-		seq_printf(seq, "%-127s\n",
+		seq_printf(seq, "%-*s\n", TMPSZ-1,
 			       "  sl  local_address rem_address   st tx_queue "
 			       "rx_queue tr tm->when retrnsmt   uid  timeout "
-			       "inode");
+			       "inode  drops");
 	else {
 		struct raw_iter_state *state = raw_seq_private(seq);
 
-		seq_printf(seq, "%-127s\n",
+		seq_printf(seq, "%-*s\n", TMPSZ-1,
 			   get_raw_sock(v, tmpbuf, state->bucket));
 	}
 	return 0;

Wang Chen

author

2007-11-14 03:18 UTC #5 lore raw

PATCH 2/2 SOCK: add raw6 drops counter

Add raw drops counter for IPv6 in /proc/net/raw6 .

Signed-off-by Wang Chen <[email protected]>

---
 raw.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

net/ipv6/raw.c +8 -7

diff -Nurp linux-2.6.24-rc2.org/net/ipv6/raw.c linux-2.6.24-rc2/net/ipv6/raw.c
@@ -354,14 +354,14 @@ static inline int rawv6_rcv_skb(struct s
 {
 	if ((raw6_sk(sk)->checksum || sk->sk_filter) &&
 	    skb_checksum_complete(skb)) {
-		/* FIXME: increment a raw6 drops counter here */
+		atomic_inc(&sk->sk_drops);	
 		kfree_skb(skb);
 		return 0;
 	}
 
 	/* Charge it to the socket. */
 	if (sock_queue_rcv_skb(sk,skb)<0) {
-		/* FIXME: increment a raw6 drops counter here */
+		atomic_inc(&sk->sk_drops);
 		kfree_skb(skb);
 		return 0;
 	}

@@ -382,6 +382,7 @@ int rawv6_rcv(struct sock *sk, struct sk
 	struct raw6_sock *rp = raw6_sk(sk);
 
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb)) {
+		atomic_inc(&sk->sk_drops);
 		kfree_skb(skb);
 		return NET_RX_DROP;
 	}

@@ -405,7 +406,7 @@ int rawv6_rcv(struct sock *sk, struct sk
 
 	if (inet->hdrincl) {
 		if (skb_checksum_complete(skb)) {
-			/* FIXME: increment a raw6 drops counter here */
+			atomic_inc(&sk->sk_drops);
 			kfree_skb(skb);
 			return 0;
 		}

@@ -496,7 +497,7 @@ csum_copy_err:
 	   as some normal condition.
 	 */
 	err = (flags&MSG_DONTWAIT) ? -EAGAIN : -EHOSTUNREACH;
-	/* FIXME: increment a raw6 drops counter here */
+	atomic_inc(&sk->sk_drops);
 	goto out;
 }

@@ -1251,7 +1252,7 @@ static void raw6_sock_seq_show(struct se
 	srcp  = inet_sk(sp)->num;
 	seq_printf(seq,
 		   "%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
-		   "%02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p\n",
+		   "%02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n",
 		   i,
 		   src->s6_addr32[0], src->s6_addr32[1],
 		   src->s6_addr32[2], src->s6_addr32[3], srcp,

@@ -1263,7 +1264,7 @@ static void raw6_sock_seq_show(struct se
 		   0, 0L, 0,
 		   sock_i_uid(sp), 0,
 		   sock_i_ino(sp),
-		   atomic_read(&sp->sk_refcnt), sp);
+		   atomic_read(&sp->sk_refcnt), sp, atomic_read(&sp->sk_drops));
 }
 
 static int raw6_seq_show(struct seq_file *seq, void *v)

@@ -1274,7 +1275,7 @@ static int raw6_seq_show(struct seq_file
 			   "local_address                         "
 			   "remote_address                        "
 			   "st tx_queue rx_queue tr tm->when retrnsmt"
-			   "   uid  timeout inode\n");
+			   "   uid  timeout inode  drops\n");
 	else
 		raw6_sock_seq_show(seq, v, raw6_seq_private(seq)->bucket);
 	return 0;

David Miller

maintainer Applied

2007-11-14 04:31 UTC #6 lore raw

Re: [PATCH 2/2] SOCK: add raw6 drops counter

From: Wang Chen <[email protected]>
Date: Wed, 14 Nov 2007 11:15:57 +0800

Quoted 3 lines

Add raw drops counter for IPv6 in /proc/net/raw6 .
Signed-off-by Wang Chen <[email protected]>

Applied to net-2.6.25, but again more whitespace problems:

Adds trailing whitespace.
diff:9: atomic_inc(&sk->sk_drops);

Wang Chen

author

2007-11-14 04:53 UTC #7 lore raw

Re: [PATCH 2/2] SOCK: add raw6 drops counter

David Miller said the following on 2007-11-14 12:31:

Quoted 12 lines

From: Wang Chen <[email protected]>
Date: Wed, 14 Nov 2007 11:15:57 +0800
Quoted 3 lines
Add raw drops counter for IPv6 in /proc/net/raw6 .
Signed-off-by Wang Chen <[email protected]>
Applied to net-2.6.25, but again more whitespace problems:
Adds trailing whitespace.
diff:9: atomic_inc(&sk->sk_drops);

Sorry for that. I will pay more attention to it.
Thank you.

Eric Dumazet

2007-11-14 05:49 UTC #8 lore raw

Re: [PATCH 2/2] SOCK: add raw6 drops counter

In reply to Wang Chen

Quoted 11 lines · Wang Chen

Add raw drops counter for IPv6 in /proc/net/raw6 .
Signed-off-by Wang Chen <[email protected]>
---
 raw.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)
diff -Nurp linux-2.6.24-rc2.org/net/ipv6/raw.c linux-2.6.24-rc2/net/ipv6/raw.c

net/ipv6/raw.c static inline int rawv6_rcv_skb(struct s · line 357

Quoted 5 lines

net/ipv6/raw.c +1 -1

@@ -354,14 +354,14 @@ static inline int rawv6_rcv_skb(struct s
 {
 	if ((raw6_sk(sk)->checksum || sk->sk_filter) &&
 	    skb_checksum_complete(skb)) {
-		/* FIXME: increment a raw6 drops counter here */
+		atomic_inc(&sk->sk_drops);

I am not sure the comment was refering to a per socket counter here.

If the frame checksum is bad, we can not be sure the socket is OK, since the garbaged bits could be in the tuple that identify the socket.

Maybe here we want to increment a global raw6 drop counter (well, for the given ipv6 instance)

Quoted 20 lines

net/ipv6/raw.c +2 -1

 		kfree_skb(skb);
 		return 0;
 	}
 
 	/* Charge it to the socket. */
 	if (sock_queue_rcv_skb(sk,skb)<0) {
-		/* FIXME: increment a raw6 drops counter here */
+		atomic_inc(&sk->sk_drops);
 		kfree_skb(skb);
 		return 0;
 	}

@@ -382,6 +382,7 @@ int rawv6_rcv(struct sock *sk, struct sk
 	struct raw6_sock *rp = raw6_sk(sk);
 
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb)) {
+		atomic_inc(&sk->sk_drops);
 		kfree_skb(skb);
 		return NET_RX_DROP;
 	}

net/ipv6/raw.c int rawv6_rcv(struct sock *sk, struct sk · line 409

Quoted 5 lines

net/ipv6/raw.c +1 -1

@@ -405,7 +406,7 @@ int rawv6_rcv(struct sock *sk, struct sk
 
 	if (inet->hdrincl) {
 		if (skb_checksum_complete(skb)) {
-			/* FIXME: increment a raw6 drops counter here */
+			atomic_inc(&sk->sk_drops);

Same remark here.

Wang Chen

author

2007-11-14 06:47 UTC #9 lore raw

Re: [PATCH 2/2] SOCK: add raw6 drops counter

Eric Dumazet said the following on 2007-11-14 13:48:

Quoted 15 lines

net/ipv6/raw.c static inline int rawv6_rcv_skb(struct s · line 357
Quoted 12 lines
diff -Nurp linux-2.6.24-rc2.org/net/ipv6/raw.c
linux-2.6.24-rc2/net/ipv6/raw.c
--- linux-2.6.24-rc2.org/net/ipv6/raw.c 2007-11-09
16:38:05.000000000 +0800
+++ linux-2.6.24-rc2/net/ipv6/raw.c 2007-11-14 09:46:54.000000000
+0800
net/ipv6/raw.c +1 -1
@@ -354,14 +354,14 @@ static inline int rawv6_rcv_skb(struct s
 {
     if ((raw6_sk(sk)->checksum || sk->sk_filter) &&
         skb_checksum_complete(skb)) {
-        /* FIXME: increment a raw6 drops counter here */
+        atomic_inc(&sk->sk_drops);   
I am not sure the comment was refering to a per socket counter here.

Quoted 6 lines

If the frame checksum is bad, we can not be sure the socket is OK, since the garbaged bits could be in the tuple that identify the socket.
Maybe here we want to increment a global raw6 drop counter (well, for the given ipv6 instance)

What the /proc/net/raw6 shows is statistical information about raw socket in IPv6 stack. And the information is per socket per row.
So I think it's better to count it to per socket.

David Miller

maintainer Applied

2007-11-14 04:30 UTC #10 lore raw

Re: [PATCH 1/2] SOCK: add raw drops counter

In reply to Wang Chen

From: Wang Chen <[email protected]>
Date: Wed, 14 Nov 2007 11:12:18 +0800

Quoted 3 lines

Add raw drops counter for IPv4 in /proc/net/raw .
Signed-off-by Wang Chen <[email protected]>

Applied to net-2.6.25, but your patch a lot of whitespace problems:

Adds trailing whitespace.
diff:9: * @sk_flags: %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
Adds trailing whitespace.
diff:19: * @sk_prot_creator: sk_prot of original sock creator (see ipv6_setsockopt,
Adds trailing whitespace.
diff:23: * @sk_err_soft: errors that don't cause failure but are the cause of a
Adds trailing whitespace.
diff:56: atomic_inc(&sk->sk_drops);
warning: 4 lines add whitespace errors.

I fixed it by hand this time, but please fix this yourself before future submissions.

Thank you.