[PATCH 0/3] UDP memory accounting and limitation (take 12)

[PATCH 0/3] UDP memory accounting and limitation (take 12)

[Hideo AOKI]

Hello,

This is the latest patch set of UDP memory accounting and limitation.

I split my patch set into accounting interface consolidation and UDP
memory accounting support. This patch set implements UDP memory
accounting support.

To use consolidated accounting functions, I matched format of udp_mem
sysctl with tcp_mem. UDP IPv6 is finally supported.

However, currently, only receive buffer accounting is implemented.

The patch set was tested on net-2.6.25 tree.


Changelog take 11 -> take 12:
 * split into accounting interface consolidation and UDP memory
   accounting support.
 * used new accounting interface
 * supported IPv6
 * dropped send buffer accounting

Changelog take 10 -> take 11:
 * stoped using spin lock in memory accounting function
 * socket lock and backlog processing were used to avoid conflict
   between receive system call processing and BH
 * revised memory accounting functions
 * stooped changing sock_queue_rcv_skb() and skb_set_owner_r()
 * added __udp_queue_rcv_skb to set proper destructor
 * removed udp_set_owner_r()
 * removed reclaim in inet_sock_destruct()

Changelog take 9 -> take 10:
 * supported using sk_forward_alloc
 * introduced several memory accounting functions with spin lock
 * changed detagram receive functions to be able to customize
   destructor
 * fixed accounting bugs in previous takes

Best regards,
Hideo Aoki

--
Hitachi Computer Products (America) Inc.

[PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min

[Hideo AOKI]

This patch adds sysctl parameters for customizing UDP memory accounting:
     /proc/sys/net/ipv4/udp_mem
     /proc/sys/net/ipv4/udp_rmem_min
     /proc/sys/net/ipv4/udp_wmem_min

Udp_mem indicates number of pages which can be used for all UDP scokets.
Each UDP packet is dropped, when the number of pages for socket buffer is
beyond udp_mem and the socket already consumes minimum buffer.

Cc: Satoshi Oshima <satoshi.oshima.fk@hitachi.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
signed-off-by: Takahiro Yasui <tyasui@redhat.com>
signed-off-by: Hideo Aoki <haoki@redhat.com>
---

 Documentation/networking/ip-sysctl.txt |   27 +++++++++++++++++++++++++++
 include/net/udp.h                      |    7 +++++++
 net/ipv4/af_inet.c                     |    3 +++
 net/ipv4/proc.c                        |    3 ++-
 net/ipv4/sysctl_net_ipv4.c             |   31 +++++++++++++++++++++++++++++++
 net/ipv4/udp.c                         |   31 +++++++++++++++++++++++++++++++
 6 files changed, 101 insertions(+), 1 deletion(-)

diff -pruN net-2.6.25-t12t19m-p4/Documentation/networking/ip-sysctl.txt net-2.6.25-t12t19m-p5/Documentation/networking/ip-sysctl.txt
--- net-2.6.25-t12t19m-p4/Documentation/networking/ip-sysctl.txt	2007-12-27 10:18:41.000000000 -0500
+++ net-2.6.25-t12t19m-p5/Documentation/networking/ip-sysctl.txt	2007-12-29 21:09:21.000000000 -0500
@@ -446,6 +446,33 @@ tcp_dma_copybreak - INTEGER
 	and CONFIG_NET_DMA is enabled.
 	Default: 4096

+UDP variables:
+
+udp_mem - vector of 3 INTEGERs: min, pressure, max
+	Number of pages allowed for queueing by all UDP sockets.
+
+	min: Below this number of pages UDP is not bothered about its
+	memory appetite. When amount of memory allocated by UDP exceeds
+	this number, UDP starts to moderate memory usage.
+
+	pressure: This value was introduced to follow format of tcp_mem.
+
+	max: Number of pages allowed for queueing by all UDP sockets.
+
+	Default is calculated at boot time from amount of available memory.
+
+udp_rmem_min - INTEGER
+	Minimal size of receive buffer used by UDP sockets in moderation.
+	Each UDP socket is able to use the size for receiving data, even if
+	total pages of UDP sockets exceed udp_mem pressure. The unit is byte.
+	Default: 4096
+
+udp_wmem_min - INTEGER
+	Minimal size of send buffer used by UDP sockets in moderation.
+	Each UDP socket is able to use the size for sending data, even if
+	total pages of UDP sockets exceed udp_mem pressure. The unit is byte.
+	Default: 4096
+
 CIPSOv4 Variables:

 cipso_cache_enable - BOOLEAN
diff -pruN net-2.6.25-t12t19m-p4/include/net/udp.h net-2.6.25-t12t19m-p5/include/net/udp.h
--- net-2.6.25-t12t19m-p4/include/net/udp.h	2007-12-27 10:18:58.000000000 -0500
+++ net-2.6.25-t12t19m-p5/include/net/udp.h	2007-12-29 21:10:48.000000000 -0500
@@ -65,6 +65,11 @@ extern rwlock_t udp_hash_lock;

 extern struct proto udp_prot;

+/* sysctl variables for udp */
+extern int sysctl_udp_mem[3];
+extern int sysctl_udp_rmem_min;
+extern int sysctl_udp_wmem_min;
+
 struct sk_buff;

 /*
@@ -198,4 +203,6 @@ extern void udp_proc_unregister(struct u
 extern int  udp4_proc_init(void);
 extern void udp4_proc_exit(void);
 #endif
+
+extern void udp_init(void);
 #endif	/* _UDP_H */
diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/af_inet.c net-2.6.25-t12t19m-p5/net/ipv4/af_inet.c
--- net-2.6.25-t12t19m-p4/net/ipv4/af_inet.c	2007-12-27 10:19:02.000000000 -0500
+++ net-2.6.25-t12t19m-p5/net/ipv4/af_inet.c	2007-12-29 21:09:21.000000000 -0500
@@ -1417,6 +1417,9 @@ static int __init inet_init(void)
 	/* Setup TCP slab cache for open requests. */
 	tcp_init();

+	/* Setup UDP memory threshold */
+	udp_init();
+
 	/* Add UDP-Lite (RFC 3828) */
 	udplite4_register();

diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
--- net-2.6.25-t12t19m-p4/net/ipv4/proc.c	2007-12-27 10:19:02.000000000 -0500
+++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c	2007-12-29 21:09:21.000000000 -0500
@@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
 		   sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
 		   tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
 		   atomic_read(&tcp_memory_allocated));
