On Mon, Jan 10, 2011 at 10:35:19PM -0600, Rob Landley wrote: > From: Rob Landley <rlandley@xxxxxxxxxxxxx> > > 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 <rlandley@xxxxxxxxxxxxx> > --- > > 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 > Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx > https://lists.linux-foundation.org/mailman/listinfo/containers -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
