Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net-next 0/2] sctp: fully support memory accounting
@ 2019-03-31  8:53 Xin Long
  2019-03-31  8:53 ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Xin Long
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Xin Long @ 2019-03-31  8:53 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: Marcelo Ricardo Leitner, Neil Horman, davem, Matteo Croce, Vladis Dronov

sctp memory accounting is added in this patchset by using
these kernel APIs on send side:

  - sk_mem_charge()
  - sk_mem_uncharge()
  - sk_wmem_schedule()
  - sk_under_memory_pressure()
  - sk_mem_reclaim()

and these on receive side:

  - sk_mem_charge()
  - sk_mem_uncharge()
  - sk_rmem_schedule()
  - sk_under_memory_pressure()
  - sk_mem_reclaim()

With sctp memory accounting, we can limit the memory allocation by
either sysctl:

  # sysctl -w net.sctp.sctp_mem="10 20 50"

or cgroup:

  # echo $((8<<14)) > \
    /sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes

When the socket is under memory pressure, the send side will block
and wait, while the receive side will renege or drop.

Xin Long (2):
  sctp: implement memory accounting on tx path
  sctp: implement memory accounting on rx path

 include/net/sctp/sctp.h |  2 +-
 net/sctp/sm_statefuns.c |  6 ++++--
 net/sctp/socket.c       | 10 ++++++++--
 net/sctp/ulpevent.c     | 19 ++++++++-----------
 net/sctp/ulpqueue.c     |  3 ++-
 5 files changed, 23 insertions(+), 17 deletions(-)

