Skip to contentSkip to conversation

SCTP: Rewrite of sctp buffer management code

PATCH Applied Vlad Yasevich 3 participants 8 messages Aug 10, 2007 – Aug 15, 2007

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 SCTP: Rewrite of sctp buffer management code

This patch introduces autotuning to the sctp buffer management code similar to the TCP. The buffer space can be grown if the advertised receive window still has room. This might happen if small message sizes are used, which is common in telecom environmens. New tunables are introduced that provide limits to buffer growth and memory pressure is entered if to much buffer spaces is used.

 include/net/sctp/sctp.h |    5 +++
 net/sctp/endpointola.c  |    1
 net/sctp/protocol.c     |   32 +++++++++++++++++++++
 net/sctp/sm_statefuns.c |   73 +++++++++++-------------------------------------
 net/sctp/socket.c       |   67 ++++++++++++++++++++++++++++++++++++--------
 net/sctp/sysctl.c       |   33 +++++++++++++++++++++
 net/sctp/ulpevent.c     |   18 +++++++++++
 net/sctp/ulpqueue.c     |    1
 8 files changed, 163 insertions(+), 67 deletions(-)
---
 net/sctp/socket.c |   68 ++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 55 insertions(+), 13 deletions(-)
net/sctp/socket.c +55 -13
index 01c6364..03579d5 100644
@@ -106,25 +106,41 @@ static int sctp_autobind(struct sock *sk);
 static void sctp_sock_migrate(struct sock *, struct sock *,
 			      struct sctp_association *, sctp_socket_type_t);
 static char *sctp_hmac_alg = SCTP_COOKIE_HMAC_ALG;
+extern int sysctl_sctp_mem[3];
+extern int sysctl_sctp_rmem[3];
+extern int sysctl_sctp_wmem[3];
 
+int sctp_memory_pressure;
+atomic_t sctp_memory_allocated;
+atomic_t sctp_sockets_allocated;
+
+static void sctp_enter_memory_pressure(void)
+{
+	sctp_memory_pressure = 1;
+}
+ 
 /* Get the sndbuf space available at the time on the association.  */
 static inline int sctp_wspace(struct sctp_association *asoc)
 {
-	struct sock *sk = asoc->base.sk;
-	int amt = 0;
+ 	int amt;
 
-	if (asoc->ep->sndbuf_policy) {
-		/* make sure that no association uses more than sk_sndbuf */
-		amt = sk->sk_sndbuf - asoc->sndbuf_used;
+	if (asoc->ep->sndbuf_policy)
+		amt = asoc->sndbuf_used;
+	else
+		amt = atomic_read(&asoc->base.sk->sk_wmem_alloc);
+
+	if (amt >= asoc->base.sk->sk_sndbuf) {
+		if (asoc->base.sk->sk_userlocks & SOCK_SNDBUF_LOCK)
+			amt = 0;
+		else {
+			amt = sk_stream_wspace(asoc->base.sk);
+			if (amt < 0)
+				amt = 0;
+		}
 	} else {
-		/* do socket level accounting */
-		amt = sk->sk_sndbuf - atomic_read(&sk->sk_wmem_alloc);
+ 		amt = asoc->base.sk->sk_sndbuf - amt;
 	}
-
-	if (amt < 0)
-		amt = 0;
-
-	return amt;
+  	return amt;
 }
 
 /* Increment the used sndbuf space count of the corresponding association by
@@ -155,6 +171,7 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
 				sizeof(struct sctp_chunk);
 
 	atomic_add(sizeof(struct sctp_chunk), &sk->sk_wmem_alloc);
+	sk_charge_skb(sk, chunk->skb);
 }
 
 /* Verify that this is a valid address. */
@@ -3314,6 +3331,7 @@ SCTP_STATIC int sctp_init_sock(struct sock *sk)
 	sp->hmac = NULL;
 
 	SCTP_DBG_OBJCNT_INC(sock);
+	atomic_inc(&sctp_sockets_allocated);
 	return 0;
 }
 
