On Mon, 2024-08-19 at 14:17 -0400, Mike Snitzer wrote:
> From: NeilBrown <neil@xxxxxxxxxx>
>
> A service created with svc_create_pooled() can be given a linked list of
> programs and all of these will be served.
>
> Using a linked list makes it cumbersome when there are several programs
> that can be optionally selected with CONFIG settings.
>
> After this patch is applied, API consumers must use only
> svc_create_pooled() when creating an RPC service that listens for more
> than one RPC program.
>
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> ---
> fs/nfsd/nfsctl.c | 2 +-
> fs/nfsd/nfsd.h | 2 +-
> fs/nfsd/nfssvc.c | 67 +++++++++++++++++--------------------
> include/linux/sunrpc/svc.h | 7 ++--
> net/sunrpc/svc.c | 68 ++++++++++++++++++++++----------------
> net/sunrpc/svc_xprt.c | 2 +-
> net/sunrpc/svcauth_unix.c | 3 +-
> 7 files changed, 79 insertions(+), 72 deletions(-)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 1c9e5b4bcb0a..64c1b4d649bc 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -2246,7 +2246,7 @@ static __net_init int nfsd_net_init(struct net *net)
> if (retval)
> goto out_repcache_error;
> memset(&nn->nfsd_svcstats, 0, sizeof(nn->nfsd_svcstats));
> - nn->nfsd_svcstats.program = &nfsd_program;
> + nn->nfsd_svcstats.program = &nfsd_programs[0];
> for (i = 0; i < sizeof(nn->nfsd_versions); i++)
> nn->nfsd_versions[i] = nfsd_support_version(i);
> for (i = 0; i < sizeof(nn->nfsd4_minorversions); i++)
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index f87a359d968f..232a873dc53a 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -85,7 +85,7 @@ struct nfsd_genl_rqstp {
> u32 rq_opnum[NFSD_MAX_OPS_PER_COMPOUND];
> };
>
> -extern struct svc_program nfsd_program;
> +extern struct svc_program nfsd_programs[];
> extern const struct svc_version nfsd_version2, nfsd_version3, nfsd_version4;
> extern struct mutex nfsd_mutex;
> extern spinlock_t nfsd_drc_lock;
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 1bec3a53e35f..5f8680ab1013 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -35,7 +35,6 @@
> #define NFSDDBG_FACILITY NFSDDBG_SVC
>
> atomic_t nfsd_th_cnt = ATOMIC_INIT(0);
> -extern struct svc_program nfsd_program;
> static int nfsd(void *vrqstp);
> #if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
> static int nfsd_acl_rpcbind_set(struct net *,
> @@ -87,16 +86,6 @@ static const struct svc_version *localio_versions[] = {
>
> #define NFSD_LOCALIO_NRVERS ARRAY_SIZE(localio_versions)
>
> -static struct svc_program nfsd_localio_program = {
> - .pg_prog = NFS_LOCALIO_PROGRAM,
> - .pg_nvers = NFSD_LOCALIO_NRVERS,
> - .pg_vers = localio_versions,
> - .pg_name = "nfslocalio",
> - .pg_class = "nfsd",
> - .pg_authenticate = &svc_set_client,
> - .pg_init_request = svc_generic_init_request,
> - .pg_rpcbind_set = svc_generic_rpcbind_set,
> -};
> #endif /* CONFIG_NFSD_LOCALIO */
>
> #if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
> @@ -109,23 +98,9 @@ static const struct svc_version *nfsd_acl_version[] = {
> # endif
> };
>
> -#define NFSD_ACL_MINVERS 2
> +#define NFSD_ACL_MINVERS 2
> #define NFSD_ACL_NRVERS ARRAY_SIZE(nfsd_acl_version)
>
> -static struct svc_program nfsd_acl_program = {
> -#if IS_ENABLED(CONFIG_NFSD_LOCALIO)
> - .pg_next = &nfsd_localio_program,
> -#endif /* CONFIG_NFSD_LOCALIO */
> - .pg_prog = NFS_ACL_PROGRAM,
> - .pg_nvers = NFSD_ACL_NRVERS,
> - .pg_vers = nfsd_acl_version,
> - .pg_name = "nfsacl",
> - .pg_class = "nfsd",
> - .pg_authenticate = &svc_set_client,
> - .pg_init_request = nfsd_acl_init_request,
> - .pg_rpcbind_set = nfsd_acl_rpcbind_set,
> -};
> -
You just added this code in patch 11.
I think it'd be clearer to reverse the order of patches 11 and 12. That
way you don't have this interim version of the localio program that
lives on the linked list.
> #endif /* defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL) */
>
> static const struct svc_version *nfsd_version[NFSD_MAXVERS+1] = {
> @@ -138,22 +113,41 @@ static const struct svc_version *nfsd_version[NFSD_MAXVERS+1] = {
> #endif
> };
>
> -struct svc_program nfsd_program = {
> -#if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
> - .pg_next = &nfsd_acl_program,
> -#else
> -#if IS_ENABLED(CONFIG_NFSD_LOCALIO)
> - .pg_next = &nfsd_localio_program,
> -#endif /* CONFIG_NFSD_LOCALIO */
> -#endif
> +struct svc_program nfsd_programs[] = {
> + {
> .pg_prog = NFS_PROGRAM, /* program number */
> .pg_nvers = NFSD_MAXVERS+1, /* nr of entries in nfsd_version */
> .pg_vers = nfsd_version, /* version table */
> .pg_name = "nfsd", /* program name */
> .pg_class = "nfsd", /* authentication class */
> - .pg_authenticate = &svc_set_client, /* export authentication */
> + .pg_authenticate = svc_set_client, /* export authentication */
> .pg_init_request = nfsd_init_request,
> .pg_rpcbind_set = nfsd_rpcbind_set,
> + },
> +#if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
> + {
> + .pg_prog = NFS_ACL_PROGRAM,
> + .pg_nvers = NFSD_ACL_NRVERS,
> + .pg_vers = nfsd_acl_version,
> + .pg_name = "nfsacl",
> + .pg_class = "nfsd",
> + .pg_authenticate = svc_set_client,
> + .pg_init_request = nfsd_acl_init_request,
> + .pg_rpcbind_set = nfsd_acl_rpcbind_set,
> + },
> +#endif /* defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL) */
> +#if IS_ENABLED(CONFIG_NFSD_LOCALIO)
> + {
> + .pg_prog = NFS_LOCALIO_PROGRAM,
> + .pg_nvers = NFSD_LOCALIO_NRVERS,
> + .pg_vers = localio_versions,
> + .pg_name = "nfslocalio",
> + .pg_class = "nfsd",
> + .pg_authenticate = svc_set_client,
> + .pg_init_request = svc_generic_init_request,
> + .pg_rpcbind_set = svc_generic_rpcbind_set,
> + }
> +#endif /* IS_ENABLED(CONFIG_NFSD_LOCALIO) */
> };
>
> bool nfsd_support_version(int vers)
> @@ -663,7 +657,8 @@ int nfsd_create_serv(struct net *net)
> if (nfsd_max_blksize == 0)
> nfsd_max_blksize = nfsd_get_default_max_blksize();
> nfsd_reset_versions(nn);
> - serv = svc_create_pooled(&nfsd_program, &nn->nfsd_svcstats,
> + serv = svc_create_pooled(nfsd_programs, ARRAY_SIZE(nfsd_programs),
> + &nn->nfsd_svcstats,
> nfsd_max_blksize, nfsd);
> if (serv == NULL)
> return -ENOMEM;
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 437672bcaa22..c7ad2fb2a155 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -67,9 +67,10 @@ enum {
> * We currently do not support more than one RPC program per daemon.
> */
> struct svc_serv {
> - struct svc_program * sv_program; /* RPC program */
> + struct svc_program * sv_programs; /* RPC programs */
> struct svc_stat * sv_stats; /* RPC statistics */
> spinlock_t sv_lock;
> + unsigned int sv_nprogs; /* Number of sv_programs */
> unsigned int sv_nrthreads; /* # of server threads */
> unsigned int sv_maxconn; /* max connections allowed or
> * '0' causing max to be based
> @@ -357,10 +358,9 @@ struct svc_process_info {
> };
>
> /*
> - * List of RPC programs on the same transport endpoint
> + * RPC program - an array of these can use the same transport endpoint
> */
> struct svc_program {
> - struct svc_program * pg_next; /* other programs (same xprt) */
> u32 pg_prog; /* program number */
> unsigned int pg_lovers; /* lowest version */
> unsigned int pg_hivers; /* highest version */
> @@ -438,6 +438,7 @@ bool svc_rqst_replace_page(struct svc_rqst *rqstp,
> void svc_rqst_release_pages(struct svc_rqst *rqstp);
> void svc_exit_thread(struct svc_rqst *);
> struct svc_serv * svc_create_pooled(struct svc_program *prog,
> + unsigned int nprog,
> struct svc_stat *stats,
> unsigned int bufsize,
> int (*threadfn)(void *data));
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index ff6f3e35b36d..b33386d249c2 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -440,10 +440,11 @@ EXPORT_SYMBOL_GPL(svc_rpcb_cleanup);
>
> static int svc_uses_rpcbind(struct svc_serv *serv)
> {
> - struct svc_program *progp;
> - unsigned int i;
> + unsigned int p, i;
> +
> + for (p = 0; p < serv->sv_nprogs; p++) {
> + struct svc_program *progp = &serv->sv_programs[p];
>
> - for (progp = serv->sv_program; progp; progp = progp->pg_next) {
> for (i = 0; i < progp->pg_nvers; i++) {
> if (progp->pg_vers[i] == NULL)
> continue;
> @@ -480,7 +481,7 @@ __svc_init_bc(struct svc_serv *serv)
> * Create an RPC service
> */
> static struct svc_serv *
> -__svc_create(struct svc_program *prog, struct svc_stat *stats,
> +__svc_create(struct svc_program *prog, int nprogs, struct svc_stat *stats,
> unsigned int bufsize, int npools, int (*threadfn)(void *data))
> {
> struct svc_serv *serv;
> @@ -491,7 +492,8 @@ __svc_create(struct svc_program *prog, struct svc_stat *stats,
> if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> return NULL;
> serv->sv_name = prog->pg_name;
> - serv->sv_program = prog;
> + serv->sv_programs = prog;
> + serv->sv_nprogs = nprogs;
> serv->sv_stats = stats;
> if (bufsize > RPCSVC_MAXPAYLOAD)
> bufsize = RPCSVC_MAXPAYLOAD;
> @@ -499,17 +501,18 @@ __svc_create(struct svc_program *prog, struct svc_stat *stats,
> serv->sv_max_mesg = roundup(serv->sv_max_payload + PAGE_SIZE, PAGE_SIZE);
> serv->sv_threadfn = threadfn;
> xdrsize = 0;
> - while (prog) {
> - prog->pg_lovers = prog->pg_nvers-1;
> - for (vers=0; vers<prog->pg_nvers ; vers++)
> - if (prog->pg_vers[vers]) {
> - prog->pg_hivers = vers;
> - if (prog->pg_lovers > vers)
> - prog->pg_lovers = vers;
> - if (prog->pg_vers[vers]->vs_xdrsize > xdrsize)
> - xdrsize = prog->pg_vers[vers]->vs_xdrsize;
> + for (i = 0; i < nprogs; i++) {
> + struct svc_program *progp = &prog[i];
> +
> + progp->pg_lovers = progp->pg_nvers-1;
> + for (vers = 0; vers < progp->pg_nvers ; vers++)
> + if (progp->pg_vers[vers]) {
> + progp->pg_hivers = vers;
> + if (progp->pg_lovers > vers)
> + progp->pg_lovers = vers;
> + if (progp->pg_vers[vers]->vs_xdrsize > xdrsize)
> + xdrsize = progp->pg_vers[vers]->vs_xdrsize;
> }
> - prog = prog->pg_next;
> }
> serv->sv_xdrsize = xdrsize;
> INIT_LIST_HEAD(&serv->sv_tempsocks);
> @@ -558,13 +561,14 @@ __svc_create(struct svc_program *prog, struct svc_stat *stats,
> struct svc_serv *svc_create(struct svc_program *prog, unsigned int bufsize,
> int (*threadfn)(void *data))
> {
> - return __svc_create(prog, NULL, bufsize, 1, threadfn);
> + return __svc_create(prog, 1, NULL, bufsize, 1, threadfn);
> }
> EXPORT_SYMBOL_GPL(svc_create);
>
> /**
> * svc_create_pooled - Create an RPC service with pooled threads
> - * @prog: the RPC program the new service will handle
> + * @prog: Array of RPC programs the new service will handle
> + * @nprogs: Number of programs in the array
> * @stats: the stats struct if desired
> * @bufsize: maximum message size for @prog
> * @threadfn: a function to service RPC requests for @prog
> @@ -572,6 +576,7 @@ EXPORT_SYMBOL_GPL(svc_create);
> * Returns an instantiated struct svc_serv object or NULL.
> */
> struct svc_serv *svc_create_pooled(struct svc_program *prog,
> + unsigned int nprogs,
> struct svc_stat *stats,
> unsigned int bufsize,
> int (*threadfn)(void *data))
> @@ -579,7 +584,7 @@ struct svc_serv *svc_create_pooled(struct svc_program *prog,
> struct svc_serv *serv;
> unsigned int npools = svc_pool_map_get();
>
> - serv = __svc_create(prog, stats, bufsize, npools, threadfn);
> + serv = __svc_create(prog, nprogs, stats, bufsize, npools, threadfn);
> if (!serv)
> goto out_err;
> serv->sv_is_pooled = true;
> @@ -602,16 +607,16 @@ svc_destroy(struct svc_serv **servp)
>
> *servp = NULL;
>
> - dprintk("svc: svc_destroy(%s)\n", serv->sv_program->pg_name);
> + dprintk("svc: svc_destroy(%s)\n", serv->sv_programs->pg_name);
> timer_shutdown_sync(&serv->sv_temptimer);
>
> /*
> * Remaining transports at this point are not expected.
> */
> WARN_ONCE(!list_empty(&serv->sv_permsocks),
> - "SVC: permsocks remain for %s\n", serv->sv_program->pg_name);
> + "SVC: permsocks remain for %s\n", serv->sv_programs->pg_name);
> WARN_ONCE(!list_empty(&serv->sv_tempsocks),
> - "SVC: tempsocks remain for %s\n", serv->sv_program->pg_name);
> + "SVC: tempsocks remain for %s\n", serv->sv_programs->pg_name);
>
> cache_clean_deferred(serv);
>
> @@ -1149,15 +1154,16 @@ int svc_register(const struct svc_serv *serv, struct net *net,
> const int family, const unsigned short proto,
> const unsigned short port)
> {
> - struct svc_program *progp;
> - unsigned int i;
> + unsigned int p, i;
> int error = 0;
>
> WARN_ON_ONCE(proto == 0 && port == 0);
> if (proto == 0 && port == 0)
> return -EINVAL;
>
> - for (progp = serv->sv_program; progp; progp = progp->pg_next) {
> + for (p = 0; p < serv->sv_nprogs; p++) {
> + struct svc_program *progp = &serv->sv_programs[p];
> +
> for (i = 0; i < progp->pg_nvers; i++) {
>
> error = progp->pg_rpcbind_set(net, progp, i,
> @@ -1209,13 +1215,14 @@ static void __svc_unregister(struct net *net, const u32 program, const u32 versi
> static void svc_unregister(const struct svc_serv *serv, struct net *net)
> {
> struct sighand_struct *sighand;
> - struct svc_program *progp;
> unsigned long flags;
> - unsigned int i;
> + unsigned int p, i;
>
> clear_thread_flag(TIF_SIGPENDING);
>
> - for (progp = serv->sv_program; progp; progp = progp->pg_next) {
> + for (p = 0; p < serv->sv_nprogs; p++) {
> + struct svc_program *progp = &serv->sv_programs[p];
> +
> for (i = 0; i < progp->pg_nvers; i++) {
> if (progp->pg_vers[i] == NULL)
> continue;
> @@ -1321,7 +1328,7 @@ svc_process_common(struct svc_rqst *rqstp)
> struct svc_process_info process;
> enum svc_auth_status auth_res;
> unsigned int aoffset;
> - int rc;
> + int pr, rc;
> __be32 *p;
>
> /* Will be turned off only when NFSv4 Sessions are used */
> @@ -1345,9 +1352,12 @@ svc_process_common(struct svc_rqst *rqstp)
> rqstp->rq_vers = be32_to_cpup(p++);
> rqstp->rq_proc = be32_to_cpup(p);
>
> - for (progp = serv->sv_program; progp; progp = progp->pg_next)
> + for (pr = 0; pr < serv->sv_nprogs; pr++) {
> + progp = &serv->sv_programs[pr];
> +
> if (rqstp->rq_prog == progp->pg_prog)
> break;
> + }
>
> /*
> * Decode auth data, and add verifier to reply buffer.
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 53ebc719ff5a..43c57124de52 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -268,7 +268,7 @@ static int _svc_xprt_create(struct svc_serv *serv, const char *xprt_name,
> spin_unlock(&svc_xprt_class_lock);
> newxprt = xcl->xcl_ops->xpo_create(serv, net, sap, len, flags);
> if (IS_ERR(newxprt)) {
> - trace_svc_xprt_create_err(serv->sv_program->pg_name,
> + trace_svc_xprt_create_err(serv->sv_programs->pg_name,
> xcl->xcl_name, sap, len,
> newxprt);
> module_put(xcl->xcl_owner);
> diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
> index 04b45588ae6f..8ca98b146ec8 100644
> --- a/net/sunrpc/svcauth_unix.c
> +++ b/net/sunrpc/svcauth_unix.c
> @@ -697,7 +697,8 @@ svcauth_unix_set_client(struct svc_rqst *rqstp)
> rqstp->rq_auth_stat = rpc_autherr_badcred;
> ipm = ip_map_cached_get(xprt);
> if (ipm == NULL)
> - ipm = __ip_map_lookup(sn->ip_map_cache, rqstp->rq_server->sv_program->pg_class,
> + ipm = __ip_map_lookup(sn->ip_map_cache,
> + rqstp->rq_server->sv_programs->pg_class,
> &sin6->sin6_addr);
>
> if (ipm == NULL)
--
Jeff Layton <jlayton@xxxxxxxxxx>
