Home » Mailing lists » Devel » Re: [PATCH] Teach cifs about network namespaces (take 2)
| Re: [PATCH] Teach cifs about network namespaces (take 2) [message #41994] |
Wed, 12 January 2011 13:57  |
Rob Landley
Messages: 19
Registered: December 2010
|
Junior Member |
|
|
On 01/11/2011 03:30 PM, Jeff Layton wrote:
> I've got a patch queued that rearranges some fields in TCP_Server_Info
> according to pahole's recommendations. You may want to base this patch
> on that.
Queued where?
> It might also be good to run pahole on this after your patch to see if
> it might be better placed...
I put it right after the network address fields affected by it. I can
stick it between:
char *hostname; /* hostname portion of UNC string */
struct socket *ssocket;
So that two consecutive pointers become three consecutive pointers. But
again, I haven't seen what you did to the structure yet...
>> spin_lock(&cifs_tcp_ses_lock);
>> list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
>> + if (HAVE_NET_NS &&
>> + cifs_net_ns(server) != current->nsproxy->net_ns)
>> + continue;
>> +
>
> This HAVE_NET_NS thing is pretty ugly.
It's a compile time constant optimized away by the compiler, and cleanly
taking out the entire dependent clause with it when it's false (because
&& only evaluates the right side when the left side is true, meaning the
if can never trigger, thus dead code elimination).
How is that ugly? It does exactly what it says. I'm not sure what the
criteria are here. (Was it better when it looked like a function rather
than a constant? I'd use the CONFIG_NET_NS symbol directly if it was 0
or 1 rather than undefined or 1, but it isn't. It's designed for use
with #ifdefs only, not from C code.)
> This is not a high-performance
> codepath. My vote would be to get rid of this and just have the useless
> test when CONFIG_NET_NS isn't set.
>
> Alternately, you could turn the comparison into a static inline or
> macro, and simply have that compile down to nothing when CONFIG_NET_NS
> isn't set.
I tried returning a constant from a static inline, it doesn't propogate
the constant far enough to do compile time dead code elimination based
on the return value under gcc 4.4.3.
As for a macro, the constant already is a macro. So adding more #ifdefs
to the headers to make the code in the function look like it's doing
something other than it actually is somehow improves matters? (I'm not
understanding the aesthetic criteria here at all.)
I think I'll just ignore the bloat. Make allnoconfig is already 800k
compressed, what's a few more bytes...
>> @@ -1754,6 +1762,8 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>> out_err_crypto_release:
>> cifs_crypto_shash_release(tcp_ses);
>>
>> + put_net(cifs_net_ns(tcp_ses));
>> +
> ^^^^
> This looks like it will oops if you end up doing the "goto
> out_err_crypto_release" after the extract_hostname call.
Hmmm... Yup, failure case when CONFIG_NET_NS enabled will dereference
the null pointer. I need to move the initialization up a few lines.
Thanks.
Rob
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containe rs
|
|
|
|
| Re: [PATCH] Teach cifs about network namespaces (take 3) [message #41997 is a reply to message #41994] |
Thu, 13 January 2011 18:55  |
Rob Landley
Messages: 19
Registered: December 2010
|
Junior Member |
|
|
From: Rob Landley <rlandley@parallels.com>
Teach cifs about network namespaces, so mounting uses adresses/routing
visible from the container rather than from init context.
Signed-off-by: Rob Landley <rlandley@parallels.com>
---
Now using net_eq(), with the initialization moved up so the error path doesn't
dereference a null on the put.
fs/cifs/cifsglob.h | 33 +++++++++++++++++++++++++++++++++
fs/cifs/connect.c | 12 ++++++++++--
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 606ca8b..54cd4ab 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -165,6 +165,9 @@ struct TCP_Server_Info {
struct socket *ssocket;
struct sockaddr_storage dstaddr;
struct sockaddr_storage srcaddr; /* locally bind to this IP */
+#ifdef CONFIG_NET_NS
+ struct net *net;
+#endif
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;
@@ -224,6 +227,36 @@ 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.
+ */
+
+#ifdef CONFIG_NET_NS
+
+static inline struct net *cifs_net_ns(struct TCP_Server_Info *srv)
+{
+ return srv->net;
+}
+
+static inline void cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
+{
+ srv->net = net;
+}
+
+#else
+
+static inline struct net *cifs_net_ns(struct TCP_Server_Info *srv)
+{
+ return &init_net;
+}
+
+static inline void cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
+{
+}
+
+#endif
+
+/*
* 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 a65d311..53679f6 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1577,6 +1577,9 @@ 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 (!net_eq(cifs_net_ns(server), current->nsproxy->net_ns))
+ continue;
+
if (!match_address(server, addr,
(struct sockaddr *)&vol->srcaddr))
continue;
@@ -1607,6 +1610,8 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
return;
}
+ put_net(cifs_net_ns(server));
+
list_del_init(&server->tcp_ses_list);
spin_unlock(&cifs_tcp_ses_lock);
@@ -1679,6 +1684,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
goto out_err;
}
+ cifs_set_net_ns(tcp_ses, get_net(current->nsproxy->net_ns));
tcp_ses->hostname = extract_hostname(volume_info->UNC);
if (IS_ERR(tcp_ses->hostname)) {
rc = PTR_ERR(tcp_ses->hostname);
@@ -1754,6 +1760,8 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
out_err_crypto_release:
cifs_crypto_shash_release(tcp_ses);
+ put_net(cifs_net_ns(tcp_ses));
+
out_err:
if (tcp_ses) {
if (!IS_ERR(tcp_ses->hostname))
@@ -2265,8 +2273,8 @@ generic_ip_connect(struct TCP_Server_Info *server)
}
if (socket == NULL) {
- rc = sock_create_kern(sfamily, SOCK_STREAM,
- IPPROTO_TCP, &socket);
+ rc = __sock_create(cifs_net_ns(server), sfamily, SOCK_STREAM,
+ IPPROTO_TCP, &socket, 1);
if (rc < 0) {
cERROR(1, "Error %d creating socket", rc);
server->ssocket = NULL;
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containe rs
|
|
|
|
Goto Forum:
Current Time: Sun Sep 07 23:16:59 GMT 2025
Total time taken to generate the page: 0.08650 seconds
|
OpenVZ Forum
Members
Search
Help
Register
Login
Home
