Received-SPF: pass (google.com: domain of linux-nfs+bounces-230-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 30 Nov 2023 13:34:40 -0500
From: Chuck Lever <chuck.lever@oracle.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: Lorenzo Bianconi <lorenzo@kernel.org>, linux-nfs@vger.kernel.org,
        lorenzo.bianconi@redhat.com, neilb@suse.de, netdev@vger.kernel.org,
        kuba@kernel.org
Subject: Re: [PATCH v5 3/3] NFSD: convert write_ports to netlink command
Message-ID: <ZWjVwJ5kv9C2s9kg@tissot.1015granger.net>
References: <cover.1701277475.git.lorenzo@kernel.org>
 <67251eabfbbccb806991e6437ebcf1cf00166017.1701277475.git.lorenzo@kernel.org>
 <7b21c962c2a6c552c9807d6f382e1097da4ba748.camel@kernel.org>
 <ZWhcc8384pf11sAu@lore-desk>
 <c87becd9af15ad9447030c17dee9c481a8d25442.camel@kernel.org>
 <e85381f3d7575d8784f54c5a3abdb60be190c4af.camel@kernel.org>
 <ZWjI6ppENVu2FPIo@tissot.1015granger.net>
 <0eaec872db97e1b7366fc2b673484358312404b0.camel@kernel.org>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0eaec872db97e1b7366fc2b673484358312404b0.camel@kernel.org>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?iso-8859-1?Q?ngCOgccZgG/ciWBWdVgwzLD7L1LXSfrumaLR39Gdr7ZUVNisoCsJ5p9F8t?=
 =?iso-8859-1?Q?zj4fqYWU2OZVvPQDvX1EcnmBV4BlafAdXJP9ZD2kkGQHJ2laBX5wLfby4V?=
 =?iso-8859-1?Q?efuBoMUQdoG2ucQKZzbXOLxZlnGaMkjmElPCb1hBfaLxlH74KM7tXDZaTa?=
 =?iso-8859-1?Q?Os1oiryJkN1VuITogHhJpdC60aqyFAD49/yn8XCCWp+HcWWs3HKkot6mzN?=
 =?iso-8859-1?Q?d9Y3FOkP2fQkJHaCGcW3h+JDaQyV1PblHjsDjJlMuEyBM6Ug8MPeNoqXYo?=
 =?iso-8859-1?Q?Qmc4rNi/PVczOgAHhUCwmZ+J93NArmbrl7jp9utqer4cvY0+lkPKka4dTA?=
 =?iso-8859-1?Q?DndYlk5K/w1gNVlZs052yHTxlttjmF0e9Wd9qInEKKHXOPqpITxMMwh6A6?=
 =?iso-8859-1?Q?y+aJhiccQNlyTOxKvsWesfFysG9L7/VLK7bfrsqwbQ1TrFAAKgZLZN3gc5?=
 =?iso-8859-1?Q?OvHB739EHKkcVNsz9FA79AY3vmABs+M5pFPnR7pW4It7l+lHqPZxf9Vu0b?=
 =?iso-8859-1?Q?051gcerlL10jCB5641PuDb9IP7IX/ZFyyAkyIB4VDUK0iZrcjpg+2etfDT?=
 =?iso-8859-1?Q?4BdzTocIPkzHhPKUR1q8ZD9W6MxjUFBeigt6wl/9EGFnoy2mGrOf0ENC1A?=
 =?iso-8859-1?Q?I6mWKJ3qIwm7JkyfUUzWuihswMpZHinn0iAI6eP7hBN5kldpk4o6vQtYF/?=
 =?iso-8859-1?Q?6DjfHhV6usq4/g1Nu7si9Lt4+lvr+dv00tizC601yqlE19Iq/KUC/jel5Z?=
 =?iso-8859-1?Q?zmyfAdqBjxp4gBgMuNPbUbfWd47LUf3OcE0wyqWmYdCisVtqIFX75vOaaG?=
 =?iso-8859-1?Q?PgD3Py15857nuOKWXw28WBWnsgatum0yee3vOZWEFD9KzTYtoLNDirBr87?=
 =?iso-8859-1?Q?vmMN4GzjZQMwxwB7X90vw3RiN7newCnI8QZPNVW0EAmX3F9sTtbEZ+iQCY?=
 =?iso-8859-1?Q?HW8kv8eVeUck0K6YIS8MAFWQbuICzv3pqE0p+xvrNDJawUYaTVbyQ4Xwex?=
 =?iso-8859-1?Q?JI4j+q/myOYeoaQm0wVvlwgIkoM/adT08Qe+qVYXIIwt1RhOAg8tK10jBC?=
 =?iso-8859-1?Q?5JhFhW/sN/bRoOWkeUSIw301mTMH7Wg+BqV5pq3IQPd4/tb1cGD6PwKsEL?=
 =?iso-8859-1?Q?D4Qle1Mwfw2tkZe9hGuwxQAqQOqO8qdHj/5v5oc6lLLmfDU8uaz2WamtIR?=
 =?iso-8859-1?Q?W5U9GlStAjxmQwFZmTWsd0jSw7ERWH+x4yIk0zb/2ETB5tmzP+fQPFnsMi?=
 =?iso-8859-1?Q?p+JFVE28E79Dd9seQOfz20k6j3anNKyBqHdBsnSXGfdc6EYyzkXYazRFHf?=
 =?iso-8859-1?Q?/3LiGJhIPB/bWoeqsu1VuTxJ5V6LA2Ngn9nQROL7/sjrmBFxqCX7xR4B3C?=
 =?iso-8859-1?Q?295WosYEY07kBmn66UInyS1ClDPYjjWoT3b0dCNzYkTSa6/Wz+Hbmvw2hX?=
 =?iso-8859-1?Q?M6MFbpSN7stGJNCho1HP+NpbLCNngRdsEWmw1a2A4iQKQUCMX0POL+Pz4E?=
 =?iso-8859-1?Q?D26KZfW8AHnD8Oes+t56wIMf6344WA0gVbSydhAwzsyx/mXVaz08W9jMLM?=
 =?iso-8859-1?Q?0WlZoFbHobkecUCpdD4A2Yyimx60FVqXOoun0FObrgTbktoASLbY1eOKCy?=
 =?iso-8859-1?Q?g+rlh0t+411j0KMCEXU6JOQm/vhwgnQhAEdlIENNt3zsBOjTT6H6hZAA?=
 =?iso-8859-1?Q?=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	pZCdgQZgPaqalTbyGX0vWyBEhCD+LjvbxMmVGhp7bVHNjmpUlDVMq0xAiwpebGNHkEK3tPhHjfVS81qpYhWcklrRDLLb0FmU+MEafyo25E9b1KQbFKQzQJkKu8zJLZ4iQKNDq4sU6wTtUAHGRxtS8kMarinwpqwGFV+dQDkIuvvyVk7mYF2JKqUkuvIJztZETyMVPL9FVIWxXvyT9KkSjlEvr5Lk9VSoYz8oCjRkaC7OsHAaOi44DRDkbTyvMhSNA9Q1zk0xRiSMrwLHu+kJyHF4oMcllkoVWDyozsQ+2/id04a+kucpUjnlq/Dqn6NSyKJPZaz3cBPmplps48s2QKpsf6C5v4daiVJkn5HxLUk7AgkPAJ8C5E5V8i+zp3vGiiXBsJGvZmYTkDX5Own2vXhjXI5y79VB4dkJELxXhbhmzbPr2vaD7yyE0wHu9zzBnEbKldR03Kynt0kxgyoQUxVQn36JCwbjvHLUwaeLtJa4Bbpd+XxM1RaajJEwhu9Ahuvi2PCexEz3S7VcH4FwGmAydugKIrJ71NYreEfdIX0RHxryyD50GTf5743NtK3wo2XVoKZfE9fFH2iyGZ68P+yonYvRqB8AcfXin2URrV8JvCON5XwCTJ3sC6B346aHiwkIeZmHqlApCz0X78D1jl3Ix9bmvyEUApN2caPO3ewIThE61EP0ATgtz2KDA74IjYT6bX1TUwLDPJLfKkRD18p+9+9l6rEMtWp6m0zNsT2jHX7B8CwXBDvhwNCvc+f1qg0FZIJHhJcjAw23Kz+irxrBnjNyB57hNHdPki7BZXU9B88rj+1qp4RalzFaUf7UtH/4+2ZGR01yQj3y7bxF5A==
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e2e2b1ec-955d-44a6-5c98-08dbf1d30587
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2023 18:34:43.7135
 (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: VWjJjAoKdKoAXrqdjKNVwwshT263UUHuekNTBEpiB5EkZ4oNf9mjo1V9LSSwznpJJA8+yWTcTeM2/PO6Y9yqug==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB6054
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-11-30_18,2023-11-30_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 phishscore=0 mlxscore=0
 malwarescore=0 adultscore=0 suspectscore=0 spamscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311060000
 definitions=main-2311300136
X-Proofpoint-GUID: PHeKgNg0K2GC5lVBEW7gyYDXg1J8Ka1Z
X-Proofpoint-ORIG-GUID: PHeKgNg0K2GC5lVBEW7gyYDXg1J8Ka1Z

On Thu, Nov 30, 2023 at 01:01:14PM -0500, Jeff Layton wrote:
> On Thu, 2023-11-30 at 12:39 -0500, Chuck Lever wrote:
> > On Thu, Nov 30, 2023 at 11:55:29AM -0500, Jeff Layton wrote:
> > > On Thu, 2023-11-30 at 11:22 -0500, Jeff Layton wrote:
> > > > On Thu, 2023-11-30 at 10:57 +0100, Lorenzo Bianconi wrote:
> > > > > > > 
> > > > > > > +/* ============== NFSD_CMD_LISTENER_START ============== */
> > > > > > > +/* NFSD_CMD_LISTENER_START - do */
> > > > > > > +struct nfsd_listener_start_req {
> > > > > > > +	struct {
> > > > > > > +		__u32 transport_name_len;
> > > > > > > +		__u32 port:1;
> > > > > > > +	} _present;
> > > > > > > +
> > > > > > > +	char *transport_name;
> > > > > > > +	__u32 port;
> > > > > > > +};
> > > > > > 
> > > > > > How do you deconfigure a listener with this interface? i.e. suppose I
> > > > > > want to stop nfsd from listening on a particular port? I think this too
> > > > > > is a place where a declarative interface would be better:
> > > > > 
> > > > > Is it possible with current APIs? as for 2/3 so far I have just added netlink
> > > > > counter for current implementation but I am fine to change the logic here to
> > > > > better APIs.
> > > > > 
> > > > > > 
> > > > 
> > > > No, I don't think you can do this with the current API at all. I
> > > > consider it a major deficiency. I don't think we want to repeat that
> > > > mistake in the new interface.
> > > > 
> > > > > > Have userland send down a list of the ports that we should currently be
> > > > > > listening on, and let the kernel do the work to match the request. Again
> > > > > > too, an empty list could mean "close everything".
> > > 
> > > Another thought: should this interface also report and allow you to
> > > specify the address to listen on?
> > > 
> > > When the write_ports interface was first created, it lacked a field for
> > > the address to listen on.?Later we added a way to just hand off a socket
> > > to the kernel to pass that info.
> > > 
> > > I think it's possible today to send down a socket that only listens on a
> > > particular address, and you have no real way to tell that with the
> > > current "ports" file.
> > 
> > All agreed, but listening on a particular address isn't something we
> > need today. (Or is it?)
> 
> It is for TCP/UDP -- see the -H option to rpc.nfsd.
> 
> > Does the socket-passing thing work for non socket-based transports
> > like RDMA? I would think that mechanism is legacy.
> 
> To the contrary actually. rpc.nfsd almost always does this today unless
> you're configuring rdma. But, that was just a convenient way to do this
> with a fd based interface.
> 
> I think with netlink, we just want to send down a list of
> (transport_name, sockaddr) pairs and let the kernel open and close the
> appropriate sockets. For RDMA, we can just fill out the sa_port field
> and ignore the rest.

Agreed, it would be cleaner to handle all transport classes the same
way and eventually deprecate the fd-passing mechanism.


> > > Should we instead plumb a complete struct sockaddr_storage (or some
> > > other suitable address structure) into this interface?
> > 
> > How difficult would it be to add this later, when we actually have a
> > specific use case?
> 
> I think we have one now. rpc.nfsd passes down sockets to the kernel for
> TCP and UDP listeners, so we need a way to send down a complete sockaddr
> for the listeners.

If we have a usage scenario, then it makes sense to try it now.


-- 
Chuck Lever