Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161034AbaDPSwG (ORCPT ); Wed, 16 Apr 2014 14:52:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37900 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755982AbaDPSwB (ORCPT ); Wed, 16 Apr 2014 14:52:01 -0400 Date: Wed, 16 Apr 2014 14:51:54 -0400 From: Vivek Goyal To: Andy Lutomirski Cc: Simo Sorce , David Miller , Tejun Heo , Daniel Walsh , "linux-kernel@vger.kernel.org" , lpoetter@redhat.com, cgroups@vger.kernel.org, kay@redhat.com, Network Development Subject: Re: [PATCH 2/2] net: Implement SO_PASSCGROUP to enable passing cgroup path Message-ID: <20140416185153.GI31074@redhat.com> References: <20140416002010.GA5035@redhat.com> <20140416.085743.1614257692560892039.davem@davemloft.net> <1397664837.19767.410.camel@willson.li.ssimo.org> <1397667766.19767.440.camel@willson.li.ssimo.org> <20140416183614.GH31074@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Apr 16, 2014 at 11:40:44AM -0700, Andy Lutomirski wrote: > On Wed, Apr 16, 2014 at 11:36 AM, Vivek Goyal wrote: > > On Wed, Apr 16, 2014 at 10:29:08AM -0700, Andy Lutomirski wrote: > > > > [..] > >> >> Admittedly cgroups aren't currently as important as uid, but if this > >> >> changes, then SO_PASSCGROUP, as currently written, will have *exactly* > >> >> the same problem. > >> > > >> > Which is easy to foil by using SO_PEERCGROUP and find out who originally > >> > opened the socket, which is why that is also available! > >> > >> Then please remove SO_PASSCGROUP. > > > > SO_PASSCGROUP is important because SO_PEERCGROUP does not work with unix > > datagram sockets. > > Right. I forgot about that. > > > > > Again going back to logging example, if some clients are logging to unix > > datagram sockets, SO_PASSCGROUP is the only option to figure out cgroup > > of client. > > Hmm. I think that, in your patch, the cgroup that is sent is the > cgroup of the caller of write/send/sendmsg. What if you changed it to > use the same cgroup that SO_PEERCRED would use? Would that still > work? No. SO_PEERCRED stores the cgroup information once at the time of connect(). After that it never changes. What if sender changes the cgroup. That information will not be captured. Also what if multiple client use the same socket fd to writer to logger? In that case too storing cgroup info in socket will not help. Cgroup is sender task's property and not client side socket's property. > > >> > >> I still haven't seen any explanation for what's wrong with requiring > >> senders to ask the kernel to transmit their cgroup. > > > > If nothing else, additional complexity and ovhead. Extra pair of messages > > need to be exchanged to request and then provide the information. > > > > How would it work in logging example? Every time logger receives a > > message, is it supposed to send another message to client to send > > SCM_CGROUP? That does not sound right. > > No -- just have the logger send the cgroup with every message. Yes, > it seems silly, but it's probably barely more expensive than with the > code in your patch. So receiver gets the cgroup messages even if it might not want to. There is no way to say "Hey don't send me SCM_CGROUP's messages". Now all loggers need to be modifed to always send SCM_CGROUP messages. And all other more complicated cases might need a different consideration and clients and servers will need to be modified accordingly. I think it is much simpler to allow passing of cgroup information and once we figure out some concrete cases where passing of that info is not desirabe, implement SO_NOPASSCGROUP and modify those *selected few corner cases* to set this flag on sockets. Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/