Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
CC:     "dan.aloni@vastdata.com" <dan.aloni@vastdata.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Anna Schumaker <Anna.Schumaker@netapp.com>,
        "trondmy@kernel.org" <trondmy@kernel.org>
Subject: Re: [PATCH] NFSD: trim reads past NFS_OFFSET_MAX
Thread-Topic: [PATCH] NFSD: trim reads past NFS_OFFSET_MAX
Thread-Index: AQHYDvfTRZNTkkSWjkWDIwR2Vc39WaxuD5oAgADuzQCAAEhDAIAAFvMAgAAd+YA=
Date:   Sat, 22 Jan 2022 20:15:14 +0000
Message-ID: <D583D8ED-8E59-4CB2-B65C-AD1C0B332DAD@oracle.com>
References: <fa9974724216c43f9bdb3fd39555d398fde11e59.camel@hammerspace.com>
 <20220121185023.260128-1-dan.aloni@vastdata.com>
 <5DD3C5DF-52EF-4C84-894C-FCBB9A0F4259@oracle.com>
 <20220122124710.4l5bzmfxhf2o2yee@gmail.com>
 <04E4C6DC-B78F-45DA-871A-296379B2D484@oracle.com>
 <b780f2adabad8beb13c0deca62561116b61e2402.camel@hammerspace.com>
In-Reply-To: <b780f2adabad8beb13c0deca62561116b61e2402.camel@hammerspace.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?UllyI1oq9/LJDCvcz36O1sgEbwZQbrOzkzi4huEaWylnY6lPVAkG6fO54gFQ?=
 =?us-ascii?Q?KwVdW3nAogFbsVmOpb3nCU8EnEvR/K4v4MIBcf0kU3VEJJ3TwDg6Kin9z7Bz?=
 =?us-ascii?Q?V50UpwRjsMVSeW1ANO9sHOm8bS5DjhLv/mOeoLnmCrMN5BTnuDLWg0wqE5vd?=
 =?us-ascii?Q?LkskfUwk7KcvHgW21I33rsamFDohib0ApHHMjLmipEGy3sDQUZFgymxBGa5Q?=
 =?us-ascii?Q?legUufFpiRzqIJJkyM3RxYBRdWtQLK46PhCYCPREdDATD70gQCEFcEfqZNrV?=
 =?us-ascii?Q?vsb7dX7vs2PYerbC8CEuWMQn+cNy7rS4kmPOba81l2gmnDJlc1Xckgzor8cU?=
 =?us-ascii?Q?Y0WHmS20si0kQIBdsM9CCTwD9LxCI8EqGvIt08PXVw2OxkzELO7LsFo4lrAS?=
 =?us-ascii?Q?JPQz4vEN9AJg6BBjkboJjURhFNs77OfsogxOkZwakltqFErIxoK99q27LTa6?=
 =?us-ascii?Q?Z0irBLdAWB/L/cqpOsrObyJu/m8Y4jvVnPgA99IjYVtXj4LSnNdaZ2Y2/PI8?=
 =?us-ascii?Q?BVtAEvdRm3YH7/gHYFtBnDb+AQfWZig4c+eSz92U8eFgBeodk0YaqFy8Ohq/?=
 =?us-ascii?Q?zEzfmcGVif+T/41w6JZsRHeaQ4LmCrmvevzWHkPr/fZcd2olPRxXv76fgMHq?=
 =?us-ascii?Q?QYTonJ4COk72b7SJeyEidUIHhIYpb4e6jubk3+xHXq45FU32++ojC/kRdcbL?=
 =?us-ascii?Q?T1YLLGjh2Ds7JKELD45HGrL1kAuna4u9nDEQbpG0PWOb3+NP1bG12hUlYP4/?=
 =?us-ascii?Q?gsjiiYqFZrRkFs3shxspRKqEXRA5dAPjdGTRHx2JmS/uf9RNsuR1e0/G/61k?=
 =?us-ascii?Q?GAwTdnvItJJiMBGe3QTUm2q38Rok69OtCEiShNqtfL3yHFlVbOE16zvXoZoI?=
 =?us-ascii?Q?tkala3wMkHfbnqXaF0zcqoFX/VfweqSu0OGp3x8UQQGs+sdtzhqSJ6IkfVAf?=
 =?us-ascii?Q?FwbfFHSpJW7fDZLb17C4dqUgGE14olASSTMF8jzVQSvIgYVvdYzvnVbmsozt?=
 =?us-ascii?Q?nzP83eqZr0cJm73YOdmsVNuJUU7Vok+b4t61K3cFQS0smX87mO+3IhIzTTQz?=
 =?us-ascii?Q?jmuXCGPkU4zy/AEi36Qny74bwrT8PEoOc7PBxSoVTW9n8hssHEMJA1ov3tCt?=
 =?us-ascii?Q?IV7htWHU2DElD0f98FQEXCAr8Gu/akKayH+RdmSEbouTSlwLCpUD9t6QKJxw?=
 =?us-ascii?Q?fEhi4RwC8xkLk53HtZkcxpczmELjiiHOKM2qrYEDz76kLkus9x8+OzGIsQxY?=
 =?us-ascii?Q?0+QpI9uSfRim35L0jj1ygJ6mapvlshZDr9jek/5RlxE/+4aaBhiI/QEU68dy?=
 =?us-ascii?Q?z8I5RcGHVAThtzd5T5JgXz2Rgo4w1zAUMpCkiSipO0NbH+TDD4nQ6gF7BfIc?=
 =?us-ascii?Q?TKwudMIUfTXDVnz+sJlxo7I4X659wTabPAVHpMnlwMr55rbvfRs8GW5Ab5NW?=
 =?us-ascii?Q?0G4ng7shXPitxMvrzMrf5LIdqMjs6CoAtcvWPDQUZdR4XLcAvpDoNwha0JRl?=
 =?us-ascii?Q?UEFIOmPx0dGae5AGSouGhkA6OiONXPD81OKTG2sPwUMAQVF9MTj9XVZBhT6Y?=
 =?us-ascii?Q?W3fx+r80yZQ1GKC7Bbt6STYRubxGJxnTxW7+0qwDLgszvZUjx60SXABucteG?=
 =?us-ascii?Q?mCgr8dAhjEFmcwsIveLK288=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <8980BE22905CB744B0C59FCE33E76441@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB4858.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 94ed39e2-a3b0-4005-eb1b-08d9dde3e675
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jan 2022 20:15:14.2801
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 6fIpT+cRD1X0rponk9VIoMlnN+8jzGiHHsz+3n/++82koX7jg5pJakVD3Ff4ZsA+30iZJ+x1ik6lq3kIjVnhow==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB4825
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10235 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxscore=0 malwarescore=0
 spamscore=0 phishscore=0 suspectscore=0 adultscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2201220144
