Skip to contentSkip to conversation

UDP memory accounting and limitation (take 6)

PATCH In review Hideo AOKI 4 participants 19 messages Oct 29, 2007 – Nov 15, 2007

Subsystems (6)
lore
Share & tools
lore

Hideo AOKI

author
#0 lore raw

cover UDP memory accounting and limitation (take 6)

Hello,

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

The number of pages for socket buffer is limited up to /proc/sys/net/ipv4/udp_mem. I removed the minimal limit number to use the feature from the former patch set (take5). And udp_init() is introduced to calculate default value.

In addition, I added /proc/sys/net/ipv4/udp_rmem and /proc/sys/net/ipv4/udp_wmem to be able to allocate minimum buffer to each socket.

As a result, UDP packet is drooped when the number of pages for socket buffer is beyond the limit and the socket already consumes minimum buffer.

Detailed change log is below.

Changelog take 5 -> take 6:

* removed minimal limit of /proc/sys/net/udp_mem
* added udp_init() for default value calculation of parameters
* added /proc/sys/net/udp_rmem and /proc/sys/net/udp_rmem
* added limitation code to ip_ufo_append_data()
* improved accounting for receiving packet
* fixed typos
* rebased to 2.6.24-rc1

Changelog take 4 -> take 5:

* removing unnessesary EXPORT_SYMBOLs
* adding minimal limit of /proc/sys/net/udp_mem
* bugfix of UDP limit affecting protocol other than UDP
* introducing __ip_check_max_skb_pages()
* using CTL_UNNUMBERED
* adding udp_mem usage to Documentation/networking/ip_sysctl.txt

Best regards,
Hideo Aoki

--
Hitachi Computer Products (America) Inc.

author
#1 lore raw

PATCH 1/5 fix send buffer check

This patch introduces sndbuf size check before memory allocation for send buffer.

--
Hideo Aoki
Hitachi Computer Products (America) Inc.

 ip_output.c |    5 +++++
 1 file changed, 5 insertions(+)
net/ipv4/ip_output.c +5 -0
diff -pruN linux-2.6.24-rc1/net/ipv4/ip_output.c linux-2.6.24-rc1-mem003-ipv4-dev-p1/net/ipv4/ip_output.c
@@ -1004,6 +1004,11 @@ alloc_new_skb:
 					frag = &skb_shinfo(skb)->frags[i];
 				}
 			} else if (i < MAX_SKB_FRAGS) {
+				if (atomic_read(&sk->sk_wmem_alloc) + PAGE_SIZE
+				    > 2 * sk->sk_sndbuf) {
+					err = -ENOBUFS;
+					goto error;
+				}
 				if (copy > PAGE_SIZE)
 					copy = PAGE_SIZE;
 				page = alloc_pages(sk->sk_allocation, 0);
#2 lore raw

Re: [PATCH 1/5] fix send buffer check

Quoted 2 lines · Hideo AOKI

This patch introduces sndbuf size check before memory allocation for send buffer.

Looks good, what about IPv6?

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author
#3 lore raw

Re: [PATCH 1/5] fix send buffer check

Hello,

I'm sorry that my response is always late.

Quoted 5 lines · Herbert Xu
Quoted 2 lines · Hideo AOKI

This patch introduces sndbuf size check before memory allocation for send buffer.

Looks good, what about IPv6?

I'm going to develop IPv6 part if IPv4 patch set is accepted.

Many thanks,
Hideo

--
Hitachi Computer Products (America) Inc.

author
#4 lore raw

PATCH 2/5 accounting unit and variable

In reply to Hideo AOKI

This patch introduces global variable for UDP memory accounting. The unit is page.

--
Hideo Aoki
Hitachi Computer Products (America) Inc.

 include/net/sock.h |    7 +++++++
 include/net/udp.h  |    2 ++
 net/ipv4/proc.c    |    3 ++-
 net/ipv4/udp.c     |    2 ++
 4 files changed, 13 insertions(+), 1 deletion(-)
include/net/sock.h +7 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p1/include/net/sock.h linux-2.6.24-rc1-mem003-ipv4-dev-p2/include/net/sock.h
@@ -727,6 +727,13 @@ static inline int sk_stream_wmem_schedul
 	       sk_stream_mem_schedule(sk, size, 0);
 }
 
