Skip to contentSkip to conversation

add rpc_status netlink support for NFSD

PATCH In review Lorenzo Bianconi 6 participants 28 messages Sep 11, 2023 – Dec 12, 2023

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

v8 cover add rpc_status netlink support for NFSD

Introduce rpc_status netlink support for NFSD in order to dump pending RPC requests debugging information from userspace.
The new code can be tested with the user-space tool reported below:
https://github.com/LorenzoBianconi/nfsd-rpc-netlink-monitor/tree/main

Changes since v7:
- introduce nfsd_server.yaml for netlink messages

Changes since v6:
- report info to user-space through netlink and get rid of the related entry in the procfs

Changes since v5:
- add missing delimiters for nfs4 compound ops
- add a header line for rpc_status handler
- do not dump rq_prog
- do not dump rq_flags in hex

Changes since v4:
- rely on acquire/release APIs and get rid of atomic operation
- fix kdoc for nfsd_rpc_status_open
- get rid of ',' as field delimiter in nfsd_rpc_status hanlder
- move nfsd_rpc_status before nfsd_v4 enum entries
- fix compilantion error if nfsdv4 is not enabled

Changes since v3:
- introduce rq_status_counter in order to detect if the RPC request is pending and RPC info are stable
- rely on __svc_print_addr to dump IP info

Changes since v2:
- minor changes in nfsd_rpc_status_show output

Changes since v1:
- rework nfsd_rpc_status_show output

Changes since RFCv1:
- riduce time holding nfsd_mutex bumping svc_serv refcoung in nfsd_rpc_status_open()
- dump rqstp->rq_stime
- add missing kdoc for nfsd_rpc_status_open()

Lorenzo Bianconi (3):
Documentation: netlink: add a YAML spec for nfsd_server
NFSD: introduce netlink rpc_status stubs
NFSD: add rpc_status netlink support

 Documentation/netlink/specs/nfsd_server.yaml |  97 +++++++++
 fs/nfsd/Makefile                             |   3 +-
 fs/nfsd/nfs_netlink_gen.c                    |  32 +++
 fs/nfsd/nfs_netlink_gen.h                    |  22 ++
 fs/nfsd/nfsctl.c                             | 204 +++++++++++++++++++
 fs/nfsd/nfsd.h                               |  16 ++
 fs/nfsd/nfssvc.c                             |  15 ++
 fs/nfsd/state.h                              |   2 -
 include/linux/sunrpc/svc.h                   |   1 +
 include/uapi/linux/nfsd_server.h             |  49 +++++
 10 files changed, 438 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/netlink/specs/nfsd_server.yaml
 create mode 100644 fs/nfsd/nfs_netlink_gen.c
 create mode 100644 fs/nfsd/nfs_netlink_gen.h
 create mode 100644 include/uapi/linux/nfsd_server.h
Signature · 1 line
2.41.0
#1 lore raw

PATCH v8 1/3 Documentation: netlink: add a YAML spec for nfsd_server

Introduce nfsd_server.yaml specs to generate uAPI and netlink code for nfsd server.
Add rpc-status specs to define message reported by the nfsd server dumping the pending RPC requests.

---
 Documentation/netlink/specs/nfsd_server.yaml | 97 ++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/netlink/specs/nfsd_server.yaml
Documentation/netlink/specs/nfsd_server.yaml new file +97 -0
new file mode 100644
index 000000000000..e681b493847b
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: nfsd_server
+
+doc:
+  nfsd server configuration over generic netlink.
+
+attribute-sets:
+  -
+    name: rpc-status-comp-op-attr
+    enum-name: nfsd-rpc-status-comp-attr
+    name-prefix: nfsd-attr-rpc-status-comp-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: op
+        type: u32
+  -
+    name: rpc-status-attr
+    enum-name: nfsd-rpc-status-attr
+    name-prefix: nfsd-attr-rpc-status-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: xid
+        type: u32
+        byte-order: big-endian
+      -
+        name: flags
+        type: u32
+      -
+        name: prog
+        type: u32
+      -
+        name: version
+        type: u8
+      -
+        name: proc
+        type: u32
+      -
+        name: service_time
+        type: s64
+      -
+        name: pad
+        type: pad
+      -
+        name: saddr4
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+      -
+        name: daddr4
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+      -
+        name: saddr6
+        type: binary
+        display-hint: ipv6
+      -
+        name: daddr6
+        type: binary
+        display-hint: ipv6
+      -
+        name: sport
+        type: u16
+        byte-order: big-endian
+      -
+        name: dport
+        type: u16
+        byte-order: big-endian
+      -
+        name: compond-op
+        type: array-nest
+        nested-attributes: rpc-status-comp-op-attr
+
+operations:
+  enum-name: nfsd-commands
+  name-prefix: nfsd-cmd-
+  list:
+    -
+      name: unspec
+      doc: unused
+      value: 0
+    -
+      name: rpc-status-get
+      doc: dump pending nfsd rpc
+      attribute-set: rpc-status-attr
+      dump:
+        pre: nfsd-server-nl-rpc-status-get-start
+        post: nfsd-server-nl-rpc-status-get-done
Signature · 1 line
2.41.0
maintainer Acked-by on v8 1/3
#2 lore raw

Re: [PATCH v8 1/3] Documentation: netlink: add a YAML spec for nfsd_server

Quoted 89 lines · Lorenzo Bianconi

Introduce nfsd_server.yaml specs to generate uAPI and netlink code for nfsd server.
Add rpc-status specs to define message reported by the nfsd server dumping the pending RPC requests.

---
 Documentation/netlink/specs/nfsd_server.yaml | 97 ++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/netlink/specs/nfsd_server.yaml
Documentation/netlink/specs/nfsd_server.yaml new file +71 -0
new file mode 100644
index 000000000000..e681b493847b
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: nfsd_server
+
+doc:
+  nfsd server configuration over generic netlink.
+
+attribute-sets:
+  -
+    name: rpc-status-comp-op-attr
+    enum-name: nfsd-rpc-status-comp-attr
+    name-prefix: nfsd-attr-rpc-status-comp-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: op
+        type: u32
+  -
+    name: rpc-status-attr
+    enum-name: nfsd-rpc-status-attr
+    name-prefix: nfsd-attr-rpc-status-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: xid
+        type: u32
+        byte-order: big-endian
+      -
+        name: flags
+        type: u32
+      -
+        name: prog
+        type: u32
+      -
+        name: version
+        type: u8
+      -
+        name: proc
+        type: u32
+      -
+        name: service_time
+        type: s64
+      -
+        name: pad
+        type: pad
+      -
+        name: saddr4
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+      -
+        name: daddr4
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+      -
+        name: saddr6
+        type: binary
+        display-hint: ipv6
+      -
+        name: daddr6
+        type: binary
+        display-hint: ipv6
+      -
+        name: sport

Documentation/netlink/specs/nfsd_server.yaml line 81

Quoted 10 lines
Documentation/netlink/specs/nfsd_server.yaml new file +10 -0
+        type: u16
+        byte-order: big-endian
+      -
+        name: dport
+        type: u16
+        byte-order: big-endian
+      -
+        name: compond-op
+        type: array-nest
+        nested-attributes: rpc-status-comp-op-attr

Is there a way to do unions in netlink? We're sending down the list of COMPOUND proc operations here for NFSv4, but it might be interesting to send down other info for other program/version/procedures in the future.

Quoted 6 lines
Documentation/netlink/specs/nfsd_server.yaml new file +6 -0
+
+operations:
+  enum-name: nfsd-commands
+  name-prefix: nfsd-cmd-
+  list:
+    -

Documentation/netlink/specs/nfsd_server.yaml line 97

Quoted 10 lines
Documentation/netlink/specs/nfsd_server.yaml new file +10 -0
+      name: unspec
+      doc: unused
+      value: 0
+    -
+      name: rpc-status-get
+      doc: dump pending nfsd rpc
+      attribute-set: rpc-status-attr
+      dump:
+        pre: nfsd-server-nl-rpc-status-get-start
+        post: nfsd-server-nl-rpc-status-get-done

Looks like a good first stab though.

#3 lore raw

Re: [PATCH v8 1/3] Documentation: netlink: add a YAML spec for nfsd_server

In reply to Lorenzo Bianconi
Quoted 11 lines · Lorenzo Bianconi

Introduce nfsd_server.yaml specs to generate uAPI and netlink code for nfsd server.
Add rpc-status specs to define message reported by the nfsd server dumping the pending RPC requests.

---
 Documentation/netlink/specs/nfsd_server.yaml | 97 ++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/netlink/specs/nfsd_server.yaml

I've had a look... the series is simple and short. Thanks!

My only quibbles right now are cosmetic and naming-related, all of which can be addressed when I apply these. So I'm going to wait for other review comments to see if we need another version or whether I can apply v8 with by-hand clean-ups.

Comments below are what I might change when applying this one. This is not (yet) a request for a new version.

Documentation/netlink/specs/nfsd_server.yaml line 3

Quoted 9 lines
Documentation/netlink/specs/nfsd_server.yaml new file +3 -0
new file mode 100644
index 000000000000..e681b493847b
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: nfsd_server

IMHO "nfsd_server" is redundant. "nfsd" should work.

Quoted 4 lines
Documentation/netlink/specs/nfsd_server.yaml new file +4 -0
+
+doc:
+  nfsd server configuration over generic netlink.
+

Documentation/netlink/specs/nfsd_server.yaml line 17

Quoted 10 lines
Documentation/netlink/specs/nfsd_server.yaml new file +10 -0
+attribute-sets:
+  -
+    name: rpc-status-comp-op-attr
+    enum-name: nfsd-rpc-status-comp-attr
+    name-prefix: nfsd-attr-rpc-status-comp-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0

I don't recall whether a zero-value definition is explicitly necessary. Maybe "value-start: 1" would work rather than these three lines? Why is zero a special attribute value?

Documentation/netlink/specs/nfsd_server.yaml line 24

Quoted 7 lines
Documentation/netlink/specs/nfsd_server.yaml new file +7 -0
+      -
+        name: op
+        type: u32
+  -
+    name: rpc-status-attr
+    enum-name: nfsd-rpc-status-attr
+    name-prefix: nfsd-attr-rpc-status-

Specifying all three of these name settings seems a bit cluttered.

Quoted 45 lines
Documentation/netlink/specs/nfsd_server.yaml new file +45 -0
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: xid
+        type: u32
+        byte-order: big-endian
+      -
+        name: flags
+        type: u32
+      -
+        name: prog
+        type: u32
+      -
+        name: version
+        type: u8
+      -
+        name: proc
+        type: u32
+      -
+        name: service_time
+        type: s64
+      -
+        name: pad
+        type: pad
+      -
+        name: saddr4
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+      -
+        name: daddr4
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+      -
+        name: saddr6
+        type: binary
+        display-hint: ipv6
+      -
+        name: daddr6
+        type: binary
+        display-hint: ipv6

Documentation/netlink/specs/nfsd_server.yaml line 79

Quoted 10 lines
Documentation/netlink/specs/nfsd_server.yaml new file +10 -0
+      -
+        name: sport
+        type: u16
+        byte-order: big-endian
+      -
+        name: dport
+        type: u16
+        byte-order: big-endian
+      -
+        name: compond-op

s/compond-op/compound-op

Documentation/netlink/specs/nfsd_server.yaml line 81

Quoted 2 lines
Documentation/netlink/specs/nfsd_server.yaml new file +2 -0
+        type: array-nest
+        nested-attributes: rpc-status-comp-op-attr

So, this is supposed to be a counted array of op numbers? Is there an existing type that could be used for this instead?

Quoted 16 lines
Documentation/netlink/specs/nfsd_server.yaml new file +16 -0
+
+operations:
+  enum-name: nfsd-commands
+  name-prefix: nfsd-cmd-
+  list:
+    -
+      name: unspec
+      doc: unused
+      value: 0
+    -
+      name: rpc-status-get
+      doc: dump pending nfsd rpc
+      attribute-set: rpc-status-attr
+      dump:
+        pre: nfsd-server-nl-rpc-status-get-start
+        post: nfsd-server-nl-rpc-status-get-done
Signature · 1 line
Chuck Lever
#4 lore raw

Re: [PATCH v8 1/3] Documentation: netlink: add a YAML spec for nfsd_server

Quoted 15 lines · Lorenzo Bianconi
Quoted 11 lines

Introduce nfsd_server.yaml specs to generate uAPI and netlink code for nfsd server.
Add rpc-status specs to define message reported by the nfsd server dumping the pending RPC requests.

---
 Documentation/netlink/specs/nfsd_server.yaml | 97 ++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/netlink/specs/nfsd_server.yaml

I've had a look... the series is simple and short. Thanks!

Quoted 7 lines

My only quibbles right now are cosmetic and naming-related, all of which can be addressed when I apply these. So I'm going to wait for other review comments to see if we need another version or whether I can apply v8 with by-hand clean-ups.

Comments below are what I might change when applying this one. This is not (yet) a request for a new version.

Hi Chuck,

thx for the review. Please let me know if I need to post a v9.

Quoted 13 lines

Documentation/netlink/specs/nfsd_server.yaml line 3

Quoted 11 lines
Documentation/netlink/specs/nfsd_server.yaml new file +3 -0
new file mode 100644
index 000000000000..e681b493847b
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: nfsd_server

IMHO "nfsd_server" is redundant. "nfsd" should work.

ack, fine

Quoted 12 lines
Quoted 12 lines
Documentation/netlink/specs/nfsd_server.yaml new file +10 -0
+
+doc:
+  nfsd server configuration over generic netlink.
+
+attribute-sets:
+  -
+    name: rpc-status-comp-op-attr
+    enum-name: nfsd-rpc-status-comp-attr
+    name-prefix: nfsd-attr-rpc-status-comp-
+    attributes:
Quoted 8 lines

Documentation/netlink/specs/nfsd_server.yaml line 17

Quoted 4 lines
Documentation/netlink/specs/nfsd_server.yaml new file +4 -0
+      -
+        name: unspec
+        type: unused
+        value: 0

I don't recall whether a zero-value definition is explicitly necessary. Maybe "value-start: 1" would work rather than these three lines? Why is zero a special attribute value?

I do not think it is mandatory, I added them just to be aligned with other netlink definitions, but we do not use them.

Quoted 10 lines

Documentation/netlink/specs/nfsd_server.yaml line 24

Quoted 9 lines
Documentation/netlink/specs/nfsd_server.yaml new file +7 -0
+      -
+        name: op
+        type: u32
+  -
+    name: rpc-status-attr
+    enum-name: nfsd-rpc-status-attr
+    name-prefix: nfsd-attr-rpc-status-
Quoted 2 lines

Specifying all three of these name settings seems a bit cluttered.

ack, I would say we can get rid of enum-name, agree?

Quoted 58 lines
Quoted 47 lines
Documentation/netlink/specs/nfsd_server.yaml new file +45 -0
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: xid
+        type: u32
+        byte-order: big-endian
+      -
+        name: flags
+        type: u32
+      -
+        name: prog
+        type: u32
+      -
+        name: version
+        type: u8
+      -
+        name: proc
+        type: u32
+      -
+        name: service_time
+        type: s64
+      -
+        name: pad
+        type: pad
+      -
+        name: saddr4
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+      -
+        name: daddr4
+        type: u32
+        byte-order: big-endian
+        display-hint: ipv4
+      -
+        name: saddr6
+        type: binary
+        display-hint: ipv6
+      -
+        name: daddr6
+        type: binary
+        display-hint: ipv6

Documentation/netlink/specs/nfsd_server.yaml line 79

Quoted 10 lines
Documentation/netlink/specs/nfsd_server.yaml new file +10 -0
+      -
+        name: sport
+        type: u16
+        byte-order: big-endian
+      -
+        name: dport
+        type: u16
+        byte-order: big-endian
+      -
+        name: compond-op
Quoted 1 line

s/compond-op/compound-op

ack

Quoted 6 lines

Documentation/netlink/specs/nfsd_server.yaml line 81

Quoted 3 lines
Documentation/netlink/specs/nfsd_server.yaml new file +2 -0
+        type: array-nest
+        nested-attributes: rpc-status-comp-op-attr

So, this is supposed to be a counted array of op numbers? Is there an existing type that could be used for this instead?

I think the attribute-sets available types are defined here:

https://github.com/torvalds/linux/blob/master/Documentation/netlink/genetlink-c.yaml#L151

Regards,
Lorenzo

Quoted 22 lines
Quoted 18 lines
Documentation/netlink/specs/nfsd_server.yaml new file +16 -0
+
+operations:
+  enum-name: nfsd-commands
+  name-prefix: nfsd-cmd-
+  list:
+    -
+      name: unspec
+      doc: unused
+      value: 0
+    -
+      name: rpc-status-get
+      doc: dump pending nfsd rpc
+      attribute-set: rpc-status-attr
+      dump:
+        pre: nfsd-server-nl-rpc-status-get-start
+        post: nfsd-server-nl-rpc-status-get-done
Signature · 1 line
Chuck Lever
maintainer
#5 lore raw

Re: [PATCH v8 1/3] Documentation: netlink: add a YAML spec for nfsd_server

In reply to Lorenzo Bianconi
Quoted 4 lines · On Mon, 11 Sep 2023 14:49:44 +0200 Lorenzo Bianconi wrote:

Introduce nfsd_server.yaml specs to generate uAPI and netlink code for nfsd server.
Add rpc-status specs to define message reported by the nfsd server dumping the pending RPC requests.

Sorry for the delay, some minor "take it or leave it" nits below.

Documentation/netlink/specs/nfsd_server.yaml line 17

Quoted 13 lines
Documentation/netlink/specs/nfsd_server.yaml new file +13 -0
+doc:
+  nfsd server configuration over generic netlink.
+
+attribute-sets:
+  -
+    name: rpc-status-comp-op-attr
+    enum-name: nfsd-rpc-status-comp-attr
+    name-prefix: nfsd-attr-rpc-status-comp-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0

the unused attrs can usually be skipped, the specs now start with value of 1 by default. Same for the unused command.

Documentation/netlink/specs/nfsd_server.yaml line 80

Quoted 7 lines
Documentation/netlink/specs/nfsd_server.yaml new file +7 -0
+      -
+        name: dport
+        type: u16
+        byte-order: big-endian
+      -
+        name: compond-op
+        type: array-nest

Avoid array-nests if you can, they are legacy (does this spec pass JSON schema validation?).

There's only one attribute in the nest, can you use

	- 
	 name: op
	 type: u32
	 multi-attr: true

?

Documentation/netlink/specs/nfsd_server.yaml line 97

Quoted 9 lines
Documentation/netlink/specs/nfsd_server.yaml new file +8 -0
+        nested-attributes: rpc-status-comp-op-attr
 
+    -
+      name: rpc-status-get
+      doc: dump pending nfsd rpc
+      attribute-set: rpc-status-attr
+      dump:
+        pre: nfsd-server-nl-rpc-status-get-start
+        post: nfsd-server-nl-rpc-status-get-done

No attributes listed? User space C codegen will need those to make sense of the commands.

#6 lore raw

Re: [PATCH v8 1/3] Documentation: netlink: add a YAML spec for nfsd_server

Hi Jakub-

Quoted 38 lines · Jakub Kicinski
Quoted 4 lines · On Mon, 11 Sep 2023 14:49:44 +0200 Lorenzo Bianconi wrote:

Introduce nfsd_server.yaml specs to generate uAPI and netlink code for nfsd server.
Add rpc-status specs to define message reported by the nfsd server dumping the pending RPC requests.

Sorry for the delay, some minor "take it or leave it" nits below.

Documentation/netlink/specs/nfsd_server.yaml line 17

Quoted 13 lines
Documentation/netlink/specs/nfsd_server.yaml new file +13 -0
+doc:
+  nfsd server configuration over generic netlink.
+
+attribute-sets:
+  -
+    name: rpc-status-comp-op-attr
+    enum-name: nfsd-rpc-status-comp-attr
+    name-prefix: nfsd-attr-rpc-status-comp-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0

the unused attrs can usually be skipped, the specs now start with value of 1 by default. Same for the unused command.

Documentation/netlink/specs/nfsd_server.yaml line 80

Quoted 7 lines
Documentation/netlink/specs/nfsd_server.yaml new file +7 -0
+      -
+        name: dport
+        type: u16
+        byte-order: big-endian
+      -
+        name: compond-op
+        type: array-nest

Avoid array-nests if you can, they are legacy (does this spec pass JSON schema validation?).

Quoted 8 lines

There's only one attribute in the nest, can you use

-
name: op
type: u32
multi-attr: true

?

I've made similar modifications to Lorenzo's original contribution. The current updated version of this spec is here:

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=nfsd-next&id=55f55c77f624066895470306898190f090e00cda

Quoted 10 lines

Documentation/netlink/specs/nfsd_server.yaml line 97

Quoted 9 lines
Documentation/netlink/specs/nfsd_server.yaml new file +8 -0
+        nested-attributes: rpc-status-comp-op-attr
 
+    -
+      name: rpc-status-get
+      doc: dump pending nfsd rpc
+      attribute-set: rpc-status-attr
+      dump:
+        pre: nfsd-server-nl-rpc-status-get-start
+        post: nfsd-server-nl-rpc-status-get-done
Quoted 2 lines

No attributes listed? User space C codegen will need those to make sense of the commands.

--
Chuck Lever

maintainer
#7 lore raw

Re: [PATCH v8 1/3] Documentation: netlink: add a YAML spec for nfsd_server

Quoted 5 lines · On Tue, 3 Oct 2023 18:40:29 +0000 Chuck Lever III wrote:

I've made similar modifications to Lorenzo's original contribution. The current updated version of this spec is here:

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=nfsd-next&id=55f55c77f624066895470306898190f090e00cda

Great! I noticed too late :)
Just the attr listing is missing in the spec, then.

#8 lore raw

Re: [PATCH v8 1/3] Documentation: netlink: add a YAML spec for nfsd_server

Quoted 13 lines · Jakub Kicinski
Quoted 5 lines · On Tue, 3 Oct 2023 18:40:29 +0000 Chuck Lever III wrote:

I've made similar modifications to Lorenzo's original contribution. The current updated version of this spec is here:

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=nfsd-next&id=55f55c77f624066895470306898190f090e00cda

Great! I noticed too late :)
Just the attr listing is missing in the spec, then.

To ensure that I understand you correctly:

Documentation/netlink/specs/nfsd.yaml +16 -0
index 403d3e3a04f3..f6a9f3da6291 100644
@@ -72,3 +72,19 @@ operations:
       dump:
         pre: nfsd-nl-rpc-status-get-start
         post: nfsd-nl-rpc-status-get-done
+        reply:
+          attributes:
+            - xid
+            - flags
+            - prog
+            - version
+            - proc
+            - service_time
+            - pad
+            - saddr4
+            - daddr4
+            - saddr6
+            - daddr6
+            - sport
+            - dport
+            - compound-ops

Yes?

--
Chuck Lever

maintainer
#9 lore raw

Re: [PATCH v8 1/3] Documentation: netlink: add a YAML spec for nfsd_server

Quoted 16 lines · On Tue, 3 Oct 2023 23:00:09 +0000 Chuck Lever III wrote:

To ensure that I understand you correctly:

Documentation/netlink/specs/nfsd.yaml +6 -0
index 403d3e3a04f3..f6a9f3da6291 100644
@@ -72,3 +72,19 @@ operations:
       dump:
         pre: nfsd-nl-rpc-status-get-start
         post: nfsd-nl-rpc-status-get-done
+        reply:
+          attributes:
+            - xid
+            - flags
+            - prog
+            - version

Documentation/netlink/specs/nfsd.yaml operations: · line 90

Quoted 10 lines
Documentation/netlink/specs/nfsd.yaml +10 -0
+            - proc
+            - service_time
+            - pad
+            - saddr4
+            - daddr4
+            - saddr6
+            - daddr6
+            - sport
+            - dport
+            - compound-ops

Yup! (the pad can be skipped since it's not rendered, anyway).

#10 lore raw

PATCH v8 2/3 NFSD: introduce netlink rpc_status stubs

In reply to Lorenzo Bianconi

Generate empty netlink stubs and uAPI through nfsd_server.yaml specs:

$./tools/net/ynl/ynl-gen-c.py --mode uapi \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o include/uapi/linux/nfsd_server.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o fs/nfsd/nfs_netlink_gen.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--source -o fs/nfsd/nfs_netlink_gen.c

---
 fs/nfsd/Makefile                 |  3 +-
 fs/nfsd/nfs_netlink_gen.c        | 32 +++++++++++++++++++++
 fs/nfsd/nfs_netlink_gen.h        | 22 ++++++++++++++
 fs/nfsd/nfsctl.c                 | 16 +++++++++++
 include/uapi/linux/nfsd_server.h | 49 ++++++++++++++++++++++++++++++++
 5 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 fs/nfsd/nfs_netlink_gen.c
 create mode 100644 fs/nfsd/nfs_netlink_gen.h
 create mode 100644 include/uapi/linux/nfsd_server.h
fs/nfsd/Makefile +2 -1
index 6fffc8f03f74..6ae1d5450bf6 100644
@@ -12,7 +12,8 @@ nfsd-y			+= trace.o
 
 nfsd-y 			+= nfssvc.o nfsctl.o nfsfh.o vfs.o \
 			   export.o auth.o lockd.o nfscache.o \
-			   stats.o filecache.o nfs3proc.o nfs3xdr.o
+			   stats.o filecache.o nfs3proc.o nfs3xdr.o \
+			   nfs_netlink_gen.o
 nfsd-$(CONFIG_NFSD_V2) += nfsproc.o nfsxdr.o
 nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
 nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
fs/nfsd/nfs_netlink_gen.c new file +32 -0
new file mode 100644
index 000000000000..4d71b80bf4a7
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "nfs_netlink_gen.h"
+
+#include <uapi/linux/nfsd_server.h>
+
+/* Ops table for nfsd_server */
+static const struct genl_split_ops nfsd_server_nl_ops[] = {
+	{
+		.cmd	= NFSD_CMD_RPC_STATUS_GET,
+		.start	= nfsd_server_nl_rpc_status_get_start,
+		.dumpit	= nfsd_server_nl_rpc_status_get_dumpit,
+		.done	= nfsd_server_nl_rpc_status_get_done,
+		.flags	= GENL_CMD_CAP_DUMP,
+	},
+};
+
+struct genl_family nfsd_server_nl_family __ro_after_init = {
+	.name		= NFSD_SERVER_FAMILY_NAME,
+	.version	= NFSD_SERVER_FAMILY_VERSION,
+	.netnsok	= true,
+	.parallel_ops	= true,
+	.module		= THIS_MODULE,
+	.split_ops	= nfsd_server_nl_ops,
+	.n_split_ops	= ARRAY_SIZE(nfsd_server_nl_ops),
+};
fs/nfsd/nfs_netlink_gen.h new file +22 -0
new file mode 100644
index 000000000000..f66b29e528c1
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_NFSD_SERVER_GEN_H
+#define _LINUX_NFSD_SERVER_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/nfsd_server.h>
+
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb);
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb);
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb);
+
+extern struct genl_family nfsd_server_nl_family;
+
+#endif /* _LINUX_NFSD_SERVER_GEN_H */
fs/nfsd/nfsctl.c +16 -0
index 33f80d289d63..1be66088849c 100644
@@ -1495,6 +1495,22 @@ static int create_proc_exports_entry(void)
 
 unsigned int nfsd_net_id;
 
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb)
+{
+	return 0;
+}
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
include/uapi/linux/nfsd_server.h new file +49 -0

49 lines not on this page · raw message

Signature · 1 line
2.41.0
maintainer Acked-by on v8 2/3
#11 lore raw

Re: [PATCH v8 2/3] NFSD: introduce netlink rpc_status stubs

Quoted 177 lines · Lorenzo Bianconi

Generate empty netlink stubs and uAPI through nfsd_server.yaml specs:

$./tools/net/ynl/ynl-gen-c.py --mode uapi \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o include/uapi/linux/nfsd_server.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o fs/nfsd/nfs_netlink_gen.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--source -o fs/nfsd/nfs_netlink_gen.c

---
 fs/nfsd/Makefile                 |  3 +-
 fs/nfsd/nfs_netlink_gen.c        | 32 +++++++++++++++++++++
 fs/nfsd/nfs_netlink_gen.h        | 22 ++++++++++++++
 fs/nfsd/nfsctl.c                 | 16 +++++++++++
 include/uapi/linux/nfsd_server.h | 49 ++++++++++++++++++++++++++++++++
 5 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 fs/nfsd/nfs_netlink_gen.c
 create mode 100644 fs/nfsd/nfs_netlink_gen.h
 create mode 100644 include/uapi/linux/nfsd_server.h
fs/nfsd/Makefile +2 -1
index 6fffc8f03f74..6ae1d5450bf6 100644
@@ -12,7 +12,8 @@ nfsd-y			+= trace.o
 
 nfsd-y 			+= nfssvc.o nfsctl.o nfsfh.o vfs.o \
 			   export.o auth.o lockd.o nfscache.o \