-	seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
+	seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
+		   atomic_read(&udp_memory_allocated));
 	seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
 	seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
 	seq_printf(seq,  "FRAG: inuse %d memory %d\n",
diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/sysctl_net_ipv4.c net-2.6.25-t12t19m-p5/net/ipv4/sysctl_net_ipv4.c
--- net-2.6.25-t12t19m-p4/net/ipv4/sysctl_net_ipv4.c	2007-12-27 10:19:02.000000000 -0500
+++ net-2.6.25-t12t19m-p5/net/ipv4/sysctl_net_ipv4.c	2007-12-29 21:09:21.000000000 -0500
@@ -19,6 +19,7 @@
 #include <net/ip.h>
 #include <net/route.h>
 #include <net/tcp.h>
+#include <net/udp.h>
 #include <net/cipso_ipv4.h>
 #include <net/inet_frag.h>

@@ -812,6 +813,36 @@ static struct ctl_table ipv4_table[] = {
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "udp_mem",
+		.data		= &sysctl_udp_mem,
+		.maxlen		= sizeof(sysctl_udp_mem),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &zero
+	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "udp_rmem_min",
+		.data		= &sysctl_udp_rmem_min,
+		.maxlen		= sizeof(sysctl_udp_rmem_min),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &zero
+	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "udp_wmem_min",
+		.data		= &sysctl_udp_wmem_min,
+		.maxlen		= sizeof(sysctl_udp_wmem_min),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &zero
+	},
 	{ .ctl_name = 0 }
 };

diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/udp.c net-2.6.25-t12t19m-p5/net/ipv4/udp.c
--- net-2.6.25-t12t19m-p4/net/ipv4/udp.c	2007-12-27 10:19:02.000000000 -0500
+++ net-2.6.25-t12t19m-p5/net/ipv4/udp.c	2007-12-29 21:12:03.000000000 -0500
@@ -82,6 +82,7 @@
 #include <asm/system.h>
 #include <asm/uaccess.h>
 #include <asm/ioctls.h>
+#include <linux/bootmem.h>
 #include <linux/types.h>
 #include <linux/fcntl.h>
 #include <linux/module.h>
@@ -118,6 +119,14 @@ EXPORT_SYMBOL(udp_stats_in6);
 struct hlist_head udp_hash[UDP_HTABLE_SIZE];
 DEFINE_RWLOCK(udp_hash_lock);

