Re: [PATCH v2 1/2] nfsd: add a Netlink dump of NFSv4 clients

[Prabhakar Pujeri <prabhakar.pujeri@dell.com>]

On Tue, 2026-09-01 at 14:02 +0000, Jeff Layton wrote:
> I'd call this nfsd_nl_find_client() as we usually use "get" in terms of
> reference counting.

Good point, renamed in v3.

> This means that table == NFSD4_NL_CLIENT_DONE means the same an
> "UNCONFIRMED". The caller shouldn't call it in that case I guess, but
> this is a lack of defensive programming.

Fixed: nfsd_nl_find_client() now selects the table via an explicit
switch and returns NULL for any unexpected value.

> In nfsd, we often use "put" with reference counting, so looking at this
> cold, this is confusingly named. Maybe change this to be named
> nfsd4_nl_client_marshal_address() ?

Renamed as suggested.

> This while loop is a bit too clever for our own good. cb->args[0] is
> effectively a state enum, so why are we incrementing through its
> values? It's looking at confirmed, then unconfirmed clients, but that's
> not evident here.

Agreed; the table ordering is now documented where the loop is, and the
comment states that args[0..2] track table, bucket, and offset. The
enum itself keeps its declaration order (confirmed, unconfirmed, done),
so the iteration order is visible at both sites.

(Alternatively, if you would prefer an explicit two-phase loop over an
array of tables rather than relying on enum order, say so and I'll
restructure it that way instead.)

Thanks for the review!
-- Prabhakar