-			   stats.o filecache.o nfs3proc.o nfs3xdr.o
+			   stats.o filecache.o nfs3proc.o nfs3xdr.o \
+			   nfs_netlink_gen.o
 nfsd-$(CONFIG_NFSD_V2) += nfsproc.o nfsxdr.o
 nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
 nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
fs/nfsd/nfs_netlink_gen.c new file +32 -0
new file mode 100644
index 000000000000..4d71b80bf4a7
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "nfs_netlink_gen.h"
+
+#include <uapi/linux/nfsd_server.h>
+
+/* Ops table for nfsd_server */
+static const struct genl_split_ops nfsd_server_nl_ops[] = {
+	{
+		.cmd	= NFSD_CMD_RPC_STATUS_GET,
+		.start	= nfsd_server_nl_rpc_status_get_start,
+		.dumpit	= nfsd_server_nl_rpc_status_get_dumpit,
+		.done	= nfsd_server_nl_rpc_status_get_done,
+		.flags	= GENL_CMD_CAP_DUMP,
+	},
+};
+
+struct genl_family nfsd_server_nl_family __ro_after_init = {
+	.name		= NFSD_SERVER_FAMILY_NAME,
+	.version	= NFSD_SERVER_FAMILY_VERSION,
+	.netnsok	= true,
+	.parallel_ops	= true,
+	.module		= THIS_MODULE,
+	.split_ops	= nfsd_server_nl_ops,
+	.n_split_ops	= ARRAY_SIZE(nfsd_server_nl_ops),
+};
fs/nfsd/nfs_netlink_gen.h new file +22 -0
new file mode 100644
index 000000000000..f66b29e528c1
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_NFSD_SERVER_GEN_H
+#define _LINUX_NFSD_SERVER_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/nfsd_server.h>
+
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb);
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb);
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb);
+
+extern struct genl_family nfsd_server_nl_family;
+
+#endif /* _LINUX_NFSD_SERVER_GEN_H */
fs/nfsd/nfsctl.c +16 -0
index 33f80d289d63..1be66088849c 100644
@@ -1495,6 +1495,22 @@ static int create_proc_exports_entry(void)
 
 unsigned int nfsd_net_id;
 
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb)
+{
+	return 0;
+}
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
include/uapi/linux/nfsd_server.h new file +39 -0
new file mode 100644
index 000000000000..c9ee00ceca3b
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN uapi header */
+
+#ifndef _UAPI_LINUX_NFSD_SERVER_H
+#define _UAPI_LINUX_NFSD_SERVER_H
+
+#define NFSD_SERVER_FAMILY_NAME		"nfsd_server"
+#define NFSD_SERVER_FAMILY_VERSION	1
+
+enum nfsd_rpc_status_comp_attr {
+	NFSD_ATTR_RPC_STATUS_COMP_UNSPEC,
+	NFSD_ATTR_RPC_STATUS_COMP_OP,
+
+	__NFSD_ATTR_RPC_STATUS_COMP_MAX,
+	NFSD_ATTR_RPC_STATUS_COMP_MAX = (__NFSD_ATTR_RPC_STATUS_COMP_MAX - 1)
+};
+
+enum nfsd_rpc_status_attr {
+	NFSD_ATTR_RPC_STATUS_UNSPEC,
+	NFSD_ATTR_RPC_STATUS_XID,
+	NFSD_ATTR_RPC_STATUS_FLAGS,
+	NFSD_ATTR_RPC_STATUS_PROG,
+	NFSD_ATTR_RPC_STATUS_VERSION,
+	NFSD_ATTR_RPC_STATUS_PROC,
+	NFSD_ATTR_RPC_STATUS_SERVICE_TIME,
+	NFSD_ATTR_RPC_STATUS_PAD,
+	NFSD_ATTR_RPC_STATUS_SADDR4,
+	NFSD_ATTR_RPC_STATUS_DADDR4,
+	NFSD_ATTR_RPC_STATUS_SADDR6,
+	NFSD_ATTR_RPC_STATUS_DADDR6,
+	NFSD_ATTR_RPC_STATUS_SPORT,
+	NFSD_ATTR_RPC_STATUS_DPORT,
+	NFSD_ATTR_RPC_STATUS_COMPOND_OP,
+
+	__NFSD_ATTR_RPC_STATUS_MAX,
+	NFSD_ATTR_RPC_STATUS_MAX = (__NFSD_ATTR_RPC_STATUS_MAX - 1)
+};

include/uapi/linux/nfsd_server.h line 49

Quoted 10 lines
include/uapi/linux/nfsd_server.h new file +10 -0
+
+enum nfsd_commands {
+	NFSD_CMD_UNSPEC,
+	NFSD_CMD_RPC_STATUS_GET,
+
+	__NFSD_CMD_MAX,
+	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
+};
+
+#endif /* _UAPI_LINUX_NFSD_SERVER_H */
#12 lore raw

Re: [PATCH v8 2/3] NFSD: introduce netlink rpc_status stubs

In reply to Lorenzo Bianconi
Quoted 11 lines · Lorenzo Bianconi

Generate empty netlink stubs and uAPI through nfsd_server.yaml specs:

$./tools/net/ynl/ynl-gen-c.py --mode uapi \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o include/uapi/linux/nfsd_server.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o fs/nfsd/nfs_netlink_gen.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--source -o fs/nfsd/nfs_netlink_gen.c

Actually there's a tool that walks the whole kernel source tree and handles any files that contain the YNL-GEN tag:

$ tools/net/ynl/ynl-regen.sh

fs/nfsd/nfs_netlink_gen.c

Quoted 32 lines
---
 fs/nfsd/Makefile                 |  3 +-
 fs/nfsd/nfs_netlink_gen.c        | 32 +++++++++++++++++++++
 fs/nfsd/nfs_netlink_gen.h        | 22 ++++++++++++++
 fs/nfsd/nfsctl.c                 | 16 +++++++++++
 include/uapi/linux/nfsd_server.h | 49 ++++++++++++++++++++++++++++++++
 5 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 fs/nfsd/nfs_netlink_gen.c
 create mode 100644 fs/nfsd/nfs_netlink_gen.h
 create mode 100644 include/uapi/linux/nfsd_server.h
fs/nfsd/Makefile +2 -1
index 6fffc8f03f74..6ae1d5450bf6 100644
@@ -12,7 +12,8 @@ nfsd-y			+= trace.o
 
 nfsd-y 			+= nfssvc.o nfsctl.o nfsfh.o vfs.o \
 			   export.o auth.o lockd.o nfscache.o \
-			   stats.o filecache.o nfs3proc.o nfs3xdr.o
+			   stats.o filecache.o nfs3proc.o nfs3xdr.o \
+			   nfs_netlink_gen.o
 nfsd-$(CONFIG_NFSD_V2) += nfsproc.o nfsxdr.o
 nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
 nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
fs/nfsd/nfs_netlink_gen.c new file +0 -0
new file mode 100644
index 000000000000..4d71b80bf4a7

I'd like a shorter file name. How about fs/nfsd/netlink.c (and below, netlink.h) ?

Quoted 146 lines
fs/nfsd/nfs_netlink_gen.c new file +32 -0
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "nfs_netlink_gen.h"
+
+#include <uapi/linux/nfsd_server.h>
+
+/* Ops table for nfsd_server */
+static const struct genl_split_ops nfsd_server_nl_ops[] = {
+	{
+		.cmd	= NFSD_CMD_RPC_STATUS_GET,
+		.start	= nfsd_server_nl_rpc_status_get_start,
+		.dumpit	= nfsd_server_nl_rpc_status_get_dumpit,
+		.done	= nfsd_server_nl_rpc_status_get_done,
+		.flags	= GENL_CMD_CAP_DUMP,
+	},
+};
+
+struct genl_family nfsd_server_nl_family __ro_after_init = {
+	.name		= NFSD_SERVER_FAMILY_NAME,
+	.version	= NFSD_SERVER_FAMILY_VERSION,
+	.netnsok	= true,
+	.parallel_ops	= true,
+	.module		= THIS_MODULE,
+	.split_ops	= nfsd_server_nl_ops,
+	.n_split_ops	= ARRAY_SIZE(nfsd_server_nl_ops),
+};
fs/nfsd/nfs_netlink_gen.h new file +22 -0
new file mode 100644
index 000000000000..f66b29e528c1
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_NFSD_SERVER_GEN_H
+#define _LINUX_NFSD_SERVER_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/nfsd_server.h>
+
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb);
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb);
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb);
+
+extern struct genl_family nfsd_server_nl_family;
+
+#endif /* _LINUX_NFSD_SERVER_GEN_H */
fs/nfsd/nfsctl.c +16 -0
index 33f80d289d63..1be66088849c 100644
@@ -1495,6 +1495,22 @@ static int create_proc_exports_entry(void)
 
 unsigned int nfsd_net_id;
 
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb)
+{
+	return 0;
+}
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
include/uapi/linux/nfsd_server.h new file +49 -0
new file mode 100644
index 000000000000..c9ee00ceca3b
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN uapi header */
+
+#ifndef _UAPI_LINUX_NFSD_SERVER_H
+#define _UAPI_LINUX_NFSD_SERVER_H
+
+#define NFSD_SERVER_FAMILY_NAME		"nfsd_server"
+#define NFSD_SERVER_FAMILY_VERSION	1
+
+enum nfsd_rpc_status_comp_attr {
+	NFSD_ATTR_RPC_STATUS_COMP_UNSPEC,
+	NFSD_ATTR_RPC_STATUS_COMP_OP,
+
+	__NFSD_ATTR_RPC_STATUS_COMP_MAX,
+	NFSD_ATTR_RPC_STATUS_COMP_MAX = (__NFSD_ATTR_RPC_STATUS_COMP_MAX - 1)
+};
+
+enum nfsd_rpc_status_attr {
+	NFSD_ATTR_RPC_STATUS_UNSPEC,
+	NFSD_ATTR_RPC_STATUS_XID,
+	NFSD_ATTR_RPC_STATUS_FLAGS,
+	NFSD_ATTR_RPC_STATUS_PROG,
+	NFSD_ATTR_RPC_STATUS_VERSION,
+	NFSD_ATTR_RPC_STATUS_PROC,
+	NFSD_ATTR_RPC_STATUS_SERVICE_TIME,
+	NFSD_ATTR_RPC_STATUS_PAD,
+	NFSD_ATTR_RPC_STATUS_SADDR4,
+	NFSD_ATTR_RPC_STATUS_DADDR4,
+	NFSD_ATTR_RPC_STATUS_SADDR6,
+	NFSD_ATTR_RPC_STATUS_DADDR6,
+	NFSD_ATTR_RPC_STATUS_SPORT,
+	NFSD_ATTR_RPC_STATUS_DPORT,
+	NFSD_ATTR_RPC_STATUS_COMPOND_OP,
+
+	__NFSD_ATTR_RPC_STATUS_MAX,
+	NFSD_ATTR_RPC_STATUS_MAX = (__NFSD_ATTR_RPC_STATUS_MAX - 1)
+};
+
+enum nfsd_commands {
+	NFSD_CMD_UNSPEC,
+	NFSD_CMD_RPC_STATUS_GET,
+
+	__NFSD_CMD_MAX,
+	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
+};
+
+#endif /* _UAPI_LINUX_NFSD_SERVER_H */
Signature · 1 line
2.41.0
Signature · 1 line
Chuck Lever
#13 lore raw

Re: [PATCH v8 2/3] NFSD: introduce netlink rpc_status stubs

Quoted 38 lines · Lorenzo Bianconi
Quoted 11 lines

Generate empty netlink stubs and uAPI through nfsd_server.yaml specs:

$./tools/net/ynl/ynl-gen-c.py --mode uapi \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o include/uapi/linux/nfsd_server.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o fs/nfsd/nfs_netlink_gen.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--source -o fs/nfsd/nfs_netlink_gen.c

Actually there's a tool that walks the whole kernel source tree and handles any files that contain the YNL-GEN tag:

$ tools/net/ynl/ynl-regen.sh
Quoted 19 lines
---
 fs/nfsd/Makefile                 |  3 +-
 fs/nfsd/nfs_netlink_gen.c        | 32 +++++++++++++++++++++
 fs/nfsd/nfs_netlink_gen.h        | 22 ++++++++++++++
 fs/nfsd/nfsctl.c                 | 16 +++++++++++
 include/uapi/linux/nfsd_server.h | 49 ++++++++++++++++++++++++++++++++
 5 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 fs/nfsd/nfs_netlink_gen.c
 create mode 100644 fs/nfsd/nfs_netlink_gen.h
 create mode 100644 include/uapi/linux/nfsd_server.h
fs/nfsd/Makefile +0 -0
index 6fffc8f03f74..6ae1d5450bf6 100644
@@ -12,7 +12,8 @@ nfsd-y			+= trace.o
 
Quoted 16 lines

fs/nfsd/nfs_netlink_gen.c

Quoted 13 lines
fs/nfsd/Makefile +2 -1
 nfsd-y 			+= nfssvc.o nfsctl.o nfsfh.o vfs.o \
 			   export.o auth.o lockd.o nfscache.o \
-			   stats.o filecache.o nfs3proc.o nfs3xdr.o
+			   stats.o filecache.o nfs3proc.o nfs3xdr.o \
+			   nfs_netlink_gen.o
 nfsd-$(CONFIG_NFSD_V2) += nfsproc.o nfsxdr.o
 nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
 nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
fs/nfsd/nfs_netlink_gen.c new file +0 -0
new file mode 100644
index 000000000000..4d71b80bf4a7

I'd like a shorter file name. How about fs/nfsd/netlink.c (and below, netlink.h) ?

ack, fine

Regards,
Lorenzo

