On Mon, Jan 10, 2011 at 10:35:19PM -0600, Rob Landley wrote:
> From: Rob Landley <[email protected]>
>
> Teach cifs about network namespaces, so mounting uses adresses and
> routing visible from a container rather than from init context.
>
> For a long drawn out test reproduction sequence, see:
>
> http://landley.livejournal.com/47024.html
> http://landley.livejournal.com/47205.html
> http://landley.livejournal.com/47476.html
>
> Signed-off-by: Rob Landley <[email protected]>
> ---
>
> fs/cifs/cifsglob.h | 32 ++++++++++++++++++++++++++++++++
> fs/cifs/connect.c | 22 +++++++++++++++++-----
> 2 files changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 7136c0c..86f31bb 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -168,6 +168,9 @@ struct TCP_Server_Info {
> struct sockaddr_in6 sockAddr6;
> } addr;
> struct sockaddr_storage srcaddr; /* locally bind to this IP */
> +#ifdef CONFIG_NET_NS
> + struct net *net;
> +#endif
I'm assuming this bit is correct -- don't know enough about CIFS to be
sure...
> wait_queue_head_t response_q;
> wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
> struct list_head pending_mid_q;
> @@ -227,6 +230,35 @@ struct TCP_Server_Info {
> };
>
> /*
> + * Macros to allow the TCP_Server_Info->net field and related code to drop
> out
> + * when CONFIG_NET_NS isn't set.
> + */
> +
> +static inline struct net *
> +cifs_net_ns(struct TCP_Server_Info *srv)
> +{
> +#ifdef CONFIG_NET_NS
> + return srv->net;
> +#else
> + return &init_net;
> +#endif
> +}
I thought style dictated we do this a different way:
#ifdef CONFIG_NET_NS
static inline struct net * cifs_net_ns(struct TCP_Server_Info *srv)
{
return srv->net;
}
<other CONFIG_NET_NS cases>
#else
static inline struct net * cifs_net_ns(struct TCP_Server_Info *srv)
{
return &init_net;
}
<other no-ops>
#endif /* CONFIG_NET_NS */
> +
> +static inline void
> +cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
> +{
> +#ifdef CONFIG_NET_NS
> + srv->net = net;
> +#endif
> +}
> +
> +#ifdef CONFIG_NET_NS
> +#define cifs_use_net_ns() (1)
> +#else
> +#define cifs_use_net_ns() (0)
> +#endif
This looks wrong -- we shouldn't need this at all. The #ifdef bits in
your patch already make all the cases below become empty/no-ops when
CONFIG_NET_NS=n.
> +
> +/*
> * Session structure. One of these for each uid session with a particular
> host
> */
> struct cifsSesInfo {
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index cc1a860..b4faef0 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1545,6 +1545,10 @@ cifs_find_tcp_session(struct sockaddr *addr, struct
> smb_vol *vol)
>
> spin_lock(&cifs_tcp_ses_lock);
> list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
> + if (cifs_use_net_ns()
> + && cifs_net_ns(server) == current->nsproxy->net_ns)
> + continue;
This looks wrong -- you want to invert part of this I think (and drop the
unnecessary cifs_use_net_ns()):
if (cifs_net_ns(server) != current->nsproxy->net_ns)
continue;
This is obvious when you note that the context below shows that we
'continue' to the next entry when the addresses don't match:
> +
> if (!match_address(server, addr,
> (struct sockaddr *)&vol->srcaddr))
> continue;
> @@ -1572,6 +1576,9 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
> return;
> }
>
> + if (cifs_use_net_ns())
> + put_net(cifs_net_ns(server));
> +
I think this should just be:
put_net(cifs_net_ns(server));
> list_del_init(&server->tcp_ses_list);
> spin_unlock(&cifs_tcp_ses_lock);
>
> @@ -1677,6 +1684,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
> sizeof(tcp_ses->srcaddr));
> ++tcp_ses->srv_count;
>
> + if (cifs_use_net_ns())
> + cifs_set_net_ns(tcp_ses, get_net(current->nsproxy->net_ns));
> +
Just use cifs_set_net_ns() because it already turns into a no-op when
CONFIG_NET_NS=n
> if (addr.ss_family == AF_INET6) {
> cFYI(1, "attempting ipv6 connect");
> /* BB should we allow ipv6 on port 139? */
> @@ -1720,6 +1730,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
> out_err_crypto_release:
> cifs_crypto_shash_release(tcp_ses);
>
> + if (cifs_use_net_ns())
> + put_net(cifs_net_ns(tcp_ses));
etc.
> +
> out_err:
> if (tcp_ses) {
> if (!IS_ERR(tcp_ses->hostname))
> @@ -2145,8 +2158,8 @@ ipv4_connect(struct TCP_Server_Info *server)
> struct socket *socket = server->ssocket;
>
> if (socket == NULL) {
> - rc = sock_create_kern(PF_INET, SOCK_STREAM,
> - IPPROTO_TCP, &socket);
> + rc = __sock_create(cifs_net_ns(server), PF_INET,
> + SOCK_STREAM, IPPROTO_TCP, &socket, 1);
> if (rc < 0) {
> cERROR(1, "Error %d creating socket", rc);
> return rc;
> @@ -2310,11 +2323,10 @@ ipv6_connect(struct TCP_Server_Info *server)
> struct socket *socket = server->ssocket;
>
> if (socket == NULL) {
> - rc = sock_create_kern(PF_INET6, SOCK_STREAM,
> - IPPROTO_TCP, &socket);
> + rc = __sock_create(cifs_net_ns(server), PF_INET6,
> + SOCK_STREAM, IPPROTO_TCP, &socket, 1);
> if (rc < 0) {
> cERROR(1, "Error %d creating ipv6 socket", rc);
> - socket = NULL;
> return rc;
> }
>
> _______________________________________________
> Containers mailing list
> [email protected]
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel
