> On Jan 24, 2024, at 3:34 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Tue, 2024-01-23 at 11:12 -0500, Chuck Lever wrote:
>> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>>
>> This enables us to ensure that testing properly covers the readv
>> paths as well as the spliced read paths, which are more commonly
>> used. Also this makes it easier to do benchmark comparisons between
>> splice and vectored reads.
>>
>> Suggested-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> Documentation/netlink/specs/nfsd.yaml | 19 +++++++
>> fs/nfsd/netlink.c | 17 ++++++
>> fs/nfsd/netlink.h | 3 +
>> fs/nfsd/netns.h | 1
>> fs/nfsd/nfsctl.c | 45 ++++++++++++++++
>> include/uapi/linux/nfsd_netlink.h | 8 +++
>> tools/net/ynl/generated/nfsd-user.c | 95 +++++++++++++++++++++++++++++++++
>> tools/net/ynl/generated/nfsd-user.h | 47 ++++++++++++++++
>> 8 files changed, 235 insertions(+)
>>
>
> I think this makes sense. How are you testing it,
So far it's only been regression testing. Nothing breaks.
> and how do you forsee
> us using this interface? Will we need a new program in nfs-utils or will
> you extend an existing one?
We could add a command line option to rpc.nfsd. That doesn't
seem convenient for automated testers, though.
>> diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
>> index 05acc73e2e33..1a3c5e78b388 100644
>> --- a/Documentation/netlink/specs/nfsd.yaml
>> +++ b/Documentation/netlink/specs/nfsd.yaml
>> @@ -62,6 +62,12 @@ attribute-sets:
>> name: compound-ops
>> type: u32
>> multi-attr: true
>> + -
>> + name: splice-read
>> + attributes:
>> + -
>> + name: enabled
>> + type: u32
>>
>> operations:
>> list:
>> @@ -87,3 +93,16 @@ operations:
>> - sport
>> - dport
>> - compound-ops
>> + -
>> + name: splice-read
>> + doc: Disable the use of splice for NFS READ operations
>> + attribute-set: splice-read
>> + flags: [ admin-perm ]
>> + do:
>> + request:
>> + attributes:
>> + - enabled
>> + dump:
>> + reply:
>> + attributes:
>> + - enabled
>> diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
>> index 0e1d635ec5f9..c47f3527d30b 100644
>> --- a/fs/nfsd/netlink.c
>> +++ b/fs/nfsd/netlink.c
>> @@ -10,6 +10,11 @@
>>
>> #include <uapi/linux/nfsd_netlink.h>
>>
>> +/* NFSD_CMD_SPLICE_READ - do */
>> +static const struct nla_policy nfsd_splice_read_nl_policy[NFSD_A_SPLICE_READ_ENABLED + 1] = {
>> + [NFSD_A_SPLICE_READ_ENABLED] = { .type = NLA_U32, },
>> +};
>> +
>> /* Ops table for nfsd */
>> static const struct genl_split_ops nfsd_nl_ops[] = {
>> {
>> @@ -19,6 +24,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
>> .done = nfsd_nl_rpc_status_get_done,
>> .flags = GENL_CMD_CAP_DUMP,
>> },
>> + {
>> + .cmd = NFSD_CMD_SPLICE_READ,
>> + .doit = nfsd_nl_splice_read_doit,
>> + .policy = nfsd_splice_read_nl_policy,
>> + .maxattr = NFSD_A_SPLICE_READ_ENABLED,
>> + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
>> + },
>> + {
>> + .cmd = NFSD_CMD_SPLICE_READ,
>> + .dumpit = nfsd_nl_splice_read_dumpit,
>> + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DUMP,
>> + },
>> };
>>
>> struct genl_family nfsd_nl_family __ro_after_init = {
>> diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
>> index d83dd6bdee92..2d96d0f093bb 100644
>> --- a/fs/nfsd/netlink.h
>> +++ b/fs/nfsd/netlink.h
>> @@ -16,6 +16,9 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
>>
>> int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
>> struct netlink_callback *cb);
>> +int nfsd_nl_splice_read_doit(struct sk_buff *skb, struct genl_info *info);
>> +int nfsd_nl_splice_read_dumpit(struct sk_buff *skb,
>> + struct netlink_callback *cb);
>>
>> extern struct genl_family nfsd_nl_family;
>>
>> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
>> index 74b4360779a1..3b9e09fecbfc 100644
>> --- a/fs/nfsd/netns.h
>> +++ b/fs/nfsd/netns.h
>> @@ -109,6 +109,7 @@ struct nfsd_net {
>>
>> bool nfsd_net_up;
>> bool lockd_up;
>> + bool spliced_reads;
>>
>> seqlock_t writeverf_lock;
>> unsigned char writeverf[8];
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index 8e6dbe9e0b65..86f466dbc784 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -1696,6 +1696,51 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
>> return 0;
>> }
>>
>> +/**
>> + * nfsd_nl_splice_read_doit - Set the value of splice_read
>> + * @skb: call buffer
>> + * @info: netlink metadata and command arguments
>> + *
>> + * Returns zero on success, or a negative errno.
>> + */
>> +int nfsd_nl_splice_read_doit(struct sk_buff *skb, struct genl_info *info)
>> +{
>> + struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
>> + u32 newval;
>> +
>> + if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SPLICE_READ_ENABLED))
>> + return -EINVAL;
>> +
>> + newval = nla_get_u32(info->attrs[NFSD_A_SPLICE_READ_ENABLED]);
>> + nn->spliced_reads = newval ? true : false;
>> + return 0;
>> +}
>> +
>> +/**
>> + * nfsd_nl_splice_read_dumpit - Return the value of splice_read
>> + * @skb: reply buffer
>> + * @cb: netlink metadata and command arguments
>> + *
>> + * Returns the size of the reply or a negative errno.
>> + */
>> +int nfsd_nl_splice_read_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
>> +{
>> + struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
>> + void *hdr;
>> +
>> + hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
>> + &nfsd_nl_family, 0, NFSD_CMD_SPLICE_READ);
>> + if (!hdr)
>> + return -ENOBUFS;
>> +
>> + if (nla_put_s32(skb, NFSD_A_SPLICE_READ_ENABLED,
>> + (nn->spliced_reads ? 1 : 0)))
>> + return -ENOBUFS;
>> +
>> + genlmsg_end(skb, hdr);
>> + return 0;
>> +}
>> +
>> /**
>> * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
>> * @net: a freshly-created network namespace
>> diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
>> index 3cd044edee5d..c2542ed18b50 100644
>> --- a/include/uapi/linux/nfsd_netlink.h
>> +++ b/include/uapi/linux/nfsd_netlink.h
>> @@ -29,8 +29,16 @@ enum {
>> NFSD_A_RPC_STATUS_MAX = (__NFSD_A_RPC_STATUS_MAX - 1)
>> };
>>
>> +enum {
>> + NFSD_A_SPLICE_READ_ENABLED = 1,
>> +
>> + __NFSD_A_SPLICE_READ_MAX,
>> + NFSD_A_SPLICE_READ_MAX = (__NFSD_A_SPLICE_READ_MAX - 1)
>> +};
>> +
>> enum {
>> NFSD_CMD_RPC_STATUS_GET = 1,
>> + NFSD_CMD_SPLICE_READ,
>>
>> __NFSD_CMD_MAX,
>> NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
>> diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
>> index 360b6448c6e9..14957bdfbe9c 100644
>> --- a/tools/net/ynl/generated/nfsd-user.c
>> +++ b/tools/net/ynl/generated/nfsd-user.c
>> @@ -15,6 +15,7 @@
>> /* Enums */
>> static const char * const nfsd_op_strmap[] = {
>> [NFSD_CMD_RPC_STATUS_GET] = "rpc-status-get",
>> + [NFSD_CMD_SPLICE_READ] = "splice-read",
>> };
>>
>> const char *nfsd_op_str(int op)
>> @@ -47,6 +48,15 @@ struct ynl_policy_nest nfsd_rpc_status_nest = {
>> .table = nfsd_rpc_status_policy,
>> };
>>
>> +struct ynl_policy_attr nfsd_splice_read_policy[NFSD_A_SPLICE_READ_MAX + 1] = {
>> + [NFSD_A_SPLICE_READ_ENABLED] = { .name = "enabled", .type = YNL_PT_U32, },
>> +};
>> +
>> +struct ynl_policy_nest nfsd_splice_read_nest = {
>> + .max_attr = NFSD_A_SPLICE_READ_MAX,
>> + .table = nfsd_splice_read_policy,
>> +};
>> +
>> /* Common nested types */
>> /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
>> /* NFSD_CMD_RPC_STATUS_GET - dump */
>> @@ -198,6 +208,91 @@ nfsd_rpc_status_get_dump(struct ynl_sock *ys)
>> return NULL;
>> }
>>
>> +/* ============== NFSD_CMD_SPLICE_READ ============== */
>> +/* NFSD_CMD_SPLICE_READ - do */
>> +void nfsd_splice_read_req_free(struct nfsd_splice_read_req *req)
>> +{
>> + free(req);
>> +}
>> +
>> +int nfsd_splice_read(struct ynl_sock *ys, struct nfsd_splice_read_req *req)
>> +{
>> + struct nlmsghdr *nlh;
>> + int err;
>> +
>> + nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_SPLICE_READ, 1);
>> + ys->req_policy = &nfsd_splice_read_nest;
>> +
>> + if (req->_present.enabled)
>> + mnl_attr_put_u32(nlh, NFSD_A_SPLICE_READ_ENABLED, req->enabled);
>> +
>> + err = ynl_exec(ys, nlh, NULL);
>> + if (err < 0)
>> + return -1;
>> +
>> + return 0;
>> +}
>> +
>> +/* NFSD_CMD_SPLICE_READ - dump */
>> +int nfsd_splice_read_rsp_dump_parse(const struct nlmsghdr *nlh, void *data)
>> +{
>> + struct nfsd_splice_read_rsp_dump *dst;
>> + struct ynl_parse_arg *yarg = data;
>> + const struct nlattr *attr;
>> +
>> + dst = yarg->data;
>> +
>> + mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
>> + unsigned int type = mnl_attr_get_type(attr);
>> +
>> + if (type == NFSD_A_SPLICE_READ_ENABLED) {
>> + if (ynl_attr_validate(yarg, attr))
>> + return MNL_CB_ERROR;
>> + dst->_present.enabled = 1;
>> + dst->enabled = mnl_attr_get_u32(attr);
>> + }
>> + }
>> +
>> + return MNL_CB_OK;
>> +}
>> +
>> +void nfsd_splice_read_rsp_list_free(struct nfsd_splice_read_rsp_list *rsp)
>> +{
>> + struct nfsd_splice_read_rsp_list *next = rsp;
>> +
>> + while ((void *)next != YNL_LIST_END) {
>> + rsp = next;
>> + next = rsp->next;
>> +
>> + free(rsp);
>> + }
>> +}
>> +
>> +struct nfsd_splice_read_rsp_list *nfsd_splice_read_dump(struct ynl_sock *ys)
>> +{
>> + struct ynl_dump_state yds = {};
>> + struct nlmsghdr *nlh;
>> + int err;
>> +
>> + yds.ys = ys;
>> + yds.alloc_sz = sizeof(struct nfsd_splice_read_rsp_list);
>> + yds.cb = nfsd_splice_read_rsp_dump_parse;
>> + yds.rsp_cmd = NFSD_CMD_SPLICE_READ;
>> + yds.rsp_policy = &nfsd_splice_read_nest;
>> +
>> + nlh = ynl_gemsg_start_dump(ys, ys->family_id, NFSD_CMD_SPLICE_READ, 1);
>> +
>> + err = ynl_exec_dump(ys, nlh, &yds);
>> + if (err < 0)
>> + goto free_list;
>> +
>> + return yds.first;
>> +
>> +free_list:
>> + nfsd_splice_read_rsp_list_free(yds.first);
>> + return NULL;
>> +}
>> +
>> const struct ynl_family ynl_nfsd_family = {
>> .name = "nfsd",
>> };
>> diff --git a/tools/net/ynl/generated/nfsd-user.h b/tools/net/ynl/generated/nfsd-user.h
>> index 989c6e209ced..5732c5a665e7 100644
>> --- a/tools/net/ynl/generated/nfsd-user.h
>> +++ b/tools/net/ynl/generated/nfsd-user.h
>> @@ -64,4 +64,51 @@ nfsd_rpc_status_get_rsp_list_free(struct nfsd_rpc_status_get_rsp_list *rsp);
>> struct nfsd_rpc_status_get_rsp_list *
>> nfsd_rpc_status_get_dump(struct ynl_sock *ys);
>>
>> +/* ============== NFSD_CMD_SPLICE_READ ============== */
>> +/* NFSD_CMD_SPLICE_READ - do */
>> +struct nfsd_splice_read_req {
>> + struct {
>> + __u32 enabled:1;
>> + } _present;
>> +
>> + __u32 enabled;
>> +};
>> +
>> +static inline struct nfsd_splice_read_req *nfsd_splice_read_req_alloc(void)
>> +{
>> + return calloc(1, sizeof(struct nfsd_splice_read_req));
>> +}
>> +void nfsd_splice_read_req_free(struct nfsd_splice_read_req *req);
>> +
>> +static inline void
>> +nfsd_splice_read_req_set_enabled(struct nfsd_splice_read_req *req,
>> + __u32 enabled)
>> +{
>> + req->_present.enabled = 1;
>> + req->enabled = enabled;
>> +}
>> +
>> +/*
>> + * Disable the use of splice for NFS READ operations
>> + */
>> +int nfsd_splice_read(struct ynl_sock *ys, struct nfsd_splice_read_req *req);
>> +
>> +/* NFSD_CMD_SPLICE_READ - dump */
>> +struct nfsd_splice_read_rsp_dump {
>> + struct {
>> + __u32 enabled:1;
>> + } _present;
>> +
>> + __u32 enabled;
>> +};
>> +
>> +struct nfsd_splice_read_rsp_list {
>> + struct nfsd_splice_read_rsp_list *next;
>> + struct nfsd_splice_read_rsp_dump obj __attribute__ ((aligned (8)));
>> +};
>> +
>> +void nfsd_splice_read_rsp_list_free(struct nfsd_splice_read_rsp_list *rsp);
>> +
>> +struct nfsd_splice_read_rsp_list *nfsd_splice_read_dump(struct ynl_sock *ys);
>> +
>> #endif /* _LINUX_NFSD_GEN_H */
>>
>>
>>
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
--
Chuck Lever
