Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CAN-5tyHegg96s7mr1YeoPbVd0UA7_cd2GEPYNWx98uUcx-0ARw@mail.gmail.com>
 <FF0659E0-8F04-4005-96D0-5D513881EDFE@oracle.com>
In-Reply-To: <FF0659E0-8F04-4005-96D0-5D513881EDFE@oracle.com>
From:   Olga Kornievskaia <aglo@umich.edu>
Date:   Tue, 10 Mar 2020 17:07:27 -0400
Message-ID: <CAN-5tyHQpS7AmPX1cDxKpD=5gyAM7+nmLX+iA29QV7sLwhoX9Q@mail.gmail.com>
Subject: Re: [RFC PATCH] fix krb5p mount not providing large enough buffer in rq_rcvsize
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

Hi Chuck,

On Tue, Mar 10, 2020 at 3:57 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> Hi Olga-
>
> > On Mar 10, 2020, at 2:58 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> >
> > Ever since commit 2c94b8eca1a26 "SUNRPC: Use au_rslack when computing
> > reply buffer size". It changed how "req->rq_rcvsize" is calculated. It
> > used to use au_cslack value which was nice and large and changed it to
> > au_rslack value which turns out to be too small.
> >
> > Since 5.1, v3 mount with sec=krb5p fails against an Ontap server
> > because client's receive buffer it too small.
>
> Can you be more specific? For instance, why is 100 bytes adequate for
> Linux servers, but not OnTAP?

I don't know why Ontap sends more data than Linux server. The
opaque_len is just a lot larger. For the first message Linux
opaque_len is 120bytes and Ontap it's 206. So it could be for instance
for FSINFO that sends the file handle, for Netapp the file handle is
44bytes and for Linux it's only 28bytes.

> Is this explanation for the current value not correct?
>
>   51 /* length of a krb5 verifier (48), plus data added before arguments when
>   52  * using integrity (two 4-byte integers): */

I'm not sure what it is suppose to be. Isn't "data before arguments"
can vary in length and thus explain why linux and onto sizes are
different?
Looking at the network trace the krb5 verifier I see is 36bytes.

> > For GSS, au_rslack is calculated from GSS_VERF_SLACK value which is
> > currently 100. And it's not enough. Changing it to 104 works and then
> > au_rslack is recalculated based on actual received mic.len and not
> > just the default buffer size.
> >
> > I would like to propose to change it to something a little larger than
> > 104, like 120 to give room if some other server might reply with
> > something even larger.
>
> Why does it need to be larger than 104?

I don't know why 100 was chosen and given that I think arguments are
taken into the account and arguments can change. I think NetApp has
changed their file handle sizes (at some point, not in the near past
but i think so?). Perhaps they might want to do that again so the size
will change again.

Honestly, I would have like for 100 to be 200 to be safe.

>
>
> > Thoughts? Will send an actual patch if no objections to this one.
> >
> > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> > index 24ca861..44ae6bc 100644
> > --- a/net/sunrpc/auth_gss/auth_gss.c
> > +++ b/net/sunrpc/auth_gss/auth_gss.c
> > @@ -50,7 +50,7 @@
> > #define GSS_CRED_SLACK         (RPC_MAX_AUTH_SIZE * 2)
> > /* length of a krb5 verifier (48), plus data added before arguments when
> >  * using integrity (two 4-byte integers): */
> > -#define GSS_VERF_SLACK         100
> > +#define GSS_VERF_SLACK         120
> >
> > static DEFINE_HASHTABLE(gss_auth_hash_table, 4);
> > static DEFINE_SPINLOCK(gss_auth_hash_lock);
>
> --
> Chuck Lever
>
>
>