@@ -3327,7 +3345,7 @@ SCTP_STATIC int sctp_destroy_sock(struct sock *sk)
 	/* Release our hold on the endpoint. */
 	ep = sctp_sk(sk)->ep;
 	sctp_endpoint_free(ep);
-
+	atomic_dec(&sctp_sockets_allocated);
 	return 0;
 }
 
@@ -5747,6 +5765,12 @@ static void sctp_wfree(struct sk_buff *skb)
 
 	atomic_sub(sizeof(struct sctp_chunk), &sk->sk_wmem_alloc);
 
+	/*
+	 * This undoes what is done via sk_charge_skb
+	 */
+	sk->sk_wmem_queued   -= skb->truesize;
+	sk->sk_forward_alloc += skb->truesize;
+
 	sock_wfree(skb);
 	__sctp_write_space(asoc);
 
@@ -5764,6 +5788,11 @@ void sctp_sock_rfree(struct sk_buff *skb)
 	struct sctp_ulpevent *event = sctp_skb2event(skb);
 
 	atomic_sub(event->rmem_len, &sk->sk_rmem_alloc);
+
+	/*
+	 * Mimic the behavior of sk_stream_rfree
+	 */
+	sk->sk_forward_alloc += event->rmem_len;
 }
 
 
@@ -6153,6 +6182,7 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 	sctp_release_sock(newsk);
 }
 
+
 /* This proto struct describes the ULP interface for SCTP.  */
 struct proto sctp_prot = {
 	.name        =	"SCTP",
@@ -6175,6 +6205,12 @@ struct proto sctp_prot = {
 	.unhash      =	sctp_unhash,
 	.get_port    =	sctp_get_port,
 	.obj_size    =  sizeof(struct sctp_sock),
+	.sysctl_mem  =  sysctl_sctp_mem,
+	.sysctl_rmem =  sysctl_sctp_rmem,
+	.sysctl_wmem =  sysctl_sctp_wmem,
+	.memory_pressure = &sctp_memory_pressure,
+	.enter_memory_pressure = sctp_enter_memory_pressure,
+	.memory_allocated = &sctp_memory_allocated,
 };
 
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
@@ -6199,5 +6235,11 @@ struct proto sctpv6_prot = {
 	.unhash		= sctp_unhash,
 	.get_port	= sctp_get_port,
 	.obj_size	= sizeof(struct sctp6_sock),
+	.sysctl_mem	= sysctl_sctp_mem,
+	.sysctl_rmem	= sysctl_sctp_rmem,
+	.sysctl_wmem	= sysctl_sctp_wmem,
+	.memory_pressure = &sctp_memory_pressure,
+	.enter_memory_pressure = sctp_enter_memory_pressure,
+	.memory_allocated = &sctp_memory_allocated,
 };
 #endif /* defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) */
Signature · 1 line
1.5.2.4
maintainer Applied
#1 lore raw

From: Vlad Yasevich <[email protected]>
Date: Fri, 10 Aug 2007 16:46:03 -0400

Quoted 9 lines

This patch introduces autotuning to the sctp buffer management code similar to the TCP. The buffer space can be grown if the advertised receive window still has room. This might happen if small message sizes are used, which is common in telecom environmens. New tunables are introduced that provide limits to buffer growth and memory pressure is entered if to much buffer spaces is used.

Applied, thanks Vlad.

Can you in the future fix up whitespace issues like this:

Adds trailing whitespace.
diff:21:
Space in indent is followed by a tab.
diff:27: int amt;
Space in indent is followed by a tab.
diff:48: amt = asoc->base.sk->sk_sndbuf - amt;
Space in indent is followed by a tab.
diff:55: return amt;

I know where the "Space in indent" cases come from, you apply the patch to merge it into a current tree, you get rejects, then you edit in the foo.rej hunks by hand into the code but forget to remove the extra leading whitespace characters.

:-)

#2 lore raw
Quoted 26 lines · David Miller