-- 
2.1.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net-next 1/2] sctp: implement memory accounting on tx path
  2019-03-31  8:53 [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
@ 2019-03-31  8:53 ` Xin Long
  2019-03-31  8:53   ` [PATCH net-next 2/2] sctp: implement memory accounting on rx path Xin Long
  2019-04-02 11:48   ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Marcelo Ricardo Leitner
  2019-03-31  8:56 ` [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Xin Long @ 2019-03-31  8:53 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: Marcelo Ricardo Leitner, Neil Horman, davem, Matteo Croce, Vladis Dronov

Now when sending packets, sk_mem_charge() and sk_mem_uncharge() have been
used to set sk_forward_alloc. We just need to call sk_wmem_schedule() to
check if the allocated should be raised, and call sk_mem_reclaim() to
check if the allocated should be reduced when it's under memory pressure.

If sk_wmem_schedule() returns false, which means no memory is allowed to
allocate, it will block and wait for memory to become available.

Note different from tcp, sctp wait_for_buf happens before allocating any
skb, so memory accounting check is done with the whole msg_len before it
too.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/socket.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 6140471..06c6f4a 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1913,7 +1913,10 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 	if (sctp_wspace(asoc) < (int)msg_len)
 		sctp_prsctp_prune(asoc, sinfo, msg_len - sctp_wspace(asoc));
 
-	if (sctp_wspace(asoc) <= 0) {
+	if (sk_under_memory_pressure(sk))
+		sk_mem_reclaim(sk);
+
+	if (sctp_wspace(asoc) <= 0 || !sk_wmem_schedule(sk, msg_len)) {
 		timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
 		err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len);
 		if (err)
@@ -8891,7 +8894,10 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
 			goto do_error;
 		if (signal_pending(current))
 			goto do_interrupted;
-		if ((int)msg_len <= sctp_wspace(asoc))
+		if (sk_under_memory_pressure(sk))
+			sk_mem_reclaim(sk);
+		if ((int)msg_len <= sctp_wspace(asoc) &&
+		    sk_wmem_schedule(sk, msg_len))
 			break;
 
 		/* Let another process have a go.  Since we are going
-- 
2.1.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net-next 2/2] sctp: implement memory accounting on rx path
  2019-03-31  8:53 ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Xin Long
@ 2019-03-31  8:53   ` Xin Long
  2019-04-02 11:48     ` Marcelo Ricardo Leitner
  2019-04-02 11:48   ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Marcelo Ricardo Leitner
  1 sibling, 1 reply; 17+ messages in thread
From: Xin Long @ 2019-03-31  8:53 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: Marcelo Ricardo Leitner, Neil Horman, davem, Matteo Croce, Vladis Dronov

sk_forward_alloc's updating is also done on rx path, but to be consistent
we change to use sk_mem_charge() in sctp_skb_set_owner_r().

In sctp_eat_data(), it's not enough to check sctp_memory_pressure only,
which doesn't work for mem_cgroup_sockets_enabled, so we change to use
sk_under_memory_pressure().

When it's under memory pressure, sk_mem_reclaim() and sk_rmem_schedule()
should be called on both RENEGE or CHUNK DELIVERY path exit the memory
pressure status as soon as possible.

Note that sk_rmem_schedule() is using datalen to make things easy there.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/sctp.h |  2 +-
 net/sctp/sm_statefuns.c |  6 ++++--
 net/sctp/ulpevent.c     | 19 ++++++++-----------
 net/sctp/ulpqueue.c     |  3 ++-
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 1d13ec3..eefdfa5 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -421,7 +421,7 @@ static inline void sctp_skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
 	/*
 	 * This mimics the behavior of skb_set_owner_r
 	 */
-	sk->sk_forward_alloc -= event->rmem_len;
+	sk_mem_charge(sk, event->rmem_len);
 }
 
 /* Tests if the list has one and only one entry. */
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index c9ae340..7dfc34b 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -6412,13 +6412,15 @@ static int sctp_eat_data(const struct sctp_association *asoc,
 	 * in sctp_ulpevent_make_rcvmsg will drop the frame if we grow our
 	 * memory usage too much
 	 */
-	if (*sk->sk_prot_creator->memory_pressure) {
+	if (sk_under_memory_pressure(sk)) {
 		if (sctp_tsnmap_has_gap(map) &&
 		    (sctp_tsnmap_get_ctsn(map) + 1) == tsn) {
 			pr_debug("%s: under pressure, reneging for tsn:%u\n",
 				 __func__, tsn);
 			deliver = SCTP_CMD_RENEGE;
-		 }
+		} else {
+			sk_mem_reclaim(sk);
+		}
 	}
 
 	/*
diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
index 8cb7d98..c2a7478 100644
--- a/net/sctp/ulpevent.c
+++ b/net/sctp/ulpevent.c
@@ -634,8 +634,9 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
 						gfp_t gfp)
 {
 	struct sctp_ulpevent *event = NULL;
-	struct sk_buff *skb;
-	size_t padding, len;
+	struct sk_buff *skb = chunk->skb;
+	struct sock *sk = asoc->base.sk;
+	size_t padding, datalen;
 	int rx_count;
 
 	/*
@@ -646,15 +647,12 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
 	if (asoc->ep->rcvbuf_policy)
 		rx_count = atomic_read(&asoc->rmem_alloc);
 	else
-		rx_count = atomic_read(&asoc->base.sk->sk_rmem_alloc);
+		rx_count = atomic_read(&sk->sk_rmem_alloc);
 
-	if (rx_count >= asoc->base.sk->sk_rcvbuf) {
+	datalen = ntohs(chunk->chunk_hdr->length);
 
-		if ((asoc->base.sk->sk_userlocks & SOCK_RCVBUF_LOCK) ||
-		    (!sk_rmem_schedule(asoc->base.sk, chunk->skb,
-				       chunk->skb->truesize)))
-			goto fail;
-	}
+	if (rx_count >= sk->sk_rcvbuf || !sk_rmem_schedule(sk, skb, datalen))
+		goto fail;
 
 	/* Clone the original skb, sharing the data.  */
 	skb = skb_clone(chunk->skb, gfp);
@@ -681,8 +679,7 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
 	 * The sender should never pad with more than 3 bytes.  The receiver
 	 * MUST ignore the padding bytes.
 	 */
-	len = ntohs(chunk->chunk_hdr->length);
-	padding = SCTP_PAD4(len) - len;
+	padding = SCTP_PAD4(datalen) - datalen;
 
 	/* Fixup cloned skb with just this chunks data.  */
 	skb_trim(skb, chunk->chunk_end - padding - skb->data);
diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index 5dde921..770ff1f 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -1106,7 +1106,8 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
 			freed += sctp_ulpq_renege_frags(ulpq, needed - freed);
 	}
 	/* If able to free enough room, accept this chunk. */
-	if (freed >= needed) {
+	if (sk_rmem_schedule(asoc->base.sk, chunk->skb, needed) &&
+	    freed >= needed) {
 		int retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
 		/*
 		 * Enter partial delivery if chunk has not been
-- 
2.1.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-03-31  8:53 [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
  2019-03-31  8:53 ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Xin Long
@ 2019-03-31  8:56 ` Xin Long
  2019-03-31 18:02   ` David Miller
  2019-03-31 19:22 ` Matteo Croce
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Xin Long @ 2019-03-31  8:56 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: Marcelo Ricardo Leitner, Neil Horman, davem, Matteo Croce, Vladis Dronov

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

Simple tests are as attached.

On Sun, Mar 31, 2019 at 4:53 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> sctp memory accounting is added in this patchset by using
> these kernel APIs on send side:
>
>   - sk_mem_charge()
>   - sk_mem_uncharge()
>   - sk_wmem_schedule()
>   - sk_under_memory_pressure()
>   - sk_mem_reclaim()
>
> and these on receive side:
>
>   - sk_mem_charge()
>   - sk_mem_uncharge()
>   - sk_rmem_schedule()
>   - sk_under_memory_pressure()
>   - sk_mem_reclaim()
>
> With sctp memory accounting, we can limit the memory allocation by
> either sysctl:
>
>   # sysctl -w net.sctp.sctp_mem="10 20 50"
>
> or cgroup:
>
>   # echo $((8<<14)) > \
>     /sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
>
> When the socket is under memory pressure, the send side will block
> and wait, while the receive side will renege or drop.
>
> Xin Long (2):
>   sctp: implement memory accounting on tx path
>   sctp: implement memory accounting on rx path
>
>  include/net/sctp/sctp.h |  2 +-
>  net/sctp/sm_statefuns.c |  6 ++++--
>  net/sctp/socket.c       | 10 ++++++++--
>  net/sctp/ulpevent.c     | 19 ++++++++-----------
>  net/sctp/ulpqueue.c     |  3 ++-
>  5 files changed, 23 insertions(+), 17 deletions(-)
>
> --
> 2.1.0
>

[-- Attachment #2: sysctl_sockets_memctl.sh --]
[-- Type: text/x-sh, Size: 1473 bytes --]

#!/bin/sh

# TOPO: local (10.73.131.158) <--> peer (10.73.131.202) [hosts not netns]

peer_run()
{
	ssh root@10.73.131.202 "$@"
	# echo " - peer run: $@"
	# echo " - print any key to continue after it's done"
	# read
}

modprobe sctp

echo "sctp sysctl memory control rmem:"
echo "--------------------------------"

echo "10 20 50 rmem limit ->"
pkill sctp_test 2> /dev/null
sysctl -w  net.sctp.sctp_mem="10 20 50"
sctp_test -H 10.73.131.158 -P 1234 -l > /dev/null 2>&1 &
sleep 3
peer_run "time sctp_test -H 10.73.131.202 -P 8000 -h 10.73.131.158 -p 1234 -s -c 5  > /dev/null 2>&1"
echo ""

echo "20 50 100 rmem limit ->"
pkill sctp_test 2> /dev/null
sysctl -w  net.sctp.sctp_mem="20 50 100"
sctp_test -H 10.73.131.158 -P 1234 -l > /dev/null 2>&1 &
sleep 3
peer_run "time sctp_test -H 10.73.131.202 -P 8000 -h 10.73.131.158 -p 1234 -s -c 5  > /dev/null 2>&1"
echo ""

echo "sctp sysctl memory control wmem:"
echo "--------------------------------"

peer_run "sctp_test -H 10.73.131.202 -P 8000 -l > /dev/null 2>&1 &"
tc qdisc add dev eth0 root netem delay 50ms
echo "10 20 50 wmem limit ->"
sysctl -w  net.sctp.sctp_mem="5 10 20"
time sctp_test -h 10.73.131.202 -p 8000 -H 10.73.131.158 -P 1234 -s -c 5  > /dev/null 2>&1
echo ""

echo "10 50 100 wmem limit ->"
sysctl -w  net.sctp.sctp_mem="10 50 100 ->"
time sctp_test -h 10.73.131.202 -p 8000 -H 10.73.131.158 -P 1234 -s -c 5  > /dev/null 2>&1
echo ""
tc qdisc delete dev eth0 root
peer_run "pkill sctp_test 2>/dev/null"

[-- Attachment #3: cgroup_sockets_memctl.sh --]
[-- Type: text/x-sh, Size: 2076 bytes --]

#!/bin/sh

# TOPO: 127.0.0.1 lo

modprobe sctp
pkill sctp_test 2> /dev/null
sctp_test -H 127.0.0.1 -P 1234 -l > /dev/null 2>&1 &
sleep 3
mkdir /sys/fs/cgroup/memory/sctp_mem > /dev/null 2>&1;

echo "sctp_wmem testing:"
echo "------------------"

echo "$((8<<14)) bytes for wmem limit ->"
echo $((8<<14)) >/sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
{
	echo $BASHPID > /sys/fs/cgroup/memory/sctp_mem/cgroup.procs
	time sctp_test -H 127.0.0.1 -P 8000 -h 127.0.0.1 -p 1234 -s -c 5  > /dev/null 2>&1
	echo ""
} &
wait $!

echo "$((8<<16)) bytes for wmem limit ->"
echo $((8<<16)) >/sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
{
	echo $BASHPID > /sys/fs/cgroup/memory/sctp_mem/cgroup.procs
	time sctp_test -H 127.0.0.1 -P 8000 -h 127.0.0.1 -p 1234 -s -c 5  > /dev/null 2>&1
	echo ""
} &
wait $!


echo "sctp_rmem testing:"
echo "------------------"

echo "$((8<<13)) bytes for rmem limit ->"
pkill sctp_test 2> /dev/null
echo $((8<<13)) >/sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
{
	echo $BASHPID > /sys/fs/cgroup/memory/sctp_mem/cgroup.procs
	sctp_test -H 127.0.0.1 -P 1234 -l > /dev/null 2>&1
} &
sleep 3
time sctp_test -H 127.0.0.1 -P 8000 -h 127.0.0.1 -p 1234 -s -c 5  > /dev/null 2>&1
echo ""

echo "$((8<<14)) bytes for rmem limit ->"
pkill sctp_test 2> /dev/null
echo $((8<<14)) >/sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
{
	echo $BASHPID > /sys/fs/cgroup/memory/sctp_mem/cgroup.procs
	sctp_test -H 127.0.0.1 -P 1234 -l > /dev/null 2>&1
} &
sleep 3
time sctp_test -H 127.0.0.1 -P 8000 -h 127.0.0.1 -p 1234 -s -c 5  > /dev/null 2>&1
echo ""

echo "sctp_wmem_and_rmem testing:"
echo "---------------------------"

echo "$((8<<14)) bytes for wmem AND rmem limit ->"
pkill sctp_test 2> /dev/null
echo $((8<<14)) >/sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
{
	echo $BASHPID > /sys/fs/cgroup/memory/sctp_mem/cgroup.procs
	sctp_test -H 127.0.0.1 -P 1234 -l > /dev/null 2>&1 &
	sleep 3
	time sctp_test -H 127.0.0.1 -P 8000 -h 127.0.0.1 -p 1234 -s -c 5  > /dev/null 2>&1
	echo ""
} &
wait

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-03-31  8:56 ` [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
@ 2019-03-31 18:02   ` David Miller
  2019-04-04  9:45     ` Xin Long
  0 siblings, 1 reply; 17+ messages in thread
From: David Miller @ 2019-03-31 18:02 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, mcroce, vdronov

From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 31 Mar 2019 16:56:40 +0800

> Simple tests are as attached.

Are these integratable into selftests somehow?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-03-31  8:53 [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
  2019-03-31  8:53 ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Xin Long
  2019-03-31  8:56 ` [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
@ 2019-03-31 19:22 ` Matteo Croce
  2019-03-31 20:10 ` Marcelo Ricardo Leitner
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Matteo Croce @ 2019-03-31 19:22 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, Neil Horman,
	David S . Miller, Vladis Dronov

On Sun, Mar 31, 2019 at 10:53 AM Xin Long <lucien.xin@gmail.com> wrote:
>
> sctp memory accounting is added in this patchset by using
> these kernel APIs on send side:
>
>   - sk_mem_charge()
>   - sk_mem_uncharge()
>   - sk_wmem_schedule()
>   - sk_under_memory_pressure()
>   - sk_mem_reclaim()
>
> and these on receive side:
>
>   - sk_mem_charge()
>   - sk_mem_uncharge()
>   - sk_rmem_schedule()
>   - sk_under_memory_pressure()
>   - sk_mem_reclaim()
>
> With sctp memory accounting, we can limit the memory allocation by
> either sysctl:
>
>   # sysctl -w net.sctp.sctp_mem="10 20 50"
>
> or cgroup:
>
>   # echo $((8<<14)) > \
>     /sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
>
> When the socket is under memory pressure, the send side will block
> and wait, while the receive side will renege or drop.
>

I have tested this series with a tool which creates lot of SCTP
sockets and writes into them, without never reading.
Before the patch I was able to escape the cgroup limit and fill the
system memory, and the OOM killer killed random processes:

[  317.348911] oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),global_oom,task_memcg=/system.slice/ifup@eth0.service,task=dhclient,pid=188,uid=0
[  317.349084] Out of memory: Killed process 188 (dhclient)
total-vm:9484kB, anon-rss:1280kB, file-rss:1424kB, shmem-rss:0kB
[  317.743943] oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),global_oom,task_memcg=/system.slice/systemd-journald.service,task=systemd-journal,pid=85,uid=0
[  317.744093] Out of memory: Killed process 85 (systemd-journal)
total-vm:24592kB, anon-rss:1024kB, file-rss:1112kB, shmem-rss:652kB
[  317.921049] oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),global_oom,task_memcg=/system.slice/cron.service,task=cron,pid=222,uid=0
[  317.921209] Out of memory: Killed process 222 (cron)
total-vm:8692kB, anon-rss:276kB, file-rss:1540kB, shmem-rss:0kB

Now the OOM killer behaves correctly and always kills the processes in
the right cgroup:

[  512.100054] Tasks state (memory values in pages):
[  512.100122] [  pid  ]   uid  tgid total_vm      rss pgtables_bytes
swapents oom_score_adj name
[  512.100256] [    838]     0   838      550      184    36864
0             0 sctprank
[  512.100452] [    841]     0   841      550       18    36864
0             0 sctprank
[  512.100573] oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),oom_memcg=/sctp,task_memcg=/sctp,task=sctprank,pid=838,uid=0
[  512.100700] Memory cgroup out of memory: Killed process 838
(sctprank) total-vm:2200kB, anon-rss:64kB, file-rss:672kB,
shmem-rss:0kB
[  512.100899] oom_reaper: reaped process 838 (sctprank), now
anon-rss:0kB, file-rss:0kB, shmem-rss:0kB