Quoted 152 lines
Quoted 148 lines
fs/nfsd/nfs_netlink_gen.c new file +32 -0
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "nfs_netlink_gen.h"
+
+#include <uapi/linux/nfsd_server.h>
+
+/* Ops table for nfsd_server */
+static const struct genl_split_ops nfsd_server_nl_ops[] = {
+	{
+		.cmd	= NFSD_CMD_RPC_STATUS_GET,
+		.start	= nfsd_server_nl_rpc_status_get_start,
+		.dumpit	= nfsd_server_nl_rpc_status_get_dumpit,
+		.done	= nfsd_server_nl_rpc_status_get_done,
+		.flags	= GENL_CMD_CAP_DUMP,
+	},
+};
+
+struct genl_family nfsd_server_nl_family __ro_after_init = {
+	.name		= NFSD_SERVER_FAMILY_NAME,
+	.version	= NFSD_SERVER_FAMILY_VERSION,
+	.netnsok	= true,
+	.parallel_ops	= true,
+	.module		= THIS_MODULE,
+	.split_ops	= nfsd_server_nl_ops,
+	.n_split_ops	= ARRAY_SIZE(nfsd_server_nl_ops),
+};
fs/nfsd/nfs_netlink_gen.h new file +22 -0
new file mode 100644
index 000000000000..f66b29e528c1
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_NFSD_SERVER_GEN_H
+#define _LINUX_NFSD_SERVER_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/nfsd_server.h>
+
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb);
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb);
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb);
+
+extern struct genl_family nfsd_server_nl_family;
+
+#endif /* _LINUX_NFSD_SERVER_GEN_H */
fs/nfsd/nfsctl.c +16 -0
index 33f80d289d63..1be66088849c 100644
@@ -1495,6 +1495,22 @@ static int create_proc_exports_entry(void)
 
 unsigned int nfsd_net_id;
 
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb)
+{
+	return 0;
+}
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
include/uapi/linux/nfsd_server.h new file +49 -0
new file mode 100644
index 000000000000..c9ee00ceca3b
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/nfsd_server.yaml */
+/* YNL-GEN uapi header */
+
+#ifndef _UAPI_LINUX_NFSD_SERVER_H
+#define _UAPI_LINUX_NFSD_SERVER_H
+
+#define NFSD_SERVER_FAMILY_NAME		"nfsd_server"
+#define NFSD_SERVER_FAMILY_VERSION	1
+
+enum nfsd_rpc_status_comp_attr {
+	NFSD_ATTR_RPC_STATUS_COMP_UNSPEC,
+	NFSD_ATTR_RPC_STATUS_COMP_OP,
+
+	__NFSD_ATTR_RPC_STATUS_COMP_MAX,
+	NFSD_ATTR_RPC_STATUS_COMP_MAX = (__NFSD_ATTR_RPC_STATUS_COMP_MAX - 1)
+};
+
+enum nfsd_rpc_status_attr {
+	NFSD_ATTR_RPC_STATUS_UNSPEC,
+	NFSD_ATTR_RPC_STATUS_XID,
+	NFSD_ATTR_RPC_STATUS_FLAGS,
+	NFSD_ATTR_RPC_STATUS_PROG,
+	NFSD_ATTR_RPC_STATUS_VERSION,
+	NFSD_ATTR_RPC_STATUS_PROC,
+	NFSD_ATTR_RPC_STATUS_SERVICE_TIME,
+	NFSD_ATTR_RPC_STATUS_PAD,
+	NFSD_ATTR_RPC_STATUS_SADDR4,
+	NFSD_ATTR_RPC_STATUS_DADDR4,
+	NFSD_ATTR_RPC_STATUS_SADDR6,
+	NFSD_ATTR_RPC_STATUS_DADDR6,
+	NFSD_ATTR_RPC_STATUS_SPORT,
+	NFSD_ATTR_RPC_STATUS_DPORT,
+	NFSD_ATTR_RPC_STATUS_COMPOND_OP,
+
+	__NFSD_ATTR_RPC_STATUS_MAX,
+	NFSD_ATTR_RPC_STATUS_MAX = (__NFSD_ATTR_RPC_STATUS_MAX - 1)
+};
+
+enum nfsd_commands {
+	NFSD_CMD_UNSPEC,
+	NFSD_CMD_RPC_STATUS_GET,
+
+	__NFSD_CMD_MAX,
+	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
+};
+
+#endif /* _UAPI_LINUX_NFSD_SERVER_H */
Signature · 1 line
2.41.0
Signature · 1 line
Chuck Lever
reviewer
#14 lore raw

Re: [PATCH v8 2/3] NFSD: introduce netlink rpc_status stubs

In reply to Lorenzo Bianconi
Quoted 14 lines · Lorenzo Bianconi

Generate empty netlink stubs and uAPI through nfsd_server.yaml specs:

$./tools/net/ynl/ynl-gen-c.py --mode uapi \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o include/uapi/linux/nfsd_server.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o fs/nfsd/nfs_netlink_gen.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--source -o fs/nfsd/nfs_netlink_gen.c

...

Quoted 14 lines
fs/nfsd/nfsctl.c +6 -0
index 33f80d289d63..1be66088849c 100644
@@ -1495,6 +1495,22 @@ static int create_proc_exports_entry(void)
 
 unsigned int nfsd_net_id;
 
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)

fs/nfsd/nfsctl.c static int create_proc_exports_entry(void) · line 1513

Quoted 10 lines
fs/nfsd/nfsctl.c +10 -0
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb)
+{
+	return 0;
+}
+

Hi Lorenzo,

W=1 build for gcc-13 and clang-16, and Smatch, complain that there is no prototype for the above functions.

Perhaps nfs_netlink_gen.h should be included in this file?

Quoted 3 lines
fs/nfsd/nfsctl.c +0 -0
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace

...

#15 lore raw

Re: [PATCH v8 2/3] NFSD: introduce netlink rpc_status stubs

Quoted 36 lines · Lorenzo Bianconi
Quoted 14 lines

Generate empty netlink stubs and uAPI through nfsd_server.yaml specs:

$./tools/net/ynl/ynl-gen-c.py --mode uapi \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o include/uapi/linux/nfsd_server.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--header -o fs/nfsd/nfs_netlink_gen.h
$./tools/net/ynl/ynl-gen-c.py --mode kernel \
--spec Documentation/netlink/specs/nfsd_server.yaml \
--source -o fs/nfsd/nfs_netlink_gen.c

...

Quoted 18 lines
fs/nfsd/nfsctl.c +10 -0
index 33f80d289d63..1be66088849c 100644
@@ -1495,6 +1495,22 @@ static int create_proc_exports_entry(void)
 
 unsigned int nfsd_net_id;
 
+int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
+{
+	return 0;
+}
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	return 0;
+}
+
Quoted 8 lines

fs/nfsd/nfsctl.c static int create_proc_exports_entry(void) · line 1513

Quoted 6 lines
fs/nfsd/nfsctl.c +6 -0
+int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
+					 struct netlink_callback *cb)
+{
+	return 0;
+}
+

Hi Lorenzo,

Hi Simon,

Quoted 5 lines

W=1 build for gcc-13 and clang-16, and Smatch, complain that there is no prototype for the above functions.

Perhaps nfs_netlink_gen.h should be included in this file?

actually I added it in patch 3/3. I will move it here. Thx.

Regards,
Lorenzo

Quoted 7 lines
Quoted 4 lines
fs/nfsd/nfsctl.c +0 -0
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace

...

#16 lore raw

PATCH v8 3/3 NFSD: add rpc_status netlink support

In reply to Lorenzo Bianconi

Introduce rpc_status netlink support for NFSD in order to dump pending RPC requests debugging information from userspace.

---
 fs/nfsd/nfsctl.c           | 192 ++++++++++++++++++++++++++++++++++++-
 fs/nfsd/nfsd.h             |  16 ++++
 fs/nfsd/nfssvc.c           |  15 +++
 fs/nfsd/state.h            |   2 -
 include/linux/sunrpc/svc.h |   1 +
 5 files changed, 222 insertions(+), 4 deletions(-)
fs/nfsd/nfsctl.c +190 -2
index 1be66088849c..b862a759ea15 100644
@@ -26,6 +26,7 @@
 #include "pnfs.h"
 #include "filecache.h"
 #include "trace.h"
+#include "nfs_netlink_gen.h"
 
 /*
  *	We have a single directory with several nodes in it.
@@ -1497,17 +1498,199 @@ unsigned int nfsd_net_id;
 
 int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
 {
-	return 0;
+	struct nfsd_net *nn = net_generic(sock_net(cb->skb->sk), nfsd_net_id);
+	int ret = -ENODEV;
+
+	mutex_lock(&nfsd_mutex);
+	if (nn->nfsd_serv) {
+		svc_get(nn->nfsd_serv);
+		ret = 0;
+	}
+	mutex_unlock(&nfsd_mutex);
+
+	return ret;
 }
 
-int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+static int nfsd_genl_rpc_status_compose_msg(struct sk_buff *skb,
+					    struct netlink_callback *cb,
+					    struct nfsd_genl_rqstp *rqstp)
 {
+	void *hdr;
+	int i;
+
+	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+			  &nfsd_server_nl_family, NLM_F_MULTI,
+			  NFSD_CMD_RPC_STATUS_GET);
+	if (!hdr)
+		return -ENOBUFS;
+
+	if (nla_put_be32(skb, NFSD_ATTR_RPC_STATUS_XID, rqstp->rq_xid) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_FLAGS, rqstp->rq_flags) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_PROG, rqstp->rq_prog) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_PROC, rqstp->rq_proc) ||
+	    nla_put_u8(skb, NFSD_ATTR_RPC_STATUS_VERSION, rqstp->rq_vers) ||
+	    nla_put_s64(skb, NFSD_ATTR_RPC_STATUS_SERVICE_TIME,
+			ktime_to_us(rqstp->rq_stime),
+			NFSD_ATTR_RPC_STATUS_PAD))
+		return -ENOBUFS;
+
+	switch (rqstp->saddr.sa_family) {
+	case AF_INET: {
+		const struct sockaddr_in *s_in, *d_in;
+
+		s_in = (const struct sockaddr_in *)&rqstp->saddr;
+		d_in = (const struct sockaddr_in *)&rqstp->daddr;
+		if (nla_put_in_addr(skb, NFSD_ATTR_RPC_STATUS_SADDR4,
+				    s_in->sin_addr.s_addr) ||
+		    nla_put_in_addr(skb, NFSD_ATTR_RPC_STATUS_DADDR4,
+				    d_in->sin_addr.s_addr) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_SPORT,
+				 s_in->sin_port) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_DPORT,
+				 d_in->sin_port))
+			return -ENOBUFS;
+		break;
+	}
+	case AF_INET6: {
+		const struct sockaddr_in6 *s_in, *d_in;
+
+		s_in = (const struct sockaddr_in6 *)&rqstp->saddr;
+		d_in = (const struct sockaddr_in6 *)&rqstp->daddr;
+		if (nla_put_in6_addr(skb, NFSD_ATTR_RPC_STATUS_SADDR6,
+				     &s_in->sin6_addr) ||
+		    nla_put_in6_addr(skb, NFSD_ATTR_RPC_STATUS_DADDR6,
+				     &d_in->sin6_addr) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_SPORT,
+				 s_in->sin6_port) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_DPORT,
+				 d_in->sin6_port))
+			return -ENOBUFS;
+		break;
+	}
+	default:
+		break;
+	}
+
+	if (rqstp->opcnt) {
+		struct nlattr *attr;
+
+		attr = nla_nest_start(skb, NFSD_ATTR_RPC_STATUS_COMPOND_OP);
+		if (!attr)
+			return -ENOBUFS;
+
+		for (i = 0; i < rqstp->opcnt; i++) {
+			struct nlattr *op_attr;
+
+			op_attr = nla_nest_start(skb, i);
+			if (!op_attr)
+				return -ENOBUFS;
+
+			if (nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_COMP_OP,
+					rqstp->opnum[i]))
+				return -ENOBUFS;
+
+			nla_nest_end(skb, op_attr);
+		}
+
+		nla_nest_end(skb, attr);
+	}
+
+	genlmsg_end(skb, hdr);
+
 	return 0;
 }
 
 int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 					 struct netlink_callback *cb)
 {
+	struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
+	int i, ret, rqstp_index;
+
+	rcu_read_lock();
+
+	for (i = 0; i < nn->nfsd_serv->sv_nrpools; i++) {
+		struct svc_rqst *rqstp;
+
+		if (i < cb->args[0]) /* already consumed */
+			continue;
+
+		rqstp_index = 0;
+		list_for_each_entry_rcu(rqstp,
+				&nn->nfsd_serv->sv_pools[i].sp_all_threads,
+				rq_all) {
+			struct nfsd_genl_rqstp genl_rqstp;
+			unsigned int status_counter;
+
+			if (rqstp_index++ < cb->args[1]) /* already consumed */
+				continue;
+			/*
+			 * Acquire rq_status_counter before parsing the rqst
+			 * fields. rq_status_counter is set to an odd value in
+			 * order to notify the consumers the rqstp fields are
+			 * meaningful.
+			 */
+			status_counter =
+				smp_load_acquire(&rqstp->rq_status_counter);
+			if (!(status_counter & 1))
+				continue;
+
+			genl_rqstp.rq_xid = rqstp->rq_xid;
+			genl_rqstp.rq_flags = rqstp->rq_flags;
+			genl_rqstp.rq_vers = rqstp->rq_vers;
+			genl_rqstp.rq_prog = rqstp->rq_prog;
+			genl_rqstp.rq_proc = rqstp->rq_proc;
+			genl_rqstp.rq_stime = rqstp->rq_stime;
+			genl_rqstp.opcnt = 0;
+			memcpy(&genl_rqstp.daddr, svc_daddr(rqstp),
+			       sizeof(struct sockaddr));
+			memcpy(&genl_rqstp.saddr, svc_addr(rqstp),
+			       sizeof(struct sockaddr));
+
+#ifdef CONFIG_NFSD_V4
+			if (rqstp->rq_vers == NFS4_VERSION &&
+			    rqstp->rq_proc == NFSPROC4_COMPOUND) {
+				/* NFSv4 compund */
+				struct nfsd4_compoundargs *args;
+				int j;
+
+				args = rqstp->rq_argp;
+				genl_rqstp.opcnt = args->opcnt;
+				for (j = 0; j < genl_rqstp.opcnt; j++)
+					genl_rqstp.opnum[j] =
+						args->ops[j].opnum;
+			}
+#endif /* CONFIG_NFSD_V4 */
+
+			/*
+			 * Acquire rq_status_counter before reporting the rqst
+			 * fields to the user.
+			 */
+			if (smp_load_acquire(&rqstp->rq_status_counter) !=
+			    status_counter)
+				continue;
+
+			ret = nfsd_genl_rpc_status_compose_msg(skb, cb,
+							       &genl_rqstp);
+			if (ret)
+				goto out;
+		}
+	}
+
+	cb->args[0] = i;
+	cb->args[1] = rqstp_index;
+	ret = skb->len;
+out:
+	rcu_read_unlock();
+
+	return ret;
+}
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	mutex_lock(&nfsd_mutex);
+	nfsd_put(sock_net(cb->skb->sk));
+	mutex_unlock(&nfsd_mutex);
+
 	return 0;
 }
 
@@ -1605,6 +1788,10 @@ static int __init init_nfsd(void)
 	retval = register_filesystem(&nfsd_fs_type);
 	if (retval)
 		goto out_free_all;