+int sysctl_udp_mem[3] __read_mostly;
+int sysctl_udp_rmem_min __read_mostly;
+int sysctl_udp_wmem_min __read_mostly;
+
+EXPORT_SYMBOL(sysctl_udp_mem);
+EXPORT_SYMBOL(sysctl_udp_rmem_min);
+EXPORT_SYMBOL(sysctl_udp_wmem_min);
+
 static inline int __udp_lib_lport_inuse(__u16 num,
 					const struct hlist_head udptable[])
 {
@@ -1460,6 +1469,9 @@ struct proto udp_prot = {
 	.hash		   = udp_lib_hash,
 	.unhash		   = udp_lib_unhash,
 	.get_port	   = udp_v4_get_port,
+	.sysctl_mem	   = sysctl_udp_mem,
+	.sysctl_wmem	   = &sysctl_udp_wmem_min,
+	.sysctl_rmem	   = &sysctl_udp_rmem_min,
 	.obj_size	   = sizeof(struct udp_sock),
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_udp_setsockopt,
@@ -1655,6 +1667,25 @@ void udp4_proc_exit(void)
 }
 #endif /* CONFIG_PROC_FS */

+void __init udp_init(void)
+{
+	unsigned long limit;
+
+	/* Set the pressure threshold up by the same strategy of TCP. It is a
+	 * fraction of global memory that is up to 1/2 at 256 MB, decreasing
+	 * toward zero with the amount of memory, with a floor of 128 pages.
+	 */
+	limit = min(nr_all_pages, 1UL<<(28-PAGE_SHIFT)) >> (20-PAGE_SHIFT);
+	limit = (limit * (nr_all_pages >> (20-PAGE_SHIFT))) >> (PAGE_SHIFT-11);
+	limit = max(limit, 128UL);
+	sysctl_udp_mem[0] = limit / 4 * 3;
+	sysctl_udp_mem[1] = limit;
+	sysctl_udp_mem[2] = sysctl_udp_mem[0] * 2;
+
+	sysctl_udp_rmem_min = SK_MEM_QUANTUM;
+	sysctl_udp_wmem_min = SK_MEM_QUANTUM;
+}
+
 EXPORT_SYMBOL(udp_disconnect);
 EXPORT_SYMBOL(udp_hash);
 EXPORT_SYMBOL(udp_hash_lock);
-- 
Hitachi Computer Products (America) Inc.

[PATCH 2/3] [UDP]: memory accounting in IPv4

[Hideo AOKI]

This patch adds UDP memory usage accounting in IPv4. Currently,
receiving buffer accounting is only supported.

This patch is also introduced memory_allocated variable for UDP protocol.

Cc: Satoshi Oshima <satoshi.oshima.fk@hitachi.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
signed-off-by: Takahiro Yasui <tyasui@redhat.com>
signed-off-by: Hideo Aoki <haoki@redhat.com>
---

 include/net/udp.h  |    2 ++
 net/ipv4/af_inet.c |    2 ++
 net/ipv4/udp.c     |   26 ++++++++++++++++++++++++--
 3 files changed, 28 insertions(+), 2 deletions(-)

diff -pruN net-2.6.25-t12t19m-p5/include/net/udp.h net-2.6.25-t12t19m-p6/include/net/udp.h
--- net-2.6.25-t12t19m-p5/include/net/udp.h	2007-12-29 21:10:48.000000000 -0500
+++ net-2.6.25-t12t19m-p6/include/net/udp.h	2007-12-29 21:48:33.000000000 -0500
@@ -65,6 +65,8 @@ extern rwlock_t udp_hash_lock;

 extern struct proto udp_prot;

+extern atomic_t udp_memory_allocated;
+
 /* sysctl variables for udp */
 extern int sysctl_udp_mem[3];
 extern int sysctl_udp_rmem_min;
diff -pruN net-2.6.25-t12t19m-p5/net/ipv4/af_inet.c net-2.6.25-t12t19m-p6/net/ipv4/af_inet.c
--- net-2.6.25-t12t19m-p5/net/ipv4/af_inet.c	2007-12-29 21:09:21.000000000 -0500
+++ net-2.6.25-t12t19m-p6/net/ipv4/af_inet.c	2007-12-29 21:47:31.000000000 -0500
@@ -139,6 +139,8 @@ void inet_sock_destruct(struct sock *sk)
 	__skb_queue_purge(&sk->sk_receive_queue);
 	__skb_queue_purge(&sk->sk_error_queue);

+	sk_mem_reclaim(sk);
+
 	if (sk->sk_type == SOCK_STREAM && sk->sk_state != TCP_CLOSE) {
 		printk("Attempt to release TCP socket in state %d %p\n",
 		       sk->sk_state, sk);
diff -pruN net-2.6.25-t12t19m-p5/net/ipv4/udp.c net-2.6.25-t12t19m-p6/net/ipv4/udp.c
--- net-2.6.25-t12t19m-p5/net/ipv4/udp.c	2007-12-29 21:12:03.000000000 -0500
+++ net-2.6.25-t12t19m-p6/net/ipv4/udp.c	2007-12-29 21:51:17.000000000 -0500
@@ -127,6 +127,9 @@ EXPORT_SYMBOL(sysctl_udp_mem);
 EXPORT_SYMBOL(sysctl_udp_rmem_min);
 EXPORT_SYMBOL(sysctl_udp_wmem_min);

+atomic_t udp_memory_allocated;
+EXPORT_SYMBOL(udp_memory_allocated);
+
 static inline int __udp_lib_lport_inuse(__u16 num,
 					const struct hlist_head udptable[])
 {
@@ -910,13 +913,17 @@ try_again:
 		err = ulen;

 out_free:
+	lock_sock(sk);
 	skb_free_datagram(sk, skb);
+	release_sock(sk);
 out:
 	return err;

 csum_copy_err:
+	lock_sock(sk);
 	if (!skb_kill_datagram(sk, skb, flags))
 		UDP_INC_STATS_USER(UDP_MIB_INERRORS, is_udplite);
+	release_sock(sk);

 	if (noblock)
 		return -EAGAIN;
@@ -1081,7 +1088,15 @@ static int __udp4_lib_mcast_deliver(stru
 				skb1 = skb_clone(skb, GFP_ATOMIC);

 			if (skb1) {
-				int ret = udp_queue_rcv_skb(sk, skb1);
+				int ret = 0;
+
+				bh_lock_sock_nested(sk);
+				if (!sock_owned_by_user(sk))
+					ret = udp_queue_rcv_skb(sk, skb1);
+				else
+					sk_add_backlog(sk, skb1);
+				bh_unlock_sock(sk);
+
 				if (ret > 0)
 					/* we should probably re-process instead
 					 * of dropping packets here. */
@@ -1174,7 +1189,13 @@ int __udp4_lib_rcv(struct sk_buff *skb,
 			       inet_iif(skb), udptable);

 	if (sk != NULL) {
-		int ret = udp_queue_rcv_skb(sk, skb);
+		int ret = 0;
+		bh_lock_sock_nested(sk);
+		if (!sock_owned_by_user(sk))
+			ret = udp_queue_rcv_skb(sk, skb);
+		else
+			sk_add_backlog(sk, skb);
+		bh_unlock_sock(sk);
 		sock_put(sk);

 		/* a return value > 0 means to resubmit the input, but
@@ -1469,6 +1490,7 @@ struct proto udp_prot = {
 	.hash		   = udp_lib_hash,
 	.unhash		   = udp_lib_unhash,
 	.get_port	   = udp_v4_get_port,
+	.memory_allocated  = &udp_memory_allocated,
 	.sysctl_mem	   = sysctl_udp_mem,
 	.sysctl_wmem	   = &sysctl_udp_wmem_min,
 	.sysctl_rmem	   = &sysctl_udp_rmem_min,
-- 
Hitachi Computer Products (America) Inc.

[PATCH 3/3] [UDP]: memory accounting in IPv6

[Hideo AOKI]

This patch adds UDP memory usage accounting in IPv6. Currently,
receiving buffer accounting is only supported.

This patch is also introduced memory_allocated variable for UDP protocol.

Cc: Satoshi Oshima <satoshi.oshima.fk@hitachi.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
signed-off-by: Takahiro Yasui <tyasui@redhat.com>
signed-off-by: Hideo Aoki <haoki@redhat.com>
---

 udp.c |   32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff -pruN net-2.6.25-t12t19m-p6/net/ipv6/udp.c net-2.6.25-t12t19m-p7/net/ipv6/udp.c
--- net-2.6.25-t12t19m-p6/net/ipv6/udp.c	2007-12-27 10:19:02.000000000 -0500
+++ net-2.6.25-t12t19m-p7/net/ipv6/udp.c	2007-12-29 21:57:41.000000000 -0500
@@ -204,13 +204,17 @@ try_again:
 		err = ulen;

 out_free:
+	lock_sock(sk);
 	skb_free_datagram(sk, skb);
+	release_sock(sk);
 out:
 	return err;

 csum_copy_err:
+	lock_sock(sk);
 	if (!skb_kill_datagram(sk, skb, flags))
 		UDP6_INC_STATS_USER(UDP_MIB_INERRORS, is_udplite);
+	release_sock(sk);

 	if (flags & MSG_DONTWAIT)
 		return -EAGAIN;
@@ -366,10 +370,21 @@ static int __udp6_lib_mcast_deliver(stru
 	while ((sk2 = udp_v6_mcast_next(sk_next(sk2), uh->dest, daddr,
 					uh->source, saddr, dif))) {
 		struct sk_buff *buff = skb_clone(skb, GFP_ATOMIC);
-		if (buff)
-			udpv6_queue_rcv_skb(sk2, buff);
+		if (buff) {
+			bh_lock_sock_nested(sk2);
+			if (!sock_owned_by_user(sk2))
+				udpv6_queue_rcv_skb(sk2, buff);
+			else
+				sk_add_backlog(sk2, buff);
+			bh_unlock_sock(sk2);
+		}
 	}
-	udpv6_queue_rcv_skb(sk, skb);
+	bh_lock_sock_nested(sk);
+	if (!sock_owned_by_user(sk))
+		udpv6_queue_rcv_skb(sk, skb);
+	else
+		sk_add_backlog(sk, skb);
+	bh_unlock_sock(sk);
 out:
 	read_unlock(&udp_hash_lock);
 	return 0;
@@ -482,7 +497,12 @@ int __udp6_lib_rcv(struct sk_buff *skb,

 	/* deliver */

-	udpv6_queue_rcv_skb(sk, skb);
+	bh_lock_sock_nested(sk);
+	if (!sock_owned_by_user(sk))
+		udpv6_queue_rcv_skb(sk, skb);
+	else
+		sk_add_backlog(sk, skb);
+	bh_unlock_sock(sk);
 	sock_put(sk);
 	return 0;

@@ -994,6 +1014,10 @@ struct proto udpv6_prot = {
 	.hash		   = udp_lib_hash,
 	.unhash		   = udp_lib_unhash,
 	.get_port	   = udp_v6_get_port,
+	.memory_allocated  = &udp_memory_allocated,
+	.sysctl_mem	   = sysctl_udp_mem,
+	.sysctl_wmem	   = &sysctl_udp_wmem_min,
+	.sysctl_rmem	   = &sysctl_udp_rmem_min,
 	.obj_size	   = sizeof(struct udp6_sock),
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_udpv6_setsockopt,
-- 
Hitachi Computer Products (America) Inc.

Re: [PATCH 2/3] [UDP]: memory accounting in IPv4

[Eric Dumazet]

Hideo AOKI a écrit :
> This patch adds UDP memory usage accounting in IPv4. Currently,
> receiving buffer accounting is only supported.
> 
> This patch is also introduced memory_allocated variable for UDP protocol.

OK, but sockstat_seq_show() should be updated so that this 
udp_memory_allocated value can be read from /proc/net/sockstat ?

Thank you

Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min

[David Miller]

From: Hideo AOKI <haoki@redhat.com>
Date: Sun, 30 Dec 2007 04:01:46 -0500

> diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
> --- net-2.6.25-t12t19m-p4/net/ipv4/proc.c	2007-12-27 10:19:02.000000000 -0500
> +++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c	2007-12-29 21:09:21.000000000 -0500
> @@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
>  		   sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
>  		   tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
>  		   atomic_read(&tcp_memory_allocated));
> -	seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
> +	seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
> +		   atomic_read(&udp_memory_allocated));
>  	seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
>  	seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
>  	seq_printf(seq,  "FRAG: inuse %d memory %d\n",

More careless patch creation.  :-/

This breaks the build because udp_memory_allocated is not added until
patch 2.

Once again I'll combine all three patches into one but I am extremely
angry about how careless and broken these two patch submissions were.

Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min

[Eric Dumazet]

David Miller a écrit :
> From: Hideo AOKI <haoki@redhat.com>
> Date: Sun, 30 Dec 2007 04:01:46 -0500
> 
>> diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
>> --- net-2.6.25-t12t19m-p4/net/ipv4/proc.c	2007-12-27 10:19:02.000000000 -0500
>> +++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c	2007-12-29 21:09:21.000000000 -0500
>> @@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
>>  		   sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
>>  		   tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
>>  		   atomic_read(&tcp_memory_allocated));
>> -	seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
>> +	seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
>> +		   atomic_read(&udp_memory_allocated));
>>  	seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
>>  	seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
>>  	seq_printf(seq,  "FRAG: inuse %d memory %d\n",
> 
> More careless patch creation.  :-/
> 
> This breaks the build because udp_memory_allocated is not added until
> patch 2.
> 
> Once again I'll combine all three patches into one but I am extremely
> angry about how careless and broken these two patch submissions were.

I am a litle bit concerned about performance of IVR servers
using SIP protocol.

On those servers, each active channel typically emits/receives 50 UDP/RTP 
frames per second. With G729 codec, each packet contains 10 bytes of payload, 
and about 40 bytes of IP/UDP/RTP encapsulation. (So these messages are very
small)

As I am currently enjoying hollidays at home, I am not able to test on my 
server farm the performance impact of this new UDP receive accounting.

If I understand well the patch, each time a packet is received (on a socket
with no previous message available in its receive queue), we are going to 
atomic_inc(&some_global_var). Then the user thread that will transfert this
message to user land will atomic_dec(&some_global_var). (Granted server is
in normal condition, ie each UDP socket holds at most one message in its
receive or transmit queue)

I have some machines with 400 active SIP channels, so that new hot cache line
will probably slow down our SMP servers, because of cache line ping pong.

I will probably setup a test next week and let you know the results.

Maybe I read the patch incorrectly, or we could add some new sysctl so that
we not try to uncharge memory if a socket 'forward_alloc' is beyond a given 
limit (say 2 pages), so that number of atomic_inc/dec on udp_memory_allocated 
(or tcp_memory_allocated) is reduced.

Thank you

Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min

[Herbert Xu]

On Mon, Dec 31, 2007 at 09:54:32AM +0100, Eric Dumazet wrote:
>
> Maybe I read the patch incorrectly, or we could add some new sysctl so that
> we not try to uncharge memory if a socket 'forward_alloc' is beyond a given 
> limit (say 2 pages), so that number of atomic_inc/dec on 
> udp_memory_allocated (or tcp_memory_allocated) is reduced.

I don't have time to go through these patches right now but
that was the whole point of using forward_alloc, i.e., avoid
touching the global for every packet.

So if it's still doing that then we're doing something wrong.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Mon, 31 Dec 2007 09:54:32 +0100

> Maybe I read the patch incorrectly, or we could add some new sysctl so that
> we not try to uncharge memory if a socket 'forward_alloc' is beyond a given 
> limit (say 2 pages), so that number of atomic_inc/dec on udp_memory_allocated 
> (or tcp_memory_allocated) is reduced.

This is what we should be striving for, using forward_alloc
as much as possible as a "cache" to avoid the atomics on
the global var as much as possible.

Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min

[Eric Dumazet]

David Miller a écrit :
> From: Eric Dumazet <dada1@cosmosbay.com>
> Date: Mon, 31 Dec 2007 09:54:32 +0100
> 
>> Maybe I read the patch incorrectly, or we could add some new sysctl so that
>> we not try to uncharge memory if a socket 'forward_alloc' is beyond a given 
>> limit (say 2 pages), so that number of atomic_inc/dec on udp_memory_allocated 
>> (or tcp_memory_allocated) is reduced.
> 
> This is what we should be striving for, using forward_alloc
> as much as possible as a "cache" to avoid the atomics on
> the global var as much as possible.

Thank you for this confirmation David, I understand now that tcp doesnt 
currently satisfy the contract.

For example, tcp_delack_timer() calls sk_mem_reclaim().

So on a machine with a lot of mostly idle sockets (but all sockets are doing 
some trafic, say one message per minute / socket), we can see :

$ grep TCP /proc/net/sockstat
TCP: inuse 1083667 orphan 8840 tw 6646 alloc 1083809 mem 262305
$ cat /proc/sys/net/ipv4/tcp_mem
2000000 3000000 4000000

so an average of 1/4 page are 'allocated' per socket :(

On this machine, we constantly change tcp_memory_allocated, even if we always 
are under tcp_mem[0] limit.

Maybe we need to introduce some mechanism to let sk_forward between 0 and 
SK_MEM_QUANTUM (inclusive).

static inline void sk_mem_reclaim_overpage(struct sock *sk)
{
         if (sk->sk_forward_alloc > SK_MEM_QUANTUM) {
                 __sk_mem_reclaim(sk);
		}
}

and use sk_mem_reclaim_overpage() instead of sk_mem_reclaim() in 
tcp_delack_timer() ?

Thank you

Small program output :

$ gcc -o prog prog.c ; ./prog
TCP: inuse 1035 orphan 0 tw 271 alloc 1203 mem 16
TCP: inuse 1035 orphan 0 tw 271 alloc 1203 mem 4016
TCP: inuse 1034 orphan 0 tw 272 alloc 1202 mem 3015
TCP: inuse 1034 orphan 0 tw 272 alloc 1202 mem 3016
TCP: inuse 1034 orphan 0 tw 272 alloc 1202 mem 3516
TCP: inuse 1034 orphan 0 tw 272 alloc 1202 mem 14 <OOPS>

$ cat prog.c
#include <sys/socket.h>
#include <string.h>
#include <stdlib.h>
#include <netinet/tcp.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <unistd.h>
#include <errno.h>
#include <asm/ioctls.h>
#include <stdio.h>

int SOCK_COUNT = 1000;
int *sockets_fd_tab;
unsigned int count;

static void open_sockets(int domain, int type)
{
         int fdlisten=-1, on = 1;
         socklen_t addrlen;
         struct sockaddr_in host, peer;

         if (domain == AF_INET && type == SOCK_STREAM) {
                 fdlisten = socket(AF_INET, type, 0);
                 setsockopt(fdlisten, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(int));
                 memset(&host, 0, sizeof(host));
                 host.sin_family = AF_INET;
                 bind(fdlisten, (struct sockaddr *)&host, sizeof(host));
                 addrlen = sizeof(host);
                 getsockname(fdlisten, (struct sockaddr *)&host, &addrlen);
                 listen(fdlisten, 5);
         }
         while (1) {
                 int res, vec[2];
                 if (domain == AF_UNIX) {
                         res = socketpair(AF_UNIX, type, 0, vec);
                         if (res == -1)
                                 break;
                 } else {
                         vec[0] = socket(AF_INET, type, 0);
                         if (vec[0] == -1)
                                 break;
                         ioctl(vec[0], FIONBIO, &on);
                         if (type == SOCK_STREAM) {
                                 connect(vec[0], (struct sockaddr *)&host, 
sizeof(host));
                                 addrlen = sizeof(peer);
                                 vec[1] = accept(fdlisten, (struct sockaddr 
*)&peer, &addrlen);
                                 if (vec[1] == -1) {
                                         close(vec[0]);
                                         break;
                                 }
                         } else {
                         }
                 }
                 sockets_fd_tab[count++] = vec[0];
                 sockets_fd_tab[count++] = vec[1];
                 if (count == SOCK_COUNT)
                         break;
         }
}

const char some_msg[1024] = "One dummy message";

static void fill_sockets()
{
         unsigned int ui;
         for (ui = 0; ui < count; ui++)
                 send(sockets_fd_tab[ui], some_msg, 100, 0);
         for (ui = 0; ui < count; ui++)
                 send(sockets_fd_tab[ui], some_msg, 100, 0);
}

static void empty_sockets()
{
         unsigned int ui;
         char buffer[4096];

         for (ui = 0; ui < count; ui++)
                 recv(sockets_fd_tab[ui], buffer, sizeof(buffer), 0);
}

static void dump_infos()
{
         system("grep TCP /proc/net/sockstat");
}

int main(int argc, char *argv[])
{
         int c;
         while ((c = getopt(argc, argv, "n:")) != EOF) {
                 if (c == 'n')
                         SOCK_COUNT = atoi(optarg);
         }
         sockets_fd_tab = malloc(SOCK_COUNT * sizeof(int));
         open_sockets(AF_INET, SOCK_STREAM);
         dump_infos();

         fill_sockets();
         dump_infos();
         sleep(1); /* to see effect of delayed acks */
         dump_infos();
         empty_sockets();
         dump_infos();

         fill_sockets();
         dump_infos();
         empty_sockets();
         sleep(1); /* to see effect of delayed acks */
         dump_infos();

         return 0;
}

Re: [PATCH 2/3] [UDP]: memory accounting in IPv4

[Hideo AOKI]

Eric Dumazet wrote:
> Hideo AOKI a écrit :
>> This patch adds UDP memory usage accounting in IPv4. Currently,
>> receiving buffer accounting is only supported.
>>
>> This patch is also introduced memory_allocated variable for UDP protocol.
> 
> OK, but sockstat_seq_show() should be updated so that this
> udp_memory_allocated value can be read from /proc/net/sockstat ?

Yes, and I updated it in the first patch of the patch set.

I thought dividing patch made easy to review. But I was wrong.
Sorry about that.

Regards,
Hideo

-- 
Hitachi Computer Products (America) Inc.

Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min

[Hideo AOKI]

David Miller wrote:
> From: Hideo AOKI <haoki@redhat.com>
> Date: Sun, 30 Dec 2007 04:01:46 -0500
> 
>> diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
>> --- net-2.6.25-t12t19m-p4/net/ipv4/proc.c	2007-12-27 10:19:02.000000000 -0500
>> +++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c	2007-12-29 21:09:21.000000000 -0500
>> @@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
>>  		   sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
>>  		   tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
>>  		   atomic_read(&tcp_memory_allocated));
>> -	seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
>> +	seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
>> +		   atomic_read(&udp_memory_allocated));
>>  	seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
>>  	seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
>>  	seq_printf(seq,  "FRAG: inuse %d memory %d\n",
> 
> More careless patch creation.  :-/
> 
> This breaks the build because udp_memory_allocated is not added until
> patch 2.

Hello David,

You are absolutely right.

> Once again I'll combine all three patches into one but I am extremely
> angry about how careless and broken these two patch submissions were.

I really apologize for wasting your time twice.
I do not do this kind mess again.

Many thanks for your support.

Best regards,
Hideo

-- 
Hitachi Computer Products (America) Inc.

Re: [PATCH 2/3] [UDP]: memory accounting in IPv4

[Eric Dumazet]

Hideo AOKI a écrit :
> Eric Dumazet wrote:
>> Hideo AOKI a écrit :
>>> This patch adds UDP memory usage accounting in IPv4. Currently,
>>> receiving buffer accounting is only supported.
>>>
>>> This patch is also introduced memory_allocated variable for UDP protocol.
>> OK, but sockstat_seq_show() should be updated so that this
>> udp_memory_allocated value can be read from /proc/net/sockstat ?
> 
> Yes, and I updated it in the first patch of the patch set.
> 
> I thought dividing patch made easy to review. But I was wrong.
> Sorry about that.

No problem Hideo, I think you did a good job with this UDP memory accounting 
and were very patient.

Thank you

Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Mon, 31 Dec 2007 15:42:42 +0100

> Maybe we need to introduce some mechanism to let sk_forward between 0 and 
> SK_MEM_QUANTUM (inclusive).
> 
> static inline void sk_mem_reclaim_overpage(struct sock *sk)
> {
>          if (sk->sk_forward_alloc > SK_MEM_QUANTUM) {
>                  __sk_mem_reclaim(sk);
> 		}
> }
> 
> and use sk_mem_reclaim_overpage() instead of sk_mem_reclaim() in 
> tcp_delack_timer() ?

This makes a lot of sense to me, it is definitely doing the wrong
thing here for the load you describe.

I've applied the following to net-2.6.25 and I am definitely
willing to put this into 2.6.24 and -stable if you think that's
worthwhile too.

[TCP]: Do not purge sk_forward_alloc entirely in tcp_delack_timer().

Otherwise we beat heavily on the global tcp_memory atomics
when all of the sockets in the system are slowly sending
perioding packet clumps.

Noticed and suggested by Eric Dumazet.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/sock.h   |    8 ++++++++
 net/ipv4/tcp_timer.c |    2 +-
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 89e04e4..d24b826 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -759,6 +759,14 @@ static inline void sk_mem_reclaim(struct sock *sk)
 		__sk_mem_reclaim(sk);
 }
 
+static inline void sk_mem_reclaim_partial(struct sock *sk)
+{
+	if (!sk_has_account(sk))
+		return;
+	if (sk->sk_forward_alloc > SK_MEM_QUANTUM)
+		__sk_mem_reclaim(sk);
+}
+
 static inline void sk_mem_charge(struct sock *sk, int size)
 {
 	if (!sk_has_account(sk))
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 17931be..803d758 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -186,7 +186,7 @@ static void tcp_delack_timer(unsigned long data)
 		goto out_unlock;
 	}
 
-	sk_mem_reclaim(sk);
+	sk_mem_reclaim_partial(sk);
 
 	if (sk->sk_state == TCP_CLOSE || !(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
 		goto out;
-- 
1.5.4.rc2.84.gf85fd