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@xxxxxxxxxxxxx>