+	retval = genl_register_family(&nfsd_server_nl_family);
+	if (retval)
+		goto out_free_all;
+
 	return 0;
 out_free_all:
 	nfsd4_destroy_laundry_wq();
@@ -1629,6 +1816,7 @@ static int __init init_nfsd(void)
 
 static void __exit exit_nfsd(void)
 {
+	genl_unregister_family(&nfsd_server_nl_family);
 	unregister_filesystem(&nfsd_fs_type);
 	nfsd4_destroy_laundry_wq();
 	unregister_cld_notifier();
fs/nfsd/nfsd.h +16 -0
index 11c14faa6c67..d787bd38c053 100644
@@ -62,6 +62,22 @@ struct readdir_cd {
 	__be32			err;	/* 0, nfserr, or nfserr_eof */
 };
 
+/* Maximum number of operations per session compound */
+#define NFSD_MAX_OPS_PER_COMPOUND	50
+
+struct nfsd_genl_rqstp {
+	struct sockaddr daddr;
+	struct sockaddr saddr;
+	unsigned long rq_flags;
+	ktime_t rq_stime;
+	__be32 rq_xid;
+	u32 rq_vers;
+	u32 rq_prog;
+	u32 rq_proc;
+	/* NFSv4 compund */
+	u32 opnum[NFSD_MAX_OPS_PER_COMPOUND];
+	u16 opcnt;
+};
 
 extern struct svc_program	nfsd_program;
 extern const struct svc_version	nfsd_version2, nfsd_version3, nfsd_version4;
fs/nfsd/nfssvc.c +15 -0
index 1582af33e204..fad34a7325b3 100644
@@ -998,6 +998,15 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream))
 		goto out_decode_err;
 
+	/*
+	 * Release rq_status_counter setting it to an odd value after the rpc
+	 * request has been properly parsed. rq_status_counter is used to
+	 * notify the consumers if the rqstp fields are stable
+	 * (rq_status_counter is odd) or not meaningful (rq_status_counter
+	 * is even).
+	 */
+	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter | 1);
+
 	rp = NULL;
 	switch (nfsd_cache_lookup(rqstp, &rp)) {
 	case RC_DOIT:
@@ -1015,6 +1024,12 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_encode(rqstp, &rqstp->rq_res_stream))
 		goto out_encode_err;
 
+	/*
+	 * Release rq_status_counter setting it to an even value after the rpc
+	 * request has been properly processed.
+	 */
+	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter + 1);
+
 	nfsd_cache_update(rqstp, rp, rqstp->rq_cachetype, statp + 1);
 out_cached_reply:
 	return 1;
fs/nfsd/state.h +0 -2
index cbddcf484dba..41bdc913fa71 100644
@@ -174,8 +174,6 @@ static inline struct nfs4_delegation *delegstateid(struct nfs4_stid *s)
 
 /* Maximum number of slots per session. 160 is useful for long haul TCP */
 #define NFSD_MAX_SLOTS_PER_SESSION     160
-/* Maximum number of operations per session compound */
-#define NFSD_MAX_OPS_PER_COMPOUND	50
 /* Maximum  session per slot cache size */
 #define NFSD_SLOT_CACHE_SIZE		2048
 /* Maximum number of NFSD_SLOT_CACHE_SIZE slots per session */
include/linux/sunrpc/svc.h +1 -0
index dbf5b21feafe..caa20defd255 100644
@@ -251,6 +251,7 @@ struct svc_rqst {
 						 * net namespace
 						 */
 	void **			rq_lease_breaker; /* The v4 client breaking a lease */
+	unsigned int		rq_status_counter; /* RPC processing counter */
 };
 
 /* bits for rq_flags */
Signature · 1 line
2.41.0
maintainer
#17 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

Quoted 95 lines · Lorenzo Bianconi

Introduce rpc_status netlink support for NFSD in order to dump pending RPC requests debugging information from userspace.

---
 fs/nfsd/nfsctl.c           | 192 ++++++++++++++++++++++++++++++++++++-
 fs/nfsd/nfsd.h             |  16 ++++
 fs/nfsd/nfssvc.c           |  15 +++
 fs/nfsd/state.h            |   2 -
 include/linux/sunrpc/svc.h |   1 +
 5 files changed, 222 insertions(+), 4 deletions(-)
fs/nfsd/nfsctl.c +62 -2
index 1be66088849c..b862a759ea15 100644
@@ -26,6 +26,7 @@
 #include "pnfs.h"
 #include "filecache.h"
 #include "trace.h"
+#include "nfs_netlink_gen.h"
 
 /*
  *	We have a single directory with several nodes in it.
@@ -1497,17 +1498,199 @@ unsigned int nfsd_net_id;
 
 int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
 {
-	return 0;
+	struct nfsd_net *nn = net_generic(sock_net(cb->skb->sk), nfsd_net_id);
+	int ret = -ENODEV;
+
+	mutex_lock(&nfsd_mutex);
+	if (nn->nfsd_serv) {
+		svc_get(nn->nfsd_serv);
+		ret = 0;
+	}
+	mutex_unlock(&nfsd_mutex);
+
+	return ret;
 }
 
-int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+static int nfsd_genl_rpc_status_compose_msg(struct sk_buff *skb,
+					    struct netlink_callback *cb,
+					    struct nfsd_genl_rqstp *rqstp)
 {
+	void *hdr;
+	int i;
+
+	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+			  &nfsd_server_nl_family, NLM_F_MULTI,
+			  NFSD_CMD_RPC_STATUS_GET);
+	if (!hdr)
+		return -ENOBUFS;
+
+	if (nla_put_be32(skb, NFSD_ATTR_RPC_STATUS_XID, rqstp->rq_xid) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_FLAGS, rqstp->rq_flags) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_PROG, rqstp->rq_prog) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_PROC, rqstp->rq_proc) ||
+	    nla_put_u8(skb, NFSD_ATTR_RPC_STATUS_VERSION, rqstp->rq_vers) ||
+	    nla_put_s64(skb, NFSD_ATTR_RPC_STATUS_SERVICE_TIME,
+			ktime_to_us(rqstp->rq_stime),
+			NFSD_ATTR_RPC_STATUS_PAD))
+		return -ENOBUFS;
+
+	switch (rqstp->saddr.sa_family) {
+	case AF_INET: {
+		const struct sockaddr_in *s_in, *d_in;
+
+		s_in = (const struct sockaddr_in *)&rqstp->saddr;
+		d_in = (const struct sockaddr_in *)&rqstp->daddr;
+		if (nla_put_in_addr(skb, NFSD_ATTR_RPC_STATUS_SADDR4,
+				    s_in->sin_addr.s_addr) ||
+		    nla_put_in_addr(skb, NFSD_ATTR_RPC_STATUS_DADDR4,
+				    d_in->sin_addr.s_addr) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_SPORT,
+				 s_in->sin_port) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_DPORT,
+				 d_in->sin_port))
+			return -ENOBUFS;
+		break;
+	}
+	case AF_INET6: {
+		const struct sockaddr_in6 *s_in, *d_in;
+
+		s_in = (const struct sockaddr_in6 *)&rqstp->saddr;
+		d_in = (const struct sockaddr_in6 *)&rqstp->daddr;
+		if (nla_put_in6_addr(skb, NFSD_ATTR_RPC_STATUS_SADDR6,
+				     &s_in->sin6_addr) ||
+		    nla_put_in6_addr(skb, NFSD_ATTR_RPC_STATUS_DADDR6,
+				     &d_in->sin6_addr) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_SPORT,
+				 s_in->sin6_port) ||

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1574

Quoted 10 lines
fs/nfsd/nfsctl.c +10 -0
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_DPORT,
+				 d_in->sin6_port))
+			return -ENOBUFS;
+		break;
+	}
+	default:
+		break;
+	}
+
+	if (rqstp->opcnt) {

It may be that we always have an opcount of 0 whenever we're running something besides NFSv4 COMPOUND, but it may be best not to count on that. I'd test for prog == NFS_PROGRAM, vers == 4, and that the proc == COMPOUND and then for the opcnt.

Quoted 156 lines
fs/nfsd/nfsctl.c +118 -0
+		struct nlattr *attr;
+
+		attr = nla_nest_start(skb, NFSD_ATTR_RPC_STATUS_COMPOND_OP);
+		if (!attr)
+			return -ENOBUFS;
+
+		for (i = 0; i < rqstp->opcnt; i++) {
+			struct nlattr *op_attr;
+
+			op_attr = nla_nest_start(skb, i);
+			if (!op_attr)
+				return -ENOBUFS;
+
+			if (nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_COMP_OP,
+					rqstp->opnum[i]))
+				return -ENOBUFS;
+
+			nla_nest_end(skb, op_attr);
+		}
+
+		nla_nest_end(skb, attr);
+	}
+
+	genlmsg_end(skb, hdr);
+
 	return 0;
 }
 
 int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 					 struct netlink_callback *cb)
 {
+	struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
+	int i, ret, rqstp_index;
+
+	rcu_read_lock();
+
+	for (i = 0; i < nn->nfsd_serv->sv_nrpools; i++) {
+		struct svc_rqst *rqstp;
+
+		if (i < cb->args[0]) /* already consumed */
+			continue;
+
+		rqstp_index = 0;
+		list_for_each_entry_rcu(rqstp,
+				&nn->nfsd_serv->sv_pools[i].sp_all_threads,
+				rq_all) {
+			struct nfsd_genl_rqstp genl_rqstp;
+			unsigned int status_counter;
+
+			if (rqstp_index++ < cb->args[1]) /* already consumed */
+				continue;
+			/*
+			 * Acquire rq_status_counter before parsing the rqst
+			 * fields. rq_status_counter is set to an odd value in
+			 * order to notify the consumers the rqstp fields are
+			 * meaningful.
+			 */
+			status_counter =
+				smp_load_acquire(&rqstp->rq_status_counter);
+			if (!(status_counter & 1))
+				continue;
+
+			genl_rqstp.rq_xid = rqstp->rq_xid;
+			genl_rqstp.rq_flags = rqstp->rq_flags;
+			genl_rqstp.rq_vers = rqstp->rq_vers;
+			genl_rqstp.rq_prog = rqstp->rq_prog;
+			genl_rqstp.rq_proc = rqstp->rq_proc;
+			genl_rqstp.rq_stime = rqstp->rq_stime;
+			genl_rqstp.opcnt = 0;
+			memcpy(&genl_rqstp.daddr, svc_daddr(rqstp),
+			       sizeof(struct sockaddr));
+			memcpy(&genl_rqstp.saddr, svc_addr(rqstp),
+			       sizeof(struct sockaddr));
+
+#ifdef CONFIG_NFSD_V4
+			if (rqstp->rq_vers == NFS4_VERSION &&
+			    rqstp->rq_proc == NFSPROC4_COMPOUND) {
+				/* NFSv4 compund */
+				struct nfsd4_compoundargs *args;
+				int j;
+
+				args = rqstp->rq_argp;
+				genl_rqstp.opcnt = args->opcnt;
+				for (j = 0; j < genl_rqstp.opcnt; j++)
+					genl_rqstp.opnum[j] =
+						args->ops[j].opnum;
+			}
+#endif /* CONFIG_NFSD_V4 */
+
+			/*
+			 * Acquire rq_status_counter before reporting the rqst
+			 * fields to the user.
+			 */
+			if (smp_load_acquire(&rqstp->rq_status_counter) !=
+			    status_counter)
+				continue;
+
+			ret = nfsd_genl_rpc_status_compose_msg(skb, cb,
+							       &genl_rqstp);
+			if (ret)
+				goto out;
+		}
+	}
+
+	cb->args[0] = i;
+	cb->args[1] = rqstp_index;
+	ret = skb->len;
+out:
+	rcu_read_unlock();
+
+	return ret;
+}
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	mutex_lock(&nfsd_mutex);
+	nfsd_put(sock_net(cb->skb->sk));
+	mutex_unlock(&nfsd_mutex);
+
 	return 0;
 }
 
@@ -1605,6 +1788,10 @@ static int __init init_nfsd(void)
 	retval = register_filesystem(&nfsd_fs_type);
 	if (retval)
 		goto out_free_all;
+	retval = genl_register_family(&nfsd_server_nl_family);
+	if (retval)
+		goto out_free_all;
+
 	return 0;
 out_free_all:
 	nfsd4_destroy_laundry_wq();
@@ -1629,6 +1816,7 @@ static int __init init_nfsd(void)
 
 static void __exit exit_nfsd(void)
 {
+	genl_unregister_family(&nfsd_server_nl_family);
 	unregister_filesystem(&nfsd_fs_type);
 	nfsd4_destroy_laundry_wq();
 	unregister_cld_notifier();
fs/nfsd/nfsd.h +7 -0
index 11c14faa6c67..d787bd38c053 100644
@@ -62,6 +62,22 @@ struct readdir_cd {
 	__be32			err;	/* 0, nfserr, or nfserr_eof */
 };
 
+/* Maximum number of operations per session compound */
+#define NFSD_MAX_OPS_PER_COMPOUND	50
+
+struct nfsd_genl_rqstp {
+	struct sockaddr daddr;
+	struct sockaddr saddr;
+	unsigned long rq_flags;

fs/nfsd/nfsd.h struct readdir_cd { · line 81

Quoted 10 lines
fs/nfsd/nfsd.h +9 -0
+	ktime_t rq_stime;
+	__be32 rq_xid;
+	u32 rq_vers;
+	u32 rq_prog;
+	u32 rq_proc;
+	/* NFSv4 compund */
+	u32 opnum[NFSD_MAX_OPS_PER_COMPOUND];
+	u16 opcnt;
+};
 

Again, I'm wondering, is there a way to pass down some sort context- specific value with netlink? It might be nice to make the two NFSv4 specific fields into a union if that can be done with netlink.

Quoted 60 lines
fs/nfsd/nfsd.h +0 -0
 extern struct svc_program	nfsd_program;
 extern const struct svc_version	nfsd_version2, nfsd_version3, nfsd_version4;
fs/nfsd/nfssvc.c +15 -0
index 1582af33e204..fad34a7325b3 100644
@@ -998,6 +998,15 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream))
 		goto out_decode_err;
 
+	/*
+	 * Release rq_status_counter setting it to an odd value after the rpc
+	 * request has been properly parsed. rq_status_counter is used to
+	 * notify the consumers if the rqstp fields are stable
+	 * (rq_status_counter is odd) or not meaningful (rq_status_counter
+	 * is even).
+	 */
+	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter | 1);
+
 	rp = NULL;
 	switch (nfsd_cache_lookup(rqstp, &rp)) {
 	case RC_DOIT:
@@ -1015,6 +1024,12 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_encode(rqstp, &rqstp->rq_res_stream))
 		goto out_encode_err;
 
