LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Paolo Abeni <pabeni@redhat.com>
To: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>, netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Eric Dumazet <edumazet@google.com>,
	Florian Westphal <fw@strlen.de>,
	linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org,
	linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org,
	sparclinux@vger.kernel.org, linux-arch@vger.kernel.org,
	Andrei Vagin <avagin@gmail.com>
Subject: Re: [PATCH] sock: allow reading and changing sk_userlocks with setsockopt
Date: Fri, 30 Jul 2021 15:13:31 +0200	[thread overview]
Message-ID: <bc53b476f8a3a1bafb73c2f5072c0bad03bc1709.camel@redhat.com> (raw)
In-Reply-To: <20210730105406.318726-1-ptikhomirov@virtuozzo.com>

On Fri, 2021-07-30 at 13:54 +0300, Pavel Tikhomirov wrote:
> SOCK_SNDBUF_LOCK and SOCK_RCVBUF_LOCK flags disable automatic socket
> buffers adjustment done by kernel (see tcp_fixup_rcvbuf() and
> tcp_sndbuf_expand()). If we've just created a new socket this adjustment
> is enabled on it, but if one changes the socket buffer size by
> setsockopt(SO_{SND,RCV}BUF*) it becomes disabled.
> 
> CRIU needs to call setsockopt(SO_{SND,RCV}BUF*) on each socket on
> restore as it first needs to increase buffer sizes for packet queues
> restore and second it needs to restore back original buffer sizes. So
> after CRIU restore all sockets become non-auto-adjustable, which can
> decrease network performance of restored applications significantly.

I'm wondering if you could just tune tcp_rmem instead?

> CRIU need to be able to restore sockets with enabled/disabled adjustment
> to the same state it was before dump, so let's add special setsockopt
> for it.
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
> Here is a corresponding CRIU commits using these new feature to fix slow
> download speed problem after migration:
> https://github.com/checkpoint-restore/criu/pull/1568 
> 
> Origin of the problem:
> 
> We have a customer in Virtuozzo who mentioned that nginx server becomes
> slower after container migration. Especially it is easy to mention when
> you wget some big file via localhost from the same container which was
> just migrated. 
>  
> By strace-ing all nginx processes I see that nginx worker process before
> c/r sends data to local wget with big chunks ~1.5Mb, but after c/r it
> only succeeds to send by small chunks ~64Kb.
> 
> Before: 
> sendfile(12, 13, [7984974] => [9425600], 11479629) = 1440626 <0.000180> 
>  
> After: 
> sendfile(8, 13, [1507275] => [1568768], 17957328) = 61493 <0.000675> 
> 
> Smaller buffer can explain the decrease in download speed. So as a POC I
> just commented out all buffer setting manipulations and that helped.
> 
> ---
>  arch/alpha/include/uapi/asm/socket.h  |  2 ++
>  arch/mips/include/uapi/asm/socket.h   |  2 ++
>  arch/parisc/include/uapi/asm/socket.h |  2 ++
>  arch/sparc/include/uapi/asm/socket.h  |  2 ++
>  include/uapi/asm-generic/socket.h     |  2 ++
>  net/core/sock.c                       | 12 ++++++++++++
>  6 files changed, 22 insertions(+)
> 
> diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> index 6b3daba60987..1dd9baf4a6c2 100644
> --- a/arch/alpha/include/uapi/asm/socket.h
> +++ b/arch/alpha/include/uapi/asm/socket.h
> @@ -129,6 +129,8 @@
>  
>  #define SO_NETNS_COOKIE		71
>  
> +#define SO_BUF_LOCK		72
> +
>  #if !defined(__KERNEL__)
>  
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
> index cdf404a831b2..1eaf6a1ca561 100644
> --- a/arch/mips/include/uapi/asm/socket.h
> +++ b/arch/mips/include/uapi/asm/socket.h
> @@ -140,6 +140,8 @@
>  
>  #define SO_NETNS_COOKIE		71
>  
> +#define SO_BUF_LOCK		72
> +
>  #if !defined(__KERNEL__)
>  
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
> index 5b5351cdcb33..8baaad52d799 100644
> --- a/arch/parisc/include/uapi/asm/socket.h
> +++ b/arch/parisc/include/uapi/asm/socket.h
> @@ -121,6 +121,8 @@
>  
>  #define SO_NETNS_COOKIE		0x4045
>  
> +#define SO_BUF_LOCK		0x4046
> +
>  #if !defined(__KERNEL__)
>  
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
> index 92675dc380fa..e80ee8641ac3 100644
> --- a/arch/sparc/include/uapi/asm/socket.h
> +++ b/arch/sparc/include/uapi/asm/socket.h
> @@ -122,6 +122,8 @@
>  
>  #define SO_NETNS_COOKIE          0x0050
>  
> +#define SO_BUF_LOCK              0x0051
> +
>  #if !defined(__KERNEL__)
>  
>  
> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> index d588c244ec2f..1f0a2b4864e4 100644
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -124,6 +124,8 @@
>  
>  #define SO_NETNS_COOKIE		71
>  
> +#define SO_BUF_LOCK		72
> +
>  #if !defined(__KERNEL__)
>  
>  #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
> diff --git a/net/core/sock.c b/net/core/sock.c
> index a3eea6e0b30a..843094f069f3 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1357,6 +1357,14 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
>  		ret = sock_bindtoindex_locked(sk, val);
>  		break;
>  
> +	case SO_BUF_LOCK:
> +		{
> +		int mask = SOCK_SNDBUF_LOCK | SOCK_RCVBUF_LOCK;

What about define a marco with the above mask, and avoid the local
variable declaration and brackets??!

Thanks!

Paolo


  reply	other threads:[~2021-07-30 13:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30 10:54 Pavel Tikhomirov
2021-07-30 13:13 ` Paolo Abeni [this message]
2021-07-30 14:21   ` Pavel Tikhomirov

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=bc53b476f8a3a1bafb73c2f5072c0bad03bc1709.camel@redhat.com \
    --to=pabeni@redhat.com \
    --cc=arnd@arndb.de \
    --cc=avagin@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=kuba@kernel.org \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ptikhomirov@virtuozzo.com \
    --cc=sparclinux@vger.kernel.org \
    --subject='Re: [PATCH] sock: allow reading and changing sk_userlocks with setsockopt' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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).