From: Vlad Yasevich <[email protected]>
Date: Fri, 10 Aug 2007 16:46:03 -0400

Quoted 9 lines

This patch introduces autotuning to the sctp buffer management code similar to the TCP. The buffer space can be grown if the advertised receive window still has room. This might happen if small message sizes are used, which is common in telecom environmens. New tunables are introduced that provide limits to buffer growth and memory pressure is entered if to much buffer spaces is used.

Applied, thanks Vlad.

Can you in the future fix up whitespace issues like this:

Adds trailing whitespace.
diff:21:
Space in indent is followed by a tab.
diff:27: int amt;
Space in indent is followed by a tab.
diff:48: amt = asoc->base.sk->sk_sndbuf - amt;
Space in indent is followed by a tab.
diff:55: return amt;

Quoted 6 lines

I know where the "Space in indent" cases come from, you apply the patch to merge it into a current tree, you get rejects, then you edit in the foo.rej hunks by hand into the code but forget to remove the extra leading whitespace characters.

:-)

Thanks for applying this Dave, but something seems to have occured during your application. The commitdiff:
http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.24.git;a=commitdiff;h=e20e5d698dcccd2e84ee2e1e482b0b203f885c00
only shows applications to socket.c, while the patch that vlad sent you changed much more than that. Not sure If I'm seeing something thats transient, but I wanted to bring it to your attention in the event that something has in fact gone awry.

Thanks & Regards to you both
Neil

Signature · 8 lines
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *[email protected]
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/
maintainer
#3 lore raw

From: Neil Horman <[email protected]>
Date: Fri, 10 Aug 2007 19:01:26 -0400

Quoted 5 lines

Thanks for applying this Dave, but something seems to have occured during your application. The commitdiff:
http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.24.git;a=commitdiff;h=e20e5d698dcccd2e84ee2e1e482b0b203f885c00
only shows applications to socket.c, while the patch that vlad sent you changed much more than that.

Vlad's posted patch only had the socket.c changes, that's the problem. I just double checked in the archives on marc.info

I'm reverting this, please sort this out.

#4 lore raw
Quoted 9 lines · David Miller

From: Neil Horman <[email protected]>
Date: Fri, 10 Aug 2007 19:01:26 -0400

Quoted 5 lines

Thanks for applying this Dave, but something seems to have occured during your application. The commitdiff:
http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.24.git;a=commitdiff;h=e20e5d698dcccd2e84ee2e1e482b0b203f885c00
only shows applications to socket.c, while the patch that vlad sent you changed much more than that.

Quoted 4 lines

Vlad's posted patch only had the socket.c changes, that's the problem. I just double checked in the archives on marc.info

I'm reverting this, please sort this out.

Vlad, this looks hosed in the same way in your lksctp-dev tree as well (Sorry I didn't notice that sooner). Can you dig the patch out of the lksctp archives? Or shall I repost it to you?

Neil

Signature · 8 lines
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *[email protected]
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/
#5 lore raw

Dave, Neil

Sorry about that. Not sure what happened to that patch. Here is the good patch with witespace cleanups.

-vlad

--

SCTP: Rewrite of sctp buffer management code

This patch introduces autotuning to the sctp buffer management code similar to the TCP. The buffer space can be grown if the advertised receive window still has room. This might happen if small message sizes are used, which is common in telecom environmens. New tunables are introduced that provide limits to buffer growth and memory pressure is entered if to much buffer spaces is used.

 include/net/sctp/sctp.h |    5 +++
 net/sctp/endpointola.c  |    1
 net/sctp/protocol.c     |   32 +++++++++++++++++++++
 net/sctp/sm_statefuns.c |   73 +++++++++++-------------------------------------
 net/sctp/socket.c       |   67 ++++++++++++++++++++++++++++++++++++--------
 net/sctp/sysctl.c       |   33 +++++++++++++++++++++
 net/sctp/ulpevent.c     |   18 +++++++++++
 net/sctp/ulpqueue.c     |    1
 8 files changed, 163 insertions(+), 67 deletions(-)
---
 include/net/sctp/sctp.h |    5 +++
 net/sctp/endpointola.c  |    1 +
 net/sctp/protocol.c     |   32 ++++++++++++++++++++
 net/sctp/sm_statefuns.c |   74 +++++++++++-----------------------------------
 net/sctp/socket.c       |   69 ++++++++++++++++++++++++++++++++++++-------
 net/sctp/sysctl.c       |   33 +++++++++++++++++++++
 net/sctp/ulpevent.c     |   18 +++++++++++
 net/sctp/ulpqueue.c     |    1 +
 8 files changed, 165 insertions(+), 68 deletions(-)
include/net/sctp/sctp.h +5 -0
index d529045..46d7d09 100644
@@ -468,6 +468,11 @@ static inline void sctp_skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
 	skb->sk = sk;
 	skb->destructor = sctp_sock_rfree;
 	atomic_add(event->rmem_len, &sk->sk_rmem_alloc);
+	/*
+	 * This mimics the behavior of
+	 * sk_stream_set_owner_r
+	 */
+	sk->sk_forward_alloc -= event->rmem_len;
 }
 
 /* Tests if the list has one and only one entry. */