+	/*
+	 * Release rq_status_counter setting it to an even value after the rpc
+	 * request has been properly processed.
+	 */
+	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter + 1);
+
 	nfsd_cache_update(rqstp, rp, rqstp->rq_cachetype, statp + 1);
 out_cached_reply:
 	return 1;
fs/nfsd/state.h +0 -2
index cbddcf484dba..41bdc913fa71 100644
@@ -174,8 +174,6 @@ static inline struct nfs4_delegation *delegstateid(struct nfs4_stid *s)
 
 /* Maximum number of slots per session. 160 is useful for long haul TCP */
 #define NFSD_MAX_SLOTS_PER_SESSION     160
-/* Maximum number of operations per session compound */
-#define NFSD_MAX_OPS_PER_COMPOUND	50
 /* Maximum  session per slot cache size */
 #define NFSD_SLOT_CACHE_SIZE		2048
 /* Maximum number of NFSD_SLOT_CACHE_SIZE slots per session */
include/linux/sunrpc/svc.h +1 -0
index dbf5b21feafe..caa20defd255 100644
@@ -251,6 +251,7 @@ struct svc_rqst {
 						 * net namespace
 						 */
 	void **			rq_lease_breaker; /* The v4 client breaking a lease */
+	unsigned int		rq_status_counter; /* RPC processing counter */
 };
 
 /* bits for rq_flags */
Signature · 1 line
Jeff Layton <[email protected]>
reviewer
#18 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

In reply to Lorenzo Bianconi
Quoted 5 lines · Lorenzo Bianconi

Introduce rpc_status netlink support for NFSD in order to dump pending RPC requests debugging information from userspace.

o
Hi Lorenzo,

some minor feedback from my side.

...

Quoted 40 lines

int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,

fs/nfsd/nfsctl.c +37 -0
 					 struct netlink_callback *cb)
 {
+	struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
+	int i, ret, rqstp_index;
+
+	rcu_read_lock();
+
+	for (i = 0; i < nn->nfsd_serv->sv_nrpools; i++) {
+		struct svc_rqst *rqstp;
+
+		if (i < cb->args[0]) /* already consumed */
+			continue;
+
+		rqstp_index = 0;
+		list_for_each_entry_rcu(rqstp,
+				&nn->nfsd_serv->sv_pools[i].sp_all_threads,
+				rq_all) {
+			struct nfsd_genl_rqstp genl_rqstp;
+			unsigned int status_counter;
+
+			if (rqstp_index++ < cb->args[1]) /* already consumed */
+				continue;
+			/*
+			 * Acquire rq_status_counter before parsing the rqst
+			 * fields. rq_status_counter is set to an odd value in
+			 * order to notify the consumers the rqstp fields are
+			 * meaningful.
+			 */
+			status_counter =
+				smp_load_acquire(&rqstp->rq_status_counter);
+			if (!(status_counter & 1))
+				continue;
+
+			genl_rqstp.rq_xid = rqstp->rq_xid;
+			genl_rqstp.rq_flags = rqstp->rq_flags;
+			genl_rqstp.rq_vers = rqstp->rq_vers;
+			genl_rqstp.rq_prog = rqstp->rq_prog;
+			genl_rqstp.rq_proc = rqstp->rq_proc;
+			genl_rqstp.rq_stime = rqstp->rq_stime;

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1652

Quoted 10 lines
fs/nfsd/nfsctl.c +10 -0
+			genl_rqstp.opcnt = 0;
+			memcpy(&genl_rqstp.daddr, svc_daddr(rqstp),
+			       sizeof(struct sockaddr));
+			memcpy(&genl_rqstp.saddr, svc_addr(rqstp),
+			       sizeof(struct sockaddr));
+
+#ifdef CONFIG_NFSD_V4
+			if (rqstp->rq_vers == NFS4_VERSION &&
+			    rqstp->rq_proc == NFSPROC4_COMPOUND) {
+				/* NFSv4 compund */

nit: compound

Quoted 18 lines
fs/nfsd/nfsctl.c +18 -0
+				struct nfsd4_compoundargs *args;
+				int j;
+
+				args = rqstp->rq_argp;
+				genl_rqstp.opcnt = args->opcnt;
+				for (j = 0; j < genl_rqstp.opcnt; j++)
+					genl_rqstp.opnum[j] =
+						args->ops[j].opnum;
+			}
+#endif /* CONFIG_NFSD_V4 */
+
+			/*
+			 * Acquire rq_status_counter before reporting the rqst
+			 * fields to the user.
+			 */
+			if (smp_load_acquire(&rqstp->rq_status_counter) !=
+			    status_counter)
+				continue;

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1680

Quoted 10 lines
fs/nfsd/nfsctl.c +10 -0
+
+			ret = nfsd_genl_rpc_status_compose_msg(skb, cb,
+							       &genl_rqstp);
+			if (ret)
+				goto out;
+		}
+	}
+
+	cb->args[0] = i;
+	cb->args[1] = rqstp_index;

I'm unsure if this is possible, but if the for loop above iterates zero times, or for all iterations (i < cb->args[0]), then rqstp_index will be used uninitialised here.

Flagged by Smatch.

Quoted 6 lines
fs/nfsd/nfsctl.c +6 -0
+	ret = skb->len;
+out:
+	rcu_read_unlock();
+
+	return ret;
+}

...

Quoted 11 lines
fs/nfsd/nfsd.h +3 -0
index 11c14faa6c67..d787bd38c053 100644
@@ -62,6 +62,22 @@ struct readdir_cd {
 	__be32			err;	/* 0, nfserr, or nfserr_eof */
 };
 
+/* Maximum number of operations per session compound */
+#define NFSD_MAX_OPS_PER_COMPOUND	50
+

fs/nfsd/nfsd.h struct readdir_cd { · line 77

Quoted 10 lines
fs/nfsd/nfsd.h +10 -0
+struct nfsd_genl_rqstp {
+	struct sockaddr daddr;
+	struct sockaddr saddr;
+	unsigned long rq_flags;
+	ktime_t rq_stime;
+	__be32 rq_xid;
+	u32 rq_vers;
+	u32 rq_prog;
+	u32 rq_proc;
+	/* NFSv4 compund */

nit: compound

Quoted 6 lines
fs/nfsd/nfsd.h +3 -0
+	u32 opnum[NFSD_MAX_OPS_PER_COMPOUND];
+	u16 opcnt;
+};
 
 extern struct svc_program	nfsd_program;
 extern const struct svc_version	nfsd_version2, nfsd_version3, nfsd_version4;

...

#19 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

[...]

Quoted 7 lines

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1652

Quoted 5 lines
fs/nfsd/nfsctl.c +5 -0
+
+#ifdef CONFIG_NFSD_V4
+			if (rqstp->rq_vers == NFS4_VERSION &&
+			    rqstp->rq_proc == NFSPROC4_COMPOUND) {
+				/* NFSv4 compund */

nit: compound

ack, I will fix it.

Quoted 25 lines
Quoted 25 lines
fs/nfsd/nfsctl.c +24 -0
+				struct nfsd4_compoundargs *args;
+				int j;
+
+				args = rqstp->rq_argp;
+				genl_rqstp.opcnt = args->opcnt;
+				for (j = 0; j < genl_rqstp.opcnt; j++)
+					genl_rqstp.opnum[j] =
+						args->ops[j].opnum;
+			}
+#endif /* CONFIG_NFSD_V4 */
+
+			/*
+			 * Acquire rq_status_counter before reporting the rqst
+			 * fields to the user.
+			 */
+			if (smp_load_acquire(&rqstp->rq_status_counter) !=
+			    status_counter)
+				continue;
+
+			ret = nfsd_genl_rpc_status_compose_msg(skb, cb,
+							       &genl_rqstp);
+			if (ret)
+				goto out;
+		}
Quoted 8 lines

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1680

Quoted 4 lines
fs/nfsd/nfsctl.c +4 -0
+	}
+
+	cb->args[0] = i;
+	cb->args[1] = rqstp_index;

I'm unsure if this is possible, but if the for loop above iterates zero times, or for all iterations (i < cb->args[0]), then rqstp_index will be used uninitialised here.

ack, thx for spotting it, I will fix it.

Regards,
Lorenzo

Quoted 45 lines

Flagged by Smatch.

Quoted 6 lines
fs/nfsd/nfsctl.c +6 -0
+	ret = skb->len;
+out:
+	rcu_read_unlock();
+
+	return ret;
+}

...

Quoted 11 lines
fs/nfsd/nfsd.h +3 -0
index 11c14faa6c67..d787bd38c053 100644
@@ -62,6 +62,22 @@ struct readdir_cd {
 	__be32			err;	/* 0, nfserr, or nfserr_eof */
 };
 
+/* Maximum number of operations per session compound */
+#define NFSD_MAX_OPS_PER_COMPOUND	50
+

fs/nfsd/nfsd.h struct readdir_cd { · line 77

Quoted 10 lines
fs/nfsd/nfsd.h +10 -0
+struct nfsd_genl_rqstp {
+	struct sockaddr daddr;
+	struct sockaddr saddr;
+	unsigned long rq_flags;
+	ktime_t rq_stime;
+	__be32 rq_xid;
+	u32 rq_vers;
+	u32 rq_prog;
+	u32 rq_proc;
+	/* NFSv4 compund */

nit: compound

Quoted 6 lines
fs/nfsd/nfsd.h +3 -0
+	u32 opnum[NFSD_MAX_OPS_PER_COMPOUND];
+	u16 opcnt;
+};
 
 extern struct svc_program	nfsd_program;
 extern const struct svc_version	nfsd_version2, nfsd_version3, nfsd_version4;

...

maintainer
#20 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

In reply to Lorenzo Bianconi

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1525

Quoted 5 lines · On Mon, 11 Sep 2023 14:49:46 +0200 Lorenzo Bianconi wrote:
fs/nfsd/nfsctl.c +5 -0
+	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+			  &nfsd_server_nl_family, NLM_F_MULTI,
+			  NFSD_CMD_RPC_STATUS_GET);
+	if (!hdr)
+		return -ENOBUFS;

Why NLM_F_MULTI? AFAIU that means "I'm splitting one object over multiple messages". 99% of the time the right thing to do is change what we consider to be "an object" rather than do F_MULTI. In theory user space should re-constitute all the NLM_F_MULTI messages into as single object, which none of YNL does today :(

#21 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

Quoted 13 lines · On Mon, 11 Sep 2023 14:49:46 +0200 Lorenzo Bianconi wrote:

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1525

Quoted 5 lines
fs/nfsd/nfsctl.c +5 -0
+	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+			  &nfsd_server_nl_family, NLM_F_MULTI,
+			  NFSD_CMD_RPC_STATUS_GET);
+	if (!hdr)
+		return -ENOBUFS;

Why NLM_F_MULTI? AFAIU that means "I'm splitting one object over multiple messages". 99% of the time the right thing to do is change what we consider to be "an object" rather than do F_MULTI. In theory user space should re-constitute all the NLM_F_MULTI messages into as single object, which none of YNL does today :(

ack, fine. I think we can get rid of it.
@chuck: do you want me to send a patch or are you taking care of it?

Regards,
Lorenzo

#22 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

Quoted 11 lines · Lorenzo Bianconi
Quoted 8 lines · On Mon, 11 Sep 2023 14:49:46 +0200 Lorenzo Bianconi wrote:
Quoted 5 lines

+ hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,

fs/nfsd/nfsctl.c +4 -0
+  &nfsd_server_nl_family, NLM_F_MULTI,
+  NFSD_CMD_RPC_STATUS_GET);
+ if (!hdr)
+ return -ENOBUFS;
Quoted 8 lines
Quoted 6 lines

Why NLM_F_MULTI? AFAIU that means "I'm splitting one object over multiple messages". 99% of the time the right thing to do is change what we consider to be "an object" rather than do F_MULTI. In theory user space should re-constitute all the NLM_F_MULTI messages into as single object, which none of YNL does today :(

ack, fine. I think we can get rid of it.
@chuck: do you want me to send a patch or are you taking care of it?

Send a (tested) patch and I can squash it into this one.

--
Chuck Lever

#23 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

Quoted 17 lines
Quoted 17 lines · Lorenzo Bianconi
Quoted 14 lines · On Mon, 11 Sep 2023 14:49:46 +0200 Lorenzo Bianconi wrote:

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1525

Quoted 5 lines

+ hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,

fs/nfsd/nfsctl.c +4 -0
+  &nfsd_server_nl_family, NLM_F_MULTI,
+  NFSD_CMD_RPC_STATUS_GET);
+ if (!hdr)
+ return -ENOBUFS;

