All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Wagner <wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
To: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: John Fastabend
	<john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>,
	Daniel Wagner
	<daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>,
	LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Cgroups <cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [BUG] Bug in netprio_cgroup and netcls_cgroup ?
Date: Mon, 21 Jan 2013 09:50:12 +0100	[thread overview]
Message-ID: <50FD0144.1000401@monom.org> (raw)
In-Reply-To: <50FCDB5C.4050608-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

Hi Li,

On 21.01.2013 07:08, Li Zefan wrote:
> I'm not a network developer, so correct me if I'm wrong.
> 
> Since commit 7955490f732c2b8
> ("net: netprio_cgroup: rework update socket logic"), sock->sk->sk_cgrp_prioidx
> is set when the socket is created, and won't be updated unless the task is
> moved to another cgroup.
> 
> Now the problem is, a socket can be _shared_ by multiple processes (fork, SCM_RIGHT).
> If we place those processes in different cgroups, and each cgroup has
> different configs, but all of the processes will send data via this socket
> with the same network priority.

Wouldn't that be addressed by 48a87cc26c13b68f6cce4e9d769fcb17a6b3e4b8

   net: netprio: fd passed in SCM_RIGHTS datagram not set correctly

    A socket fd passed in a SCM_RIGHTS datagram was not getting
    updated with the new tasks cgrp prioidx. This leaves IO on
    the socket tagged with the old tasks priority.

    To fix this add a check in the scm recvmsg path to update the
    sock cgrp prioidx with the new tasks value.

As I read this this should work for net_prio.

> Similar with cls cgroup.

but not for cls_cgroup because I forgot to apply that fix as well. Just
a sec, about to
send a patch for this.

Thanks for looking into this!

cheers,
daniel



WARNING: multiple messages have this Message-ID (diff)
From: Daniel Wagner <wagi@monom.org>
To: Li Zefan <lizefan@huawei.com>
Cc: John Fastabend <john.r.fastabend@intel.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	Daniel Wagner <daniel.wagner@bmw-carit.de>,
	LKML <linux-kernel@vger.kernel.org>,
	netdev@vger.kernel.org, Cgroups <cgroups@vger.kernel.org>
Subject: Re: [BUG] Bug in netprio_cgroup and netcls_cgroup ?
Date: Mon, 21 Jan 2013 09:50:12 +0100	[thread overview]
Message-ID: <50FD0144.1000401@monom.org> (raw)
In-Reply-To: <50FCDB5C.4050608@huawei.com>

Hi Li,

On 21.01.2013 07:08, Li Zefan wrote:
> I'm not a network developer, so correct me if I'm wrong.
> 
> Since commit 7955490f732c2b8
> ("net: netprio_cgroup: rework update socket logic"), sock->sk->sk_cgrp_prioidx
> is set when the socket is created, and won't be updated unless the task is
> moved to another cgroup.
> 
> Now the problem is, a socket can be _shared_ by multiple processes (fork, SCM_RIGHT).
> If we place those processes in different cgroups, and each cgroup has
> different configs, but all of the processes will send data via this socket
> with the same network priority.

Wouldn't that be addressed by 48a87cc26c13b68f6cce4e9d769fcb17a6b3e4b8

   net: netprio: fd passed in SCM_RIGHTS datagram not set correctly

    A socket fd passed in a SCM_RIGHTS datagram was not getting
    updated with the new tasks cgrp prioidx. This leaves IO on
    the socket tagged with the old tasks priority.

    To fix this add a check in the scm recvmsg path to update the
    sock cgrp prioidx with the new tasks value.

As I read this this should work for net_prio.

> Similar with cls cgroup.

but not for cls_cgroup because I forgot to apply that fix as well. Just
a sec, about to
send a patch for this.

Thanks for looking into this!

cheers,
daniel




  parent reply	other threads:[~2013-01-21  8:50 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-21  6:08 [BUG] Bug in netprio_cgroup and netcls_cgroup ? Li Zefan
2013-01-21  6:08 ` Li Zefan
2013-01-21  6:08 ` Li Zefan
     [not found] ` <50FCDB5C.4050608-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-01-21  8:50   ` Daniel Wagner [this message]
2013-01-21  8:50     ` Daniel Wagner
     [not found]     ` <50FD0144.1000401-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2013-01-21  9:01       ` Li Zefan
2013-01-21  9:01         ` Li Zefan
2013-01-21  9:01         ` Li Zefan
     [not found]         ` <50FD0402.6060400-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-01-21  9:21           ` John Fastabend
2013-01-21  9:21             ` John Fastabend
     [not found]             ` <50FD0893.1050805-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-01-21  9:52               ` Li Zefan
2013-01-21  9:52                 ` Li Zefan
2013-01-21  9:52                 ` Li Zefan
2013-01-21  9:27           ` Daniel Wagner
2013-01-21  9:27             ` Daniel Wagner
2013-01-21  9:57             ` Li Zefan
2013-01-21  9:57               ` Li Zefan
2013-01-21 17:18               ` John Fastabend
     [not found]                 ` <50FD786E.4050108-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-01-22 10:09                   ` Daniel Wagner
2013-01-22 10:09                     ` Daniel Wagner
2013-01-22 10:09                     ` Daniel Wagner
2013-01-23  0:02                     ` John Fastabend
     [not found]                       ` <50FF287C.70906-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-01-23  9:24                         ` Daniel Wagner
2013-01-23  9:24                           ` Daniel Wagner
2013-01-25  8:39                     ` Li Zefan
2013-01-25  8:39                       ` Li Zefan

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=50FD0144.1000401@monom.org \
    --to=wagi-kqcpca+x3s7ytjvyw6ydsg@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org \
    --cc=john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.