Re: [PATCH 1/2] Traffic control cgroups subsystem

[Thomas Graf <tgraf@suug.ch>]

* Ranjit Manomohan <ranjitm@google.com> 2008-09-10 10:42
> +void cgroup_tc_set_sock_classid(struct sock *sk)
> +{
> +	if (sk)
> +		sk->sk_cgroup_classid = cgroup_tc_classid(current);
> +}
> +
> @@ -1170,6 +1171,8 @@ static int __sock_create(struct net *net, int family, int type, int protocol,
>  	if (err < 0)
>  		goto out_module_put;
> 
> +	cgroup_tc_set_sock_classid(sock->sk);
> +
>  	/*
>  	 * Now to bump the refcnt of the [loadable] module that owns this
>  	 * socket at sock_release time we decrement its refcnt.
> @@ -1444,6 +1447,8 @@ asmlinkage long sys_accept(int fd, struct sockaddr __user *upeer_sockaddr,
>  	if (err < 0)
>  		goto out_fd;
> 
> +	cgroup_tc_set_sock_classid(newsock->sk);
> +
>  	if (upeer_sockaddr) {
>  		if (newsock->ops->getname(newsock, (struct sockaddr *)address,
>  					  &len, 2) < 0) {

The big disadvantage of this method is that it does not allow to change
the classid for sockets which already exist. It inherits the classid
at socket creation time and then sticks to it. So if you want to follow
this approach I'd suggest to at least store a reference to the cgroup
state and reference count it properly.

As for the locking that you mentioned in the other thread. IMHO it is
not possible to lookup a socket without taking at least one lock, but
I might be wrong there. Actually I think it will take even more locks
as different locks are used to f.e. protect listening and established
tcp sockets.