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:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v1] SUNRPC: Ensure backchannel transports are marked
 connected
Thread-Topic: [PATCH v1] SUNRPC: Ensure backchannel transports are marked
 connected
Thread-Index: AQHXlPXcGICcXaW3sESgYQmy74r7pat6ysSAgAAEDgA=
Date:   Thu, 19 Aug 2021 13:16:09 +0000
Message-ID: <6D4FFB37-B5CB-410B-A3C9-AAC92F611520@oracle.com>
References: <162937592206.2298.13447589794033256951.stgit@klimt.1015granger.net>
 <ed3fbd005a9a2e3a6217085ebe05e80cd78766ba.camel@hammerspace.com>
In-Reply-To: <ed3fbd005a9a2e3a6217085ebe05e80cd78766ba.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?Miu1BEVFLh7VfDEgZMcHYqZhyuSNKMADkRSvvvMwqwkhwh/nHM8ci7YEPQiG?=
 =?us-ascii?Q?YAnGerunag0RFTy3M6mD7SwwbJNPt2mz3Sc/xELolWIE2QPVYXE6LuqGWZ//?=
 =?us-ascii?Q?YGGwKrB4PZlysgFJXbubTQXv96EO3B2Q0CujptbXlNW7orBrwD+WKIgidjO3?=
 =?us-ascii?Q?6OQ7unmj1vbpCr//lAOuJ5Jt0lgYMF3GPZmVsQPWx3ZAUxnUUEDLg2F88ZxN?=
 =?us-ascii?Q?NU99flVfHVG6NYLu+gemOw5ZmVxzI/biDffvYPCXNurkiyo6g15SB4TEfKX2?=
 =?us-ascii?Q?AR7RAQOkJWyfJnS6wlyldC5ww6dfUIVnqy73NUhbbo6Tq98X4dvA/HfySMOt?=
 =?us-ascii?Q?SZkaWvpgyu4/zYDvcnyOoZgaYLyODMcExatOek07PV48nJ5Q2BG6mRORTAI0?=
 =?us-ascii?Q?6/ZeQa4Vpcb4T8dFFHKnYQMnCtgY7Ffc0qUfcQRltxAvmLDCo49/2NVsoRJA?=
 =?us-ascii?Q?+El5a7LXuC0t9qnv8zMmUilBff8VrPKJ7WmwHHQ4vjL0hs4gNKDOQ+N1O2Pb?=
 =?us-ascii?Q?SGNp9wsbVqSgLTx/VqXEg8TUCNzStOvOP33Y6BJfEkgLEhz6/G5rv4VkUMm8?=
 =?us-ascii?Q?KPyCJnwf4pu/psHraG/lQ0taaTpynk9JmR95rGXATy1YJAEzmrXdMshsepWs?=
 =?us-ascii?Q?dZrnGvZYV8sHM3J8vNnJNcjQcUSS8u4nReKyfLYYOW1QsgAVz4QKxaue8yHT?=
 =?us-ascii?Q?ZrAvZti3htz8eB5PWRw2jA9tqrc4XX9DgwQkLQCtcCn2lag9dOGxdTD8B3L8?=
 =?us-ascii?Q?YWEWdogR8HBjm4H3fbPrKYgg1NDz7xS0gU3g1mBCleHzAI7TZclLuRpaAQHA?=
 =?us-ascii?Q?aKr7MEvpzO+d3gczJIb/Lo2A2kz/Y0fAfC5vWky5kQqvpwx3IxpGUFlsFMKs?=
 =?us-ascii?Q?NmJ8N8bEaXUL4G2L+t6LMMay2kOcb6Lw0TpLpBQQ0gXQgxG5JgFWqcAsG93D?=
 =?us-ascii?Q?w97EIXxNP7C5G/8da9llIDZVLiuyO/7h/DXo3IYO5ZkDFfx0DEMukcolZOzt?=
 =?us-ascii?Q?mx76yJ/EFXOgReKZbtWZ0+NSIjKXqkgFNXzEpRneqbxRglzXmMFTAJFIuHnQ?=
 =?us-ascii?Q?Dzxl1K7Uy0+r3ef/Q1EKdz3efvR6MFSae4XZm7wIENQ8fhUIgxtHQHMiEkWw?=
 =?us-ascii?Q?Og8mqYOTHcH8i5dcxrwH2PXdQuLZTeDGW3MbrrI2ZuZ2RqabAiMkc3+qFWW8?=
 =?us-ascii?Q?7UUj2zbhWI/C2uaVdr7FYnjUC27uFi+/4UrBrJCjBCbAMNyiQFRKNsYfLBh6?=
 =?us-ascii?Q?VkmMYLKYmxp9LgXnjtTskXKJE+EcHmT53wwmoRBO3B82w1SAkm1/6Ot4PFK2?=
 =?us-ascii?Q?+YYb8moRznVjc2xYMep6msGh?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6D4BDEF97FFE3E4783D4110614F8D4AE@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: SJ0PR10MB4688.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 511a52c8-5536-4856-a3b1-08d963138291
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Aug 2021 13:16:09.4651
 (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: AumACF6xFf0hPVTCsTYBxwNgNi2ECT0oQh0lnwompQBN1XEqr44cqLJhJkboo3mOEKlydecgeMDcjIcOH4eWRA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4447
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10080 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 spamscore=0 adultscore=0
 suspectscore=0 phishscore=0 mlxlogscore=999 malwarescore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2107140000
 definitions=main-2108190077
X-Proofpoint-GUID: hW-LWU2SYns77UQ1igbYHwcwyrBODXVz
X-Proofpoint-ORIG-GUID: hW-LWU2SYns77UQ1igbYHwcwyrBODXVz
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Aug 19, 2021, at 9:01 AM, Trond Myklebust <trondmy@hammerspace.com> wr=
ote:
>=20
> On Thu, 2021-08-19 at 08:29 -0400, Chuck Lever wrote:
>> With NFSv4.1+ on RDMA, backchannel recovery appears not to work.
>>=20
>> xprt_setup_xxx_bc() is invoked by the client's first CREATE_SESSION
>> operation, and it always marks the rpc_clnt's transport as
>> connected.
>>=20
>> On a subsequent CREATE_SESSION, if rpc_create() is called and
>> xpt_bc_xprt is populated, it might not be connected (for instance,
>> if a backchannel fault has occurred). Ensure that code path returns
>> a connected xprt also.
>>=20
>> Reported-by: Timo Rothenpieler <timo@rothenpieler.org>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>  net/sunrpc/clnt.c |    1 +
>>  1 file changed, 1 insertion(+)
>>=20
>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>> index 8b4de70e8ead..570480a649a3 100644
>> --- a/net/sunrpc/clnt.c
>> +++ b/net/sunrpc/clnt.c
>> @@ -535,6 +535,7 @@ struct rpc_clnt *rpc_create(struct
>> rpc_create_args *args)
>>                 xprt =3D args->bc_xprt->xpt_bc_xprt;
>>                 if (xprt) {
>>                         xprt_get(xprt);
>> +                       xprt_set_connected(xprt);
>>                         return rpc_create_xprt(args, xprt);
>>                 }
>>         }
>>=20
>>=20
>=20
> No. This is wrong. If the connection got disconnected, then the client
> needs to reconnect and build a new connection altogether. We can't just
> make pretend that the old connection still exists.

The patch description is not clear: the client has not disconnected.
The forward channel is functioning properly, and the server has set
SEQ4_STATUS_BACKCHANNEL_FAULT.

To recover, the client sends a DESTROY_SESSION / CREATE_SESSION pair
on the existing connection. On the server, setup_callback_client()
invokes rpc_create() again -- it's this step that is failing during
the second CREATE_SESSION on a connection because the old xprt
is returned but it's still marked disconnected.

An alternative would be to ensure that setup_callback_client()
always puts xpt_bc_xprt before it invokes rpc_create(). But it
looked to me like rpc_create() already has a bunch of logic to
deal with an existing xpt_bc_xprt.


--
Chuck Lever