netdev - Re: [PATCH net-next 0/2] sctp: fully support memory accounting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190402114123.GB17593@hmswarspite.think-freely.org>
Date:   Tue, 2 Apr 2019 07:41:23 -0400
From:   Neil Horman <nhorman@...driver.com>
To:     Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc:     Xin Long <lucien.xin@...il.com>,
        network dev <netdev@...r.kernel.org>,
        linux-sctp@...r.kernel.org, davem@...emloft.net,
        Matteo Croce <mcroce@...hat.com>,
        Vladis Dronov <vdronov@...hat.com>
Subject: Re: [PATCH net-next 0/2] sctp: fully support memory accounting

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@...driver.com>