+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}
+
 /* Used by processes to "lock" a socket state, so that
  * interrupts and bottom half handlers won't change it
  * from under us. It essentially blocks any incoming
include/net/udp.h +2 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p1/include/net/udp.h linux-2.6.24-rc1-mem003-ipv4-dev-p2/include/net/udp.h
@@ -65,6 +65,8 @@ extern rwlock_t udp_hash_lock;
 
 extern struct proto udp_prot;
 
+extern atomic_t udp_memory_allocated;
+
 struct sk_buff;
 
 /*
net/ipv4/proc.c +2 -1
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p1/net/ipv4/proc.c linux-2.6.24-rc1-mem003-ipv4-dev-p2/net/ipv4/proc.c
@@ -67,7 +67,8 @@ static int sockstat_seq_show(struct seq_
 		   fold_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", fold_prot_inuse(&udp_prot));
+	seq_printf(seq, "UDP: inuse %d mem %d\n", fold_prot_inuse(&udp_prot),
+		   atomic_read(&udp_memory_allocated));
 	seq_printf(seq, "UDPLITE: inuse %d\n", fold_prot_inuse(&udplite_prot));
 	seq_printf(seq, "RAW: inuse %d\n", fold_prot_inuse(&raw_prot));
 	seq_printf(seq,  "FRAG: inuse %d memory %d\n",
net/ipv4/udp.c +2 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p1/net/ipv4/udp.c linux-2.6.24-rc1-mem003-ipv4-dev-p2/net/ipv4/udp.c
@@ -114,6 +114,8 @@ DEFINE_SNMP_STAT(struct udp_mib, udp_sta
 struct hlist_head udp_hash[UDP_HTABLE_SIZE];
 DEFINE_RWLOCK(udp_hash_lock);
 
+atomic_t udp_memory_allocated;
+
 static inline int __udp_lib_lport_inuse(__u16 num,
 					const struct hlist_head udptable[])
 {
#5 lore raw

Re: [PATCH 2/5] accounting unit and variable

include/net/sock.h static inline int sk_stream_wmem_schedul · line 735

Quoted 7 lines · Hideo AOKI
include/net/sock.h +6 -0
+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}

Does this really have to be int? Unsigned would let the compiler optimise this to a simple shift.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author
#6 lore raw

Re: [PATCH 2/5] accounting unit and variable

Hello,

Quoted 11 lines · Herbert Xu

include/net/sock.h static inline int sk_stream_wmem_schedul · line 735

Quoted 7 lines · Hideo AOKI
include/net/sock.h +6 -0
+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}

Does this really have to be int? Unsigned would let the compiler optimise this to a simple shift.

Thank you for the comment.

This inline function is used to calculate the first argument of atomic_add() and atomic_sub(). Since the argument is int, I believe that using int is better than using unsigned int.

Best regards,
Hideo

--
Hitachi Computer Products (America) Inc.

#7 lore raw

Re: [PATCH 2/5] accounting unit and variable

Quoted 13 lines · Hideo AOKI
Quoted 10 lines · Herbert Xu

include/net/sock.h static inline int sk_stream_wmem_schedul · line 735

Quoted 7 lines
include/net/sock.h +6 -0
+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}

Does this really have to be int? Unsigned would let the compiler optimise this to a simple shift.

Quoted 5 lines

Thank you for the comment.

This inline function is used to calculate the first argument of atomic_add() and atomic_sub(). Since the argument is int, I believe that using int is better than using unsigned int.

That doesn't really answer my question. Is this quantity ever negative? If not you should make it unsigned for the reason I gave above.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#8 lore raw

Re: [PATCH 2/5] accounting unit and variable

In reply to Hideo AOKI

From: Hideo AOKI <[email protected]>
Date: Tue, 13 Nov 2007 22:27:13 -0500

Quoted 13 lines
Quoted 11 lines · Herbert Xu

include/net/sock.h static inline int sk_stream_wmem_schedul · line 735

Quoted 7 lines · Hideo AOKI
include/net/sock.h +6 -0
+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}

Does this really have to be int? Unsigned would let the compiler optimise this to a simple shift.

Quoted 5 lines

Thank you for the comment.

This inline function is used to calculate the first argument of atomic_add() and atomic_sub(). Since the argument is int, I believe that using int is better than using unsigned int.

If you know the values will always be positive, as you will know here, it is OK to us unsigned int here and avoids the unacceptable expensive divide instruction.

Please fix this.

author
#9 lore raw

Re: [PATCH 2/5] accounting unit and variable

Quoted 20 lines · David Miller

From: Hideo AOKI <[email protected]>
Date: Tue, 13 Nov 2007 22:27:13 -0500

Quoted 11 lines
Quoted 10 lines · Herbert Xu

include/net/sock.h static inline int sk_stream_wmem_schedul · line 735

Quoted 7 lines · Hideo AOKI
include/net/sock.h +6 -0
+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}

Does this really have to be int? Unsigned would let the compiler optimise this to a simple shift.

Quoted 5 lines

Thank you for the comment.

This inline function is used to calculate the first argument of atomic_add() and atomic_sub(). Since the argument is int, I believe that using int is better than using unsigned int.

Quoted 5 lines

If you know the values will always be positive, as you will know here, it is OK to us unsigned int here and avoids the unacceptable expensive divide instruction.

Please fix this.

Hello,

Thanks for your comments. I finally understood.

I'll fix it and resubmit the patch as soon as possible.

Regards,
Hideo

--
Hitachi Computer Products (America) Inc.

author
#10 lore raw

Re: [PATCH 2/5] accounting unit and variable

Quoted 25 lines · Hideo AOKI
Quoted 24 lines · David Miller

From: Hideo AOKI <[email protected]>
Date: Tue, 13 Nov 2007 22:27:13 -0500

Quoted 11 lines
Quoted 10 lines · Herbert Xu

include/net/sock.h static inline int sk_stream_wmem_schedul · line 735

Quoted 7 lines · Hideo AOKI
include/net/sock.h +6 -0
+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+    return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}

Does this really have to be int? Unsigned would let the compiler optimise this to a simple shift.

Quoted 5 lines

Thank you for the comment.

This inline function is used to calculate the first argument of atomic_add() and atomic_sub(). Since the argument is int, I believe that using int is better than using unsigned int.

If you know the values will always be positive, as you will know here, it is OK to us unsigned int here and avoids the unacceptable expensive divide instruction.

Quoted 5 lines
Quoted 1 line

Please fix this.

Thanks for your comments. I finally understood.

I'll fix it and resubmit the patch as soon as possible.

Let me propose the following code to use a shift instruction instead of a divide instruction. I confirmed that the code could remove a divide instruction, however, I would like to have comment on this implementation.

+#define SK_DATAGRAM_MEM_QUANTUM ((unsigned int)PAGE_SIZE) +
+static inline int sk_datagram_pages(int amt)
+{
+ /* Cast to unsigned as an optimization, since amt is always positive. */
+ return DIV_ROUND_UP((unsigned int)amt, SK_DATAGRAM_MEM_QUANTUM);
+}
+

