All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Serge E. Hallyn" <serue@us.ibm.com>
To: Dan Smith <danms@us.ibm.com>
Cc: containers@lists.osdl.org,
	Vlad Yasevich <vladislav.yasevich@hp.com>,
	netdev@vger.kernel.org, David Miller <davem@davemloft.net>
Subject: Re: [PATCH] [RFC] C/R: inet4 and inet6 unicast routes (v2)
Date: Fri, 30 Apr 2010 13:19:46 -0500	[thread overview]
Message-ID: <20100430181946.GA26761@us.ibm.com> (raw)
In-Reply-To: <1272646855-17327-1-git-send-email-danms@us.ibm.com>

Quoting Dan Smith (danms@us.ibm.com):
> +static int temp_netns_enter(struct net *net)
> +{
> +	int ret;
> +	struct net *tmp_netns;
> +
> +	ret = copy_namespaces(CLONE_NEWNET, current);
> +	if (ret)
> +		return ret;

Actually there is one problem here - copy_namespaces() is
specifically used only by clone() and it expects tsk to
not yet be live.  So it just does

	tsk->nsproxy = new_ns

Since you're doing this on current which is live, it would
have to use rcu_assign_pointer() to be safe.

So I'm afraid you're going to have to do a slightly uglier
thing where you unshare_nsproxy_namespaces() and then
switch_task_namespaces() to the new nsproxy.

> +
> +	tmp_netns = current->nsproxy->net_ns;
> +	get_net(net);
> +	current->nsproxy->net_ns = net;
> +	put_net(tmp_netns);
> +
> +	return 0;
> +}

Otherwise it looks good to me.  My only other comment would be to soothe
readers' anxieties by putting a comment right here explaining that
switch_task_namespaces() will drop your ref to current->nsproxy->net_ns,
and that you had never dropped the ref to prev so it will be safe.

> +static void temp_netns_exit(struct nsproxy *prev)
> +{
> +	switch_task_namespaces(current, prev);
> +}

thanks,
-serge

  reply	other threads:[~2010-04-30 18:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-30 17:00 [PATCH] [RFC] C/R: inet4 and inet6 unicast routes (v2) Dan Smith
2010-04-30 18:19 ` Serge E. Hallyn [this message]
2010-04-30 18:25   ` Dan Smith
2010-04-30 18:37     ` Serge E. Hallyn
2010-04-30 20:35 ` Daniel Lezcano
2010-04-30 21:24   ` Dan Smith
     [not found]     ` <87bpd0zl9l.fsf-FLMGYpZoEPULwtHQx/6qkW3U47Q5hpJU@public.gmane.org>
2010-05-01  0:26       ` jamal
2010-05-03 14:21         ` Dan Smith
2010-05-03 20:34           ` jamal
2010-05-01  2:02     ` Oren Laadan
     [not found]   ` <4BDB3F07.2030900-GANU6spQydw@public.gmane.org>
2010-05-01  1:42     ` Oren Laadan

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=20100430181946.GA26761@us.ibm.com \
    --to=serue@us.ibm.com \
    --cc=containers@lists.osdl.org \
    --cc=danms@us.ibm.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=vladislav.yasevich@hp.com \
    /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.