Netdev Archive on
help / color / mirror / Atom feed
From: "" <>
To: Paolo Abeni <>,
	"David S . Miller" <>,
	Eric Dumazet <>
Subject: Re: [PATCH] net/sock: don't drop udp packets if udp_mem[2] not reached
Date: Wed, 9 Sep 2020 11:24:33 +0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, Sep 08, 2020 at 10:46:13AM +0200, Paolo Abeni wrote:
>On Tue, 2020-09-08 at 11:15 +0800, wrote:
>> Actually, with more udp sockets, I can always make it large
>> enough to exceed udp_mem[0], and drop packets before udp_mem[1]
>> and udp_mem[2].
>Sure, but with enough sockets you can also exceeeds any limits ;).
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index 6c5c6b18eff4..fed8211d8dbe 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -2648,6 +2648,12 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
>>                                  atomic_read(&sk->sk_rmem_alloc) +
>>                                  sk->sk_forward_alloc))
>>                         return 1;
>> +       } else {
>> +               /* for prots without memory_pressure callbacks, we should not
>> +                * drop until hard limit reached
>> +                */
>> +               if (allocated <= sk_prot_mem_limits(sk, 2))
>> +                       return 1;
>At this point, the above condition is always true, due to an earlier
>check. Additionally, accepting any value below udp_mem[2] would make
>the previous checks to allow a minimum per socket memory useless.

Thanks for your reply !

Yeah, this is wrong :O

>You can obtain the same result setting udp_mem[0] = udp_mem[2], without
>any kernel change.
>But with this change applied you can't guarantee anymore a minimum per
>socket amount of memory.

Yeah, understood.

>I think you are possibly mislead by your own comment: the point is that
>we should never allow allocation above the hard limit, but the protocol
>is allowed to drop as soon as the memory allocated raises above the
>lower limit.


>Note that the current behavior is correctly documented
>in Documentation/networking/ip-sysctl.rst.
>Your problem must be solved in another way e.g. raising udp_mem[0] -
>and keeping udp_mem[2] above that value.

I think it's somehow not quite fit what Documentation/networking/ip-sysctl.rst

    min: Below this number of pages UDP is not bothered about its
    memory appetite. When amount of memory allocated by UDP exceeds
    this number, UDP starts to moderate memory usage.

It says UDP starts to moderate memory usage when exceeds min, but here
it is actually dropping all the packets unless the socket is under minimum
, which means udp_mem[0] is almost equal to the hard limit, and
udp_mem[1]/udp_mem[2] is useless.

Do you think it's usefull if we move the per socket memory check to the
common path for both with and without memory_pressure protocols ? So
only those sockets who comsume most of the memory will got dropped before
hard limit. Something like this:

  if (sk_has_memory_pressure(sk)) {

          if (!sk_under_memory_pressure(sk))
                  return 1;
  alloc = sk_sockets_allocated_read_positive(sk);
  if (sk_prot_mem_limits(sk, 2) > alloc *
      sk_mem_pages(sk->sk_wmem_queued +
                   atomic_read(&sk->sk_rmem_alloc) +
          return 1;

Sorry for bothering again :)


      reply	other threads:[~2020-09-09  3:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-07 14:44 Dust Li
2020-09-07 17:18 ` Paolo Abeni
2020-09-08  3:15   `
2020-09-08  8:46     ` Paolo Abeni
2020-09-09  3:24       ` [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \
    --subject='Re: [PATCH] net/sock: don'\''t drop udp packets if udp_mem[2] not reached' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).