Please let me know if there is proper coding style.

I'm re-testing the whole patch set right now. If there is no problem, I'll resubmit new patch set, which includes this fix, tomorrow.

Many thanks,
Hideo

Signature · 1 line
Hitachi Computer Products (America) Inc.
#11 lore raw

Re: [PATCH 2/5] accounting unit and variable

include/net/sock.h static inline int sk_stream_wmem_schedul · line 736

Quoted 10 lines · Hideo AOKI
include/net/sock.h +5 -0
+#define SK_DATAGRAM_MEM_QUANTUM ((unsigned int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	/* Cast to unsigned as an optimization, since amt is always 

positive. */

include/net/sock.h +3 -0
+	return DIV_ROUND_UP((unsigned int)amt, SK_DATAGRAM_MEM_QUANTUM);
+}
+

Thanks, this looks OK to me.

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author
#12 lore raw

Re: [PATCH 2/5] accounting unit and variable

Quoted 12 lines · Herbert Xu

Review

Quoted 9 lines · Hideo AOKI
(quoted diff) +5 -0
+#define SK_DATAGRAM_MEM_QUANTUM ((unsigned int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	/* Cast to unsigned as an optimization, since amt is always 

positive. */

(quoted diff) +3 -0
+	return DIV_ROUND_UP((unsigned int)amt, SK_DATAGRAM_MEM_QUANTUM);
+}
+

Thanks, this looks OK to me.

Hello,

Thank you for reviewing. Then, I'll send take 8 patch set later.

Regards,
Hideo

Signature · 1 line
Hitachi Computer Products (America) Inc.
author
#13 lore raw

PATCH 3/5 memory accounting

In reply to Hideo AOKI

This patch adds UDP memory usage accounting in IPv4.

--
Hideo Aoki
Hitachi Computer Products (America) Inc.

 af_inet.c   |   30 +++++++++++++++++++++++++++++-
 ip_output.c |   25 ++++++++++++++++++++++---
 udp.c       |    9 +++++++++
 3 files changed, 60 insertions(+), 4 deletions(-)
