From: Olaf Kirch <[email protected]>
The NFSACL patches introduced support for multiple RPC services
listening on the same transport. However, only the first of these
services was registered with portmapper. This was perfectly fine
for nfsacl, as you traditionally do not want these to show up in a
portmapper listing.
The patch below changes the default behavior to always register
all services listening on a given transport, but retains the
old behavior for nfsacl services.
Signed-off-by: Olaf Kirch <[email protected]>
Signed-off-by: Neil Brown <[email protected]>
### Diffstat output
./fs/nfsd/nfs2acl.c | 1 +
./fs/nfsd/nfs3acl.c | 1 +
./include/linux/sunrpc/svc.h | 3 +++
./net/sunrpc/svc.c | 37 +++++++++++++++++++++++--------------
4 files changed, 28 insertions(+), 14 deletions(-)
diff .prev/fs/nfsd/nfs2acl.c ./fs/nfsd/nfs2acl.c
--- .prev/fs/nfsd/nfs2acl.c 2006-09-01 12:22:10.000000000 +1000
+++ ./fs/nfsd/nfs2acl.c 2006-09-01 12:22:11.000000000 +1000
@@ -333,4 +333,5 @@ struct svc_version nfsd_acl_version2 = {
.vs_proc = nfsd_acl_procedures2,
.vs_dispatch = nfsd_dispatch,
.vs_xdrsize = NFS3_SVC_XDRSIZE,
+ .vs_hidden = 1,
};
diff .prev/fs/nfsd/nfs3acl.c ./fs/nfsd/nfs3acl.c
--- .prev/fs/nfsd/nfs3acl.c 2006-09-01 12:22:10.000000000 +1000
+++ ./fs/nfsd/nfs3acl.c 2006-09-01 12:22:11.000000000 +1000
@@ -263,5 +263,6 @@ struct svc_version nfsd_acl_version3 = {
.vs_proc = nfsd_acl_procedures3,
.vs_dispatch = nfsd_dispatch,
.vs_xdrsize = NFS3_SVC_XDRSIZE,
+ .vs_hidden = 1,
};
diff .prev/include/linux/sunrpc/svc.h ./include/linux/sunrpc/svc.h
--- .prev/include/linux/sunrpc/svc.h 2006-09-01 12:22:10.000000000 +1000
+++ ./include/linux/sunrpc/svc.h 2006-09-01 12:22:11.000000000 +1000
@@ -304,6 +304,9 @@ struct svc_version {
struct svc_procedure * vs_proc; /* per-procedure info */
u32 vs_xdrsize; /* xdrsize needed for this version */
+ unsigned int vs_hidden : 1; /* Don't register with portmapper.
+ * Only used for nfsacl so far. */
+
/* Override dispatch function (e.g. when caching replies).
* A return value of 0 means drop the request.
* vs_dispatch == NULL means use default dispatcher.
diff .prev/net/sunrpc/svc.c ./net/sunrpc/svc.c
--- .prev/net/sunrpc/svc.c 2006-09-01 12:22:11.000000000 +1000
+++ ./net/sunrpc/svc.c 2006-09-01 12:22:11.000000000 +1000
@@ -644,23 +644,32 @@ svc_register(struct svc_serv *serv, int
unsigned long flags;
int i, error = 0, dummy;
- progp = serv->sv_program;
-
- dprintk("RPC: svc_register(%s, %s, %d)\n",
- progp->pg_name, proto == IPPROTO_UDP? "udp" : "tcp", port);
-
if (!port)
clear_thread_flag(TIF_SIGPENDING);
- for (i = 0; i < progp->pg_nvers; i++) {
- if (progp->pg_vers[i] == NULL)
- continue;
- error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
- if (error < 0)
- break;
- if (port && !dummy) {
- error = -EACCES;
- break;
+ 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;
+
+ dprintk("RPC: svc_register(%s, %s, %d, %d)%s\n",
+ progp->pg_name,
+ proto == IPPROTO_UDP? "udp" : "tcp",
+ port,
+ i,
+ progp->pg_vers[i]->vs_hidden?
+ " (but not telling portmap)" : "");
+
+ if (progp->pg_vers[i]->vs_hidden)
+ continue;
+
+ error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
+ if (error < 0)
+ break;
+ if (port && !dummy) {
+ error = -EACCES;
+ break;
+ }
}
}
NeilBrown wrote:
> From: Olaf Kirch <[email protected]>
>
> The NFSACL patches introduced support for multiple RPC services
> listening on the same transport. However, only the first of these
> services was registered with portmapper. This was perfectly fine
> for nfsacl, as you traditionally do not want these to show up in a
> portmapper listing.
>
> The patch below changes the default behavior to always register
> all services listening on a given transport, but retains the
> old behavior for nfsacl services.
>
> Signed-off-by: Olaf Kirch <[email protected]>
> Signed-off-by: Neil Brown <[email protected]>
>
> ### Diffstat output
> ./fs/nfsd/nfs2acl.c | 1 +
> ./fs/nfsd/nfs3acl.c | 1 +
> ./include/linux/sunrpc/svc.h | 3 +++
> ./net/sunrpc/svc.c | 37 +++++++++++++++++++++++--------------
> 4 files changed, 28 insertions(+), 14 deletions(-)
>
> diff .prev/fs/nfsd/nfs2acl.c ./fs/nfsd/nfs2acl.c
> --- .prev/fs/nfsd/nfs2acl.c 2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs2acl.c 2006-09-01 12:22:11.000000000 +1000
> @@ -333,4 +333,5 @@ struct svc_version nfsd_acl_version2 = {
> .vs_proc = nfsd_acl_procedures2,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> + .vs_hidden = 1,
> };
>
> diff .prev/fs/nfsd/nfs3acl.c ./fs/nfsd/nfs3acl.c
> --- .prev/fs/nfsd/nfs3acl.c 2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs3acl.c 2006-09-01 12:22:11.000000000 +1000
> @@ -263,5 +263,6 @@ struct svc_version nfsd_acl_version3 = {
> .vs_proc = nfsd_acl_procedures3,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> + .vs_hidden = 1,
> };
>
>
> diff .prev/include/linux/sunrpc/svc.h ./include/linux/sunrpc/svc.h
> --- .prev/include/linux/sunrpc/svc.h 2006-09-01 12:22:10.000000000 +1000
> +++ ./include/linux/sunrpc/svc.h 2006-09-01 12:22:11.000000000 +1000
> @@ -304,6 +304,9 @@ struct svc_version {
> struct svc_procedure * vs_proc; /* per-procedure info */
> u32 vs_xdrsize; /* xdrsize needed for this version */
>
> + unsigned int vs_hidden : 1; /* Don't register with portmapper.
> + * Only used for nfsacl so far. */
> +
> /* Override dispatch function (e.g. when caching replies).
> * A return value of 0 means drop the request.
> * vs_dispatch == NULL means use default dispatcher.
>
> diff .prev/net/sunrpc/svc.c ./net/sunrpc/svc.c
> --- .prev/net/sunrpc/svc.c 2006-09-01 12:22:11.000000000 +1000
> +++ ./net/sunrpc/svc.c 2006-09-01 12:22:11.000000000 +1000
> @@ -644,23 +644,32 @@ svc_register(struct svc_serv *serv, int
> unsigned long flags;
> int i, error = 0, dummy;
>
> - progp = serv->sv_program;
> -
> - dprintk("RPC: svc_register(%s, %s, %d)\n",
> - progp->pg_name, proto == IPPROTO_UDP? "udp" : "tcp", port);
> -
> if (!port)
> clear_thread_flag(TIF_SIGPENDING);
>
> - for (i = 0; i < progp->pg_nvers; i++) {
> - if (progp->pg_vers[i] == NULL)
> - continue;
> - error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> - if (error < 0)
> - break;
> - if (port && !dummy) {
> - error = -EACCES;
> - break;
> + 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;
> +
> + dprintk("RPC: svc_register(%s, %s, %d, %d)%s\n",
> + progp->pg_name,
> + proto == IPPROTO_UDP? "udp" : "tcp",
> + port,
> + i,
> + progp->pg_vers[i]->vs_hidden?
> + " (but not telling portmap)" : "");
> +
> + if (progp->pg_vers[i]->vs_hidden)
> + continue;
> +
> + error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> + if (error < 0)
> + break;
> + if (port && !dummy) {
> + error = -EACCES;
> + break;
> + }
> }
> }
Just out of curiosity, why does the dprintk say "svc_register", but the
code calls rpc_register?
Thanx...
ps
NeilBrown wrote:
> From: Olaf Kirch <[email protected]>
>
> The NFSACL patches introduced support for multiple RPC services
> listening on the same transport. However, only the first of these
> services was registered with portmapper. This was perfectly fine
> for nfsacl, as you traditionally do not want these to show up in a
> portmapper listing.
>
> The patch below changes the default behavior to always register
> all services listening on a given transport, but retains the
> old behavior for nfsacl services.
>
> Signed-off-by: Olaf Kirch <[email protected]>
> Signed-off-by: Neil Brown <[email protected]>
>
> ### Diffstat output
> ./fs/nfsd/nfs2acl.c | 1 +
> ./fs/nfsd/nfs3acl.c | 1 +
> ./include/linux/sunrpc/svc.h | 3 +++
> ./net/sunrpc/svc.c | 37 +++++++++++++++++++++++--------------
> 4 files changed, 28 insertions(+), 14 deletions(-)
>
> diff .prev/fs/nfsd/nfs2acl.c ./fs/nfsd/nfs2acl.c
> --- .prev/fs/nfsd/nfs2acl.c 2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs2acl.c 2006-09-01 12:22:11.000000000 +1000
> @@ -333,4 +333,5 @@ struct svc_version nfsd_acl_version2 = {
> .vs_proc = nfsd_acl_procedures2,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> + .vs_hidden = 1,
> };
>
> diff .prev/fs/nfsd/nfs3acl.c ./fs/nfsd/nfs3acl.c
> --- .prev/fs/nfsd/nfs3acl.c 2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs3acl.c 2006-09-01 12:22:11.000000000 +1000
> @@ -263,5 +263,6 @@ struct svc_version nfsd_acl_version3 = {
> .vs_proc = nfsd_acl_procedures3,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> + .vs_hidden = 1,
> };
>
>
> diff .prev/include/linux/sunrpc/svc.h ./include/linux/sunrpc/svc.h
> --- .prev/include/linux/sunrpc/svc.h 2006-09-01 12:22:10.000000000 +1000
> +++ ./include/linux/sunrpc/svc.h 2006-09-01 12:22:11.000000000 +1000
> @@ -304,6 +304,9 @@ struct svc_version {
> struct svc_procedure * vs_proc; /* per-procedure info */
> u32 vs_xdrsize; /* xdrsize needed for this version */
>
> + unsigned int vs_hidden : 1; /* Don't register with portmapper.
> + * Only used for nfsacl so far. */
> +
> /* Override dispatch function (e.g. when caching replies).
> * A return value of 0 means drop the request.
> * vs_dispatch == NULL means use default dispatcher.
>
> diff .prev/net/sunrpc/svc.c ./net/sunrpc/svc.c
> --- .prev/net/sunrpc/svc.c 2006-09-01 12:22:11.000000000 +1000
> +++ ./net/sunrpc/svc.c 2006-09-01 12:22:11.000000000 +1000
> @@ -644,23 +644,32 @@ svc_register(struct svc_serv *serv, int
> unsigned long flags;
> int i, error = 0, dummy;
>
> - progp = serv->sv_program;
> -
> - dprintk("RPC: svc_register(%s, %s, %d)\n",
> - progp->pg_name, proto == IPPROTO_UDP? "udp" : "tcp", port);
> -
> if (!port)
> clear_thread_flag(TIF_SIGPENDING);
>
> - for (i = 0; i < progp->pg_nvers; i++) {
> - if (progp->pg_vers[i] == NULL)
> - continue;
> - error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> - if (error < 0)
> - break;
> - if (port && !dummy) {
> - error = -EACCES;
> - break;
> + 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;
> +
> + dprintk("RPC: svc_register(%s, %s, %d, %d)%s\n",
> + progp->pg_name,
> + proto == IPPROTO_UDP? "udp" : "tcp",
> + port,
> + i,
> + progp->pg_vers[i]->vs_hidden?
> + " (but not telling portmap)" : "");
> +
> + if (progp->pg_vers[i]->vs_hidden)
> + continue;
> +
> + error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> + if (error < 0)
> + break;
> + if (port && !dummy) {
> + error = -EACCES;
> + break;
> + }
> }
> }
While I'm thinking about it a little more, why not register NFS_ACL with
portmap/rpcbind? That would make it pingable via rpcinfo.
Thanx...
ps
On Fri, Sep 01, 2006 at 09:29:01AM -0400, Peter Staubach wrote:
> While I'm thinking about it a little more, why not register NFS_ACL with
> portmap/rpcbind? That would make it pingable via rpcinfo.
I can't say, but I dimly recall that this was intentional, and I
did not want to change this behavior. Andreas Gruenbacher should
know.
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
[email protected] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
On 9/1/06, NeilBrown <[email protected]> wrote:
>
> From: Olaf Kirch <[email protected]>
>
> The NFSACL patches introduced support for multiple RPC services
> listening on the same transport. However, only the first of these
> services was registered with portmapper. This was perfectly fine
> for nfsacl, as you traditionally do not want these to show up in a
> portmapper listing.
>
> The patch below changes the default behavior to always register
> all services listening on a given transport, but retains the
> old behavior for nfsacl services.
I don't like this. The idea that multiple RPC services are listening
on the same port is a total hack. What other service might use this
besides NFSACL?
Does the reference implementation (Solaris) behave this way?
> Signed-off-by: Olaf Kirch <[email protected]>
> Signed-off-by: Neil Brown <[email protected]>
>
> ### Diffstat output
> ./fs/nfsd/nfs2acl.c | 1 +
> ./fs/nfsd/nfs3acl.c | 1 +
> ./include/linux/sunrpc/svc.h | 3 +++
> ./net/sunrpc/svc.c | 37 +++++++++++++++++++++++--------------
> 4 files changed, 28 insertions(+), 14 deletions(-)
>
> diff .prev/fs/nfsd/nfs2acl.c ./fs/nfsd/nfs2acl.c
> --- .prev/fs/nfsd/nfs2acl.c 2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs2acl.c 2006-09-01 12:22:11.000000000 +1000
> @@ -333,4 +333,5 @@ struct svc_version nfsd_acl_version2 = {
> .vs_proc = nfsd_acl_procedures2,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> + .vs_hidden = 1,
> };
>
> diff .prev/fs/nfsd/nfs3acl.c ./fs/nfsd/nfs3acl.c
> --- .prev/fs/nfsd/nfs3acl.c 2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs3acl.c 2006-09-01 12:22:11.000000000 +1000
> @@ -263,5 +263,6 @@ struct svc_version nfsd_acl_version3 = {
> .vs_proc = nfsd_acl_procedures3,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> + .vs_hidden = 1,
> };
>
>
> diff .prev/include/linux/sunrpc/svc.h ./include/linux/sunrpc/svc.h
> --- .prev/include/linux/sunrpc/svc.h 2006-09-01 12:22:10.000000000 +1000
> +++ ./include/linux/sunrpc/svc.h 2006-09-01 12:22:11.000000000 +1000
> @@ -304,6 +304,9 @@ struct svc_version {
> struct svc_procedure * vs_proc; /* per-procedure info */
> u32 vs_xdrsize; /* xdrsize needed for this version */
>
> + unsigned int vs_hidden : 1; /* Don't register with portmapper.
> + * Only used for nfsacl so far. */
> +
> /* Override dispatch function (e.g. when caching replies).
> * A return value of 0 means drop the request.
> * vs_dispatch == NULL means use default dispatcher.
>
> diff .prev/net/sunrpc/svc.c ./net/sunrpc/svc.c
> --- .prev/net/sunrpc/svc.c 2006-09-01 12:22:11.000000000 +1000
> +++ ./net/sunrpc/svc.c 2006-09-01 12:22:11.000000000 +1000
> @@ -644,23 +644,32 @@ svc_register(struct svc_serv *serv, int
> unsigned long flags;
> int i, error = 0, dummy;
>
> - progp = serv->sv_program;
> -
> - dprintk("RPC: svc_register(%s, %s, %d)\n",
> - progp->pg_name, proto == IPPROTO_UDP? "udp" : "tcp", port);
> -
> if (!port)
> clear_thread_flag(TIF_SIGPENDING);
>
> - for (i = 0; i < progp->pg_nvers; i++) {
> - if (progp->pg_vers[i] == NULL)
> - continue;
> - error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> - if (error < 0)
> - break;
> - if (port && !dummy) {
> - error = -EACCES;
> - break;
> + 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;
> +
> + dprintk("RPC: svc_register(%s, %s, %d, %d)%s\n",
> + progp->pg_name,
> + proto == IPPROTO_UDP? "udp" : "tcp",
> + port,
> + i,
> + progp->pg_vers[i]->vs_hidden?
> + " (but not telling portmap)" : "");
> +
> + if (progp->pg_vers[i]->vs_hidden)
> + continue;
> +
> + error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> + if (error < 0)
> + break;
> + if (port && !dummy) {
> + error = -EACCES;
> + break;
> + }
> }
> }
>
>
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> NFS maillist - [email protected]
> https://lists.sourceforge.net/lists/listinfo/nfs
>
--
"We who cut mere stones must always be envisioning cathedrals"
-- Quarry worker's creed
On Fri, Sep 01, 2006 at 11:31:25AM -0400, Chuck Lever wrote:
> I don't like this. The idea that multiple RPC services are listening
> on the same port is a total hack. What other service might use this
> besides NFSACL?
Why do you consider this a hack? I have always felt that librpc requiring
you to open separate ports for every program you register was a poor
design. The RPC header contains the program number, and the RPC code
is fully capable of demuxing incoming requests. So I do not think it is
a hack at all.
And yes, Solaris NFSACL resides on 2049 too.
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
[email protected] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
On 9/1/06, Olaf Kirch <[email protected]> wrote:
> On Fri, Sep 01, 2006 at 11:31:25AM -0400, Chuck Lever wrote:
> > I don't like this. The idea that multiple RPC services are listening
> > on the same port is a total hack. What other service might use this
> > besides NFSACL?
>
> Why do you consider this a hack? I have always felt that librpc requiring
> you to open separate ports for every program you register was a poor
> design. The RPC header contains the program number, and the RPC code
> is fully capable of demuxing incoming requests. So I do not think it is
> a hack at all.
>
> And yes, Solaris NFSACL resides on 2049 too.
I meant "Does Solaris advertise NFSACL on 2049 via the portmapper?"
--
"We who cut mere stones must always be envisioning cathedrals"
-- Quarry worker's creed
On Fri, 2006-09-01 at 11:31 -0400, Chuck Lever wrote:
> I don't like this. The idea that multiple RPC services are listening
> on the same port is a total hack. What other service might use this
> besides NFSACL?
Ermm... Any RPC server process that doesn't want to register with the
portmapper. The NFSv4 callback channel is one obvious candidate.
Cheers,
Trond
Chuck Lever wrote:
> On 9/1/06, Olaf Kirch <[email protected]> wrote:
>
>> On Fri, Sep 01, 2006 at 11:31:25AM -0400, Chuck Lever wrote:
>>
>>> I don't like this. The idea that multiple RPC services are listening
>>> on the same port is a total hack. What other service might use this
>>> besides NFSACL?
>>>
>> Why do you consider this a hack? I have always felt that librpc requiring
>> you to open separate ports for every program you register was a poor
>> design. The RPC header contains the program number, and the RPC code
>> is fully capable of demuxing incoming requests. So I do not think it is
>> a hack at all.
>>
>> And yes, Solaris NFSACL resides on 2049 too.
>>
>
> I meant "Does Solaris advertise NFSACL on 2049 via the portmapper?"
Yes, the Solaris server registers the NFS_ACL service with the rpcbind
daemon. And, the NFS_ACL protocol is defined to use port 2049. Please
see nfs_acl.x (/usr/include/rpcsvc/nfs_acl.x) for details.
ps