From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Bruce Fields <bfields@fieldses.org>
CC:     Dai Ngo <dai.ngo@oracle.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH RFC v6 2/2] nfsd: Initial implementation of NFSv4
 Courteous Server
Thread-Topic: [PATCH RFC v6 2/2] nfsd: Initial implementation of NFSv4
 Courteous Server
Thread-Index: AQHX6ssTAZRABZ1HqkCYuoltcOnBFawl4NyAgAAeY4CAAAz7AIAABiuAgAGDrIA=
Date:   Tue, 7 Dec 2021 22:00:22 +0000
Message-ID: <DEB6A7B8-0772-487F-8861-BEB924259860@oracle.com>
References: <20211206175942.47326-1-dai.ngo@oracle.com>
 <20211206175942.47326-3-dai.ngo@oracle.com>
 <4025C4F6-258F-4A2E-8715-05FD77052275@oracle.com>
 <01923a7c-bb49-c004-8a35-dbbae718e374@oracle.com>
 <242C2259-2CF0-406F-B313-23D6D923C76F@oracle.com>
 <20211206225249.GE20244@fieldses.org>
In-Reply-To: <20211206225249.GE20244@fieldses.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?n7dzw3uD8rFDtczg/J7UpAyI7OyMn9xQBcpusxhfVsyfWGq/mydTiXpE3k+o?=
 =?us-ascii?Q?/IlEVuaWyvNwJrp7RBEVQGdzzvRp1PEWWUEHRkqCSz0aTpc+9G9zAad98zfp?=
 =?us-ascii?Q?YwWcyet04LVOE8vI7/PyPFgiE2V+XD7YRva5ZPS+GeS5K98kWjLg7ABR4j76?=
 =?us-ascii?Q?IJrZRKWpe1DKlRt5Hc5/nKKTkc3zKdu6uR98Kaf0LaVIeOxIP4jhnXzHZAVz?=
 =?us-ascii?Q?p3Z9o3Q+BMTCludbflluPDRrkNpwRqlODWBpUsXICVXPwwM7hKWwNzNnwQWg?=
 =?us-ascii?Q?jQRlmGm4uykGJAVQtXO07KHseIRhKjdgE/olvUnqbaYWDn+HuNuxBF3Jg1zs?=
 =?us-ascii?Q?k6/qpjX/NICWiQRhiAzYO8hHSou3bDMUkyaGtcYILGBcE04b71AICeVGccek?=
 =?us-ascii?Q?7wzQeF+9hfwPvRgJsn6cBDc/MhwdOGeklWHb5xH/kRtoA6bpkbp0QnWyTsDX?=
 =?us-ascii?Q?slLC+vPRRHlcKUkJNOArIbCN/XtGdLbjbAVDsf5Jz3THGsy5JttNktDH1Dae?=
 =?us-ascii?Q?bN8Kp9QVDUH2nNL/5FFZuE1dl1y8t9QdXIW8trbm/v/L04n+fME4lPZbZuAv?=
 =?us-ascii?Q?n+eF5lKr+xqNPr86f14InFsqNLpJbKCIVj2eYLpftcUTjBTmftZtUxakdGs9?=
 =?us-ascii?Q?DAqTZgZHJLHc34MX/eJR56UpaxFUCWw9TjFmryPH1R4HGw1ZwzieOcHW/9wX?=
 =?us-ascii?Q?R/zXSQvAXMd5A5hBNisw59uJ18J6/7isM6FbuxMHqoPYTmTZXCDHPSBbvBe2?=
 =?us-ascii?Q?HyrEk0geH118x69P7HOv4YYnI9Z3Wo+fyH1OAOHBZ8Zzzv5Jd3VEdAaMHtV2?=
 =?us-ascii?Q?M1jKFCU//sMIR7EB2bP7nCk+PGGTYO8nsgVo+UXf1UsY7BAybL4JfQNp2I1u?=
 =?us-ascii?Q?ZSjXlGfY15LfBkQb+yhAIXo6FdQfgdaIaLrGKKBdkzxl1NI+7RER7eNOpo7i?=
 =?us-ascii?Q?3Ca+P3VXJVsiWcrl/bcnSWsEGeM2C9e0m2ybyTMaTk7mn5lHh6ntcFfVdIWz?=
 =?us-ascii?Q?0BacMOF3/jn8WN0UiPJXjaGEmqaoPC/0eGCMqPup9jiU73Z20LUW8XM8dovk?=
 =?us-ascii?Q?TLAip9IBxj8Kp4HTXicOG49vYmOLYfbt7VLagrbQr2NtrgwDUFdHMopsjBq9?=
 =?us-ascii?Q?X9m322jMQr2Ua5S6eOMK6V8hD+YcYsSNdgLUZ3tesfDBBg71RzzPYbTUQquv?=
 =?us-ascii?Q?RHYnfU5xpWwaCrcKgwh+kLdYa2TcHkE82sU4KUzqj+a2pOTQrbehSffhgO5l?=
 =?us-ascii?Q?BK7Im+3PWsQuak1WJMQWccV62gml02hfDGPXQXAzqHvETb4P4S4PMjYPZ64d?=
 =?us-ascii?Q?HI7X5gVfjh2SPO7LhmOutNTFaAt6l1WeyIseHjqNxTFVbZbzk62twCz7ed2Z?=
 =?us-ascii?Q?VE4veOEkKZpVaecpqCiMZ5a6ZylIg/5FljdyY8WHJXGjp1DQBQIWzaRgGIWJ?=
 =?us-ascii?Q?EDTMpBohsS5JRY9QvZGuzZVpWa3IqQGYat9ClhslTgHuw7j+kdwvM7s9hkPp?=
 =?us-ascii?Q?N7FQg81GcJ0HYvkFTZvYkE8pj4Etc0iGQ9ZwtiupGLjIcIu5XV2WzLx5TwYg?=
 =?us-ascii?Q?1uUvEZdxhtObycJAHF0B1G4KsqSHfk8ke9KhGdTx2mwTO4DaaoszgHRuGVA9?=
 =?us-ascii?Q?m16OiB7kvO+FypnBy7KPnSY=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5E21BCF4B00DF642B3B6CA6B46B2A8C9@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: ea157984-d9de-4b03-3d26-08d9b9ccf754
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Dec 2021 22:00:22.2450
 (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: QKj2TFilIkMAJSzdvQJyv/HfdHhLlXWooqBk/wZypubb8U6taqvu7dq98sHQDbHHj8IIzj3s4ivv0BE4Gu2+zA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB5145
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10191 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 suspectscore=0
 adultscore=0 mlxlogscore=999 bulkscore=0 malwarescore=0 mlxscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2112070134
X-Proofpoint-GUID: rSgY64D-tJbotaPZnO5O-b9D7G3tlgp9
X-Proofpoint-ORIG-GUID: rSgY64D-tJbotaPZnO5O-b9D7G3tlgp9
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Dec 6, 2021, at 5:52 PM, Bruce Fields <bfields@fieldses.org> wrote:
>=20
> On Mon, Dec 06, 2021 at 10:30:45PM +0000, Chuck Lever III wrote:
>> OK, this is really confusing.
>>=20
>> 5142         set_deny(open->op_share_deny, stp);
>> 5143         fp->fi_share_deny |=3D (open->op_share_deny & NFS4_SHARE_DE=
NY_BOTH);
>>=20
>> Here set_deny() is treating the contents of open->op_share_deny
>> as bit positions, but then upon return NFS4_SHARE_DENY_BOTH
>> is used directly as a bit mask. Am I reading this correctly?
>>=20
>> But that's not your problem, so I'll let that be.
>=20
> This is weird but intentional.
>=20
> For most practical purposes, fi_share_deny is all that matters.
>=20
> BUT, there is also this language in the spec for OPEN_DOWNGRADE:
>=20
> 	https://datatracker.ietf.org/doc/html/rfc5661#section-18.18.3
>=20
> 	The bits in share_deny SHOULD equal the union of the share_deny
> 	bits specified for some subset of the OPENs in effect for the
> 	current open-owner on the current file.
>=20
> 	If the above constraints are not respected, the server SHOULD
> 	return the error NFS4ERR_INVAL.
>=20
> If you open a file twice, once with DENY_READ, once with DENY_WRITE,
> then that is not *quite* the same as opening it once with DENY_BOTH.  In
> the former case, you're allowed to, for example, downgrade to DENY_READ.
> In the latter, you're not.
>=20
> So if we want to the server to follow that SHOULD, we need to remember
> not only that the union of all the DENYs so far, you also need to
> remember the different DENY modes that different OPENs were done with.
>=20
> So, we also keep the st_deny_bmap with that information.
>=20
> The same goes for allow bits (hence there's also an st_access_bmap).
>=20
> It's arguably a lot of extra busy work just for one SHOULD that has no
> justification other than just to be persnickety about client
> behavior....

Thanks for clarifying! If you are feeling industrious, it would be nice
for this to be documented somewhere in the source code....


--
Chuck Lever