net/ipv4/af_inet.c +29 -1
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p2/net/ipv4/af_inet.c linux-2.6.24-rc1-mem003-ipv4-dev-p3/net/ipv4/af_inet.c
@@ -126,13 +126,41 @@ extern void ip_mc_drop_socket(struct soc
 static struct list_head inetsw[SOCK_MAX];
 static DEFINE_SPINLOCK(inetsw_lock);
 
+/**
+ *	__skb_queue_purge_and_sub_memory_allocated
+ *		- empty a list and subtruct memory allocation counter
+ *	@sk:   sk
+ *	@list: list to empty
+ *	Delete all buffers on an &sk_buff list and subtruct the
+ *	truesize of the sk_buff for memory accounting. Each buffer
+ *	is removed from the list and one reference dropped. This
+ *	function does not take the list lock and the caller must
+ *	hold the relevant locks to use it.
+ */
+static inline void __skb_queue_purge_and_sub_memory_allocated(struct sock *sk,
+					struct sk_buff_head *list)
+{
+	struct sk_buff *skb;
+	int purged_skb_size = 0;
+	while ((skb = __skb_dequeue(list)) != NULL) {
+		purged_skb_size += sk_datagram_pages(skb->truesize);
+		kfree_skb(skb);
+	}
+	atomic_sub(purged_skb_size, sk->sk_prot->memory_allocated);
+}
+
 /* New destruction routine */
 
 void inet_sock_destruct(struct sock *sk)
 {
 	struct inet_sock *inet = inet_sk(sk);
 
-	__skb_queue_purge(&sk->sk_receive_queue);
+	if (sk->sk_prot->memory_allocated && sk->sk_type != SOCK_STREAM)
+		__skb_queue_purge_and_sub_memory_allocated(sk,
+				&sk->sk_receive_queue);
+	else
+		__skb_queue_purge(&sk->sk_receive_queue);
+
 	__skb_queue_purge(&sk->sk_error_queue);
 
 	if (sk->sk_type == SOCK_STREAM && sk->sk_state != TCP_CLOSE) {
net/ipv4/ip_output.c +22 -3
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p2/net/ipv4/ip_output.c linux-2.6.24-rc1-mem003-ipv4-dev-p3/net/ipv4/ip_output.c
@@ -743,6 +743,8 @@ static inline int ip_ufo_append_data(str
 		/* specify the length of each IP datagram fragment*/
 		skb_shinfo(skb)->gso_size = mtu - fragheaderlen;
 		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
+		atomic_add(sk_datagram_pages(skb->truesize),
+			   sk->sk_prot->memory_allocated);
 		__skb_queue_tail(&sk->sk_write_queue, skb);
 
 		return 0;
@@ -924,6 +926,9 @@ alloc_new_skb:
 			}
 			if (skb == NULL)
 				goto error;
+			if (sk->sk_prot->memory_allocated)
+				atomic_add(sk_datagram_pages(skb->truesize),
+					   sk->sk_prot->memory_allocated);
 
 			/*
 			 *	Fill in the control structures
@@ -1023,6 +1028,8 @@ alloc_new_skb:
 				frag = &skb_shinfo(skb)->frags[i];
 				skb->truesize += PAGE_SIZE;
 				atomic_add(PAGE_SIZE, &sk->sk_wmem_alloc);
+				if (sk->sk_prot->memory_allocated)
+					atomic_inc(sk->sk_prot->memory_allocated);
 			} else {
 				err = -EMSGSIZE;
 				goto error;
@@ -1123,7 +1130,9 @@ ssize_t	ip_append_page(struct sock *sk, 
 			if (unlikely(!skb)) {
 				err = -ENOBUFS;
 				goto error;
-			}
+			} else if (sk->sk_prot->memory_allocated)
+				atomic_add(sk_datagram_pages(skb->truesize),
+					   sk->sk_prot->memory_allocated);
 
 			/*
 			 *	Fill in the control structures
@@ -1202,13 +1211,14 @@ int ip_push_pending_frames(struct sock *
 	struct iphdr *iph;
 	__be16 df = 0;
 	__u8 ttl;
-	int err = 0;
+	int err = 0, send_page_size;
 
 	if ((skb = __skb_dequeue(&sk->sk_write_queue)) == NULL)
 		goto out;
 	tail_skb = &(skb_shinfo(skb)->frag_list);
 
 	/* move skb->data to ip header from ext header */
+	send_page_size = sk_datagram_pages(skb->truesize);
 	if (skb->data < skb_network_header(skb))
 		__skb_pull(skb, skb_network_offset(skb));
 	while ((tmp_skb = __skb_dequeue(&sk->sk_write_queue)) != NULL) {
@@ -1218,6 +1228,7 @@ int ip_push_pending_frames(struct sock *
 		skb->len += tmp_skb->len;
 		skb->data_len += tmp_skb->len;
 		skb->truesize += tmp_skb->truesize;
+		send_page_size += sk_datagram_pages(tmp_skb->truesize);
 		__sock_put(tmp_skb->sk);
 		tmp_skb->destructor = NULL;
 		tmp_skb->sk = NULL;
@@ -1273,6 +1284,8 @@ int ip_push_pending_frames(struct sock *
 	/* Netfilter gets whole the not fragmented skb. */
 	err = NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL,
 		      skb->dst->dev, dst_output);
+	if (sk->sk_prot->memory_allocated)
+		atomic_sub(send_page_size, sk->sk_prot->memory_allocated);
 	if (err) {
 		if (err > 0)
 			err = inet->recverr ? net_xmit_errno(err) : 0;
@@ -1302,9 +1315,15 @@ void ip_flush_pending_frames(struct sock
 {
 	struct inet_sock *inet = inet_sk(sk);
 	struct sk_buff *skb;
+	int num_flush_mem = 0;
 
-	while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL)
+	while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL) {
+		num_flush_mem += sk_datagram_pages(skb->truesize);
 		kfree_skb(skb);
+	}
+
+	if (sk->sk_prot->memory_allocated)
+		atomic_sub(num_flush_mem, sk->sk_prot->memory_allocated);
 
 	inet->cork.flags &= ~IPCORK_OPT;
 	kfree(inet->cork.opt);
net/ipv4/udp.c +9 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p2/net/ipv4/udp.c linux-2.6.24-rc1-mem003-ipv4-dev-p3/net/ipv4/udp.c
@@ -894,6 +894,9 @@ try_again:
 
 out_free:
 	skb_free_datagram(sk, skb);
+	atomic_sub(sk_datagram_pages(skb->truesize),
+		   sk->sk_prot->memory_allocated);
+
 out:
 	return err;
 
@@ -901,6 +904,8 @@ csum_copy_err:
 	UDP_INC_STATS_BH(UDP_MIB_INERRORS, is_udplite);
 
 	skb_kill_datagram(sk, skb, flags);
+	atomic_sub(sk_datagram_pages(skb->truesize),
+		   sk->sk_prot->memory_allocated);
 
 	if (noblock)
 		return -EAGAIN;
@@ -1025,6 +1030,9 @@ int udp_queue_rcv_skb(struct sock * sk, 
 		goto drop;
 	}
 
+	atomic_add(sk_datagram_pages(skb->truesize),
+		   sk->sk_prot->memory_allocated);
+
 	UDP_INC_STATS_BH(UDP_MIB_INDATAGRAMS, up->pcflag);
 	return 0;
 
@@ -1449,6 +1457,7 @@ struct proto udp_prot = {
 	.hash		   = udp_lib_hash,
 	.unhash		   = udp_lib_unhash,
 	.get_port	   = udp_v4_get_port,
+	.memory_allocated  = &udp_memory_allocated,
 	.obj_size	   = sizeof(struct udp_sock),
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_udp_setsockopt,
#14 lore raw

Re: [PATCH 3/5] memory accounting

Quoted 9 lines · Hideo AOKI

This patch adds UDP memory usage accounting in IPv4.

--
Hideo Aoki
Hitachi Computer Products (America) Inc.

This looks fine for now. Although in future I'd prefer this to use the forward alloc model used by stream sockets. In fact we should be able to share most of that code too.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author
#15 lore raw

PATCH 4/5 memory limitation by using udp_mem

In reply to Hideo AOKI

This patch introduces memory limitation for UDP.

--
Hideo Aoki
Hitachi Computer Products (America) Inc.

 Documentation/networking/ip-sysctl.txt |    6 ++++
 include/net/udp.h                      |    3 ++
 net/ipv4/af_inet.c                     |    3 ++
 net/ipv4/ip_output.c                   |   47 ++++++++++++++++++++++++++++++---
 net/ipv4/sysctl_net_ipv4.c             |   11 +++++++
 net/ipv4/udp.c                         |   24 ++++++++++++++++
 6 files changed, 91 insertions(+), 3 deletions(-)
Documentation/networking/ip-sysctl.txt +6 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p3/Documentation/networking/ip-sysctl.txt linux-2.6.24-rc1-mem003-ipv4-dev-p4/Documentation/networking/ip-sysctl.txt
@@ -446,6 +446,12 @@ tcp_dma_copybreak - INTEGER
 	and CONFIG_NET_DMA is enabled.
 	Default: 4096
 
+UDP variables:
+
+udp_mem - INTEGER
+	Number of pages allowed for queueing by all UDP sockets.
+	Default is calculated at boot time from amount of available memory.
+
 CIPSOv4 Variables:
 
 cipso_cache_enable - BOOLEAN
include/net/udp.h +3 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p3/include/net/udp.h linux-2.6.24-rc1-mem003-ipv4-dev-p4/include/net/udp.h
@@ -66,6 +66,7 @@ extern rwlock_t udp_hash_lock;
 extern struct proto udp_prot;
 
 extern atomic_t udp_memory_allocated;
+extern int sysctl_udp_mem;
 
 struct sk_buff;
 
@@ -175,4 +176,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 */
net/ipv4/af_inet.c +3 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p3/net/ipv4/af_inet.c linux-2.6.24-rc1-mem003-ipv4-dev-p4/net/ipv4/af_inet.c
@@ -1446,6 +1446,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();
 
net/ipv4/ip_output.c +44 -3

102 lines not on this page · raw message

net/ipv4/sysctl_net_ipv4.c +11 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p3/net/ipv4/sysctl_net_ipv4.c linux-2.6.24-rc1-mem003-ipv4-dev-p4/net/ipv4/sysctl_net_ipv4.c
@@ -18,6 +18,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>
 
@@ -885,6 +886,16 @@ 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 = 0 }
 };
 
net/ipv4/udp.c +24 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p3/net/ipv4/udp.c linux-2.6.24-rc1-mem003-ipv4-dev-p4/net/ipv4/udp.c
@@ -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>
@@ -115,6 +116,7 @@ struct hlist_head udp_hash[UDP_HTABLE_SI
 DEFINE_RWLOCK(udp_hash_lock);
 
 atomic_t udp_memory_allocated;
+int sysctl_udp_mem __read_mostly;
 
 static inline int __udp_lib_lport_inuse(__u16 num,
 					const struct hlist_head udptable[])
@@ -1023,6 +1025,13 @@ int udp_queue_rcv_skb(struct sock * sk, 
 			goto drop;
 	}
 
+	if ((atomic_read(sk->sk_prot->memory_allocated)
+		       + sk_datagram_pages(skb->truesize))
+		> sk->sk_prot->sysctl_mem[0]) {
+		UDP_INC_STATS_BH(UDP_MIB_RCVBUFERRORS, up->pcflag);
+		goto drop;
+	}
+
 	if ((rc = sock_queue_rcv_skb(sk,skb)) < 0) {
 		/* Note that an ENOMEM error is charged twice */
 		if (rc == -ENOMEM)
@@ -1458,6 +1467,7 @@ struct proto udp_prot = {
 	.unhash		   = udp_lib_unhash,
 	.get_port	   = udp_v4_get_port,
 	.memory_allocated  = &udp_memory_allocated,
+	.sysctl_mem	   = &sysctl_udp_mem,
 	.obj_size	   = sizeof(struct udp_sock),
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_udp_setsockopt,
@@ -1652,6 +1662,20 @@ 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 = limit / 2 * 3;
+}
+
 EXPORT_SYMBOL(udp_disconnect);
 EXPORT_SYMBOL(udp_hash);
 EXPORT_SYMBOL(udp_hash_lock);
author
#16 lore raw

PATCH 5/5 introduce udp_rmem and udp_wmem

In reply to Hideo AOKI

This patch added /proc/sys/net/udp_rmem and /proc/sys/net/udp_rmem. Each UDP packet is drooped when the number of pages for socket buffer is beyond the limit and the socket already consumes minimum buffer.

--
Hideo Aoki
Hitachi Computer Products (America) Inc.

Cc Satoshi Oshima <[email protected]>
 Documentation/networking/ip-sysctl.txt |   12 ++++++++++++
 include/net/udp.h                      |    4 ++++
 net/ipv4/ip_output.c                   |    4 +++-
 net/ipv4/sysctl_net_ipv4.c             |   20 ++++++++++++++++++++
 net/ipv4/udp.c                         |   13 +++++++++++--
 5 files changed, 50 insertions(+), 3 deletions(-)
Documentation/networking/ip-sysctl.txt +12 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p4/Documentation/networking/ip-sysctl.txt linux-2.6.24-rc1-mem003-ipv4-dev-p5/Documentation/networking/ip-sysctl.txt
@@ -452,6 +452,18 @@ udp_mem - INTEGER
 	Number of pages allowed for queueing by all UDP sockets.
 	Default is calculated at boot time from amount of available memory.
 
+udp_rmem - INTEGER
+	Minimal size of receive buffer used by UDP sockets. Each UDP socket
+	is able to use the size for receiving data, even if total pages of UDP
+	sockets exceed udp_mem. The unit is byte.
+	Default: 4096
+
+udp_wmem - INTEGER
+	Minimal size of send buffer used by UDP sockets. Each UDP socket is
+	able to use the size for sending data, even if total pages of UDP
+	sockets exceed udp_mem. The unit is byte.
+	Default: 4096
+
 CIPSOv4 Variables:
 
 cipso_cache_enable - BOOLEAN
include/net/udp.h +4 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p4/include/net/udp.h linux-2.6.24-rc1-mem003-ipv4-dev-p5/include/net/udp.h
@@ -66,7 +66,11 @@ 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;
+extern int sysctl_udp_rmem;
+extern int sysctl_udp_wmem;
 
 struct sk_buff;
 
net/ipv4/ip_output.c +3 -1
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p4/net/ipv4/ip_output.c linux-2.6.24-rc1-mem003-ipv4-dev-p5/net/ipv4/ip_output.c
@@ -705,7 +705,9 @@ static inline int __ip_check_max_skb_pag
 	switch(sk->sk_protocol) {
 	case IPPROTO_UDP:
 		if (atomic_read(sk->sk_prot->memory_allocated) + size
-		    > sk->sk_prot->sysctl_mem[0])
+		    > sk->sk_prot->sysctl_mem[0] &&
+		    atomic_read(&sk->sk_wmem_alloc) + size
+		    > sk->sk_prot->sysctl_wmem[0])
 			return -ENOBUFS;
 		/* Fall through */	
 	default:
net/ipv4/sysctl_net_ipv4.c +20 -0
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p4/net/ipv4/sysctl_net_ipv4.c linux-2.6.24-rc1-mem003-ipv4-dev-p5/net/ipv4/sysctl_net_ipv4.c
@@ -896,6 +896,26 @@ ctl_table ipv4_table[] = {
 		.strategy	= &sysctl_intvec,
 		.extra1		= &zero
 	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "udp_rmem",
+		.data		= &sysctl_udp_rmem,
+		.maxlen		= sizeof(sysctl_udp_rmem),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &zero
+	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "udp_wmem",
+		.data		= &sysctl_udp_wmem,
+		.maxlen		= sizeof(sysctl_udp_wmem),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &zero
+	},
 	{ .ctl_name = 0 }
 };
 
net/ipv4/udp.c +11 -2
diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p4/net/ipv4/udp.c linux-2.6.24-rc1-mem003-ipv4-dev-p5/net/ipv4/udp.c
@@ -117,6 +117,8 @@ DEFINE_RWLOCK(udp_hash_lock);
 
 atomic_t udp_memory_allocated;
 int sysctl_udp_mem __read_mostly;
+int sysctl_udp_rmem __read_mostly;
+int sysctl_udp_wmem __read_mostly;
 
 static inline int __udp_lib_lport_inuse(__u16 num,
 					const struct hlist_head udptable[])
@@ -1026,8 +1028,10 @@ int udp_queue_rcv_skb(struct sock * sk, 
 	}
 
 	if ((atomic_read(sk->sk_prot->memory_allocated)
-		       + sk_datagram_pages(skb->truesize))
-		> sk->sk_prot->sysctl_mem[0]) {
+	     + sk_datagram_pages(skb->truesize))
+	    > sk->sk_prot->sysctl_mem[0] &&
+	    atomic_read(&sk->sk_rmem_alloc) + skb->truesize 
+	    > sk->sk_prot->sysctl_rmem[0]) {
 		UDP_INC_STATS_BH(UDP_MIB_RCVBUFERRORS, up->pcflag);
 		goto drop;
 	}
@@ -1468,6 +1472,8 @@ struct proto udp_prot = {
 	.get_port	   = udp_v4_get_port,
 	.memory_allocated  = &udp_memory_allocated,
 	.sysctl_mem	   = &sysctl_udp_mem,
+	.sysctl_wmem	   = &sysctl_udp_wmem,
+	.sysctl_rmem	   = &sysctl_udp_rmem,
 	.obj_size	   = sizeof(struct udp_sock),
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_udp_setsockopt,
@@ -1674,6 +1680,9 @@ void __init udp_init(void)
 	limit = (limit * (nr_all_pages >> (20-PAGE_SHIFT))) >> (PAGE_SHIFT-11);
 	limit = max(limit, 128UL);
 	sysctl_udp_mem = limit / 2 * 3;
+
+	sysctl_udp_rmem = SK_DATAGRAM_MEM_QUANTUM;
+	sysctl_udp_wmem = SK_DATAGRAM_MEM_QUANTUM;
 }
 
 EXPORT_SYMBOL(udp_disconnect);
#17 lore raw

Re: [PATCH 5/5] introduce udp_rmem and udp_wmem

Quoted 3 lines · Hideo AOKI

This patch added /proc/sys/net/udp_rmem and /proc/sys/net/udp_rmem. Each UDP packet is drooped when the number of pages for socket buffer is beyond the limit and the socket already consumes minimum buffer.

I think you meant /proc/sys/net/ipv4/udp_{r,w}mem above.

Patch not in-lined making replying more difficult.

Cutting and pasting:

Quoted 12 lines

diff -pruN linux-2.6.24-rc1-mem003-ipv4-dev-p4/Documentation/networking/ip-sysctl.txt linux-2.6.24-rc1-mem003-ipv4-dev-p5/Documentation/networking/ip-sysctl.txt

Documentation/networking/ip-sysctl.txt +5 -0
@@ -452,6 +452,18 @@ udp_mem - INTEGER
 	Number of pages allowed for queueing by all UDP sockets.
 	Default is calculated at boot time from amount of available memory.
 
+udp_rmem - INTEGER
+	Minimal size of receive buffer used by UDP sockets. Each UDP socket
+	is able to use the size for receiving data, even if total pages of UDP
+	sockets exceed udp_mem. The unit is byte.
+	Default: 4096

Documentation/networking/ip-sysctl.txt udp_mem - INTEGER · line 469

Quoted 10 lines
Documentation/networking/ip-sysctl.txt +7 -0
+
+udp_wmem - INTEGER
+	Minimal size of send buffer used by UDP sockets. Each UDP socket is
+	able to use the size for sending data, even if total pages of UDP
+	sockets exceed udp_mem. The unit is byte.
+	Default: 4096
+
 CIPSOv4 Variables:
 
 cipso_cache_enable - BOOLEAN

I think either the above should be renamed to udp_{r,w}mem_min, or they should be changed to a 3-tuple like tcp_{r,w}mem, and the code refactored accordingly (but then what to do about /proc/sys/net/core/{r,w}mem_max).

-Bill

author
#18 lore raw

Re: [PATCH 5/5] introduce udp_rmem and udp_wmem

Hello,

I'm sorry to not respond quickly.

Quoted 7 lines · Bill Fink
Quoted 3 lines · Hideo AOKI

This patch added /proc/sys/net/udp_rmem and /proc/sys/net/udp_rmem. Each UDP packet is drooped when the number of pages for socket buffer is beyond the limit and the socket already consumes minimum buffer.

I think you meant /proc/sys/net/ipv4/udp_{r,w}mem above.

You're right. New parameters are added to /proc/sys/net/ipv4.

Quoted 1 line

Patch not in-lined making replying more difficult.

I apologize for the inconvenient. I'll send patch as in-line next time.

Quoted 14 lines
Quoted 14 lines
Quoted 1 line

+udp_rmem - INTEGER

Documentation/networking/ip-sysctl.txt +11 -0
+	Minimal size of receive buffer used by UDP sockets. Each UDP socket
+	is able to use the size for receiving data, even if total pages of UDP
+	sockets exceed udp_mem. The unit is byte.
+	Default: 4096
+
+udp_wmem - INTEGER
+	Minimal size of send buffer used by UDP sockets. Each UDP socket is
+	able to use the size for sending data, even if total pages of UDP
+	sockets exceed udp_mem. The unit is byte.
+	Default: 4096
+
 CIPSOv4 Variables:
Quoted 8 lines
Quoted 1 line

cipso_cache_enable - BOOLEAN

I think either the above should be renamed to udp_{r,w}mem_min, or they should be changed to a 3-tuple like tcp_{r,w}mem, and the code refactored accordingly (but then what to do about /proc/sys/net/core/{r,w}mem_max).

-Bill

I understood. Then, I'll rename them to udp_{r,w}mem_min next take. Please let me know if there is any suggestions.

Many thanks,
Hideo

Signature · 1 line
Hitachi Computer Products (America) Inc.