* [PATCH 1/2] CIFS: Simplify ipv*_connect functions into one
@ 2010-11-14 11:29 Pavel Shilovsky
[not found] ` <1289734155-3613-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Pavel Shilovsky @ 2010-11-14 11:29 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Make connect logic more ip-protocol independent. Also move RFC1001 stuff into
separate function.
Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
fs/cifs/connect.c | 303 ++++++++++++++++++-----------------------------------
1 files changed, 100 insertions(+), 203 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 88c84a3..f0d0c1a 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -108,8 +108,7 @@ struct smb_vol {
struct nls_table *local_nls;
};
-static int ipv4_connect(struct TCP_Server_Info *server);
-static int ipv6_connect(struct TCP_Server_Info *server);
+static int ip_connect(struct TCP_Server_Info *server);
/*
* cifs tcp session reconnection
@@ -186,10 +185,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
while ((server->tcpStatus != CifsExiting) &&
(server->tcpStatus != CifsGood)) {
try_to_freeze();
- if (server->addr.sockAddr6.sin6_family == AF_INET6)
- rc = ipv6_connect(server);
- else
- rc = ipv4_connect(server);
+ rc = ip_connect(server);
if (rc) {
cFYI(1, "reconnect error %d", rc);
msleep(3000);
@@ -1592,12 +1588,11 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
/* other OS never observed in Wild doing 139 with v6 */
memcpy(&tcp_ses->addr.sockAddr6, sin_server6,
sizeof(struct sockaddr_in6));
- rc = ipv6_connect(tcp_ses);
- } else {
+ } else
memcpy(&tcp_ses->addr.sockAddr, sin_server,
sizeof(struct sockaddr_in));
- rc = ipv4_connect(tcp_ses);
- }
+
+ rc = ip_connect(tcp_ses);
if (rc < 0) {
cERROR(1, "Error connecting to socket. Aborting operation");
goto out_err;
@@ -1997,15 +1992,91 @@ static void rfc1002mangle(char *target, char *source, unsigned int length)
}
+static int
+ip_rfc1001_connect(struct TCP_Server_Info *server)
+{
+ int rc = 0;
+ /* some servers require RFC1001 sessinit before sending
+ negprot - BB check reconnection in case where second
+ sessinit is sent but no second negprot */
+ struct rfc1002_session_packet *ses_init_buf;
+ struct smb_hdr *smb_buf;
+ ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
+ GFP_KERNEL);
+ if (ses_init_buf) {
+ ses_init_buf->trailer.session_req.called_len = 32;
+
+ if (server->server_RFC1001_name &&
+ server->server_RFC1001_name[0] != 0)
+ rfc1002mangle(ses_init_buf->trailer.
+ session_req.called_name,
+ server->server_RFC1001_name,
+ RFC1001_NAME_LEN_WITH_NULL);
+ else
+ rfc1002mangle(ses_init_buf->trailer.
+ session_req.called_name,
+ DEFAULT_CIFS_CALLED_NAME,
+ RFC1001_NAME_LEN_WITH_NULL);
+
+ ses_init_buf->trailer.session_req.calling_len = 32;
+
+ /* calling name ends in null (byte 16) from old smb
+ convention. */
+ if (server->workstation_RFC1001_name &&
+ server->workstation_RFC1001_name[0] != 0)
+ rfc1002mangle(ses_init_buf->trailer.
+ session_req.calling_name,
+ server->workstation_RFC1001_name,
+ RFC1001_NAME_LEN_WITH_NULL);
+ else
+ rfc1002mangle(ses_init_buf->trailer.
+ session_req.calling_name,
+ "LINUX_CIFS_CLNT",
+ RFC1001_NAME_LEN_WITH_NULL);
+
+ ses_init_buf->trailer.session_req.scope1 = 0;
+ ses_init_buf->trailer.session_req.scope2 = 0;
+ smb_buf = (struct smb_hdr *)ses_init_buf;
+
+ /* sizeof RFC1002_SESSION_REQUEST with no scope */
+ smb_buf->smb_buf_length = 0x81000044;
+ rc = smb_send(server, smb_buf, 0x44);
+ kfree(ses_init_buf);
+ msleep(1); /* RFC1001 layer in at least one server
+ requires very short break before negprot
+ presumably because not expecting negprot
+ to follow so fast. This is a simple
+ solution that works without
+ complicating the code and causes no
+ significant slowing down on mount
+ for everyone else */
+ }
+ /* else the negprot may still work without this
+ even though malloc failed */
+
+ return rc;
+}
static int
-ipv4_connect(struct TCP_Server_Info *server)
+ip_connect(struct TCP_Server_Info *server)
{
int rc = 0;
- int val;
- bool connected = false;
- __be16 orig_port = 0;
+ bool using_ipv6 = false;
+ unsigned short int *sport;
+ int slen;
struct socket *socket = server->ssocket;
+ struct sockaddr *saddr;
+
+ if (server->addr.sockAddr6.sin6_family == AF_INET6) {
+ sport = &server->addr.sockAddr6.sin6_port;
+ using_ipv6 = true;
+ slen = sizeof(struct sockaddr_in6);
+ saddr = (struct sockaddr *) &server->addr.sockAddr6;
+ } else {
+ sport = &server->addr.sockAddr.sin_port;
+ slen = sizeof(struct sockaddr_in);
+ saddr = (struct sockaddr *) &server->addr.sockAddr;
+ }
if (socket == NULL) {
rc = sock_create_kern(PF_INET, SOCK_STREAM,
@@ -2019,54 +2090,30 @@ ipv4_connect(struct TCP_Server_Info *server)
cFYI(1, "Socket created");
server->ssocket = socket;
socket->sk->sk_allocation = GFP_NOFS;
- cifs_reclassify_socket4(socket);
- }
-
- /* user overrode default port */
- if (server->addr.sockAddr.sin_port) {
- rc = socket->ops->connect(socket, (struct sockaddr *)
- &server->addr.sockAddr,
- sizeof(struct sockaddr_in), 0);
- if (rc >= 0)
- connected = true;
- }
-
- if (!connected) {
- /* save original port so we can retry user specified port
- later if fall back ports fail this time */
- orig_port = server->addr.sockAddr.sin_port;
-
- /* do not retry on the same port we just failed on */
- if (server->addr.sockAddr.sin_port != htons(CIFS_PORT)) {
- server->addr.sockAddr.sin_port = htons(CIFS_PORT);
- rc = socket->ops->connect(socket,
- (struct sockaddr *)
- &server->addr.sockAddr,
- sizeof(struct sockaddr_in), 0);
- if (rc >= 0)
- connected = true;
- }
+ if (using_ipv6)
+ cifs_reclassify_socket4(socket);
+ else
+ cifs_reclassify_socket6(socket);
}
- if (!connected) {
- server->addr.sockAddr.sin_port = htons(RFC1001_PORT);
- rc = socket->ops->connect(socket, (struct sockaddr *)
- &server->addr.sockAddr,
- sizeof(struct sockaddr_in), 0);
- if (rc >= 0)
- connected = true;
+
+ if (*sport)
+ /* user specify port manually */
+ rc = socket->ops->connect(socket, saddr, slen, 0);
+ else {
+ /* try with 445 port at first */
+ *sport = htons(CIFS_PORT);
+ rc = socket->ops->connect(socket, saddr, slen, 0);
}
- /* give up here - unless we want to retry on different
- protocol families some day */
- if (!connected) {
- if (orig_port)
- server->addr.sockAddr.sin_port = orig_port;
+ if (rc < 0) {
cFYI(1, "Error %d connecting to server via ipv4", rc);
sock_release(socket);
server->ssocket = NULL;
return rc;
}
+ if (*sport == htons(RFC1001_PORT))
+ rc = ip_rfc1001_connect(server);
/*
* Eventually check for other socket options to change from
@@ -2085,7 +2132,7 @@ ipv4_connect(struct TCP_Server_Info *server)
}
if (server->tcp_nodelay) {
- val = 1;
+ int val = 1;
rc = kernel_setsockopt(socket, SOL_TCP, TCP_NODELAY,
(char *)&val, sizeof(val));
if (rc)
@@ -2096,156 +2143,6 @@ ipv4_connect(struct TCP_Server_Info *server)
socket->sk->sk_sndbuf,
socket->sk->sk_rcvbuf, socket->sk->sk_rcvtimeo);
- /* send RFC1001 sessinit */
- if (server->addr.sockAddr.sin_port == htons(RFC1001_PORT)) {
- /* some servers require RFC1001 sessinit before sending
- negprot - BB check reconnection in case where second
- sessinit is sent but no second negprot */
- struct rfc1002_session_packet *ses_init_buf;
- struct smb_hdr *smb_buf;
- ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
- GFP_KERNEL);
- if (ses_init_buf) {
- ses_init_buf->trailer.session_req.called_len = 32;
- if (server->server_RFC1001_name &&
- server->server_RFC1001_name[0] != 0)
- rfc1002mangle(ses_init_buf->trailer.
- session_req.called_name,
- server->server_RFC1001_name,
- RFC1001_NAME_LEN_WITH_NULL);
- else
- rfc1002mangle(ses_init_buf->trailer.
- session_req.called_name,
- DEFAULT_CIFS_CALLED_NAME,
- RFC1001_NAME_LEN_WITH_NULL);
-
- ses_init_buf->trailer.session_req.calling_len = 32;
-
- /* calling name ends in null (byte 16) from old smb
- convention. */
- if (server->workstation_RFC1001_name &&
- server->workstation_RFC1001_name[0] != 0)
- rfc1002mangle(ses_init_buf->trailer.
- session_req.calling_name,
- server->workstation_RFC1001_name,
- RFC1001_NAME_LEN_WITH_NULL);
- else
- rfc1002mangle(ses_init_buf->trailer.
- session_req.calling_name,
- "LINUX_CIFS_CLNT",
- RFC1001_NAME_LEN_WITH_NULL);
-
- ses_init_buf->trailer.session_req.scope1 = 0;
- ses_init_buf->trailer.session_req.scope2 = 0;
- smb_buf = (struct smb_hdr *)ses_init_buf;
- /* sizeof RFC1002_SESSION_REQUEST with no scope */
- smb_buf->smb_buf_length = 0x81000044;
- rc = smb_send(server, smb_buf, 0x44);
- kfree(ses_init_buf);
- msleep(1); /* RFC1001 layer in at least one server
- requires very short break before negprot
- presumably because not expecting negprot
- to follow so fast. This is a simple
- solution that works without
- complicating the code and causes no
- significant slowing down on mount
- for everyone else */
- }
- /* else the negprot may still work without this
- even though malloc failed */
-
- }
-
- return rc;
-}
-
-static int
-ipv6_connect(struct TCP_Server_Info *server)
-{
- int rc = 0;
- int val;
- bool connected = false;
- __be16 orig_port = 0;
- struct socket *socket = server->ssocket;
-
- if (socket == NULL) {
- rc = sock_create_kern(PF_INET6, SOCK_STREAM,
- IPPROTO_TCP, &socket);
- if (rc < 0) {
- cERROR(1, "Error %d creating ipv6 socket", rc);
- socket = NULL;
- return rc;
- }
-
- /* BB other socket options to set KEEPALIVE, NODELAY? */
- cFYI(1, "ipv6 Socket created");
- server->ssocket = socket;
- socket->sk->sk_allocation = GFP_NOFS;
- cifs_reclassify_socket6(socket);
- }
-
- /* user overrode default port */
- if (server->addr.sockAddr6.sin6_port) {
- rc = socket->ops->connect(socket,
- (struct sockaddr *) &server->addr.sockAddr6,
- sizeof(struct sockaddr_in6), 0);
- if (rc >= 0)
- connected = true;
- }
-
- if (!connected) {
- /* save original port so we can retry user specified port
- later if fall back ports fail this time */
-
- orig_port = server->addr.sockAddr6.sin6_port;
- /* do not retry on the same port we just failed on */
- if (server->addr.sockAddr6.sin6_port != htons(CIFS_PORT)) {
- server->addr.sockAddr6.sin6_port = htons(CIFS_PORT);
- rc = socket->ops->connect(socket, (struct sockaddr *)
- &server->addr.sockAddr6,
- sizeof(struct sockaddr_in6), 0);
- if (rc >= 0)
- connected = true;
- }
- }
- if (!connected) {
- server->addr.sockAddr6.sin6_port = htons(RFC1001_PORT);
- rc = socket->ops->connect(socket, (struct sockaddr *)
- &server->addr.sockAddr6,
- sizeof(struct sockaddr_in6), 0);
- if (rc >= 0)
- connected = true;
- }
-
- /* give up here - unless we want to retry on different
- protocol families some day */
- if (!connected) {
- if (orig_port)
- server->addr.sockAddr6.sin6_port = orig_port;
- cFYI(1, "Error %d connecting to server via ipv6", rc);
- sock_release(socket);
- server->ssocket = NULL;
- return rc;
- }
-
- /*
- * Eventually check for other socket options to change from
- * the default. sock_setsockopt not used because it expects
- * user space buffer
- */
- socket->sk->sk_rcvtimeo = 7 * HZ;
- socket->sk->sk_sndtimeo = 5 * HZ;
-
- if (server->tcp_nodelay) {
- val = 1;
- rc = kernel_setsockopt(socket, SOL_TCP, TCP_NODELAY,
- (char *)&val, sizeof(val));
- if (rc)
- cFYI(1, "set TCP_NODELAY socket option error %d", rc);
- }
-
- server->ssocket = socket;
-
return rc;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] CIFS: Add match_port check during looking for existing connection
[not found] ` <1289734155-3613-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-11-14 11:29 ` Pavel Shilovsky
[not found] ` <1289734155-3613-2-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-11-14 18:46 ` [PATCH 1/2] CIFS: Simplify ipv*_connect functions into one Pavel Shilovsky
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Shilovsky @ 2010-11-14 11:29 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
If we have a share mounted by non-standard port and try to mount another share
on the same host with standard port, we connect the first share again that's wrong.
This patch fixes this bug.
Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
fs/cifs/connect.c | 38 ++++++++++++++++++++++++++++++++------
1 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index f0d0c1a..483ccb2 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1371,6 +1371,35 @@ cifs_parse_mount_options(char *options, const char *devname,
}
static bool
+match_port(struct TCP_Server_Info *server, struct sockaddr *addr)
+{
+ struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
+ struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
+ unsigned short int port;
+
+ /* search for a connection with the default port if user doesn't
+ specify the port manually */
+ port = htons(CIFS_PORT);
+
+ switch (addr->sa_family) {
+ case AF_INET:
+ if (addr4->sin_port)
+ port = addr4->sin_port;
+ if (port != server->addr.sockAddr.sin_port)
+ return false;
+ break;
+ case AF_INET6:
+ if (addr6->sin6_port)
+ port = addr6->sin6_port;
+ if (port != server->addr.sockAddr6.sin6_port)
+ return false;
+ break;
+ }
+
+ return true;
+}
+
+static bool
match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
{
struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
@@ -1381,9 +1410,6 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
if (addr4->sin_addr.s_addr !=
server->addr.sockAddr.sin_addr.s_addr)
return false;
- if (addr4->sin_port &&
- addr4->sin_port != server->addr.sockAddr.sin_port)
- return false;
break;
case AF_INET6:
if (!ipv6_addr_equal(&addr6->sin6_addr,
@@ -1392,9 +1418,6 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
if (addr6->sin6_scope_id !=
server->addr.sockAddr6.sin6_scope_id)
return false;
- if (addr6->sin6_port &&
- addr6->sin6_port != server->addr.sockAddr6.sin6_port)
- return false;
break;
}
@@ -1468,6 +1491,9 @@ cifs_find_tcp_session(struct sockaddr *addr, struct smb_vol *vol)
if (!match_address(server, addr))
continue;
+ if (!match_port(server, addr))
+ continue;
+
if (!match_security(server, vol))
continue;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] CIFS: Simplify ipv*_connect functions into one
[not found] ` <1289734155-3613-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-11-14 11:29 ` [PATCH 2/2] CIFS: Add match_port check during looking for existing connection Pavel Shilovsky
@ 2010-11-14 18:46 ` Pavel Shilovsky
[not found] ` <1289760378-8715-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Shilovsky @ 2010-11-14 18:46 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Make connect logic more ip-protocol independent. Also move RFC1001 stuff into
separate function.
Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
fs/cifs/connect.c | 311 ++++++++++++++++++-----------------------------------
1 files changed, 103 insertions(+), 208 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 251a17c..01ebcc4 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -114,8 +114,7 @@ struct smb_vol {
#define TLINK_ERROR_EXPIRE (1 * HZ)
#define TLINK_IDLE_EXPIRE (600 * HZ)
-static int ipv4_connect(struct TCP_Server_Info *server);
-static int ipv6_connect(struct TCP_Server_Info *server);
+static int ip_connect(struct TCP_Server_Info *server);
static void tlink_rb_insert(struct rb_root *root, struct tcon_link *new_tlink);
static void cifs_prune_tlinks(struct work_struct *work);
@@ -199,10 +198,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
while ((server->tcpStatus != CifsExiting) &&
(server->tcpStatus != CifsGood)) {
try_to_freeze();
- if (server->addr.sockAddr6.sin6_family == AF_INET6)
- rc = ipv6_connect(server);
- else
- rc = ipv4_connect(server);
+ rc = ip_connect(server);
if (rc) {
cFYI(1, "reconnect error %d", rc);
msleep(3000);
@@ -1668,12 +1664,11 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
/* other OS never observed in Wild doing 139 with v6 */
memcpy(&tcp_ses->addr.sockAddr6, sin_server6,
sizeof(struct sockaddr_in6));
- rc = ipv6_connect(tcp_ses);
- } else {
+ } else
memcpy(&tcp_ses->addr.sockAddr, sin_server,
sizeof(struct sockaddr_in));
- rc = ipv4_connect(tcp_ses);
- }
+
+ rc = ip_connect(tcp_ses);
if (rc < 0) {
cERROR(1, "Error connecting to socket. Aborting operation");
goto out_err_crypto_release;
@@ -2121,19 +2116,97 @@ bind_socket(struct TCP_Server_Info *server)
}
static int
-ipv4_connect(struct TCP_Server_Info *server)
+ip_rfc1001_connect(struct TCP_Server_Info *server)
+{
+ int rc = 0;
+ /* some servers require RFC1001 sessinit before sending
+ negprot - BB check reconnection in case where second
+ sessinit is sent but no second negprot */
+ struct rfc1002_session_packet *ses_init_buf;
+ struct smb_hdr *smb_buf;
+ ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
+ GFP_KERNEL);
+ if (ses_init_buf) {
+ ses_init_buf->trailer.session_req.called_len = 32;
+
+ if (server->server_RFC1001_name &&
+ server->server_RFC1001_name[0] != 0)
+ rfc1002mangle(ses_init_buf->trailer.
+ session_req.called_name,
+ server->server_RFC1001_name,
+ RFC1001_NAME_LEN_WITH_NULL);
+ else
+ rfc1002mangle(ses_init_buf->trailer.
+ session_req.called_name,
+ DEFAULT_CIFS_CALLED_NAME,
+ RFC1001_NAME_LEN_WITH_NULL);
+
+ ses_init_buf->trailer.session_req.calling_len = 32;
+
+ /* calling name ends in null (byte 16) from old smb
+ convention. */
+ if (server->workstation_RFC1001_name &&
+ server->workstation_RFC1001_name[0] != 0)
+ rfc1002mangle(ses_init_buf->trailer.
+ session_req.calling_name,
+ server->workstation_RFC1001_name,
+ RFC1001_NAME_LEN_WITH_NULL);
+ else
+ rfc1002mangle(ses_init_buf->trailer.
+ session_req.calling_name,
+ "LINUX_CIFS_CLNT",
+ RFC1001_NAME_LEN_WITH_NULL);
+
+ ses_init_buf->trailer.session_req.scope1 = 0;
+ ses_init_buf->trailer.session_req.scope2 = 0;
+ smb_buf = (struct smb_hdr *)ses_init_buf;
+
+ /* sizeof RFC1002_SESSION_REQUEST with no scope */
+ smb_buf->smb_buf_length = 0x81000044;
+ rc = smb_send(server, smb_buf, 0x44);
+ kfree(ses_init_buf);
+ msleep(1); /* RFC1001 layer in at least one server
+ requires very short break before negprot
+ presumably because not expecting negprot
+ to follow so fast. This is a simple
+ solution that works without
+ complicating the code and causes no
+ significant slowing down on mount
+ for everyone else */
+ }
+ /* else the negprot may still work without this
+ even though malloc failed */
+
+ return rc;
+}
+
+static int
+ip_connect(struct TCP_Server_Info *server)
{
int rc = 0;
- int val;
- bool connected = false;
- __be16 orig_port = 0;
+ bool using_ipv6 = false;
+ unsigned short int *sport;
+ int slen;
struct socket *socket = server->ssocket;
+ struct sockaddr *saddr;
+
+ if (server->addr.sockAddr6.sin6_family == AF_INET6) {
+ sport = &server->addr.sockAddr6.sin6_port;
+ using_ipv6 = true;
+ slen = sizeof(struct sockaddr_in6);
+ saddr = (struct sockaddr *) &server->addr.sockAddr6;
+ } else {
+ sport = &server->addr.sockAddr.sin_port;
+ slen = sizeof(struct sockaddr_in);
+ saddr = (struct sockaddr *) &server->addr.sockAddr;
+ }
if (socket == NULL) {
rc = sock_create_kern(PF_INET, SOCK_STREAM,
IPPROTO_TCP, &socket);
if (rc < 0) {
cERROR(1, "Error %d creating socket", rc);
+ server->ssocket = NULL;
return rc;
}
@@ -2141,59 +2214,32 @@ ipv4_connect(struct TCP_Server_Info *server)
cFYI(1, "Socket created");
server->ssocket = socket;
socket->sk->sk_allocation = GFP_NOFS;
- cifs_reclassify_socket4(socket);
+ if (using_ipv6)
+ cifs_reclassify_socket4(socket);
+ else
+ cifs_reclassify_socket6(socket);
}
rc = bind_socket(server);
if (rc < 0)
return rc;
- /* user overrode default port */
- if (server->addr.sockAddr.sin_port) {
- rc = socket->ops->connect(socket, (struct sockaddr *)
- &server->addr.sockAddr,
- sizeof(struct sockaddr_in), 0);
- if (rc >= 0)
- connected = true;
- }
-
- if (!connected) {
- /* save original port so we can retry user specified port
- later if fall back ports fail this time */
- orig_port = server->addr.sockAddr.sin_port;
-
- /* do not retry on the same port we just failed on */
- if (server->addr.sockAddr.sin_port != htons(CIFS_PORT)) {
- server->addr.sockAddr.sin_port = htons(CIFS_PORT);
- rc = socket->ops->connect(socket,
- (struct sockaddr *)
- &server->addr.sockAddr,
- sizeof(struct sockaddr_in), 0);
- if (rc >= 0)
- connected = true;
- }
+ if (*sport)
+ /* user specify port manually */
+ rc = socket->ops->connect(socket, saddr, slen, 0);
+ else {
+ /* try with 445 port at first */
+ *sport = htons(CIFS_PORT);
+ rc = socket->ops->connect(socket, saddr, slen, 0);
}
- if (!connected) {
- server->addr.sockAddr.sin_port = htons(RFC1001_PORT);
- rc = socket->ops->connect(socket, (struct sockaddr *)
- &server->addr.sockAddr,
- sizeof(struct sockaddr_in), 0);
- if (rc >= 0)
- connected = true;
- }
-
- /* give up here - unless we want to retry on different
- protocol families some day */
- if (!connected) {
- if (orig_port)
- server->addr.sockAddr.sin_port = orig_port;
- cFYI(1, "Error %d connecting to server via ipv4", rc);
+
+ if (rc < 0) {
+ cFYI(1, "Error %d connecting to server", rc);
sock_release(socket);
server->ssocket = NULL;
return rc;
}
-
/*
* Eventually check for other socket options to change from
* the default. sock_setsockopt not used because it expects
@@ -2211,7 +2257,7 @@ ipv4_connect(struct TCP_Server_Info *server)
}
if (server->tcp_nodelay) {
- val = 1;
+ int val = 1;
rc = kernel_setsockopt(socket, SOL_TCP, TCP_NODELAY,
(char *)&val, sizeof(val));
if (rc)
@@ -2222,159 +2268,8 @@ ipv4_connect(struct TCP_Server_Info *server)
socket->sk->sk_sndbuf,
socket->sk->sk_rcvbuf, socket->sk->sk_rcvtimeo);
- /* send RFC1001 sessinit */
- if (server->addr.sockAddr.sin_port == htons(RFC1001_PORT)) {
- /* some servers require RFC1001 sessinit before sending
- negprot - BB check reconnection in case where second
- sessinit is sent but no second negprot */
- struct rfc1002_session_packet *ses_init_buf;
- struct smb_hdr *smb_buf;
- ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
- GFP_KERNEL);
- if (ses_init_buf) {
- ses_init_buf->trailer.session_req.called_len = 32;
- if (server->server_RFC1001_name &&
- server->server_RFC1001_name[0] != 0)
- rfc1002mangle(ses_init_buf->trailer.
- session_req.called_name,
- server->server_RFC1001_name,
- RFC1001_NAME_LEN_WITH_NULL);
- else
- rfc1002mangle(ses_init_buf->trailer.
- session_req.called_name,
- DEFAULT_CIFS_CALLED_NAME,
- RFC1001_NAME_LEN_WITH_NULL);
-
- ses_init_buf->trailer.session_req.calling_len = 32;
-
- /* calling name ends in null (byte 16) from old smb
- convention. */
- if (server->workstation_RFC1001_name &&
- server->workstation_RFC1001_name[0] != 0)
- rfc1002mangle(ses_init_buf->trailer.
- session_req.calling_name,
- server->workstation_RFC1001_name,
- RFC1001_NAME_LEN_WITH_NULL);
- else
- rfc1002mangle(ses_init_buf->trailer.
- session_req.calling_name,
- "LINUX_CIFS_CLNT",
- RFC1001_NAME_LEN_WITH_NULL);
-
- ses_init_buf->trailer.session_req.scope1 = 0;
- ses_init_buf->trailer.session_req.scope2 = 0;
- smb_buf = (struct smb_hdr *)ses_init_buf;
- /* sizeof RFC1002_SESSION_REQUEST with no scope */
- smb_buf->smb_buf_length = 0x81000044;
- rc = smb_send(server, smb_buf, 0x44);
- kfree(ses_init_buf);
- msleep(1); /* RFC1001 layer in at least one server
- requires very short break before negprot
- presumably because not expecting negprot
- to follow so fast. This is a simple
- solution that works without
- complicating the code and causes no
- significant slowing down on mount
- for everyone else */
- }
- /* else the negprot may still work without this
- even though malloc failed */
-
- }
-
- return rc;
-}
-
-static int
-ipv6_connect(struct TCP_Server_Info *server)
-{
- int rc = 0;
- int val;
- bool connected = false;
- __be16 orig_port = 0;
- struct socket *socket = server->ssocket;
-
- if (socket == NULL) {
- rc = sock_create_kern(PF_INET6, SOCK_STREAM,
- IPPROTO_TCP, &socket);
- if (rc < 0) {
- cERROR(1, "Error %d creating ipv6 socket", rc);
- socket = NULL;
- return rc;
- }
-
- /* BB other socket options to set KEEPALIVE, NODELAY? */
- cFYI(1, "ipv6 Socket created");
- server->ssocket = socket;
- socket->sk->sk_allocation = GFP_NOFS;
- cifs_reclassify_socket6(socket);
- }
-
- rc = bind_socket(server);
- if (rc < 0)
- return rc;
-
- /* user overrode default port */
- if (server->addr.sockAddr6.sin6_port) {
- rc = socket->ops->connect(socket,
- (struct sockaddr *) &server->addr.sockAddr6,
- sizeof(struct sockaddr_in6), 0);
- if (rc >= 0)
- connected = true;
- }
-
- if (!connected) {
- /* save original port so we can retry user specified port
- later if fall back ports fail this time */
-
- orig_port = server->addr.sockAddr6.sin6_port;
- /* do not retry on the same port we just failed on */
- if (server->addr.sockAddr6.sin6_port != htons(CIFS_PORT)) {
- server->addr.sockAddr6.sin6_port = htons(CIFS_PORT);
- rc = socket->ops->connect(socket, (struct sockaddr *)
- &server->addr.sockAddr6,
- sizeof(struct sockaddr_in6), 0);
- if (rc >= 0)
- connected = true;
- }
- }
- if (!connected) {
- server->addr.sockAddr6.sin6_port = htons(RFC1001_PORT);
- rc = socket->ops->connect(socket, (struct sockaddr *)
- &server->addr.sockAddr6,
- sizeof(struct sockaddr_in6), 0);
- if (rc >= 0)
- connected = true;
- }
-
- /* give up here - unless we want to retry on different
- protocol families some day */
- if (!connected) {
- if (orig_port)
- server->addr.sockAddr6.sin6_port = orig_port;
- cFYI(1, "Error %d connecting to server via ipv6", rc);
- sock_release(socket);
- server->ssocket = NULL;
- return rc;
- }
-
- /*
- * Eventually check for other socket options to change from
- * the default. sock_setsockopt not used because it expects
- * user space buffer
- */
- socket->sk->sk_rcvtimeo = 7 * HZ;
- socket->sk->sk_sndtimeo = 5 * HZ;
-
- if (server->tcp_nodelay) {
- val = 1;
- rc = kernel_setsockopt(socket, SOL_TCP, TCP_NODELAY,
- (char *)&val, sizeof(val));
- if (rc)
- cFYI(1, "set TCP_NODELAY socket option error %d", rc);
- }
-
- server->ssocket = socket;
+ if (*sport == htons(RFC1001_PORT))
+ rc = ip_rfc1001_connect(server);
return rc;
}
--
1.7.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] CIFS: Add match_port check during looking for an existing connection
[not found] ` <1289734155-3613-2-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-11-14 18:49 ` Pavel Shilovsky
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Shilovsky @ 2010-11-14 18:49 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
If we have a share mounted by non-standard port and try to mount another share
on the same host with standard port, we connect to the first share again -
that's wrong. This patch fixes this bug.
Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
fs/cifs/connect.c | 37 +++++++++++++++++++++++++++++++------
1 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 01ebcc4..7130dd7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1435,6 +1435,34 @@ srcip_matches(struct sockaddr *srcaddr, struct sockaddr *rhs)
}
}
+static bool
+match_port(struct TCP_Server_Info *server, struct sockaddr *addr)
+{
+ struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
+ struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
+ unsigned short int port;
+
+ /* search for a connection with the default port if user doesn't
+ specify the port manually */
+ port = htons(CIFS_PORT);
+
+ switch (addr->sa_family) {
+ case AF_INET:
+ if (addr4->sin_port)
+ port = addr4->sin_port;
+ if (port != server->addr.sockAddr.sin_port)
+ return false;
+ break;
+ case AF_INET6:
+ if (addr6->sin6_port)
+ port = addr6->sin6_port;
+ if (port != server->addr.sockAddr6.sin6_port)
+ return false;
+ break;
+ }
+
+ return true;
+}
static bool
match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
@@ -1448,9 +1476,6 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
if (addr4->sin_addr.s_addr !=
server->addr.sockAddr.sin_addr.s_addr)
return false;
- if (addr4->sin_port &&
- addr4->sin_port != server->addr.sockAddr.sin_port)
- return false;
break;
case AF_INET6:
if (!ipv6_addr_equal(&addr6->sin6_addr,
@@ -1459,9 +1484,6 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
if (addr6->sin6_scope_id !=
server->addr.sockAddr6.sin6_scope_id)
return false;
- if (addr6->sin6_port &&
- addr6->sin6_port != server->addr.sockAddr6.sin6_port)
- return false;
break;
}
@@ -1530,6 +1552,9 @@ cifs_find_tcp_session(struct sockaddr *addr, struct smb_vol *vol)
(struct sockaddr *)&vol->srcaddr))
continue;
+ if (!match_port(server, addr))
+ continue;
+
if (!match_security(server, vol))
continue;
--
1.7.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] CIFS: Simplify ipv*_connect functions into one
[not found] ` <1289760378-8715-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-11-15 12:18 ` Jeff Layton
[not found] ` <20101115071824.15eac25d-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Layton @ 2010-11-15 12:18 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Sun, 14 Nov 2010 21:46:18 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Make connect logic more ip-protocol independent. Also move RFC1001 stuff into
> separate function.
>
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Nice cleanup so far. Some comments below:
> ---
> fs/cifs/connect.c | 311 ++++++++++++++++++-----------------------------------
> 1 files changed, 103 insertions(+), 208 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 251a17c..01ebcc4 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -114,8 +114,7 @@ struct smb_vol {
> #define TLINK_ERROR_EXPIRE (1 * HZ)
> #define TLINK_IDLE_EXPIRE (600 * HZ)
>
> -static int ipv4_connect(struct TCP_Server_Info *server);
> -static int ipv6_connect(struct TCP_Server_Info *server);
> +static int ip_connect(struct TCP_Server_Info *server);
> static void tlink_rb_insert(struct rb_root *root, struct tcon_link *new_tlink);
> static void cifs_prune_tlinks(struct work_struct *work);
>
> @@ -199,10 +198,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
> while ((server->tcpStatus != CifsExiting) &&
> (server->tcpStatus != CifsGood)) {
> try_to_freeze();
> - if (server->addr.sockAddr6.sin6_family == AF_INET6)
> - rc = ipv6_connect(server);
> - else
> - rc = ipv4_connect(server);
> + rc = ip_connect(server);
> if (rc) {
> cFYI(1, "reconnect error %d", rc);
> msleep(3000);
> @@ -1668,12 +1664,11 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
> /* other OS never observed in Wild doing 139 with v6 */
> memcpy(&tcp_ses->addr.sockAddr6, sin_server6,
> sizeof(struct sockaddr_in6));
> - rc = ipv6_connect(tcp_ses);
> - } else {
> + } else
> memcpy(&tcp_ses->addr.sockAddr, sin_server,
> sizeof(struct sockaddr_in));
> - rc = ipv4_connect(tcp_ses);
> - }
> +
> + rc = ip_connect(tcp_ses);
> if (rc < 0) {
> cERROR(1, "Error connecting to socket. Aborting operation");
> goto out_err_crypto_release;
> @@ -2121,19 +2116,97 @@ bind_socket(struct TCP_Server_Info *server)
> }
>
> static int
> -ipv4_connect(struct TCP_Server_Info *server)
> +ip_rfc1001_connect(struct TCP_Server_Info *server)
> +{
> + int rc = 0;
> + /* some servers require RFC1001 sessinit before sending
> + negprot - BB check reconnection in case where second
> + sessinit is sent but no second negprot */
> + struct rfc1002_session_packet *ses_init_buf;
> + struct smb_hdr *smb_buf;
> + ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
> + GFP_KERNEL);
> + if (ses_init_buf) {
> + ses_init_buf->trailer.session_req.called_len = 32;
> +
> + if (server->server_RFC1001_name &&
> + server->server_RFC1001_name[0] != 0)
> + rfc1002mangle(ses_init_buf->trailer.
> + session_req.called_name,
> + server->server_RFC1001_name,
> + RFC1001_NAME_LEN_WITH_NULL);
> + else
> + rfc1002mangle(ses_init_buf->trailer.
> + session_req.called_name,
> + DEFAULT_CIFS_CALLED_NAME,
> + RFC1001_NAME_LEN_WITH_NULL);
> +
> + ses_init_buf->trailer.session_req.calling_len = 32;
> +
> + /* calling name ends in null (byte 16) from old smb
> + convention. */
> + if (server->workstation_RFC1001_name &&
> + server->workstation_RFC1001_name[0] != 0)
> + rfc1002mangle(ses_init_buf->trailer.
> + session_req.calling_name,
> + server->workstation_RFC1001_name,
> + RFC1001_NAME_LEN_WITH_NULL);
> + else
> + rfc1002mangle(ses_init_buf->trailer.
> + session_req.calling_name,
> + "LINUX_CIFS_CLNT",
> + RFC1001_NAME_LEN_WITH_NULL);
> +
> + ses_init_buf->trailer.session_req.scope1 = 0;
> + ses_init_buf->trailer.session_req.scope2 = 0;
> + smb_buf = (struct smb_hdr *)ses_init_buf;
> +
> + /* sizeof RFC1002_SESSION_REQUEST with no scope */
> + smb_buf->smb_buf_length = 0x81000044;
> + rc = smb_send(server, smb_buf, 0x44);
> + kfree(ses_init_buf);
> + msleep(1); /* RFC1001 layer in at least one server
> + requires very short break before negprot
> + presumably because not expecting negprot
> + to follow so fast. This is a simple
> + solution that works without
> + complicating the code and causes no
> + significant slowing down on mount
> + for everyone else */
> + }
> + /* else the negprot may still work without this
> + even though malloc failed */
> +
> + return rc;
> +}
> +
> +static int
> +ip_connect(struct TCP_Server_Info *server)
> {
> int rc = 0;
> - int val;
> - bool connected = false;
> - __be16 orig_port = 0;
> + bool using_ipv6 = false;
> + unsigned short int *sport;
> + int slen;
> struct socket *socket = server->ssocket;
> + struct sockaddr *saddr;
> +
> + if (server->addr.sockAddr6.sin6_family == AF_INET6) {
> + sport = &server->addr.sockAddr6.sin6_port;
> + using_ipv6 = true;
> + slen = sizeof(struct sockaddr_in6);
> + saddr = (struct sockaddr *) &server->addr.sockAddr6;
> + } else {
> + sport = &server->addr.sockAddr.sin_port;
> + slen = sizeof(struct sockaddr_in);
> + saddr = (struct sockaddr *) &server->addr.sockAddr;
> + }
>
> if (socket == NULL) {
> rc = sock_create_kern(PF_INET, SOCK_STREAM,
> IPPROTO_TCP, &socket);
> if (rc < 0) {
> cERROR(1, "Error %d creating socket", rc);
> + server->ssocket = NULL;
> return rc;
> }
>
> @@ -2141,59 +2214,32 @@ ipv4_connect(struct TCP_Server_Info *server)
> cFYI(1, "Socket created");
> server->ssocket = socket;
> socket->sk->sk_allocation = GFP_NOFS;
> - cifs_reclassify_socket4(socket);
> + if (using_ipv6)
> + cifs_reclassify_socket4(socket);
> + else
> + cifs_reclassify_socket6(socket);
^^^^^^^^
Isn't that backwards?
> }
>
> rc = bind_socket(server);
> if (rc < 0)
> return rc;
>
> - /* user overrode default port */
> - if (server->addr.sockAddr.sin_port) {
> - rc = socket->ops->connect(socket, (struct sockaddr *)
> - &server->addr.sockAddr,
> - sizeof(struct sockaddr_in), 0);
> - if (rc >= 0)
> - connected = true;
> - }
> -
> - if (!connected) {
> - /* save original port so we can retry user specified port
> - later if fall back ports fail this time */
> - orig_port = server->addr.sockAddr.sin_port;
> -
> - /* do not retry on the same port we just failed on */
> - if (server->addr.sockAddr.sin_port != htons(CIFS_PORT)) {
> - server->addr.sockAddr.sin_port = htons(CIFS_PORT);
> - rc = socket->ops->connect(socket,
> - (struct sockaddr *)
> - &server->addr.sockAddr,
> - sizeof(struct sockaddr_in), 0);
> - if (rc >= 0)
> - connected = true;
> - }
> + if (*sport)
> + /* user specify port manually */
> + rc = socket->ops->connect(socket, saddr, slen, 0);
> + else {
> + /* try with 445 port at first */
> + *sport = htons(CIFS_PORT);
> + rc = socket->ops->connect(socket, saddr, slen, 0);
^^^^^^^^^
You could collapse that down to:
if (*sport = 0)
*sport = htons(CIFS_PORT);
rc = socket->ops->connect(socket, saddr, slen, 0);
Eliminating unnecessary conditional code makes for a smaller
resulting binary and makes the code easier to read.
> }
> - if (!connected) {
> - server->addr.sockAddr.sin_port = htons(RFC1001_PORT);
> - rc = socket->ops->connect(socket, (struct sockaddr *)
> - &server->addr.sockAddr,
> - sizeof(struct sockaddr_in), 0);
> - if (rc >= 0)
> - connected = true;
> - }
> -
> - /* give up here - unless we want to retry on different
> - protocol families some day */
> - if (!connected) {
> - if (orig_port)
> - server->addr.sockAddr.sin_port = orig_port;
> - cFYI(1, "Error %d connecting to server via ipv4", rc);
> +
> + if (rc < 0) {
> + cFYI(1, "Error %d connecting to server", rc);
> sock_release(socket);
> server->ssocket = NULL;
> return rc;
> }
>
> -
> /*
> * Eventually check for other socket options to change from
> * the default. sock_setsockopt not used because it expects
> @@ -2211,7 +2257,7 @@ ipv4_connect(struct TCP_Server_Info *server)
> }
>
> if (server->tcp_nodelay) {
> - val = 1;
> + int val = 1;
> rc = kernel_setsockopt(socket, SOL_TCP, TCP_NODELAY,
> (char *)&val, sizeof(val));
> if (rc)
> @@ -2222,159 +2268,8 @@ ipv4_connect(struct TCP_Server_Info *server)
> socket->sk->sk_sndbuf,
> socket->sk->sk_rcvbuf, socket->sk->sk_rcvtimeo);
>
> - /* send RFC1001 sessinit */
> - if (server->addr.sockAddr.sin_port == htons(RFC1001_PORT)) {
> - /* some servers require RFC1001 sessinit before sending
> - negprot - BB check reconnection in case where second
> - sessinit is sent but no second negprot */
> - struct rfc1002_session_packet *ses_init_buf;
> - struct smb_hdr *smb_buf;
> - ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
> - GFP_KERNEL);
> - if (ses_init_buf) {
> - ses_init_buf->trailer.session_req.called_len = 32;
> - if (server->server_RFC1001_name &&
> - server->server_RFC1001_name[0] != 0)
> - rfc1002mangle(ses_init_buf->trailer.
> - session_req.called_name,
> - server->server_RFC1001_name,
> - RFC1001_NAME_LEN_WITH_NULL);
> - else
> - rfc1002mangle(ses_init_buf->trailer.
> - session_req.called_name,
> - DEFAULT_CIFS_CALLED_NAME,
> - RFC1001_NAME_LEN_WITH_NULL);
> -
> - ses_init_buf->trailer.session_req.calling_len = 32;
> -
> - /* calling name ends in null (byte 16) from old smb
> - convention. */
> - if (server->workstation_RFC1001_name &&
> - server->workstation_RFC1001_name[0] != 0)
> - rfc1002mangle(ses_init_buf->trailer.
> - session_req.calling_name,
> - server->workstation_RFC1001_name,
> - RFC1001_NAME_LEN_WITH_NULL);
> - else
> - rfc1002mangle(ses_init_buf->trailer.
> - session_req.calling_name,
> - "LINUX_CIFS_CLNT",
> - RFC1001_NAME_LEN_WITH_NULL);
> -
> - ses_init_buf->trailer.session_req.scope1 = 0;
> - ses_init_buf->trailer.session_req.scope2 = 0;
> - smb_buf = (struct smb_hdr *)ses_init_buf;
> - /* sizeof RFC1002_SESSION_REQUEST with no scope */
> - smb_buf->smb_buf_length = 0x81000044;
> - rc = smb_send(server, smb_buf, 0x44);
> - kfree(ses_init_buf);
> - msleep(1); /* RFC1001 layer in at least one server
> - requires very short break before negprot
> - presumably because not expecting negprot
> - to follow so fast. This is a simple
> - solution that works without
> - complicating the code and causes no
> - significant slowing down on mount
> - for everyone else */
> - }
> - /* else the negprot may still work without this
> - even though malloc failed */
> -
> - }
> -
> - return rc;
> -}
> -
> -static int
> -ipv6_connect(struct TCP_Server_Info *server)
> -{
> - int rc = 0;
> - int val;
> - bool connected = false;
> - __be16 orig_port = 0;
> - struct socket *socket = server->ssocket;
> -
> - if (socket == NULL) {
> - rc = sock_create_kern(PF_INET6, SOCK_STREAM,
> - IPPROTO_TCP, &socket);
> - if (rc < 0) {
> - cERROR(1, "Error %d creating ipv6 socket", rc);
> - socket = NULL;
> - return rc;
> - }
> -
> - /* BB other socket options to set KEEPALIVE, NODELAY? */
> - cFYI(1, "ipv6 Socket created");
> - server->ssocket = socket;
> - socket->sk->sk_allocation = GFP_NOFS;
> - cifs_reclassify_socket6(socket);
> - }
> -
> - rc = bind_socket(server);
> - if (rc < 0)
> - return rc;
> -
> - /* user overrode default port */
> - if (server->addr.sockAddr6.sin6_port) {
> - rc = socket->ops->connect(socket,
> - (struct sockaddr *) &server->addr.sockAddr6,
> - sizeof(struct sockaddr_in6), 0);
> - if (rc >= 0)
> - connected = true;
> - }
> -
> - if (!connected) {
> - /* save original port so we can retry user specified port
> - later if fall back ports fail this time */
> -
> - orig_port = server->addr.sockAddr6.sin6_port;
> - /* do not retry on the same port we just failed on */
> - if (server->addr.sockAddr6.sin6_port != htons(CIFS_PORT)) {
> - server->addr.sockAddr6.sin6_port = htons(CIFS_PORT);
> - rc = socket->ops->connect(socket, (struct sockaddr *)
> - &server->addr.sockAddr6,
> - sizeof(struct sockaddr_in6), 0);
> - if (rc >= 0)
> - connected = true;
> - }
> - }
> - if (!connected) {
> - server->addr.sockAddr6.sin6_port = htons(RFC1001_PORT);
> - rc = socket->ops->connect(socket, (struct sockaddr *)
> - &server->addr.sockAddr6,
> - sizeof(struct sockaddr_in6), 0);
> - if (rc >= 0)
> - connected = true;
> - }
> -
> - /* give up here - unless we want to retry on different
> - protocol families some day */
> - if (!connected) {
> - if (orig_port)
> - server->addr.sockAddr6.sin6_port = orig_port;
> - cFYI(1, "Error %d connecting to server via ipv6", rc);
> - sock_release(socket);
> - server->ssocket = NULL;
> - return rc;
> - }
> -
> - /*
> - * Eventually check for other socket options to change from
> - * the default. sock_setsockopt not used because it expects
> - * user space buffer
> - */
> - socket->sk->sk_rcvtimeo = 7 * HZ;
> - socket->sk->sk_sndtimeo = 5 * HZ;
> -
> - if (server->tcp_nodelay) {
> - val = 1;
> - rc = kernel_setsockopt(socket, SOL_TCP, TCP_NODELAY,
> - (char *)&val, sizeof(val));
> - if (rc)
> - cFYI(1, "set TCP_NODELAY socket option error %d", rc);
> - }
> -
> - server->ssocket = socket;
> + if (*sport == htons(RFC1001_PORT))
> + rc = ip_rfc1001_connect(server);
>
> return rc;
> }
With these changes, where does the port get automatically set to
RFC1001_PORT if the CIFS_PORT connect fails? It looks like that part
got ripped out, but not replaced.
That said, it seems like it might be better to separate policy
from function here. The socket connect code really has no
business deciding what port to use. Might it be better to
instead have a generic function that takes a fully filled-out sockaddr,
and then have another function that calls it and decides what ports
to try?
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] CIFS: Simplify ipv*_connect functions into one
[not found] ` <20101115071824.15eac25d-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2010-11-15 13:25 ` Pavel Shilovsky
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Shilovsky @ 2010-11-15 13:25 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
2010/11/15 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> On Sun, 14 Nov 2010 21:46:18 +0300
> Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> Make connect logic more ip-protocol independent. Also move RFC1001 stuff into
>> separate function.
>>
>> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Nice cleanup so far. Some comments below:
Thank you for the review!
>
>> ---
>> fs/cifs/connect.c | 311 ++++++++++++++++++-----------------------------------
>> 1 files changed, 103 insertions(+), 208 deletions(-)
>>
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 251a17c..01ebcc4 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -114,8 +114,7 @@ struct smb_vol {
>> #define TLINK_ERROR_EXPIRE (1 * HZ)
>> #define TLINK_IDLE_EXPIRE (600 * HZ)
>>
>> -static int ipv4_connect(struct TCP_Server_Info *server);
>> -static int ipv6_connect(struct TCP_Server_Info *server);
>> +static int ip_connect(struct TCP_Server_Info *server);
>> static void tlink_rb_insert(struct rb_root *root, struct tcon_link *new_tlink);
>> static void cifs_prune_tlinks(struct work_struct *work);
>>
>> @@ -199,10 +198,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
>> while ((server->tcpStatus != CifsExiting) &&
>> (server->tcpStatus != CifsGood)) {
>> try_to_freeze();
>> - if (server->addr.sockAddr6.sin6_family == AF_INET6)
>> - rc = ipv6_connect(server);
>> - else
>> - rc = ipv4_connect(server);
>> + rc = ip_connect(server);
>> if (rc) {
>> cFYI(1, "reconnect error %d", rc);
>> msleep(3000);
>> @@ -1668,12 +1664,11 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>> /* other OS never observed in Wild doing 139 with v6 */
>> memcpy(&tcp_ses->addr.sockAddr6, sin_server6,
>> sizeof(struct sockaddr_in6));
>> - rc = ipv6_connect(tcp_ses);
>> - } else {
>> + } else
>> memcpy(&tcp_ses->addr.sockAddr, sin_server,
>> sizeof(struct sockaddr_in));
>> - rc = ipv4_connect(tcp_ses);
>> - }
>> +
>> + rc = ip_connect(tcp_ses);
>> if (rc < 0) {
>> cERROR(1, "Error connecting to socket. Aborting operation");
>> goto out_err_crypto_release;
>> @@ -2121,19 +2116,97 @@ bind_socket(struct TCP_Server_Info *server)
>> }
>>
>> static int
>> -ipv4_connect(struct TCP_Server_Info *server)
>> +ip_rfc1001_connect(struct TCP_Server_Info *server)
>> +{
>> + int rc = 0;
>> + /* some servers require RFC1001 sessinit before sending
>> + negprot - BB check reconnection in case where second
>> + sessinit is sent but no second negprot */
>> + struct rfc1002_session_packet *ses_init_buf;
>> + struct smb_hdr *smb_buf;
>> + ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
>> + GFP_KERNEL);
>> + if (ses_init_buf) {
>> + ses_init_buf->trailer.session_req.called_len = 32;
>> +
>> + if (server->server_RFC1001_name &&
>> + server->server_RFC1001_name[0] != 0)
>> + rfc1002mangle(ses_init_buf->trailer.
>> + session_req.called_name,
>> + server->server_RFC1001_name,
>> + RFC1001_NAME_LEN_WITH_NULL);
>> + else
>> + rfc1002mangle(ses_init_buf->trailer.
>> + session_req.called_name,
>> + DEFAULT_CIFS_CALLED_NAME,
>> + RFC1001_NAME_LEN_WITH_NULL);
>> +
>> + ses_init_buf->trailer.session_req.calling_len = 32;
>> +
>> + /* calling name ends in null (byte 16) from old smb
>> + convention. */
>> + if (server->workstation_RFC1001_name &&
>> + server->workstation_RFC1001_name[0] != 0)
>> + rfc1002mangle(ses_init_buf->trailer.
>> + session_req.calling_name,
>> + server->workstation_RFC1001_name,
>> + RFC1001_NAME_LEN_WITH_NULL);
>> + else
>> + rfc1002mangle(ses_init_buf->trailer.
>> + session_req.calling_name,
>> + "LINUX_CIFS_CLNT",
>> + RFC1001_NAME_LEN_WITH_NULL);
>> +
>> + ses_init_buf->trailer.session_req.scope1 = 0;
>> + ses_init_buf->trailer.session_req.scope2 = 0;
>> + smb_buf = (struct smb_hdr *)ses_init_buf;
>> +
>> + /* sizeof RFC1002_SESSION_REQUEST with no scope */
>> + smb_buf->smb_buf_length = 0x81000044;
>> + rc = smb_send(server, smb_buf, 0x44);
>> + kfree(ses_init_buf);
>> + msleep(1); /* RFC1001 layer in at least one server
>> + requires very short break before negprot
>> + presumably because not expecting negprot
>> + to follow so fast. This is a simple
>> + solution that works without
>> + complicating the code and causes no
>> + significant slowing down on mount
>> + for everyone else */
>> + }
>> + /* else the negprot may still work without this
>> + even though malloc failed */
>> +
>> + return rc;
>> +}
>> +
>> +static int
>> +ip_connect(struct TCP_Server_Info *server)
>> {
>> int rc = 0;
>> - int val;
>> - bool connected = false;
>> - __be16 orig_port = 0;
>> + bool using_ipv6 = false;
>> + unsigned short int *sport;
>> + int slen;
>> struct socket *socket = server->ssocket;
>> + struct sockaddr *saddr;
>> +
>> + if (server->addr.sockAddr6.sin6_family == AF_INET6) {
>> + sport = &server->addr.sockAddr6.sin6_port;
>> + using_ipv6 = true;
>> + slen = sizeof(struct sockaddr_in6);
>> + saddr = (struct sockaddr *) &server->addr.sockAddr6;
>> + } else {
>> + sport = &server->addr.sockAddr.sin_port;
>> + slen = sizeof(struct sockaddr_in);
>> + saddr = (struct sockaddr *) &server->addr.sockAddr;
>> + }
>>
>> if (socket == NULL) {
>> rc = sock_create_kern(PF_INET, SOCK_STREAM,
>> IPPROTO_TCP, &socket);
>> if (rc < 0) {
>> cERROR(1, "Error %d creating socket", rc);
>> + server->ssocket = NULL;
>> return rc;
>> }
>>
>> @@ -2141,59 +2214,32 @@ ipv4_connect(struct TCP_Server_Info *server)
>> cFYI(1, "Socket created");
>> server->ssocket = socket;
>> socket->sk->sk_allocation = GFP_NOFS;
>> - cifs_reclassify_socket4(socket);
>> + if (using_ipv6)
>> + cifs_reclassify_socket4(socket);
>> + else
>> + cifs_reclassify_socket6(socket);
> ^^^^^^^^
> Isn't that backwards?
Yes, you are right.
>> }
>>
>> rc = bind_socket(server);
>> if (rc < 0)
>> return rc;
>>
>> - /* user overrode default port */
>> - if (server->addr.sockAddr.sin_port) {
>> - rc = socket->ops->connect(socket, (struct sockaddr *)
>> - &server->addr.sockAddr,
>> - sizeof(struct sockaddr_in), 0);
>> - if (rc >= 0)
>> - connected = true;
>> - }
>> -
>> - if (!connected) {
>> - /* save original port so we can retry user specified port
>> - later if fall back ports fail this time */
>> - orig_port = server->addr.sockAddr.sin_port;
>> -
>> - /* do not retry on the same port we just failed on */
>> - if (server->addr.sockAddr.sin_port != htons(CIFS_PORT)) {
>> - server->addr.sockAddr.sin_port = htons(CIFS_PORT);
>> - rc = socket->ops->connect(socket,
>> - (struct sockaddr *)
>> - &server->addr.sockAddr,
>> - sizeof(struct sockaddr_in), 0);
>> - if (rc >= 0)
>> - connected = true;
>> - }
>> + if (*sport)
>> + /* user specify port manually */
>> + rc = socket->ops->connect(socket, saddr, slen, 0);
>> + else {
>> + /* try with 445 port at first */
>> + *sport = htons(CIFS_PORT);
>> + rc = socket->ops->connect(socket, saddr, slen, 0);
> ^^^^^^^^^
> You could collapse that down to:
>
> if (*sport = 0)
> *sport = htons(CIFS_PORT);
> rc = socket->ops->connect(socket, saddr, slen, 0);
>
> Eliminating unnecessary conditional code makes for a smaller
> resulting binary and makes the code easier to read.
Looks ok - I will change this too.
>
>> }
>> - if (!connected) {
>> - server->addr.sockAddr.sin_port = htons(RFC1001_PORT);
>> - rc = socket->ops->connect(socket, (struct sockaddr *)
>> - &server->addr.sockAddr,
>> - sizeof(struct sockaddr_in), 0);
>> - if (rc >= 0)
>> - connected = true;
>> - }
>> -
>> - /* give up here - unless we want to retry on different
>> - protocol families some day */
>> - if (!connected) {
>> - if (orig_port)
>> - server->addr.sockAddr.sin_port = orig_port;
>> - cFYI(1, "Error %d connecting to server via ipv4", rc);
>> +
>> + if (rc < 0) {
>> + cFYI(1, "Error %d connecting to server", rc);
>> sock_release(socket);
>> server->ssocket = NULL;
>> return rc;
>> }
>>
>> -
>> /*
>> * Eventually check for other socket options to change from
>> * the default. sock_setsockopt not used because it expects
>> @@ -2211,7 +2257,7 @@ ipv4_connect(struct TCP_Server_Info *server)
>> }
>>
>> if (server->tcp_nodelay) {
>> - val = 1;
>> + int val = 1;
>> rc = kernel_setsockopt(socket, SOL_TCP, TCP_NODELAY,
>> (char *)&val, sizeof(val));
>> if (rc)
>> @@ -2222,159 +2268,8 @@ ipv4_connect(struct TCP_Server_Info *server)
>> socket->sk->sk_sndbuf,
>> socket->sk->sk_rcvbuf, socket->sk->sk_rcvtimeo);
>>
>> - /* send RFC1001 sessinit */
>> - if (server->addr.sockAddr.sin_port == htons(RFC1001_PORT)) {
>> - /* some servers require RFC1001 sessinit before sending
>> - negprot - BB check reconnection in case where second
>> - sessinit is sent but no second negprot */
>> - struct rfc1002_session_packet *ses_init_buf;
>> - struct smb_hdr *smb_buf;
>> - ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
>> - GFP_KERNEL);
>> - if (ses_init_buf) {
>> - ses_init_buf->trailer.session_req.called_len = 32;
>> - if (server->server_RFC1001_name &&
>> - server->server_RFC1001_name[0] != 0)
>> - rfc1002mangle(ses_init_buf->trailer.
>> - session_req.called_name,
>> - server->server_RFC1001_name,
>> - RFC1001_NAME_LEN_WITH_NULL);
>> - else
>> - rfc1002mangle(ses_init_buf->trailer.
>> - session_req.called_name,
>> - DEFAULT_CIFS_CALLED_NAME,
>> - RFC1001_NAME_LEN_WITH_NULL);
>> -
>> - ses_init_buf->trailer.session_req.calling_len = 32;
>> -
>> - /* calling name ends in null (byte 16) from old smb
>> - convention. */
>> - if (server->workstation_RFC1001_name &&
>> - server->workstation_RFC1001_name[0] != 0)
>> - rfc1002mangle(ses_init_buf->trailer.
>> - session_req.calling_name,
>> - server->workstation_RFC1001_name,
>> - RFC1001_NAME_LEN_WITH_NULL);
>> - else
>> - rfc1002mangle(ses_init_buf->trailer.
>> - session_req.calling_name,
>> - "LINUX_CIFS_CLNT",
>> - RFC1001_NAME_LEN_WITH_NULL);
>> -
>> - ses_init_buf->trailer.session_req.scope1 = 0;
>> - ses_init_buf->trailer.session_req.scope2 = 0;
>> - smb_buf = (struct smb_hdr *)ses_init_buf;
>> - /* sizeof RFC1002_SESSION_REQUEST with no scope */
>> - smb_buf->smb_buf_length = 0x81000044;
>> - rc = smb_send(server, smb_buf, 0x44);
>> - kfree(ses_init_buf);
>> - msleep(1); /* RFC1001 layer in at least one server
>> - requires very short break before negprot
>> - presumably because not expecting negprot
>> - to follow so fast. This is a simple
>> - solution that works without
>> - complicating the code and causes no
>> - significant slowing down on mount
>> - for everyone else */
>> - }
>> - /* else the negprot may still work without this
>> - even though malloc failed */
>> -
>> - }
>> -
>> - return rc;
>> -}
>> -
>> -static int
>> -ipv6_connect(struct TCP_Server_Info *server)
>> -{
>> - int rc = 0;
>> - int val;
>> - bool connected = false;
>> - __be16 orig_port = 0;
>> - struct socket *socket = server->ssocket;
>> -
>> - if (socket == NULL) {
>> - rc = sock_create_kern(PF_INET6, SOCK_STREAM,
>> - IPPROTO_TCP, &socket);
>> - if (rc < 0) {
>> - cERROR(1, "Error %d creating ipv6 socket", rc);
>> - socket = NULL;
>> - return rc;
>> - }
>> -
>> - /* BB other socket options to set KEEPALIVE, NODELAY? */
>> - cFYI(1, "ipv6 Socket created");
>> - server->ssocket = socket;
>> - socket->sk->sk_allocation = GFP_NOFS;
>> - cifs_reclassify_socket6(socket);
>> - }
>> -
>> - rc = bind_socket(server);
>> - if (rc < 0)
>> - return rc;
>> -
>> - /* user overrode default port */
>> - if (server->addr.sockAddr6.sin6_port) {
>> - rc = socket->ops->connect(socket,
>> - (struct sockaddr *) &server->addr.sockAddr6,
>> - sizeof(struct sockaddr_in6), 0);
>> - if (rc >= 0)
>> - connected = true;
>> - }
>> -
>> - if (!connected) {
>> - /* save original port so we can retry user specified port
>> - later if fall back ports fail this time */
>> -
>> - orig_port = server->addr.sockAddr6.sin6_port;
>> - /* do not retry on the same port we just failed on */
>> - if (server->addr.sockAddr6.sin6_port != htons(CIFS_PORT)) {
>> - server->addr.sockAddr6.sin6_port = htons(CIFS_PORT);
>> - rc = socket->ops->connect(socket, (struct sockaddr *)
>> - &server->addr.sockAddr6,
>> - sizeof(struct sockaddr_in6), 0);
>> - if (rc >= 0)
>> - connected = true;
>> - }
>> - }
>> - if (!connected) {
>> - server->addr.sockAddr6.sin6_port = htons(RFC1001_PORT);
>> - rc = socket->ops->connect(socket, (struct sockaddr *)
>> - &server->addr.sockAddr6,
>> - sizeof(struct sockaddr_in6), 0);
>> - if (rc >= 0)
>> - connected = true;
>> - }
>> -
>> - /* give up here - unless we want to retry on different
>> - protocol families some day */
>> - if (!connected) {
>> - if (orig_port)
>> - server->addr.sockAddr6.sin6_port = orig_port;
>> - cFYI(1, "Error %d connecting to server via ipv6", rc);
>> - sock_release(socket);
>> - server->ssocket = NULL;
>> - return rc;
>> - }
>> -
>> - /*
>> - * Eventually check for other socket options to change from
>> - * the default. sock_setsockopt not used because it expects
>> - * user space buffer
>> - */
>> - socket->sk->sk_rcvtimeo = 7 * HZ;
>> - socket->sk->sk_sndtimeo = 5 * HZ;
>> -
>> - if (server->tcp_nodelay) {
>> - val = 1;
>> - rc = kernel_setsockopt(socket, SOL_TCP, TCP_NODELAY,
>> - (char *)&val, sizeof(val));
>> - if (rc)
>> - cFYI(1, "set TCP_NODELAY socket option error %d", rc);
>> - }
>> -
>> - server->ssocket = socket;
>> + if (*sport == htons(RFC1001_PORT))
>> + rc = ip_rfc1001_connect(server);
>>
>> return rc;
>> }
>
> With these changes, where does the port get automatically set to
> RFC1001_PORT if the CIFS_PORT connect fails? It looks like that part
> got ripped out, but not replaced.
I did it before you commented that it should be done according to
mount.cifs man page. So, ok, I will change this too.
>
> That said, it seems like it might be better to separate policy
> from function here. The socket connect code really has no
> business deciding what port to use. Might it be better to
> instead have a generic function that takes a fully filled-out sockaddr,
> and then have another function that calls it and decides what ports
> to try?
Good idea. I will try to add smth like that.
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-15 13:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-14 11:29 [PATCH 1/2] CIFS: Simplify ipv*_connect functions into one Pavel Shilovsky
[not found] ` <1289734155-3613-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-11-14 11:29 ` [PATCH 2/2] CIFS: Add match_port check during looking for existing connection Pavel Shilovsky
[not found] ` <1289734155-3613-2-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-11-14 18:49 ` [PATCH 2/2] CIFS: Add match_port check during looking for an " Pavel Shilovsky
2010-11-14 18:46 ` [PATCH 1/2] CIFS: Simplify ipv*_connect functions into one Pavel Shilovsky
[not found] ` <1289760378-8715-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-11-15 12:18 ` Jeff Layton
[not found] ` <20101115071824.15eac25d-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2010-11-15 13:25 ` Pavel Shilovsky
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.