Series ACK.

Tested-by: Matteo Croce <mcroce@redhat.com>


--
Matteo Croce
per aspera ad upstream

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-03-31  8:53 [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
                   ` (2 preceding siblings ...)
  2019-03-31 19:22 ` Matteo Croce
@ 2019-03-31 20:10 ` Marcelo Ricardo Leitner
  2019-03-31 21:04   ` David Miller
  2019-04-01  7:25 ` Xin Long
  2019-04-01 11:31 ` Neil Horman
  5 siblings, 1 reply; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-03-31 20:10 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, Neil Horman, davem, Matteo Croce, Vladis Dronov

On Sun, Mar 31, 2019 at 04:53:45PM +0800, Xin Long wrote:
> sctp memory accounting is added in this patchset by using
> these kernel APIs on send side:

Please give me till tomorrow to review this one.

Thanks,
Marcelo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-03-31 20:10 ` Marcelo Ricardo Leitner
@ 2019-03-31 21:04   ` David Miller
  0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2019-03-31 21:04 UTC (permalink / raw)
  To: marcelo.leitner; +Cc: lucien.xin, netdev, linux-sctp, nhorman, mcroce, vdronov

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Sun, 31 Mar 2019 17:10:48 -0300

> On Sun, Mar 31, 2019 at 04:53:45PM +0800, Xin Long wrote:
>> sctp memory accounting is added in this patchset by using
>> these kernel APIs on send side:
> 
> Please give me till tomorrow to review this one.

No problem.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-03-31  8:53 [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
                   ` (3 preceding siblings ...)
  2019-03-31 20:10 ` Marcelo Ricardo Leitner
@ 2019-04-01  7:25 ` Xin Long
  2019-04-01 11:31 ` Neil Horman
  5 siblings, 0 replies; 17+ messages in thread
From: Xin Long @ 2019-04-01  7:25 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: Marcelo Ricardo Leitner, Neil Horman, davem, Matteo Croce, Vladis Dronov

On Sun, Mar 31, 2019 at 4:53 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> sctp memory accounting is added in this patchset by using
> these kernel APIs on send side:
>
>   - sk_mem_charge()
>   - sk_mem_uncharge()
>   - sk_wmem_schedule()
>   - sk_under_memory_pressure()
>   - sk_mem_reclaim()
>
> and these on receive side:
>
>   - sk_mem_charge()
>   - sk_mem_uncharge()
>   - sk_rmem_schedule()
>   - sk_under_memory_pressure()
>   - sk_mem_reclaim()
>
> With sctp memory accounting, we can limit the memory allocation by
> either sysctl:
>
>   # sysctl -w net.sctp.sctp_mem="10 20 50"
>
> or cgroup:
>
>   # echo $((8<<14)) > \
>     /sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
>
> When the socket is under memory pressure, the send side will block
> and wait, while the receive side will renege or drop.
>
> Xin Long (2):
>   sctp: implement memory accounting on tx path
>   sctp: implement memory accounting on rx path
>
>  include/net/sctp/sctp.h |  2 +-
>  net/sctp/sm_statefuns.c |  6 ++++--
>  net/sctp/socket.c       | 10 ++++++++--
>  net/sctp/ulpevent.c     | 19 ++++++++-----------
>  net/sctp/ulpqueue.c     |  3 ++-
>  5 files changed, 23 insertions(+), 17 deletions(-)
>
> --
> 2.1.0
>
Reported-by: Matteo Croce <mcroce@redhat.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-03-31  8:53 [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
                   ` (4 preceding siblings ...)
  2019-04-01  7:25 ` Xin Long
@ 2019-04-01 11:31 ` Neil Horman
  2019-04-02  3:36   ` Marcelo Ricardo Leitner
  5 siblings, 1 reply; 17+ messages in thread
From: Neil Horman @ 2019-04-01 11:31 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem,
	Matteo Croce, Vladis Dronov

On Sun, Mar 31, 2019 at 04:53:45PM +0800, Xin Long wrote:
> sctp memory accounting is added in this patchset by using
> these kernel APIs on send side:
> 
>   - sk_mem_charge()
>   - sk_mem_uncharge()
>   - sk_wmem_schedule()
>   - sk_under_memory_pressure()
>   - sk_mem_reclaim()
> 
> and these on receive side:
> 
>   - sk_mem_charge()
>   - sk_mem_uncharge()
>   - sk_rmem_schedule()
>   - sk_under_memory_pressure()
>   - sk_mem_reclaim()
> 
> With sctp memory accounting, we can limit the memory allocation by
> either sysctl:
> 
>   # sysctl -w net.sctp.sctp_mem="10 20 50"
> 
> or cgroup:
> 
>   # echo $((8<<14)) > \
>     /sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
> 
> When the socket is under memory pressure, the send side will block
> and wait, while the receive side will renege or drop.
> 
> Xin Long (2):
>   sctp: implement memory accounting on tx path
>   sctp: implement memory accounting on rx path
> 
>  include/net/sctp/sctp.h |  2 +-
>  net/sctp/sm_statefuns.c |  6 ++++--
>  net/sctp/socket.c       | 10 ++++++++--
>  net/sctp/ulpevent.c     | 19 ++++++++-----------
>  net/sctp/ulpqueue.c     |  3 ++-
>  5 files changed, 23 insertions(+), 17 deletions(-)
> 
> -- 
> 2.1.0
> 
> 
I don't have a problem with either of these patches in terms of altering memory
accounting, but SCTP has the notion of accounting buffers based on either
sockets space or association space (based on the sndbuf_policy and rcvbuf_policy
sysctls).  This patch eliminates them.  I don't see this patch addressing either
the removal of that functionality (as the proposed accounting scheme renders
those sysctls useless and ignored, which may cause regressions in some
environments), nor does it address the possibiliy of one association starving
others on the same socket when they share the same socket level accounting.  I
think you need to look how to address that (either by re-adding the ability to
account in either case based on the sysctls, or deprecating eliminating the
sysctls and addressing the starvation issue.

Best
Neil


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-04-01 11:31 ` Neil Horman
@ 2019-04-02  3:36   ` Marcelo Ricardo Leitner
  2019-04-02 11:41     ` Neil Horman
  0 siblings, 1 reply; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-04-02  3:36 UTC (permalink / raw)
  To: Neil Horman
  Cc: Xin Long, network dev, linux-sctp, davem, Matteo Croce, Vladis Dronov

On Mon, Apr 01, 2019 at 07:31:10AM -0400, Neil Horman wrote:
> On Sun, Mar 31, 2019 at 04:53:45PM +0800, Xin Long wrote:
> > sctp memory accounting is added in this patchset by using
> > these kernel APIs on send side:
> > 
> >   - sk_mem_charge()
> >   - sk_mem_uncharge()
> >   - sk_wmem_schedule()
> >   - sk_under_memory_pressure()
> >   - sk_mem_reclaim()
> > 
> > and these on receive side:
> > 
> >   - sk_mem_charge()
> >   - sk_mem_uncharge()
> >   - sk_rmem_schedule()
> >   - sk_under_memory_pressure()
> >   - sk_mem_reclaim()
> > 
> > With sctp memory accounting, we can limit the memory allocation by
> > either sysctl:
> > 
> >   # sysctl -w net.sctp.sctp_mem="10 20 50"
> > 
> > or cgroup:
> > 
> >   # echo $((8<<14)) > \
> >     /sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
> > 
> > When the socket is under memory pressure, the send side will block
> > and wait, while the receive side will renege or drop.
> > 
> > Xin Long (2):
> >   sctp: implement memory accounting on tx path
> >   sctp: implement memory accounting on rx path
> > 
> >  include/net/sctp/sctp.h |  2 +-
> >  net/sctp/sm_statefuns.c |  6 ++++--
> >  net/sctp/socket.c       | 10 ++++++++--
> >  net/sctp/ulpevent.c     | 19 ++++++++-----------
> >  net/sctp/ulpqueue.c     |  3 ++-
> >  5 files changed, 23 insertions(+), 17 deletions(-)
> > 
> > -- 
> > 2.1.0
> > 
> > 
> I don't have a problem with either of these patches in terms of altering memory
> accounting, but SCTP has the notion of accounting buffers based on either
> sockets space or association space (based on the sndbuf_policy and rcvbuf_policy
> sysctls).  This patch eliminates them.  I don't see this patch addressing either
> the removal of that functionality (as the proposed accounting scheme renders
> those sysctls useless and ignored, which may cause regressions in some
> environments), nor does it address the possibiliy of one association starving
> others on the same socket when they share the same socket level accounting.  I
> think you need to look how to address that (either by re-adding the ability to
> account in either case based on the sysctls, or deprecating eliminating the
> sysctls and addressing the starvation issue.

That's not how I'm reading these. All original conditions are still
there while these patches are adding a couple of restrictions more.
What that means is that they are adding a ceiling to it, even if the
limits are per socket or per assoc. Considering the idea of the cgroup
limit being added here, it makes sense to me. If the cgroup is
configured to allow at most X MB, it doesn't matter how that is
allocated. That's a sysadmin task then, to adjust the other sysctls
(net.sctp.sctp_mem & cia) and balance the usage, be it per socket or
per asoc.

Cheers,
  Marcelo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-04-02  3:36   ` Marcelo Ricardo Leitner
@ 2019-04-02 11:41     ` Neil Horman
  0 siblings, 0 replies; 17+ messages in thread
From: Neil Horman @ 2019-04-02 11:41 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Xin Long, network dev, linux-sctp, davem, Matteo Croce, Vladis Dronov

On Tue, Apr 02, 2019 at 12:36:10AM -0300, Marcelo Ricardo Leitner wrote:
> On Mon, Apr 01, 2019 at 07:31:10AM -0400, Neil Horman wrote:
> > On Sun, Mar 31, 2019 at 04:53:45PM +0800, Xin Long wrote:
> > > sctp memory accounting is added in this patchset by using
> > > these kernel APIs on send side:
> > > 
> > >   - sk_mem_charge()
> > >   - sk_mem_uncharge()
> > >   - sk_wmem_schedule()
> > >   - sk_under_memory_pressure()
> > >   - sk_mem_reclaim()
> > > 
> > > and these on receive side:
> > > 
> > >   - sk_mem_charge()
> > >   - sk_mem_uncharge()
> > >   - sk_rmem_schedule()
> > >   - sk_under_memory_pressure()
> > >   - sk_mem_reclaim()
> > > 
> > > With sctp memory accounting, we can limit the memory allocation by
> > > either sysctl:
> > > 
> > >   # sysctl -w net.sctp.sctp_mem="10 20 50"
> > > 
> > > or cgroup:
> > > 
> > >   # echo $((8<<14)) > \
> > >     /sys/fs/cgroup/memory/sctp_mem/memory.kmem.tcp.limit_in_bytes
> > > 
> > > When the socket is under memory pressure, the send side will block
> > > and wait, while the receive side will renege or drop.
> > > 
> > > Xin Long (2):
> > >   sctp: implement memory accounting on tx path
> > >   sctp: implement memory accounting on rx path
> > > 
> > >  include/net/sctp/sctp.h |  2 +-
> > >  net/sctp/sm_statefuns.c |  6 ++++--
> > >  net/sctp/socket.c       | 10 ++++++++--
> > >  net/sctp/ulpevent.c     | 19 ++++++++-----------
> > >  net/sctp/ulpqueue.c     |  3 ++-
> > >  5 files changed, 23 insertions(+), 17 deletions(-)
> > > 
> > > -- 
> > > 2.1.0
> > > 
> > > 
> > I don't have a problem with either of these patches in terms of altering memory
> > accounting, but SCTP has the notion of accounting buffers based on either
> > sockets space or association space (based on the sndbuf_policy and rcvbuf_policy
> > sysctls).  This patch eliminates them.  I don't see this patch addressing either
> > the removal of that functionality (as the proposed accounting scheme renders
> > those sysctls useless and ignored, which may cause regressions in some
> > environments), nor does it address the possibiliy of one association starving
> > others on the same socket when they share the same socket level accounting.  I
> > think you need to look how to address that (either by re-adding the ability to
> > account in either case based on the sysctls, or deprecating eliminating the
> > sysctls and addressing the starvation issue.
> 
> That's not how I'm reading these. All original conditions are still
> there while these patches are adding a couple of restrictions more.
> What that means is that they are adding a ceiling to it, even if the
> limits are per socket or per assoc. Considering the idea of the cgroup
> limit being added here, it makes sense to me. If the cgroup is
> configured to allow at most X MB, it doesn't matter how that is
> allocated. That's a sysadmin task then, to adjust the other sysctls
> (net.sctp.sctp_mem & cia) and balance the usage, be it per socket or
> per asoc.
> 

You're right, I had the sense on the conditional backwards in my head. My bad

Series

Acked-by: Neil Horman <nhorman@tuxdriver.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 1/2] sctp: implement memory accounting on tx path
  2019-03-31  8:53 ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Xin Long
  2019-03-31  8:53   ` [PATCH net-next 2/2] sctp: implement memory accounting on rx path Xin Long
@ 2019-04-02 11:48   ` Marcelo Ricardo Leitner
  1 sibling, 0 replies; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-04-02 11:48 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, Neil Horman, davem, Matteo Croce, Vladis Dronov

On Sun, Mar 31, 2019 at 04:53:46PM +0800, Xin Long wrote:
> Now when sending packets, sk_mem_charge() and sk_mem_uncharge() have been
> used to set sk_forward_alloc. We just need to call sk_wmem_schedule() to
> check if the allocated should be raised, and call sk_mem_reclaim() to
> check if the allocated should be reduced when it's under memory pressure.
> 
> If sk_wmem_schedule() returns false, which means no memory is allowed to
> allocate, it will block and wait for memory to become available.
> 
> Note different from tcp, sctp wait_for_buf happens before allocating any
> skb, so memory accounting check is done with the whole msg_len before it
> too.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/socket.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 6140471..06c6f4a 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1913,7 +1913,10 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
>  	if (sctp_wspace(asoc) < (int)msg_len)
>  		sctp_prsctp_prune(asoc, sinfo, msg_len - sctp_wspace(asoc));
>  
> -	if (sctp_wspace(asoc) <= 0) {
> +	if (sk_under_memory_pressure(sk))
> +		sk_mem_reclaim(sk);
> +
> +	if (sctp_wspace(asoc) <= 0 || !sk_wmem_schedule(sk, msg_len)) {
>  		timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
>  		err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len);
>  		if (err)
> @@ -8891,7 +8894,10 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
>  			goto do_error;
>  		if (signal_pending(current))
>  			goto do_interrupted;
> -		if ((int)msg_len <= sctp_wspace(asoc))
> +		if (sk_under_memory_pressure(sk))
> +			sk_mem_reclaim(sk);
> +		if ((int)msg_len <= sctp_wspace(asoc) &&
> +		    sk_wmem_schedule(sk, msg_len))
>  			break;
>  
>  		/* Let another process have a go.  Since we are going
> -- 
> 2.1.0
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 2/2] sctp: implement memory accounting on rx path
  2019-03-31  8:53   ` [PATCH net-next 2/2] sctp: implement memory accounting on rx path Xin Long
@ 2019-04-02 11:48     ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-04-02 11:48 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, Neil Horman, davem, Matteo Croce, Vladis Dronov

On Sun, Mar 31, 2019 at 04:53:47PM +0800, Xin Long wrote:
> sk_forward_alloc's updating is also done on rx path, but to be consistent
> we change to use sk_mem_charge() in sctp_skb_set_owner_r().
> 
> In sctp_eat_data(), it's not enough to check sctp_memory_pressure only,
> which doesn't work for mem_cgroup_sockets_enabled, so we change to use
> sk_under_memory_pressure().
> 
> When it's under memory pressure, sk_mem_reclaim() and sk_rmem_schedule()
> should be called on both RENEGE or CHUNK DELIVERY path exit the memory
> pressure status as soon as possible.
> 
> Note that sk_rmem_schedule() is using datalen to make things easy there.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/net/sctp/sctp.h |  2 +-
>  net/sctp/sm_statefuns.c |  6 ++++--
>  net/sctp/ulpevent.c     | 19 ++++++++-----------
>  net/sctp/ulpqueue.c     |  3 ++-
>  4 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index 1d13ec3..eefdfa5 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -421,7 +421,7 @@ static inline void sctp_skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
>  	/*
>  	 * This mimics the behavior of skb_set_owner_r
>  	 */
> -	sk->sk_forward_alloc -= event->rmem_len;
> +	sk_mem_charge(sk, event->rmem_len);
>  }
>  
>  /* Tests if the list has one and only one entry. */
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index c9ae340..7dfc34b 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -6412,13 +6412,15 @@ static int sctp_eat_data(const struct sctp_association *asoc,
>  	 * in sctp_ulpevent_make_rcvmsg will drop the frame if we grow our
>  	 * memory usage too much
>  	 */
> -	if (*sk->sk_prot_creator->memory_pressure) {
> +	if (sk_under_memory_pressure(sk)) {
>  		if (sctp_tsnmap_has_gap(map) &&
>  		    (sctp_tsnmap_get_ctsn(map) + 1) == tsn) {
>  			pr_debug("%s: under pressure, reneging for tsn:%u\n",
>  				 __func__, tsn);
>  			deliver = SCTP_CMD_RENEGE;
> -		 }
> +		} else {
> +			sk_mem_reclaim(sk);
> +		}
>  	}
>  
>  	/*
> diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
> index 8cb7d98..c2a7478 100644
> --- a/net/sctp/ulpevent.c
> +++ b/net/sctp/ulpevent.c
> @@ -634,8 +634,9 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
>  						gfp_t gfp)
>  {
>  	struct sctp_ulpevent *event = NULL;
> -	struct sk_buff *skb;
> -	size_t padding, len;
> +	struct sk_buff *skb = chunk->skb;
> +	struct sock *sk = asoc->base.sk;
> +	size_t padding, datalen;
>  	int rx_count;
>  
>  	/*
> @@ -646,15 +647,12 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
>  	if (asoc->ep->rcvbuf_policy)
>  		rx_count = atomic_read(&asoc->rmem_alloc);
>  	else
> -		rx_count = atomic_read(&asoc->base.sk->sk_rmem_alloc);
> +		rx_count = atomic_read(&sk->sk_rmem_alloc);
>  
> -	if (rx_count >= asoc->base.sk->sk_rcvbuf) {
> +	datalen = ntohs(chunk->chunk_hdr->length);
>  
> -		if ((asoc->base.sk->sk_userlocks & SOCK_RCVBUF_LOCK) ||
> -		    (!sk_rmem_schedule(asoc->base.sk, chunk->skb,
> -				       chunk->skb->truesize)))
> -			goto fail;
> -	}
> +	if (rx_count >= sk->sk_rcvbuf || !sk_rmem_schedule(sk, skb, datalen))
> +		goto fail;
>  
>  	/* Clone the original skb, sharing the data.  */
>  	skb = skb_clone(chunk->skb, gfp);
> @@ -681,8 +679,7 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
>  	 * The sender should never pad with more than 3 bytes.  The receiver
>  	 * MUST ignore the padding bytes.
>  	 */
> -	len = ntohs(chunk->chunk_hdr->length);
> -	padding = SCTP_PAD4(len) - len;
> +	padding = SCTP_PAD4(datalen) - datalen;
>  
>  	/* Fixup cloned skb with just this chunks data.  */
>  	skb_trim(skb, chunk->chunk_end - padding - skb->data);
> diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
> index 5dde921..770ff1f 100644
> --- a/net/sctp/ulpqueue.c
> +++ b/net/sctp/ulpqueue.c
> @@ -1106,7 +1106,8 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>  			freed += sctp_ulpq_renege_frags(ulpq, needed - freed);
>  	}
>  	/* If able to free enough room, accept this chunk. */
> -	if (freed >= needed) {
> +	if (sk_rmem_schedule(asoc->base.sk, chunk->skb, needed) &&
> +	    freed >= needed) {
>  		int retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
>  		/*
>  		 * Enter partial delivery if chunk has not been
> -- 
> 2.1.0
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-03-31 18:02   ` David Miller
@ 2019-04-04  9:45     ` Xin Long
  2019-04-04 17:38       ` David Miller
  0 siblings, 1 reply; 17+ messages in thread
From: Xin Long @ 2019-04-04  9:45 UTC (permalink / raw)
  To: David Miller
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, Neil Horman,
	Matteo Croce, Vladis Dronov

On Mon, Apr 1, 2019 at 2:02 AM David Miller <davem@davemloft.net> wrote:
>
> From: Xin Long <lucien.xin@gmail.com>
> Date: Sun, 31 Mar 2019 16:56:40 +0800
>
> > Simple tests are as attached.
>
> Are these integratable into selftests somehow?
We're thinking about adding some sctp tests into selftests,
but these ones are not good to be the first one, I think.
sysctl_sockets_memctl.sh is a multi-host test, even netns can't work for it.

Each subcomponent seems to have its own test case in other git repo,
can I ask what kind of tests should I put into kernel selftests in the future?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-04-04  9:45     ` Xin Long
@ 2019-04-04 17:38       ` David Miller
  2019-04-05  4:49         ` Xin Long
  0 siblings, 1 reply; 17+ messages in thread
From: David Miller @ 2019-04-04 17:38 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, mcroce, vdronov

From: Xin Long <lucien.xin@gmail.com>
Date: Thu, 4 Apr 2019 17:45:07 +0800

> We're thinking about adding some sctp tests into selftests,
> but these ones are not good to be the first one, I think.
> sysctl_sockets_memctl.sh is a multi-host test, even netns can't work for it.

You can't set per-netns memory controls appropriately to make the
tests work?

> Each subcomponent seems to have its own test case in other git repo,
> can I ask what kind of tests should I put into kernel selftests in
> the future?

It would be ideal that we have a decent base set of SCTP tests and
then when bug fixes are added, we get a unit test with the fix.

I guess a good baseline would be testing basic comminucation between
SCTP sockets in different netns, and then adding tests for setting
the various socket options and making sure the socket option had the
desired effect.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/2] sctp: fully support memory accounting
  2019-04-04 17:38       ` David Miller
@ 2019-04-05  4:49         ` Xin Long
  0 siblings, 0 replies; 17+ messages in thread
From: Xin Long @ 2019-04-05  4:49 UTC (permalink / raw)
  To: David Miller
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, Neil Horman,
	Matteo Croce, Vladis Dronov

On Fri, Apr 5, 2019 at 1:38 AM David Miller <davem@davemloft.net> wrote:
>
> From: Xin Long <lucien.xin@gmail.com>
> Date: Thu, 4 Apr 2019 17:45:07 +0800
>
> > We're thinking about adding some sctp tests into selftests,
> > but these ones are not good to be the first one, I think.
> > sysctl_sockets_memctl.sh is a multi-host test, even netns can't work for it.
>
> You can't set per-netns memory controls appropriately to make the
> tests work?
static atomic_long_t sctp_memory_allocated;

struct proto sctp_prot = {
.memory_allocated = &sctp_memory_allocated,

sysctl limitation is per-netns, but the counter 'sctp_memory_allocated' is not.
It means other netns buffer allocation will affect the current netns.

>
> > Each subcomponent seems to have its own test case in other git repo,
> > can I ask what kind of tests should I put into kernel selftests in
> > the future?
>
> It would be ideal that we have a decent base set of SCTP tests and
> then when bug fixes are added, we get a unit test with the fix.
>
> I guess a good baseline would be testing basic comminucation between
> SCTP sockets in different netns, and then adding tests for setting
> the various socket options and making sure the socket option had the
> desired effect.
got your point. thanks

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, back to index

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-31  8:53 [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
2019-03-31  8:53 ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Xin Long
2019-03-31  8:53   ` [PATCH net-next 2/2] sctp: implement memory accounting on rx path Xin Long
2019-04-02 11:48     ` Marcelo Ricardo Leitner
2019-04-02 11:48   ` [PATCH net-next 1/2] sctp: implement memory accounting on tx path Marcelo Ricardo Leitner
2019-03-31  8:56 ` [PATCH net-next 0/2] sctp: fully support memory accounting Xin Long
2019-03-31 18:02   ` David Miller
2019-04-04  9:45     ` Xin Long
2019-04-04 17:38       ` David Miller
2019-04-05  4:49         ` Xin Long
2019-03-31 19:22 ` Matteo Croce
2019-03-31 20:10 ` Marcelo Ricardo Leitner
2019-03-31 21:04   ` David Miller
2019-04-01  7:25 ` Xin Long
2019-04-01 11:31 ` Neil Horman
2019-04-02  3:36   ` Marcelo Ricardo Leitner
2019-04-02 11:41     ` Neil Horman

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox