OpenVZ Forum


Home » Mailing lists » Devel » [PATCH 1/4] NFS: make nfs_client_list per net ns
[PATCH 1/4] NFS: make nfs_client_list per net ns [message #45043] Mon, 23 January 2012 17:26 Go to next message
Stanislav Kinsbursky is currently offline  Stanislav Kinsbursky
Messages: 683
Registered: October 2011
Senior Member
From: *parallels.com
This patch splits global list of NFS clients into per-net-ns array of lists.
This looks more strict and clearer.
BTW, this patch also makes "/proc/fs/nfsfs/servers" entry content depends on
/proc mount owner pid namespace. See below for details.

NOTE: few words about how was /proc/fs/nfsfs/ entries content show per network
namespace done. This is a little bit tricky and not the best is could be. But
it's cheap (proper fix for /proc conteinerization is a hard nut to crack).
The idea is simple: take proper network namespace from pid namespace
child reaper nsproxy of /proc/ mount creator.
This actually means, that if there are 2 containers with different net
namespace sharing pid namespace, then read of /proc/fs/nfsfs/ entries will
always return content, taken from net namespace of pid namespace creator task
(and thus second namespace set wil be unvisible).

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>

---
fs/nfs/client.c | 38 +++++++++++++++++++++++++++-----------
fs/nfs/idmap.c | 5 ++---
fs/nfs/inode.c | 1 +
fs/nfs/internal.h | 2 +-
fs/nfs/netns.h | 1 +
5 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 98af1cb..058eb9b 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -39,6 +39,8 @@
#include <net/ipv6.h>
#include <linux/nfs_xdr.h>
#include <linux/sunrpc/bc_xprt.h>
+#include <linux/nsproxy.h>
+#include <linux/pid_namespace.h>

#include <asm/system.h>

@@ -49,11 +51,11 @@
#include "internal.h"
#include "fscache.h"
#include "pnfs.h"
+#include "netns.h"

#define NFSDBG_FACILITY NFSDBG_CLIENT

DEFINE_SPINLOCK(nfs_client_lock);
-LIST_HEAD(nfs_client_list);
static LIST_HEAD(nfs_volume_list);
static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
#ifdef CONFIG_NFS_V4
@@ -464,8 +466,9 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
{
struct nfs_client *clp;
const struct sockaddr *sap = data->addr;
+ struct nfs_net *nn = net_generic(data->net, nfs_net_id);

- list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
+ list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
const struct sockaddr *clap = (struct sockaddr *)&clp->cl_addr;
/* Don't match clients that failed to initialise properly */
if (clp->cl_cons_state < 0)
@@ -483,9 +486,6 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
/* Match the full socket address */
if (!nfs_sockaddr_cmp(sap, clap))
continue;
- /* Match network namespace */
- if (clp->net != data->net)
- continue;

atomic_inc(&clp->cl_count);
return clp;
@@ -506,6 +506,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
{
struct nfs_client *clp, *new = NULL;
int error;
+ struct nfs_net *nn = net_generic(cl_init->net, nfs_net_id);

dprintk("--> nfs_get_client(%s,v%u)\n",
cl_init->hostname ?: "", cl_init->rpc_ops->version);
@@ -531,7 +532,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
/* install a new client and return with it unready */
install_client:
clp = new;
- list_add(&clp->cl_share_link, &nfs_client_list);
+ list_add(&clp->cl_share_link, &nn->nfs_client_list);
spin_unlock(&nfs_client_lock);

error = cl_init->rpc_ops->init_client(clp, timeparms, ip_addr,
@@ -1227,9 +1228,10 @@ nfs4_find_client_sessionid(const struct sockaddr *addr,
struct nfs4_sessionid *sid)
{
struct nfs_client *clp;
+ struct nfs_net *nn = net_generic(&init_net, nfs_net_id);

spin_lock(&nfs_client_lock);
- list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
+ list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
if (nfs4_cb_match_client(addr, clp, 1) == false)
continue;

@@ -1757,6 +1759,13 @@ out_free_server:
return ERR_PTR(error);
}

+void nfs_clients_init(struct net *net)
+{
+ struct nfs_net *nn = net_generic(net, nfs_net_id);
+
+ INIT_LIST_HEAD(&nn->nfs_client_list);
+}
+
#ifdef CONFIG_PROC_FS
static struct proc_dir_entry *proc_fs_nfs;

@@ -1810,13 +1819,15 @@ static int nfs_server_list_open(struct inode *inode, struct file *file)
{
struct seq_file *m;
int ret;
+ struct pid_namespace *pid_ns = file->f_dentry->d_sb->s_fs_info;
+ struct net *net = pid_ns->child_reaper->nsproxy->net_ns;

ret = seq_open(file, &nfs_server_list_ops);
if (ret < 0)
return ret;

m = file->private_data;
- m->private = PDE(inode)->data;
+ m->private = net;

return 0;
}
@@ -1826,9 +1837,11 @@ static int nfs_server_list_open(struct inode *inode, struct file *file)
*/
static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
{
+ struct nfs_net *nn = net_generic(m->private, nfs_net_id);
+
/* lock the list against modification */
spin_lock(&nfs_client_lock);
- return seq_list_start_head(&nfs_client_list, *_pos);
+ return seq_list_start_head(&nn->nfs_client_list, *_pos);
}

/*
@@ -1836,7 +1849,9 @@ static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
*/
static void *nfs_server_list_next(struct seq_file *p, void *v, loff_t *pos)
{
- return seq_list_next(v, &nfs_client_list, pos);
+ struct nfs_net *nn = net_generic(p->private, nfs_net_id);
+
+ return seq_list_next(v, &nn->nfs_client_list, pos);
}

/*
@@ -1853,9 +1868,10 @@ static void nfs_server_list_stop(struct seq_file *p, void *v)
static int nfs_server_list_show(struct seq_file *m, void *v)
{
struct nfs_client *clp;
+ struct nfs_net *nn = net_generic(m->private, nfs_net_id);

/* display header on line 1 */
- if (v == &nfs_client_list) {
+ if (v == &nn->nfs_client_list) {
seq_puts(m, "NV SERVER PORT USE HOSTNAME\n");
return 0;
}
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index ff084d2..92deaf8 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -571,13 +571,12 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
void *ptr)
{
struct super_block *sb = ptr;
+ struct nfs_net *nn = net_generic(sb->s_fs_info, nfs_net_id);
struct nfs_client *clp;
int error = 0;

spin_lock(&nfs_client_lock);
- list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
- if (clp->net != sb->s_fs_info)
- continue;
+ list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
if (clp->rpc_ops != &nfs_v4_clientops)
continue;
error = __rpc_pipefs_event(clp, event, sb);
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index d2c760e..e8f81e5 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1558,6 +1558,7 @@ EXPORT_SYMBOL_GPL(nfs_net_id);

static int nfs_net_init(struct net *net)
{
+ nfs_clients_init(net);
return nfs_dns_resolver_cache_init(net);
}

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index cdb121d..a9ae806 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -146,6 +146,7 @@ extern void nfs_umount(const struct nfs_mount_request *info);

/* client.c */
extern const struct rpc_program nfs_program;
+extern void nfs_clients_init(struct net *net);

extern void nfs_cleanup_cb_ident_idr(void);
extern void nfs_put_client(struct nfs_client *);
@@ -183,7 +184,6 @@ static inline void nfs_fs_proc_exit(void)
#endif
#ifdef CONFIG_NFS_V4
extern spinlock_t nfs_client_lock;
-extern struct list_head nfs_client_list;
#endif

/* nfs4namespace.c */
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index 39ae4ca..feb33c3 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -7,6 +7,7 @@
struct nfs_net {
struct cache_detail *nfs_dns_resolve;
struct rpc_pipe *bl_device_pipe;
+ struct list_head nfs_client_list;
};

extern int nfs_net_id;
Re: [PATCH 1/4] NFS: make nfs_client_list per net ns [message #45181 is a reply to message #45043] Tue, 07 February 2012 15:34 Go to previous messageGo to next message
Myklebust, Trond is currently offline  Myklebust, Trond
Messages: 52
Registered: November 2011
Member
From: 10.104.60*
On Tue, 2012-02-07 at 10:32 -0500, Bryan Schumaker wrote:
> I fixed up my bisect and identified this patch instead. The problem I'm seeing is:
>
> CC [M] fs/nfs/delegation.o
> CC [M] fs/nfs/idmap.o
> fs/nfs/idmap.c: In function 'rpc_pipefs_event':
> fs/nfs/idmap.c:535:9: error: implicit declaration of function 'net_generic' [-Werror=implicit-function-declaration]
> fs/nfs/idmap.c:535:50: error: 'nfs_net_id' undeclared (first use in this function)
> fs/nfs/idmap.c:535:50: note: each undeclared identifier is reported only once for each function it appears in
> fs/nfs/idmap.c:540:82: error: dereferencing pointer to incomplete type
> fs/nfs/idmap.c:540:224: error: dereferencing pointer to incomplete type
> cc1: some warnings being treated as errors
>
> make[2]: *** [fs/nfs/idmap.o] Error 1
> make[1]: *** [fs/nfs] Error 2
> make: *** [fs] Error 2
>
> I think you need to add: #include "netns.h" to idmap.c.
>
> - Bryan

That should already be there in the 'devel' branch. See commit
17347d03c008e2f504c33bb4905cdad0abc01319.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
Re: [PATCH 1/4] NFS: make nfs_client_list per net ns [message #45191 is a reply to message #45043] Tue, 07 February 2012 15:32 Go to previous messageGo to next message
Bryan Schumaker is currently offline  Bryan Schumaker
Messages: 6
Registered: February 2012
Junior Member
From: *parallels.com
I fixed up my bisect and identified this patch instead. The problem I'm seeing is:

CC [M] fs/nfs/delegation.o
CC [M] fs/nfs/idmap.o
fs/nfs/idmap.c: In function 'rpc_pipefs_event':
fs/nfs/idmap.c:535:9: error: implicit declaration of function 'net_generic' [-Werror=implicit-function-declaration]
fs/nfs/idmap.c:535:50: error: 'nfs_net_id' undeclared (first use in this function)
fs/nfs/idmap.c:535:50: note: each undeclared identifier is reported only once for each function it appears in
fs/nfs/idmap.c:540:82: error: dereferencing pointer to incomplete type
fs/nfs/idmap.c:540:224: error: dereferencing pointer to incomplete type
cc1: some warnings being treated as errors

make[2]: *** [fs/nfs/idmap.o] Error 1
make[1]: *** [fs/nfs] Error 2
make: *** [fs] Error 2

I think you need to add: #include "netns.h" to idmap.c.

- Bryan

On 01/23/12 12:26, Stanislav Kinsbursky wrote:

> This patch splits global list of NFS clients into per-net-ns array of lists.
> This looks more strict and clearer.
> BTW, this patch also makes "/proc/fs/nfsfs/servers" entry content depends on
> /proc mount owner pid namespace. See below for details.
>
> NOTE: few words about how was /proc/fs/nfsfs/ entries content show per network
> namespace done. This is a little bit tricky and not the best is could be. But
> it's cheap (proper fix for /proc conteinerization is a hard nut to crack).
> The idea is simple: take proper network namespace from pid namespace
> child reaper nsproxy of /proc/ mount creator.
> This actually means, that if there are 2 containers with different net
> namespace sharing pid namespace, then read of /proc/fs/nfsfs/ entries will
> always return content, taken from net namespace of pid namespace creator task
> (and thus second namespace set wil be unvisible).
>
> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
>
> ---
> fs/nfs/client.c | 38 +++++++++++++++++++++++++++-----------
> fs/nfs/idmap.c | 5 ++---
> fs/nfs/inode.c | 1 +
> fs/nfs/internal.h | 2 +-
> fs/nfs/netns.h | 1 +
> 5 files changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 98af1cb..058eb9b 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -39,6 +39,8 @@
> #include <net/ipv6.h>
> #include <linux/nfs_xdr.h>
> #include <linux/sunrpc/bc_xprt.h>
> +#include <linux/nsproxy.h>
> +#include <linux/pid_namespace.h>
>
> #include <asm/system.h>
>
> @@ -49,11 +51,11 @@
> #include "internal.h"
> #include "fscache.h"
> #include "pnfs.h"
> +#include "netns.h"
>
> #define NFSDBG_FACILITY NFSDBG_CLIENT
>
> DEFINE_SPINLOCK(nfs_client_lock);
> -LIST_HEAD(nfs_client_list);
> static LIST_HEAD(nfs_volume_list);
> static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
> #ifdef CONFIG_NFS_V4
> @@ -464,8 +466,9 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
> {
> struct nfs_client *clp;
> const struct sockaddr *sap = data->addr;
> + struct nfs_net *nn = net_generic(data->net, nfs_net_id);
>
> - list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
> + list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
> const struct sockaddr *clap = (struct sockaddr *)&clp->cl_addr;
> /* Don't match clients that failed to initialise properly */
> if (clp->cl_cons_state < 0)
> @@ -483,9 +486,6 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
> /* Match the full socket address */
> if (!nfs_sockaddr_cmp(sap, clap))
> continue;
> - /* Match network namespace */
> - if (clp->net != data->net)
> - continue;
>
> atomic_inc(&clp->cl_count);
> return clp;
> @@ -506,6 +506,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
> {
> struct nfs_client *clp, *new = NULL;
> int error;
> + struct nfs_net *nn = net_generic(cl_init->net, nfs_net_id);
>
> dprintk("--> nfs_get_client(%s,v%u)\n",
> cl_init->hostname ?: "", cl_init->rpc_ops->version);
> @@ -531,7 +532,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
> /* install a new client and return with it unready */
> install_client:
> clp = new;
> - list_add(&clp->cl_share_link, &nfs_client_list);
> + list_add(&clp->cl_share_link, &nn->nfs_client_list);
> spin_unlock(&nfs_client_lock);
>
> error = cl_init->rpc_ops->init_client(clp, timeparms, ip_addr,
> @@ -1227,9 +1228,10 @@ nfs4_find_client_sessionid(const struct sockaddr *addr,
> struct nfs4_sessionid *sid)
> {
> struct nfs_client *clp;
> + struct nfs_net *nn = net_generic(&init_net, nfs_net_id);
>
> spin_lock(&nfs_client_lock);
> - list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
> + list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
> if (nfs4_cb_match_client(addr, clp, 1) == false)
> continue;
>
> @@ -1757,6 +1759,13 @@ out_free_server:
> return ERR_PTR(error);
> }
>
> +void nfs_clients_init(struct net *net)
> +{
> + struct nfs_net *nn = net_generic(net, nfs_net_id);
> +
> + INIT_LIST_HEAD(&nn->nfs_client_list);
> +}
> +
> #ifdef CONFIG_PROC_FS
> static struct proc_dir_entry *proc_fs_nfs;
>
> @@ -1810,13 +1819,15 @@ static int nfs_server_list_open(struct inode *inode, struct file *file)
> {
> struct seq_file *m;
> int ret;
> + struct pid_namespace *pid_ns = file->f_dentry->d_sb->s_fs_info;
> + struct net *net = pid_ns->child_reaper->nsproxy->net_ns;
>
> ret = seq_open(file, &nfs_server_list_ops);
> if (ret < 0)
> return ret;
>
> m = file->private_data;
> - m->private = PDE(inode)->data;
> + m->private = net;
>
> return 0;
> }
> @@ -1826,9 +1837,11 @@ static int nfs_server_list_open(struct inode *inode, struct file *file)
> */
> static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
> {
> + struct nfs_net *nn = net_generic(m->private, nfs_net_id);
> +
> /* lock the list against modification */
> spin_lock(&nfs_client_lock);
> - return seq_list_start_head(&nfs_client_list, *_pos);
> + return seq_list_start_head(&nn->nfs_client_list, *_pos);
> }
>
> /*
> @@ -1836,7 +1849,9 @@ static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
> */
> static void *nfs_server_list_next(struct seq_file *p, void *v, loff_t *pos)
> {
> - return seq_list_next(v, &nfs_client_list, pos);
> + struct nfs_net *nn = net_generic(p->private, nfs_net_id);
> +
> + return seq_list_next(v, &nn->nfs_client_list, pos);
> }
>
> /*
> @@ -1853,9 +1868,10 @@ static void nfs_server_list_stop(struct seq_file *p, void *v)
> static int nfs_server_list_show(struct seq_file *m, void *v)
> {
> struct nfs_client *clp;
> + struct nfs_net *nn = net_generic(m->private, nfs_net_id);
>
> /* display header on line 1 */
> - if (v == &nfs_client_list) {
> + if (v == &nn->nfs_client_list) {
> seq_puts(m, "NV SERVER PORT USE HOSTNAME\n");
> return 0;
> }
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index ff084d2..92deaf8 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -571,13 +571,12 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
> void *ptr)
> {
> struct super_block *sb = ptr;
> + struct nfs_net *nn = net_generic(sb->s_fs_info, nfs_net_id);
> struct nfs_client *clp;
> int error = 0;
>
> spin_lock(&nfs_client_lock);
> - list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
> - if (clp->net != sb->s_fs_info)
> - continue;
> + list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
> if (clp->rpc_ops != &nfs_v4_clientops)
> continue;
> error = __rpc_pipefs_event(clp, event, sb);
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index d2c760e..e8f81e5 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -1558,6 +1558,7 @@ EXPORT_SYMBOL_GPL(nfs_net_id);
>
> static int nfs_net_init(struct net *net)
> {
> + nfs_clients_init(net);
> return nfs_dns_resolver_cache_init(net);
> }
>
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index cdb121d..a9ae806 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -146,6 +146,7 @@ extern void nfs_umount(const struct nfs_mount_request *info);
>
> /* client.c */
> extern const struct rpc_program nfs_program;
> +extern void nfs_clients_init(struct net *net);
>
> extern void nfs_cleanup_cb_ident_idr(void);
> extern void nfs_put_client(struct nfs_client *);
> @@ -183,7 +184,6 @@ static inline void nfs_fs_proc_exit(void)
> #endif
> #ifdef CONFIG_NFS_V4
> extern spinlock_t nfs_client_lock;
> -extern struct list_head nfs_client_list;
> #endif
>
> /* nfs4namespace.c */
> diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
> index 39ae4ca..feb33c3 100644
> --- a/fs/nfs/netns.h
> +++ b/fs/nfs/netns.h
> @@ -7,6 +7,7 @@
> struct nfs_net {
> struct cache_detail *nfs_dns_resolve;
> struct rpc_pipe *bl_device_pipe;
> + struct list_head nfs_client_list;
> };
>
> extern int nfs_net_id;
>
> --
> To unsubsc
...

Re: [PATCH 1/4] NFS: make nfs_client_list per net ns [message #45193 is a reply to message #45181] Tue, 07 February 2012 15:44 Go to previous message
Bryan Schumaker is currently offline  Bryan Schumaker
Messages: 6
Registered: February 2012
Junior Member
From: *parallels.com
On 02/07/12 10:34, Myklebust, Trond wrote:

> On Tue, 2012-02-07 at 10:32 -0500, Bryan Schumaker wrote:
>> I fixed up my bisect and identified this patch instead. The problem I'm seeing is:
>>
>> CC [M] fs/nfs/delegation.o
>> CC [M] fs/nfs/idmap.o
>> fs/nfs/idmap.c: In function 'rpc_pipefs_event':
>> fs/nfs/idmap.c:535:9: error: implicit declaration of function 'net_generic' [-Werror=implicit-function-declaration]
>> fs/nfs/idmap.c:535:50: error: 'nfs_net_id' undeclared (first use in this function)
>> fs/nfs/idmap.c:535:50: note: each undeclared identifier is reported only once for each function it appears in
>> fs/nfs/idmap.c:540:82: error: dereferencing pointer to incomplete type
>> fs/nfs/idmap.c:540:224: error: dereferencing pointer to incomplete type
>> cc1: some warnings being treated as errors
>>
>> make[2]: *** [fs/nfs/idmap.o] Error 1
>> make[1]: *** [fs/nfs] Error 2
>> make: *** [fs] Error 2
>>
>> I think you need to add: #include "netns.h" to idmap.c.
>>
>> - Bryan
>
> That should already be there in the 'devel' branch. See commit
> 17347d03c008e2f504c33bb4905cdad0abc01319.
>


Yes, but once I fixed this I was able to find the initial problem I was searching for (I have no idea how git led me to the wrong patch on my first attempt).

- Bryan
Previous Topic: [PATCH 4/4] NFS: make nfs_client_lock per net ns
Next Topic: [PATCH] NFS: fix nfs4_find_client_sessionid() arguments list
Goto Forum:
  


Current Time: Sat Sep 22 14:53:36 GMT 2018