X-Proofpoint-ORIG-GUID: vJCN9K9sLtO2bB20lw-5X28soLfJrB_3
X-Proofpoint-GUID: vJCN9K9sLtO2bB20lw-5X28soLfJrB_3
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jan 22, 2022, at 1:27 PM, Trond Myklebust <trondmy@hammerspace.com> wr=
ote:
>=20
> On Sat, 2022-01-22 at 17:05 +0000, Chuck Lever III wrote:
>>=20
>>> On Jan 22, 2022, at 7:47 AM, Dan Aloni <dan.aloni@vastdata.com>
>>> wrote:
>>>=20
>>> On Fri, Jan 21, 2022 at 10:32:28PM +0000, Chuck Lever III wrote:
>>>>> On Jan 21, 2022, at 1:50 PM, Dan Aloni <dan.aloni@vastdata.com>
>>>>> wrote:
>>>>>=20
>>>>> Due to change 8cfb9015280d ("NFS: Always provide aligned
>>>>> buffers to the
>>>>> RPC read layers"), a read of 0xfff is aligned up to server
>>>>> rsize of
>>>>> 0x1000.
>>>>>=20
>>>>> As a result, in a test where the server has a file of size
>>>>> 0x7fffffffffffffff, and the client tries to read from the
>>>>> offset
>>>>> 0x7ffffffffffff000, the read causes loff_t overflow in the
>>>>> server and it
>>>>> returns an NFS code of EINVAL to the client. The client as a
>>>>> result
>>>>> indefinitely retries the request.
>>>>=20
>>>> An infinite loop in this case is a client bug.
>>>>=20
>>>> Section 3.3.6 of RFC 1813 permits the NFSv3 READ procedure
>>>> to return NFS3ERR_INVAL. The READ entry in Table 6 of RFC
>>>> 5661 permits the NFSv4 READ operation to return
>>>> NFS4ERR_INVAL.
>>>>=20
>>>> Was the client side fix for this issue rejected?
>>>=20
>>> Yeah, see Trond's response in
>>>=20
>>> =20
>>> https://lore.kernel.org/linux-nfs/fa9974724216c43f9bdb3fd39555d398fde11=
e59.camel@hammerspace.com/
>>>=20
>>> So it is both a client and server bugs?
>>=20
>> Splitting hairs, but yes there are issues on both sides
>> IMO. Bad behavior due to bugs on both sides is actually
>> not uncommon.
>>=20
>> Trond is correct that the server is not dealing totally
>> correctly with the range of values in a READ request.
>>=20
>> However, as I pointed out, the specification permits NFS
>> servers to return NFS[34]ERR_INVAL on READ. And in fact,
>> there is already code in the NFSv4 READ path that returns
>> INVAL, for example:
>>=20
>>  785         if (read->rd_offset >=3D OFFSET_MAX)
>>  786                 return nfserr_inval;
>>=20
>> I'm not sure the specifications describe precisely when
>> the server /must/ return INVAL, but the client needs to
>> be prepared to handle it reasonably. If INVAL results in
>> an infinite loop, then that's a client bug.
>>=20
>> IMO changing the alignment for that case is a band-aid.
>> The underlying looping behavior is what is the root
>> problem. (So... I agree with Trond's NACK, but for
>> different reasons).
>=20
> If I'm reading Dan's test case correctly, the client is trying to read
> a full page of 0x1000 bytes starting at offset 0x7fffffffffffff000.
> That means the end offset for that read is 0x7fffffffffffff000 + 0x1000
> - 1 =3D 0x7fffffffffffffff.
>=20
> IOW: as far as the server is concerned, there is no loff_t overflow on
> either the start or end offset and so there is no reason for it to
> return NFS4ERR_INVAL.

Yep, I agree there's server misbehavior, and I think Dan's
server fix is on point.

I would like to know why the client is looping, though. INVAL
is a valid response the Linux server already uses in other
cases and by itself should not trigger a READ retry.

After checking the relevant XDR definitions, an NFS READ error
response doesn't include the EOF flag, so I'm a little mystified
why the client would need to retry after receiving INVAL.


--
Chuck Lever