Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Igor Mammedov <imammedo@redhat.com>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1] NFSD: Decode NFSv4 birth time attribute
Thread-Topic: [PATCH v1] NFSD: Decode NFSv4 birth time attribute
Thread-Index: AQHYlI1Yu1IMkeKgwEG1/w/Jb2AUD615alaAgAABEoA=
Date:   Mon, 11 Jul 2022 17:18:38 +0000
Message-ID: <A4F0C111-B2EB-4325-AC6A-4A80BD19DA43@oracle.com>
References: <165747876458.1259.8334435718280903102.stgit@bazille.1015granger.net>
 <20220711191447.1046538c@redhat.com>
In-Reply-To: <20220711191447.1046538c@redhat.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?vh0NiVN8mq3kVoc93spNvVMoa1JadeuDBfPokDfLKJXjoYlrt5onkPnc4FBp?=
 =?us-ascii?Q?F49Tkp6ZeIxHu0MyBxHAry1YDTs1vKCCzeHAxzok7NMkbi7btJ/kQG3Hh5pu?=
 =?us-ascii?Q?Unkd51lMlZZlHiokXXoI/ygxAt4a786F3zkK1OVKXCQTtQ7GRJp8A5FjhesC?=
 =?us-ascii?Q?GubD/Xh60SSBoIyteQ3h8SeqKZKx6ngiLfEYA2lW7u9PmuaLtNrBw5clGGX0?=
 =?us-ascii?Q?MaZqOkww5Y+vrJsuGy2+EtPRzBD9XAoiqvUzy+ruCsjyXGVZ1FykvJIKKAEv?=
 =?us-ascii?Q?mC8pJ4JqQHpxfu4RutP8S5+jWmfKslOgLgkNGfL9bl9WDInc4eaSn2XpbkP5?=
 =?us-ascii?Q?8ghw2hNGUme5CnaK/aKedI8E6MJHyds6hZhmLnxNBPJk91Pmf18jM/OTNSyF?=
 =?us-ascii?Q?L2xwCnQmcRyvCepmncGLRoOHR5wu7zkCvOX7GvH4/9UdhlcWgTIxUJfvBJRa?=
 =?us-ascii?Q?Cm2SZn+o9360mWyD376/O8riYpMngWdNMNNyMYMOb9zWY8SidDxCJHMKuVe4?=
 =?us-ascii?Q?JfUnj1UYsRJpClH0jqq/XEA4xoZsVcO6Pk8SsKoIW0qFKnxsINx1/l7GJGMm?=
 =?us-ascii?Q?+6vzCZIOlvA8vTqSV1qfLPb1bZM6DwSipz4BME5if0jCXvTResIkO8pPkfR2?=
 =?us-ascii?Q?IOyrSfPI0Tgt4vMLwVekyvLmKKayo1YDbD6MqH/AkrpiyKQIWu/IhS/G6t7y?=
 =?us-ascii?Q?OxeM9QcpSt/fyYXqvKzbMp3uQFMct7Je8JixVHU+TfBzdQD5fDVx6vDULoiT?=
 =?us-ascii?Q?wGsXgScfIwNgLoWqFtKKAAGlobf31VV7Zixwlu5Gxl9eeJhgJl4K7s7DivWY?=
 =?us-ascii?Q?eVz68sb/KkHXfVOTWulYAZCPreg63SzkhXfEd0Lsg4Hc1laM7ImhuNreBm/9?=
 =?us-ascii?Q?4BglPBdyocJgK0eIDJrKhW8VDuWc5XNXb7B8M/ntiSFjzMKVaIufCsfL1ERB?=
 =?us-ascii?Q?E61DkdPmRpeOzXjXvaYpvKwywtCNqiLcBItpoXBLgiPQTzeYEVDde/GDNl1B?=
 =?us-ascii?Q?8l9tVYIudc8L4B1ZKC6hz86lDU6x7Af1q2kt+MpTmPjhTffbd02nN7fZSZVK?=
 =?us-ascii?Q?Ns3EpOk77r9Nlu2Ve/FcDdtnbjXBrmYPhr5BCKQ0sy8VmpDMTBJtDc+Id0Et?=
 =?us-ascii?Q?5vMvoNiQNIXYFb1oaIg7iptNzESHBJ95kS4FduBEF6AOliKohYpAcL/qL6d9?=
 =?us-ascii?Q?DV8XLP0u78Wp+VyLXwhp5gAgewe2B/U/+L4G5csWkSRjd37cF+YKCRYhGwEE?=
 =?us-ascii?Q?ylRY0Hht/Pw8yoA+nVr1qDFpJ0YBBFoCNZQ4Xu+Q7kz2wwgnWjZ+GQKBpCzf?=
 =?us-ascii?Q?4OffwYx3XCeAgSjke+sK44St75zjyxVOz2EMPJz3e4LCIfdd5gkHeEV77//C?=
 =?us-ascii?Q?3yNVw89KL9Fy6B0489LMZz+bXLZ4DeEaNB/scUoVcNplB6Abn3ST10AJBc5e?=
 =?us-ascii?Q?W1dGM5XSgFH2MDIuwE+9YA0K2lkpUnZLx0RDxD9rmrwNPSlxL0JoekdWe4OQ?=
 =?us-ascii?Q?lHUAsFph/iohklVgzJbmlCqjzPvYbo0v2IAtyX4bBhKqAg8nZ3t1nBoqOzBh?=
 =?us-ascii?Q?0+vXxTfISdq2je9qstuwTeRNjB4NGb3wg2Gu1M6hHLiS/3UY1Ht6O2sHbc89?=
 =?us-ascii?Q?mg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <84857339ABFA7B47A2CEBF8AAA67F371@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: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e2910f67-7c65-427c-ef99-08da63616512
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Jul 2022 17:18:38.4907
 (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: voYDYdIroaQrH63nXfXhO0Kvt/WD+14xpx//rE6W3NNALl3yUl9wg4MuSy9f/0yFkYHxDS6WFjVhgo5d9kHKgg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB3960
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-11_21:2022-07-08,2022-07-11 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 phishscore=0
 suspectscore=0 mlxlogscore=999 mlxscore=0 spamscore=0 malwarescore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2206140000 definitions=main-2207110074
X-Proofpoint-GUID: yyAd4WPxuVcBdwcRxLWK4wISTWXjCeOK
X-Proofpoint-ORIG-GUID: yyAd4WPxuVcBdwcRxLWK4wISTWXjCeOK
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jul 11, 2022, at 1:14 PM, Igor Mammedov <imammedo@redhat.com> wrote:
>=20
> On Sun, 10 Jul 2022 14:46:04 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
>=20
>> NFSD has advertised support for the NFSv4 time_create attribute
>> since commit e377a3e698fb ("nfsd: Add support for the birth time
>> attribute").
>>=20
>> Igor Mammedov reports that Mac OS clients attempt to set the NFSv4
>> birth time attribute via OPEN(CREATE) and SETATTR if the server
>> indicates that it supports it, but since the above commit was
>> merged, those attempts now fail.
>>=20
>> Table 5 in RFC 8881 lists the time_create attribute as one that can
>> be both set and retrieved, but the above commit did not add server
>> support for clients to provide a time_create attribute. IMO that's
>> a bug in our implementation of the NFSv4 protocol, which this commit
>> addresses.
>>=20
>> Whether NFSD silently ignores the new birth time or actually sets it
>> is another matter. I haven't found another filesystem service in the
>> Linux kernel that enables users or clients to modify a file's birth
>> time attribute.
>>=20
>> This commit reflects my (perhaps incorrect) understanding of whether
>> Linux users can set a file's birth time. NFSD will now recognize a
>> time_create attribute but it ignores its value. It clears the
>> time_create bit in the returned attribute bitmask to indicate that
>> the value was not used.
>>=20
>> Reported-by: Igor Mammedov <imammedo@redhat.com>
>> Fixes: e377a3e698fb ("nfsd: Add support for the birth time attribute")
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>=20
> Thanks for fixing it,
> tested 'touch', 'cp', 'tar' within CLI and copying file with Finder
>=20
> Tested-by: Igor Mammedov <imammedo@redhat.com>

Thanks!


> on tangent:
> when copying file from Mac (used 'cp') there is a delay ~4sec/file
> 'cp' does first triggers create then extra open and then setattr
> which returns
> SETATTR Status: NFS4ERR_DELAY
> after which the client stalls for a few seconds before repeating setattr.
> So question is what makes server unhappy to trigger this error
> and if it could be fixed on server side.
>=20
> it seems to affect other methods of copying. So if one extracted
> an archive with multiple files or copied multiple files, that
> would be a pain.
>=20
> With vers=3D3 copying is 'instant'
> with linux client and vers=3D4.0 copying is 'instant' as well but it
> doesn't use the same call sequence.
>=20
> PS:
> it is not regression (I think slowness was there for a long time)

A network capture would help diagnose this further, but it
sounds like it's delegation-related.


>> ---
>> fs/nfsd/nfs4xdr.c | 9 +++++++++
>> fs/nfsd/nfsd.h | 3 ++-
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>=20
>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>> index 61b2aae81abb..2acea7792bb2 100644
>> --- a/fs/nfsd/nfs4xdr.c
>> +++ b/fs/nfsd/nfs4xdr.c
>> @@ -470,6 +470,15 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp=
, u32 *bmval, u32 bmlen,
>> 			return nfserr_bad_xdr;
>> 		}
>> 	}
>> +	if (bmval[1] & FATTR4_WORD1_TIME_CREATE) {
>> +		struct timespec64 ts;
>> +
>> +		/* No Linux filesystem supports setting this attribute. */
>> +		bmval[1] &=3D ~FATTR4_WORD1_TIME_CREATE;
>> +		status =3D nfsd4_decode_nfstime4(argp, &ts);
>> +		if (status)
>> +			return status;
>> +	}
>> 	if (bmval[1] & FATTR4_WORD1_TIME_MODIFY_SET) {
>> 		u32 set_it;
>>=20
>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>> index 847b482155ae..9a8b09afc173 100644
>> --- a/fs/nfsd/nfsd.h
>> +++ b/fs/nfsd/nfsd.h
>> @@ -465,7 +465,8 @@ static inline bool nfsd_attrs_supported(u32 minorver=
sion, const u32 *bmval)
>> 	(FATTR4_WORD0_SIZE | FATTR4_WORD0_ACL)
>> #define NFSD_WRITEABLE_ATTRS_WORD1 \
>> 	(FATTR4_WORD1_MODE | FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP \
>> -	| FATTR4_WORD1_TIME_ACCESS_SET | FATTR4_WORD1_TIME_MODIFY_SET)
>> +	| FATTR4_WORD1_TIME_ACCESS_SET | FATTR4_WORD1_TIME_CREATE \
>> +	| FATTR4_WORD1_TIME_MODIFY_SET)
>> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
>> #define MAYBE_FATTR4_WORD2_SECURITY_LABEL \
>> 	FATTR4_WORD2_SECURITY_LABEL

--
Chuck Lever