2010/11/16 Jeff Layton <jlayton@xxxxxxxxxx>:
> On Tue, 16 Nov 2010 13:17:55 -0500
> Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
>> On Tue, 16 Nov 2010 10:48:29 +0300
>> Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>>
>> > Make connect logic more ip-protocol independent. Also move RFC1001 stuff into
>> > separate function.
>> >
>> > Signed-off-by: Pavel Shilovsky <piastryyy@xxxxxxxxx>
>> > ---
>> > fs/cifs/connect.c | 314 ++++++++++++++++++++---------------------------------
>> > 1 files changed, 116 insertions(+), 198 deletions(-)
>> >
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index 251a17c..8bdf1cc 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -114,8 +114,8 @@ 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 int generic_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 +199,10 @@ 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);
>> > +
>> > + /* on reconnect we should try only the port we connected
>> > + before */
>> > + rc = generic_ip_connect(server);
>> > if (rc) {
>> > cFYI(1, "reconnect error %d", rc);
>> > msleep(3000);
>> > @@ -1668,12 +1668,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 +2120,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
>> > +generic_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 +2218,24 @@ 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_socket6(socket);
>> > + else
>> > + cifs_reclassify_socket4(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 (!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);
>> > + rc = socket->ops->connect(socket, saddr, slen, 0);
>> > + 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 +2253,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,161 +2264,37 @@ 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 */
>> > -
>> > - }
>> > + if (*sport == htons(RFC1001_PORT))
>> > + rc = ip_rfc1001_connect(server);
>> >
>> > return rc;
>> > }
>> >
>> > static int
>> > -ipv6_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;
>> > - struct socket *socket = server->ssocket;
>> > + unsigned short int *sport;
>> >
>> > - 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;
>> > - }
>> > + if (server->addr.sockAddr6.sin6_family == AF_INET6)
>> > + sport = &server->addr.sockAddr6.sin6_port;
>> > + else
>> > + sport = &server->addr.sockAddr.sin_port;
>> >
>> > - /* 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);
>> > - }
>> > + if (*sport == 0) {
>> > + int rc;
>> >
>> > - rc = bind_socket(server);
>> > - if (rc < 0)
>> > - return rc;
>> > + /* try with 445 port at first */
>> > + *sport = htons(CIFS_PORT);
>> >
>> > - /* 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);
>> > + rc = generic_ip_connect(server);
>> > 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;
>> > + return rc;
>> >
>> > - 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);
>> > + /* if it failed, try with 139 port */
>> > + *sport = htons(RFC1001_PORT);
>> > }
>> >
>> > - server->ssocket = socket;
>> > -
>> > - return rc;
>> > + return generic_ip_connect(server);
>> > }
>> >
>> > void reset_cifs_unix_caps(int xid, struct cifsTconInfo *tcon,
>>
>> Nice cleanup.
>>
>> Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
>
> Actually...my ACK on this still stands if you don't want to do it, but
> there is a "nice to have" change that could go with this.
>
> It would be nice if we didn't have this goofy "addr" union in the
> TCP_Server_Info, and just had a standard sockaddr_storage embedded
> inside it instead. It would mean quite a bit less conditional code, I
> think...
>
> You're already touching about half the code that would be affected by
> such a change with this patch. Care to do the rest?
Ok, good idea. But I think it should be another separate patch that we
can apply after the patch above.
--
Best regards,
Pavel Shilovsky.
--
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