Why NLM_F_MULTI? AFAIU that means "I'm splitting one object over multiple messages". 99% of the time the right thing to do is change what we consider to be "an object" rather than do F_MULTI. In theory user space should re-constitute all the NLM_F_MULTI messages into as single object, which none of YNL does today :(

Quoted 4 lines
Quoted 2 lines

ack, fine. I think we can get rid of it.
@chuck: do you want me to send a patch or are you taking care of it?

Send a (tested) patch and I can squash it into this one.

ack, I will do.

Regards,
Lorenzo

Quoted 6 lines

--
Chuck Lever

maintainer
#24 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

In reply to Lorenzo Bianconi
Quoted 32 lines · Lorenzo Bianconi

Introduce rpc_status netlink support for NFSD in order to dump pending RPC requests debugging information from userspace.

---
 fs/nfsd/nfsctl.c           | 192 ++++++++++++++++++++++++++++++++++++-
 fs/nfsd/nfsd.h             |  16 ++++
 fs/nfsd/nfssvc.c           |  15 +++
 fs/nfsd/state.h            |   2 -
 include/linux/sunrpc/svc.h |   1 +
 5 files changed, 222 insertions(+), 4 deletions(-)
fs/nfsd/nfsctl.c +3 -1
index 1be66088849c..b862a759ea15 100644
@@ -26,6 +26,7 @@
 #include "pnfs.h"
 #include "filecache.h"
 #include "trace.h"
+#include "nfs_netlink_gen.h"
 
 /*
  *	We have a single directory with several nodes in it.
@@ -1497,17 +1498,199 @@ unsigned int nfsd_net_id;
 
 int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
 {
-	return 0;
+	struct nfsd_net *nn = net_generic(sock_net(cb->skb->sk), nfsd_net_id);
+	int ret = -ENODEV;

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1512

Quoted 10 lines
fs/nfsd/nfsctl.c +9 -0
+
+	mutex_lock(&nfsd_mutex);
+	if (nn->nfsd_serv) {
+		svc_get(nn->nfsd_serv);
+		ret = 0;
+	}
+	mutex_unlock(&nfsd_mutex);
+
+	return ret;
 }

I think there is a potential race above. Once you've dropped the nfsd_mutex, there is no guarantee that the nn->nfsd_serv will still be set when you come back to put the serv. That means that we could oops when we hit the _done method below.

Is it possible to stash a pointer to the serv while we hold the reference?

Quoted 175 lines
fs/nfsd/nfsctl.c +166 -1
-int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+static int nfsd_genl_rpc_status_compose_msg(struct sk_buff *skb,
+					    struct netlink_callback *cb,
+					    struct nfsd_genl_rqstp *rqstp)
 {
+	void *hdr;
+	int i;
+
+	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+			  &nfsd_server_nl_family, NLM_F_MULTI,
+			  NFSD_CMD_RPC_STATUS_GET);
+	if (!hdr)
+		return -ENOBUFS;
+
+	if (nla_put_be32(skb, NFSD_ATTR_RPC_STATUS_XID, rqstp->rq_xid) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_FLAGS, rqstp->rq_flags) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_PROG, rqstp->rq_prog) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_PROC, rqstp->rq_proc) ||
+	    nla_put_u8(skb, NFSD_ATTR_RPC_STATUS_VERSION, rqstp->rq_vers) ||
+	    nla_put_s64(skb, NFSD_ATTR_RPC_STATUS_SERVICE_TIME,
+			ktime_to_us(rqstp->rq_stime),
+			NFSD_ATTR_RPC_STATUS_PAD))
+		return -ENOBUFS;
+
+	switch (rqstp->saddr.sa_family) {
+	case AF_INET: {
+		const struct sockaddr_in *s_in, *d_in;
+
+		s_in = (const struct sockaddr_in *)&rqstp->saddr;
+		d_in = (const struct sockaddr_in *)&rqstp->daddr;
+		if (nla_put_in_addr(skb, NFSD_ATTR_RPC_STATUS_SADDR4,
+				    s_in->sin_addr.s_addr) ||
+		    nla_put_in_addr(skb, NFSD_ATTR_RPC_STATUS_DADDR4,
+				    d_in->sin_addr.s_addr) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_SPORT,
+				 s_in->sin_port) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_DPORT,
+				 d_in->sin_port))
+			return -ENOBUFS;
+		break;
+	}
+	case AF_INET6: {
+		const struct sockaddr_in6 *s_in, *d_in;
+
+		s_in = (const struct sockaddr_in6 *)&rqstp->saddr;
+		d_in = (const struct sockaddr_in6 *)&rqstp->daddr;
+		if (nla_put_in6_addr(skb, NFSD_ATTR_RPC_STATUS_SADDR6,
+				     &s_in->sin6_addr) ||
+		    nla_put_in6_addr(skb, NFSD_ATTR_RPC_STATUS_DADDR6,
+				     &d_in->sin6_addr) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_SPORT,
+				 s_in->sin6_port) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_DPORT,
+				 d_in->sin6_port))
+			return -ENOBUFS;
+		break;
+	}
+	default:
+		break;
+	}
+
+	if (rqstp->opcnt) {
+		struct nlattr *attr;
+
+		attr = nla_nest_start(skb, NFSD_ATTR_RPC_STATUS_COMPOND_OP);
+		if (!attr)
+			return -ENOBUFS;
+
+		for (i = 0; i < rqstp->opcnt; i++) {
+			struct nlattr *op_attr;
+
+			op_attr = nla_nest_start(skb, i);
+			if (!op_attr)
+				return -ENOBUFS;
+
+			if (nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_COMP_OP,
+					rqstp->opnum[i]))
+				return -ENOBUFS;
+
+			nla_nest_end(skb, op_attr);
+		}
+
+		nla_nest_end(skb, attr);
+	}
+
+	genlmsg_end(skb, hdr);
+
 	return 0;
 }
 
 int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 					 struct netlink_callback *cb)
 {
+	struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
+	int i, ret, rqstp_index;
+
+	rcu_read_lock();
+
+	for (i = 0; i < nn->nfsd_serv->sv_nrpools; i++) {
+		struct svc_rqst *rqstp;
+
+		if (i < cb->args[0]) /* already consumed */
+			continue;
+
+		rqstp_index = 0;
+		list_for_each_entry_rcu(rqstp,
+				&nn->nfsd_serv->sv_pools[i].sp_all_threads,
+				rq_all) {
+			struct nfsd_genl_rqstp genl_rqstp;
+			unsigned int status_counter;
+
+			if (rqstp_index++ < cb->args[1]) /* already consumed */
+				continue;
+			/*
+			 * Acquire rq_status_counter before parsing the rqst
+			 * fields. rq_status_counter is set to an odd value in
+			 * order to notify the consumers the rqstp fields are
+			 * meaningful.
+			 */
+			status_counter =
+				smp_load_acquire(&rqstp->rq_status_counter);
+			if (!(status_counter & 1))
+				continue;
+
+			genl_rqstp.rq_xid = rqstp->rq_xid;
+			genl_rqstp.rq_flags = rqstp->rq_flags;
+			genl_rqstp.rq_vers = rqstp->rq_vers;
+			genl_rqstp.rq_prog = rqstp->rq_prog;
+			genl_rqstp.rq_proc = rqstp->rq_proc;
+			genl_rqstp.rq_stime = rqstp->rq_stime;
+			genl_rqstp.opcnt = 0;
+			memcpy(&genl_rqstp.daddr, svc_daddr(rqstp),
+			       sizeof(struct sockaddr));
+			memcpy(&genl_rqstp.saddr, svc_addr(rqstp),
+			       sizeof(struct sockaddr));
+
+#ifdef CONFIG_NFSD_V4
+			if (rqstp->rq_vers == NFS4_VERSION &&
+			    rqstp->rq_proc == NFSPROC4_COMPOUND) {
+				/* NFSv4 compund */
+				struct nfsd4_compoundargs *args;
+				int j;
+
+				args = rqstp->rq_argp;
+				genl_rqstp.opcnt = args->opcnt;
+				for (j = 0; j < genl_rqstp.opcnt; j++)
+					genl_rqstp.opnum[j] =
+						args->ops[j].opnum;
+			}
+#endif /* CONFIG_NFSD_V4 */
+
+			/*
+			 * Acquire rq_status_counter before reporting the rqst
+			 * fields to the user.
+			 */
+			if (smp_load_acquire(&rqstp->rq_status_counter) !=
+			    status_counter)
+				continue;
+
+			ret = nfsd_genl_rpc_status_compose_msg(skb, cb,
+							       &genl_rqstp);
+			if (ret)
+				goto out;
+		}
+	}
+
+	cb->args[0] = i;
+	cb->args[1] = rqstp_index;
+	ret = skb->len;
+out:
+	rcu_read_unlock();
+
+	return ret;
+}

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1696

Quoted 10 lines
fs/nfsd/nfsctl.c +7 -0
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	mutex_lock(&nfsd_mutex);
+	nfsd_put(sock_net(cb->skb->sk));
+	mutex_unlock(&nfsd_mutex);
+
 	return 0;
 }
 

I think there is a potential race above. Once you've

Quoted 104 lines
fs/nfsd/nfsctl.c +5 -0
@@ -1605,6 +1788,10 @@ static int __init init_nfsd(void)
 	retval = register_filesystem(&nfsd_fs_type);
 	if (retval)
 		goto out_free_all;
+	retval = genl_register_family(&nfsd_server_nl_family);
+	if (retval)
+		goto out_free_all;
+
 	return 0;
 out_free_all:
 	nfsd4_destroy_laundry_wq();
@@ -1629,6 +1816,7 @@ static int __init init_nfsd(void)
 
 static void __exit exit_nfsd(void)
 {
+	genl_unregister_family(&nfsd_server_nl_family);
 	unregister_filesystem(&nfsd_fs_type);
 	nfsd4_destroy_laundry_wq();
 	unregister_cld_notifier();
fs/nfsd/nfsd.h +16 -0
index 11c14faa6c67..d787bd38c053 100644
@@ -62,6 +62,22 @@ struct readdir_cd {
 	__be32			err;	/* 0, nfserr, or nfserr_eof */
 };
 
+/* Maximum number of operations per session compound */
+#define NFSD_MAX_OPS_PER_COMPOUND	50
+
+struct nfsd_genl_rqstp {
+	struct sockaddr daddr;
+	struct sockaddr saddr;
+	unsigned long rq_flags;
+	ktime_t rq_stime;
+	__be32 rq_xid;
+	u32 rq_vers;
+	u32 rq_prog;
+	u32 rq_proc;
+	/* NFSv4 compund */
+	u32 opnum[NFSD_MAX_OPS_PER_COMPOUND];
+	u16 opcnt;
+};
 
 extern struct svc_program	nfsd_program;
 extern const struct svc_version	nfsd_version2, nfsd_version3, nfsd_version4;
fs/nfsd/nfssvc.c +15 -0
index 1582af33e204..fad34a7325b3 100644
@@ -998,6 +998,15 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream))
 		goto out_decode_err;
 
+	/*
+	 * Release rq_status_counter setting it to an odd value after the rpc
+	 * request has been properly parsed. rq_status_counter is used to
+	 * notify the consumers if the rqstp fields are stable
+	 * (rq_status_counter is odd) or not meaningful (rq_status_counter
+	 * is even).
+	 */
+	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter | 1);
+
 	rp = NULL;
 	switch (nfsd_cache_lookup(rqstp, &rp)) {
 	case RC_DOIT:
@@ -1015,6 +1024,12 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_encode(rqstp, &rqstp->rq_res_stream))
 		goto out_encode_err;
 
+	/*
+	 * Release rq_status_counter setting it to an even value after the rpc
+	 * request has been properly processed.
+	 */
+	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter + 1);
+
 	nfsd_cache_update(rqstp, rp, rqstp->rq_cachetype, statp + 1);
 out_cached_reply:
 	return 1;
fs/nfsd/state.h +0 -2
index cbddcf484dba..41bdc913fa71 100644
@@ -174,8 +174,6 @@ static inline struct nfs4_delegation *delegstateid(struct nfs4_stid *s)
 
 /* Maximum number of slots per session. 160 is useful for long haul TCP */
 #define NFSD_MAX_SLOTS_PER_SESSION     160
-/* Maximum number of operations per session compound */
-#define NFSD_MAX_OPS_PER_COMPOUND	50
 /* Maximum  session per slot cache size */
 #define NFSD_SLOT_CACHE_SIZE		2048
 /* Maximum number of NFSD_SLOT_CACHE_SIZE slots per session */
include/linux/sunrpc/svc.h +1 -0
index dbf5b21feafe..caa20defd255 100644
@@ -251,6 +251,7 @@ struct svc_rqst {
 						 * net namespace
 						 */
 	void **			rq_lease_breaker; /* The v4 client breaking a lease */
+	unsigned int		rq_status_counter; /* RPC processing counter */
 };
 
 /* bits for rq_flags */
Signature · 1 line
Jeff Layton <[email protected]>
maintainer
#25 lore raw

Re: [PATCH v8 3/3] NFSD: add rpc_status netlink support

Quoted 49 lines · Jeff Layton
Quoted 32 lines · Lorenzo Bianconi

Introduce rpc_status netlink support for NFSD in order to dump pending RPC requests debugging information from userspace.

---
 fs/nfsd/nfsctl.c           | 192 ++++++++++++++++++++++++++++++++++++-
 fs/nfsd/nfsd.h             |  16 ++++
 fs/nfsd/nfssvc.c           |  15 +++
 fs/nfsd/state.h            |   2 -
 include/linux/sunrpc/svc.h |   1 +
 5 files changed, 222 insertions(+), 4 deletions(-)
fs/nfsd/nfsctl.c +3 -1
index 1be66088849c..b862a759ea15 100644
@@ -26,6 +26,7 @@
 #include "pnfs.h"
 #include "filecache.h"
 #include "trace.h"