net/sctp/endpointola.c +1 -0
index 1404a9e..aba9258 100644
@@ -103,6 +103,7 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
 
 	/* Use SCTP specific send buffer space queues.  */
 	ep->sndbuf_policy = sctp_sndbuf_policy;
+
 	sk->sk_write_space = sctp_write_space;
 	sock_set_flag(sk, SOCK_USE_WRITE_QUEUE);
 
net/sctp/protocol.c +32 -0
index e98579b..a746ebf 100644
@@ -51,6 +51,7 @@
 #include <linux/netdevice.h>
 #include <linux/inetdevice.h>
 #include <linux/seq_file.h>
+#include <linux/bootmem.h>
 #include <net/protocol.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
@@ -82,6 +83,10 @@ static struct sctp_af *sctp_af_v6_specific;
 struct kmem_cache *sctp_chunk_cachep __read_mostly;
 struct kmem_cache *sctp_bucket_cachep __read_mostly;
 
+extern int sysctl_sctp_mem[3];
+extern int sysctl_sctp_rmem[3];
+extern int sysctl_sctp_wmem[3];
+
 /* Return the address of the control sock. */
 struct sock *sctp_get_ctl_sock(void)
 {
@@ -969,6 +974,8 @@ SCTP_STATIC __init int sctp_init(void)
 	int i;
 	int status = -EINVAL;
 	unsigned long goal;
+	unsigned long limit;
+	int max_share;
 	int order;
 
 	/* SCTP_DEBUG sanity check. */
@@ -1059,6 +1066,31 @@ SCTP_STATIC __init int sctp_init(void)
 	/* Initialize handle used for association ids. */
 	idr_init(&sctp_assocs_id);
 
+	/* Set the pressure threshold to be 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.
+	 * Note this initalizes the data in sctpv6_prot too
+	 * Unabashedly stolen from tcp_init
+	 */
+	limit = min(num_physpages, 1UL<<(28-PAGE_SHIFT)) >> (20-PAGE_SHIFT);
+	limit = (limit * (num_physpages >> (20-PAGE_SHIFT))) >> (PAGE_SHIFT-11);
+	limit = max(limit, 128UL);
+	sysctl_sctp_mem[0] = limit / 4 * 3;
+	sysctl_sctp_mem[1] = limit;
+	sysctl_sctp_mem[2] = sysctl_sctp_mem[0] * 2;
+
+	/* Set per-socket limits to no more than 1/128 the pressure threshold*/
+	limit = (sysctl_sctp_mem[1]) << (PAGE_SHIFT - 7);
+	max_share = min(4UL*1024*1024, limit);
+
+	sysctl_sctp_rmem[0] = PAGE_SIZE; /* give each asoc 1 page min */
+	sysctl_sctp_rmem[1] = (1500 *(sizeof(struct sk_buff) + 1));
+	sysctl_sctp_rmem[2] = max(sysctl_sctp_rmem[1], max_share);
+
+	sysctl_sctp_wmem[0] = SK_STREAM_MEM_QUANTUM;
+	sysctl_sctp_wmem[1] = 16*1024;
+	sysctl_sctp_wmem[2] = max(64*1024, max_share);
+
 	/* Size and allocate the association hash table.
 	 * The methodology is similar to that of the tcp hash tables.
 	 */
net/sctp/sm_statefuns.c +18 -56

100 lines not on this page · raw message

net/sctp/socket.c +57 -12

128 lines not on this page · raw message

net/sctp/sysctl.c +33 -0
index e2c679b..ba75ef4 100644
@@ -52,6 +52,15 @@ static int int_max = INT_MAX;
 static long sack_timer_min = 1;
 static long sack_timer_max = 500;
 
+int sysctl_sctp_mem[3];
+int sysctl_sctp_rmem[3];
+int sysctl_sctp_wmem[3];
+
+/*
+ * per assoc memory limitationf for sends
+ */
+int sysctl_sctp_wmem[3];
+
 static ctl_table sctp_table[] = {
 	{
 		.ctl_name	= NET_SCTP_RTO_INITIAL,
@@ -226,6 +235,30 @@ static ctl_table sctp_table[] = {
 		.extra1         = &sack_timer_min,
 		.extra2         = &sack_timer_max,
 	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "sctp_mem",
+		.data		= &sysctl_sctp_mem,
+		.maxlen		= sizeof(sysctl_sctp_mem),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "sctp_rmem",
+		.data		= &sysctl_sctp_rmem,
+		.maxlen		= sizeof(sysctl_sctp_rmem),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "sctp_wmem",
+		.data		= &sysctl_sctp_wmem,
+		.maxlen		= sizeof(sysctl_sctp_wmem),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
 	{ .ctl_name = 0 }
 };
 
net/sctp/ulpevent.c +18 -0
index bfecb35..5dc094b 100644
@@ -685,6 +685,24 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
 	struct sctp_ulpevent *event = NULL;
 	struct sk_buff *skb;
 	size_t padding, len;
+	int rx_count;
+
+	/*
+	 * check to see if we need to make space for this
+	 * new skb, expand the rcvbuffer if needed, or drop
+	 * the frame
+	 */
+	if (asoc->ep->rcvbuf_policy)
+		rx_count = atomic_read(&asoc->rmem_alloc);
+	else
+		rx_count = atomic_read(&asoc->base.sk->sk_rmem_alloc);
+
+	if (rx_count >= asoc->base.sk->sk_rcvbuf) {
+
+		if ((asoc->base.sk->sk_userlocks & SOCK_RCVBUF_LOCK) ||
+		   (!sk_stream_rmem_schedule(asoc->base.sk, chunk->skb)))
+			goto fail;
+	}
 
 	/* Clone the original skb, sharing the data.  */
 	skb = skb_clone(chunk->skb, gfp);
net/sctp/ulpqueue.c +1 -0
index 34eb977..33e49b5 100644
@@ -978,6 +978,7 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
 		sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
 	}
 
+	sk_stream_mem_reclaim(asoc->base.sk);
 	return;
 }
 
Signature · 1 line
1.5.2.4
#6 lore raw
Quoted 7 lines · Vlad Yasevich

Dave, Neil

Sorry about that. Not sure what happened to that patch. Here is the good patch with witespace cleanups.

-vlad

Thank you Vlad, that looks much better. Not sure what happened either Regards
Neil

Signature · 8 lines
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *[email protected]
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/
maintainer Applied
#7 lore raw
In reply to Vlad Yasevich

From: Vlad Yasevich <[email protected]>
Date: Mon, 13 Aug 2007 09:24:00 -0400

Quoted 2 lines

Sorry about that. Not sure what happened to that patch. Here is the good patch with witespace cleanups.

Applied to net-2.6.24, thanks for fixing this patch up.