Re: [PATCH 1/2] CIFS: Simplify ipv*_connect functions into one (try #2)

[Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>]

On Mon, 22 Nov 2010 22:26:59 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> 2010/11/22 Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
> > On Tue, 16 Nov 2010 10:48:29 +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>
> >> ---
> >>  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);
> >
> >                        ^^^^^
> > I finally got a chance to give this a try today. It's broken due to the
> > above statement. If we get an IPv6 address, we need to set the family
> > correctly in sock_create_kern.
> >
> > Pavel, care to fix this?
> 
> No problem. Should I create separate patch or change this one?

Please fix this one. I don't believe Steve has merged it yet.

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>