+#include "nfs_netlink_gen.h"
 
 /*
  *	We have a single directory with several nodes in it.
@@ -1497,17 +1498,199 @@ unsigned int nfsd_net_id;
 
 int nfsd_server_nl_rpc_status_get_start(struct netlink_callback *cb)
 {
-	return 0;
+	struct nfsd_net *nn = net_generic(sock_net(cb->skb->sk), nfsd_net_id);
+	int ret = -ENODEV;

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1512

Quoted 10 lines
fs/nfsd/nfsctl.c +9 -0
+
+	mutex_lock(&nfsd_mutex);
+	if (nn->nfsd_serv) {
+		svc_get(nn->nfsd_serv);
+		ret = 0;
+	}
+	mutex_unlock(&nfsd_mutex);
+
+	return ret;
 }

I think there is a potential race above. Once you've dropped the nfsd_mutex, there is no guarantee that the nn->nfsd_serv will still be set when you come back to put the serv. That means that we could oops when we hit the _done method below.

Quoted 3 lines

Is it possible to stash a pointer to the serv while we hold the reference?

Actually, it looks like Neil may have already fixed this in the series he sent on Oct 29th. See:

[PATCH 3/5] nfsd: hold nfsd_mutex across entire netlink operation

Another reason to go ahead and get that series in...

Quoted 294 lines
Quoted 175 lines
fs/nfsd/nfsctl.c +166 -1
-int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+static int nfsd_genl_rpc_status_compose_msg(struct sk_buff *skb,
+					    struct netlink_callback *cb,
+					    struct nfsd_genl_rqstp *rqstp)
 {
+	void *hdr;
+	int i;
+
+	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+			  &nfsd_server_nl_family, NLM_F_MULTI,
+			  NFSD_CMD_RPC_STATUS_GET);
+	if (!hdr)
+		return -ENOBUFS;
+
+	if (nla_put_be32(skb, NFSD_ATTR_RPC_STATUS_XID, rqstp->rq_xid) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_FLAGS, rqstp->rq_flags) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_PROG, rqstp->rq_prog) ||
+	    nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_PROC, rqstp->rq_proc) ||
+	    nla_put_u8(skb, NFSD_ATTR_RPC_STATUS_VERSION, rqstp->rq_vers) ||
+	    nla_put_s64(skb, NFSD_ATTR_RPC_STATUS_SERVICE_TIME,
+			ktime_to_us(rqstp->rq_stime),
+			NFSD_ATTR_RPC_STATUS_PAD))
+		return -ENOBUFS;
+
+	switch (rqstp->saddr.sa_family) {
+	case AF_INET: {
+		const struct sockaddr_in *s_in, *d_in;
+
+		s_in = (const struct sockaddr_in *)&rqstp->saddr;
+		d_in = (const struct sockaddr_in *)&rqstp->daddr;
+		if (nla_put_in_addr(skb, NFSD_ATTR_RPC_STATUS_SADDR4,
+				    s_in->sin_addr.s_addr) ||
+		    nla_put_in_addr(skb, NFSD_ATTR_RPC_STATUS_DADDR4,
+				    d_in->sin_addr.s_addr) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_SPORT,
+				 s_in->sin_port) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_DPORT,
+				 d_in->sin_port))
+			return -ENOBUFS;
+		break;
+	}
+	case AF_INET6: {
+		const struct sockaddr_in6 *s_in, *d_in;
+
+		s_in = (const struct sockaddr_in6 *)&rqstp->saddr;
+		d_in = (const struct sockaddr_in6 *)&rqstp->daddr;
+		if (nla_put_in6_addr(skb, NFSD_ATTR_RPC_STATUS_SADDR6,
+				     &s_in->sin6_addr) ||
+		    nla_put_in6_addr(skb, NFSD_ATTR_RPC_STATUS_DADDR6,
+				     &d_in->sin6_addr) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_SPORT,
+				 s_in->sin6_port) ||
+		    nla_put_be16(skb, NFSD_ATTR_RPC_STATUS_DPORT,
+				 d_in->sin6_port))
+			return -ENOBUFS;
+		break;
+	}
+	default:
+		break;
+	}
+
+	if (rqstp->opcnt) {
+		struct nlattr *attr;
+
+		attr = nla_nest_start(skb, NFSD_ATTR_RPC_STATUS_COMPOND_OP);
+		if (!attr)
+			return -ENOBUFS;
+
+		for (i = 0; i < rqstp->opcnt; i++) {
+			struct nlattr *op_attr;
+
+			op_attr = nla_nest_start(skb, i);
+			if (!op_attr)
+				return -ENOBUFS;
+
+			if (nla_put_u32(skb, NFSD_ATTR_RPC_STATUS_COMP_OP,
+					rqstp->opnum[i]))
+				return -ENOBUFS;
+
+			nla_nest_end(skb, op_attr);
+		}
+
+		nla_nest_end(skb, attr);
+	}
+
+	genlmsg_end(skb, hdr);
+
 	return 0;
 }
 
 int nfsd_server_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 					 struct netlink_callback *cb)
 {
+	struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
+	int i, ret, rqstp_index;
+
+	rcu_read_lock();
+
+	for (i = 0; i < nn->nfsd_serv->sv_nrpools; i++) {
+		struct svc_rqst *rqstp;
+
+		if (i < cb->args[0]) /* already consumed */
+			continue;
+
+		rqstp_index = 0;
+		list_for_each_entry_rcu(rqstp,
+				&nn->nfsd_serv->sv_pools[i].sp_all_threads,
+				rq_all) {
+			struct nfsd_genl_rqstp genl_rqstp;
+			unsigned int status_counter;
+
+			if (rqstp_index++ < cb->args[1]) /* already consumed */
+				continue;
+			/*
+			 * Acquire rq_status_counter before parsing the rqst
+			 * fields. rq_status_counter is set to an odd value in
+			 * order to notify the consumers the rqstp fields are
+			 * meaningful.
+			 */
+			status_counter =
+				smp_load_acquire(&rqstp->rq_status_counter);
+			if (!(status_counter & 1))
+				continue;
+
+			genl_rqstp.rq_xid = rqstp->rq_xid;
+			genl_rqstp.rq_flags = rqstp->rq_flags;
+			genl_rqstp.rq_vers = rqstp->rq_vers;
+			genl_rqstp.rq_prog = rqstp->rq_prog;
+			genl_rqstp.rq_proc = rqstp->rq_proc;
+			genl_rqstp.rq_stime = rqstp->rq_stime;
+			genl_rqstp.opcnt = 0;
+			memcpy(&genl_rqstp.daddr, svc_daddr(rqstp),
+			       sizeof(struct sockaddr));
+			memcpy(&genl_rqstp.saddr, svc_addr(rqstp),
+			       sizeof(struct sockaddr));
+
+#ifdef CONFIG_NFSD_V4
+			if (rqstp->rq_vers == NFS4_VERSION &&
+			    rqstp->rq_proc == NFSPROC4_COMPOUND) {
+				/* NFSv4 compund */
+				struct nfsd4_compoundargs *args;
+				int j;
+
+				args = rqstp->rq_argp;
+				genl_rqstp.opcnt = args->opcnt;
+				for (j = 0; j < genl_rqstp.opcnt; j++)
+					genl_rqstp.opnum[j] =
+						args->ops[j].opnum;
+			}
+#endif /* CONFIG_NFSD_V4 */
+
+			/*
+			 * Acquire rq_status_counter before reporting the rqst
+			 * fields to the user.
+			 */
+			if (smp_load_acquire(&rqstp->rq_status_counter) !=
+			    status_counter)
+				continue;
+
+			ret = nfsd_genl_rpc_status_compose_msg(skb, cb,
+							       &genl_rqstp);
+			if (ret)
+				goto out;
+		}
+	}
+
+	cb->args[0] = i;
+	cb->args[1] = rqstp_index;
+	ret = skb->len;
+out:
+	rcu_read_unlock();
+
+	return ret;
+}

fs/nfsd/nfsctl.c unsigned int nfsd_net_id; · line 1696

Quoted 10 lines
fs/nfsd/nfsctl.c +7 -0
+
+int nfsd_server_nl_rpc_status_get_done(struct netlink_callback *cb)
+{
+	mutex_lock(&nfsd_mutex);
+	nfsd_put(sock_net(cb->skb->sk));
+	mutex_unlock(&nfsd_mutex);
+
 	return 0;
 }
 

I think there is a potential race above. Once you've

Quoted 104 lines
fs/nfsd/nfsctl.c +5 -0
@@ -1605,6 +1788,10 @@ static int __init init_nfsd(void)
 	retval = register_filesystem(&nfsd_fs_type);
 	if (retval)
 		goto out_free_all;
+	retval = genl_register_family(&nfsd_server_nl_family);
+	if (retval)
+		goto out_free_all;
+
 	return 0;
 out_free_all:
 	nfsd4_destroy_laundry_wq();
@@ -1629,6 +1816,7 @@ static int __init init_nfsd(void)
 
 static void __exit exit_nfsd(void)
 {
+	genl_unregister_family(&nfsd_server_nl_family);
 	unregister_filesystem(&nfsd_fs_type);
 	nfsd4_destroy_laundry_wq();
 	unregister_cld_notifier();
fs/nfsd/nfsd.h +16 -0
index 11c14faa6c67..d787bd38c053 100644
@@ -62,6 +62,22 @@ struct readdir_cd {
 	__be32			err;	/* 0, nfserr, or nfserr_eof */
 };
 
+/* Maximum number of operations per session compound */
+#define NFSD_MAX_OPS_PER_COMPOUND	50
+
+struct nfsd_genl_rqstp {
+	struct sockaddr daddr;
+	struct sockaddr saddr;
+	unsigned long rq_flags;
+	ktime_t rq_stime;
+	__be32 rq_xid;
+	u32 rq_vers;
+	u32 rq_prog;
+	u32 rq_proc;
+	/* NFSv4 compund */
+	u32 opnum[NFSD_MAX_OPS_PER_COMPOUND];
+	u16 opcnt;
+};
 
 extern struct svc_program	nfsd_program;
 extern const struct svc_version	nfsd_version2, nfsd_version3, nfsd_version4;
fs/nfsd/nfssvc.c +15 -0
index 1582af33e204..fad34a7325b3 100644
@@ -998,6 +998,15 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream))
 		goto out_decode_err;
 
+	/*
+	 * Release rq_status_counter setting it to an odd value after the rpc
+	 * request has been properly parsed. rq_status_counter is used to
+	 * notify the consumers if the rqstp fields are stable
+	 * (rq_status_counter is odd) or not meaningful (rq_status_counter
+	 * is even).
+	 */
+	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter | 1);
+
 	rp = NULL;
 	switch (nfsd_cache_lookup(rqstp, &rp)) {
 	case RC_DOIT:
@@ -1015,6 +1024,12 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	if (!proc->pc_encode(rqstp, &rqstp->rq_res_stream))
 		goto out_encode_err;
 
+	/*
+	 * Release rq_status_counter setting it to an even value after the rpc
+	 * request has been properly processed.
+	 */
+	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter + 1);
+
 	nfsd_cache_update(rqstp, rp, rqstp->rq_cachetype, statp + 1);
 out_cached_reply:
 	return 1;
fs/nfsd/state.h +0 -2
index cbddcf484dba..41bdc913fa71 100644
@@ -174,8 +174,6 @@ static inline struct nfs4_delegation *delegstateid(struct nfs4_stid *s)
 
 /* Maximum number of slots per session. 160 is useful for long haul TCP */
 #define NFSD_MAX_SLOTS_PER_SESSION     160
-/* Maximum number of operations per session compound */
-#define NFSD_MAX_OPS_PER_COMPOUND	50
 /* Maximum  session per slot cache size */
 #define NFSD_SLOT_CACHE_SIZE		2048
 /* Maximum number of NFSD_SLOT_CACHE_SIZE slots per session */
include/linux/sunrpc/svc.h +1 -0
index dbf5b21feafe..caa20defd255 100644
@@ -251,6 +251,7 @@ struct svc_rqst {
 						 * net namespace
 						 */
 	void **			rq_lease_breaker; /* The v4 client breaking a lease */
+	unsigned int		rq_status_counter; /* RPC processing counter */
 };
 
 /* bits for rq_flags */
Signature · 1 line
Jeff Layton <[email protected]>
#26 lore raw
In reply to Lorenzo Bianconi
Quoted 49 lines · Lorenzo Bianconi

Introduce rpc_status netlink support for NFSD in order to dump pending RPC requests debugging information from userspace.
The new code can be tested with the user-space tool reported below:
https://github.com/LorenzoBianconi/nfsd-rpc-netlink-monitor/tree/main

Changes since v7:
- introduce nfsd_server.yaml for netlink messages

Changes since v6:
- report info to user-space through netlink and get rid of the related entry in the procfs

Changes since v5:
- add missing delimiters for nfs4 compound ops
- add a header line for rpc_status handler
- do not dump rq_prog
- do not dump rq_flags in hex

Changes since v4:
- rely on acquire/release APIs and get rid of atomic operation
- fix kdoc for nfsd_rpc_status_open
- get rid of ',' as field delimiter in nfsd_rpc_status hanlder
- move nfsd_rpc_status before nfsd_v4 enum entries
- fix compilantion error if nfsdv4 is not enabled

Changes since v3:
- introduce rq_status_counter in order to detect if the RPC request is pending and RPC info are stable
- rely on __svc_print_addr to dump IP info

Changes since v2:
- minor changes in nfsd_rpc_status_show output

Changes since v1:
- rework nfsd_rpc_status_show output

Changes since RFCv1:
- riduce time holding nfsd_mutex bumping svc_serv refcoung in nfsd_rpc_status_open()
- dump rqstp->rq_stime
- add missing kdoc for nfsd_rpc_status_open()

Lorenzo Bianconi (3):
Documentation: netlink: add a YAML spec for nfsd_server
NFSD: introduce netlink rpc_status stubs
NFSD: add rpc_status netlink support

Quoted 15 lines
 Documentation/netlink/specs/nfsd_server.yaml |  97 +++++++++
 fs/nfsd/Makefile                             |   3 +-
 fs/nfsd/nfs_netlink_gen.c                    |  32 +++
 fs/nfsd/nfs_netlink_gen.h                    |  22 ++
 fs/nfsd/nfsctl.c                             | 204 +++++++++++++++++++
 fs/nfsd/nfsd.h                               |  16 ++
 fs/nfsd/nfssvc.c                             |  15 ++
 fs/nfsd/state.h                              |   2 -
 include/linux/sunrpc/svc.h                   |   1 +
 include/uapi/linux/nfsd_server.h             |  49 +++++
 10 files changed, 438 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/netlink/specs/nfsd_server.yaml
 create mode 100644 fs/nfsd/nfs_netlink_gen.c
 create mode 100644 fs/nfsd/nfs_netlink_gen.h
 create mode 100644 include/uapi/linux/nfsd_server.h

Hi Lorenzo -

I've applied these three to nfsd-next with the following changes:

- Renaming as we discussed
- Replaced the nested compound_op attribute -- may require some user space tooling changes
- Simon's Smatch bug fixed
- Squashed 1/3 and 2/3 into one patch
- Added Closes/Acked-by etc

If you spot a bug, send patches against nfsd-next and I can squash them in.

I was wondering if you have a little more time to try adding one or two control cmds. write_threads and v4_end_grace might be simple ones to start with. No problem if you are "done" with this project, I can add these over time.

Signature · 1 line
Chuck Lever
#27 lore raw

[...]

Quoted 18 lines
Quoted 7 lines
 Documentation/netlink/specs/nfsd_server.yaml |  97 +++++++++
 fs/nfsd/Makefile                             |   3 +-
 fs/nfsd/nfs_netlink_gen.c                    |  32 +++
 fs/nfsd/nfs_netlink_gen.h                    |  22 ++
 fs/nfsd/nfsctl.c                             | 204 +++++++++++++++++++
 fs/nfsd/nfsd.h                               |  16 ++
 fs/nfsd/nfssvc.c                             |  15 ++
Quoted 8 lines
 fs/nfsd/state.h                              |   2 -
 include/linux/sunrpc/svc.h                   |   1 +
 include/uapi/linux/nfsd_server.h             |  49 +++++
 10 files changed, 438 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/netlink/specs/nfsd_server.yaml
 create mode 100644 fs/nfsd/nfs_netlink_gen.c
 create mode 100644 fs/nfsd/nfs_netlink_gen.h
 create mode 100644 include/uapi/linux/nfsd_server.h

Hi Lorenzo -

Quoted 8 lines

I've applied these three to nfsd-next with the following changes:

- Renaming as we discussed
- Replaced the nested compound_op attribute -- may require some user space tooling changes
- Simon's Smatch bug fixed
- Squashed 1/3 and 2/3 into one patch
- Added Closes/Acked-by etc

Hi Chuck,

Thanks for addressing the points above.

Quoted 3 lines

If you spot a bug, send patches against nfsd-next and I can squash them in.

ack, I will do

Quoted 5 lines

I was wondering if you have a little more time to try adding one or two control cmds. write_threads and v4_end_grace might be simple ones to start with. No problem if you are "done" with this project, I can add these over time.

sure, no worries. I will look into them soon :)

Regards,
Lorenzo

Quoted 3 lines
Signature · 2 lines
-- 
Chuck Lever