On Fri, Nov 8, 2024 at 12:35 AM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
>
> meetakshisetiyaoss@xxxxxxxxx writes:
>
> > @@ -2245,6 +2269,7 @@ struct cifs_ses *
> > cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
> > {
> > int rc = 0;
> > + int retries = 0;
> > unsigned int xid;
> > struct cifs_ses *ses;
> > struct sockaddr_in *addr = (struct sockaddr_in *)&server->dstaddr;
> > @@ -2263,6 +2288,8 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
> > cifs_dbg(FYI, "Session needs reconnect\n");
> >
> > mutex_lock(&ses->session_mutex);
> > +
> > +retry_old_session:
> > rc = cifs_negotiate_protocol(xid, ses, server);
> > if (rc) {
> > mutex_unlock(&ses->session_mutex);
> > @@ -2275,6 +2302,13 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
> > rc = cifs_setup_session(xid, ses, server,
> > ctx->local_nls);
> > if (rc) {
> > + if (((rc == -EACCES) || (rc == -EKEYEXPIRED) ||
> > + (rc == -EKEYREVOKED)) && !retries && ses->password2) {
> > + retries++;
> > + cifs_info("Session reconnect failed, retrying with alternate password\n");
>
> Please don't add more noisy messages over reconnect. Remember that if
> SMB session doesn't get re-established, there will be flood enough on
> dmesg with "Send error in SessSetup = ..." messages on every 2s that
> already pisses off users and customers.
>
Perhaps we could do a cifs_dbg instead of cifs_info.
But Paulo, the problem here is that we retry every 2s. I think we
should address that instead.
One way is to do an exponential backoff every time we retry.
I'd also want to understand why we need the reconnect work? Why not
always do smb2_reconnect when someone does filesystem calls on the
mount point?
That way, we avoid unnecessary retries altogether. @Steve French your opinions?
> > + swap(ses->password, ses->password2);
> > + goto retry_old_session;
> > + }
> > mutex_unlock(&ses->session_mutex);
> > /* problem -- put our reference */
> > cifs_put_smb_ses(ses);
> > @@ -2350,6 +2384,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
> > ses->chans_need_reconnect = 1;
> > spin_unlock(&ses->chan_lock);
> >
> > +retry_new_session:
> > mutex_lock(&ses->session_mutex);
> > rc = cifs_negotiate_protocol(xid, ses, server);
> > if (!rc)
> > @@ -2362,8 +2397,16 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
> > sizeof(ses->smb3signingkey));
> > spin_unlock(&ses->chan_lock);
> >
> > - if (rc)
> > - goto get_ses_fail;
> > + if (rc) {
> > + if (((rc == -EACCES) || (rc == -EKEYEXPIRED) ||
> > + (rc == -EKEYREVOKED)) && !retries && ses->password2) {
> > + retries++;
> > + cifs_info("Session setup failed, retrying with alternate password\n");
>
> Ditto.
>
> > + swap(ses->password, ses->password2);
> > + goto retry_new_session;
> > + } else
> > + goto get_ses_fail;
> > + }
> >
> > /*
> > * success, put it on the list and add it as first channel
> > --
> > 2.46.0.46.g406f326d27
--
Regards,
Shyam
