Update the write handler for the portlist file to allow creating new
listening endpoints on a transport. The general form of the string is:
<transport_name><space><port number>
For example:
echo "tcp 2049" > /proc/fs/nfsd/portlist
This is intended to support the creation of a listening endpoint for
RDMA transports without adding #ifdef code to the nfssvc.c file.
Transports can also be removed as follows:
'-'<transport_name><space><port number>
For example:
echo "-tcp 2049" > /proc/fs/nfsd/portlist
Attempting to add a listener with an invalid transport string results
in EPROTONOSUPPORT and a perror string of "Protocol not supported".
Attempting to remove an non-existent listener (.e.g. bad proto or port)
results in ENOTCONN and a perror string of
"Transport endpoint is not connected"
Signed-off-by: Tom Tucker <[email protected]>
---
fs/nfsd/nfsctl.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
net/sunrpc/svc_xprt.c | 1 +
2 files changed, 52 insertions(+), 1 deletions(-)
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 77dc989..5b9ed0e 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -540,7 +540,7 @@ static ssize_t write_ports(struct file *file, char *buf, size_t size)
}
return err < 0 ? err : 0;
}
- if (buf[0] == '-') {
+ if (buf[0] == '-' && isdigit(buf[1])) {
char *toclose = kstrdup(buf+1, GFP_KERNEL);
int len = 0;
if (!toclose)
@@ -554,6 +554,56 @@ static ssize_t write_ports(struct file *file, char *buf, size_t size)
kfree(toclose);
return len;
}
+ /*
+ * Add a transport listener by writing it's transport name
+ */
+ if (isalpha(buf[0])) {
+ int err;
+ char transport[16];
+ int port;
+ if (sscanf(buf, "%15s %4d", transport, &port) == 2) {
+ err = nfsd_create_serv();
+ if (!err) {
+ if (svc_find_xprt(nfsd_serv, transport,
+ AF_UNSPEC, port))
+ return -EADDRINUSE;
+
+ err = svc_create_xprt(nfsd_serv,
+ transport, port,
+ SVC_SOCK_ANONYMOUS);
+ if (err == -ENOENT)
+ /* Give a reasonable perror msg for
+ * bad transport string */
+ err = -EPROTONOSUPPORT;
+ }
+ return err < 0 ? err : 0;
+ }
+ }
+ /*
+ * Remove a transport by writing it's transport name and port number
+ */
+ if (buf[0] == '-' && isalpha(buf[1])) {
+ struct svc_xprt *xprt;
+ int err = -EINVAL;
+ char transport[16];
+ int port;
+ if (sscanf(&buf[1], "%15s %4d", transport, &port) == 2) {
+ if (port == 0)
+ return -EINVAL;
+ lock_kernel();
+ if (nfsd_serv) {
+ xprt = svc_find_xprt(nfsd_serv, transport,
+ AF_UNSPEC, port);
+ if (xprt) {
+ svc_close_xprt(xprt);
+ err = 0;
+ } else
+ err = -ENOTCONN;
+ }
+ unlock_kernel();
+ return err < 0 ? err : 0;
+ }
+ }
return -EINVAL;
}
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 9815c6f..6708aa2 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -827,6 +827,7 @@ void svc_close_xprt(struct svc_xprt *xprt)
clear_bit(XPT_BUSY, &xprt->xpt_flags);
svc_xprt_put(xprt);
}
+EXPORT_SYMBOL_GPL(svc_close_xprt);
void svc_close_all(struct list_head *xprt_list)
{
On Thu, 2008-01-03 at 16:13 -0500, J. Bruce Fields wrote:
> On Fri, Dec 21, 2007 at 11:52:30AM -0600, Tom Tucker wrote:
[...snip...]
> > > > + AF_UNSPEC, port);
> > > > + if (xprt) {
> > > > + svc_close_xprt(xprt);
> > > > + err = 0;
> > >
> > > Hm, also: I'd have thought that svc_find_xprt should be incrementing the
> > > reference count on the returned xprt, if we're expecting the caller to
> > > do anything with it other than check it against NULL.
>
> ... there's still a race here, isn't there?
>
I think it's a good idea to have the svc_find_xprt service take a
reference. I'll code this up....
> --b.
On Fri, Dec 21, 2007 at 11:52:30AM -0600, Tom Tucker wrote:
>
> On Fri, 2007-12-14 at 18:51 -0500, J. Bruce Fields wrote:
> > On Tue, Dec 11, 2007 at 05:33:16PM -0600, Tom Tucker wrote:
> > >
> > > Update the write handler for the portlist file to allow creating new
> > > listening endpoints on a transport. The general form of the string is:
> > >
> > > <transport_name><space><port number>
> > >
> > > For example:
> > >
> > > echo "tcp 2049" > /proc/fs/nfsd/portlist
> > >
> > > This is intended to support the creation of a listening endpoint for
> > > RDMA transports without adding #ifdef code to the nfssvc.c file.
> > >
> > > Transports can also be removed as follows:
> > >
> > > '-'<transport_name><space><port number>
> > >
> > > For example:
> > >
> > > echo "-tcp 2049" > /proc/fs/nfsd/portlist
> > >
> > > Attempting to add a listener with an invalid transport string results
> > > in EPROTONOSUPPORT and a perror string of "Protocol not supported".
> > >
> > > Attempting to remove an non-existent listener (.e.g. bad proto or port)
> > > results in ENOTCONN and a perror string of
> > > "Transport endpoint is not connected"
> > >
> > > Signed-off-by: Tom Tucker <[email protected]>
> > > ---
> > >
> > > fs/nfsd/nfsctl.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
> > > net/sunrpc/svc_xprt.c | 1 +
> > > 2 files changed, 52 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > > index 77dc989..5b9ed0e 100644
> > > --- a/fs/nfsd/nfsctl.c
> > > +++ b/fs/nfsd/nfsctl.c
> > > @@ -540,7 +540,7 @@ static ssize_t write_ports(struct file *file, char *buf, size_t size)
> > > }
> > > return err < 0 ? err : 0;
> > > }
> > > - if (buf[0] == '-') {
> > > + if (buf[0] == '-' && isdigit(buf[1])) {
> > > char *toclose = kstrdup(buf+1, GFP_KERNEL);
> > > int len = 0;
> > > if (!toclose)
> > > @@ -554,6 +554,56 @@ static ssize_t write_ports(struct file *file, char *buf, size_t size)
> > > kfree(toclose);
> > > return len;
> > > }
> > > + /*
> > > + * Add a transport listener by writing it's transport name
> > > + */
> > > + if (isalpha(buf[0])) {
> > > + int err;
> > > + char transport[16];
> > > + int port;
> > > + if (sscanf(buf, "%15s %4d", transport, &port) == 2) {
> > > + err = nfsd_create_serv();
> > > + if (!err) {
> > > + if (svc_find_xprt(nfsd_serv, transport,
> > > + AF_UNSPEC, port))
> > > + return -EADDRINUSE;
> >
> > Shouldn't svc_create_xprt do this check for us itself? (And if it
> > doesn't, isn't there some minor race between two writers both checking,
> > finding nothing, and then both svc_create_xprt()ing?)
> >
>
> You're right. fixed.
Thanks! But:
>
> > > +
> > > + err = svc_create_xprt(nfsd_serv,
> > > + transport, port,
> > > + SVC_SOCK_ANONYMOUS);
> > > + if (err == -ENOENT)
> > > + /* Give a reasonable perror msg for
> > > + * bad transport string */
> > > + err = -EPROTONOSUPPORT;
> > > + }
> > > + return err < 0 ? err : 0;
> > > + }
> > > + }
> > > + /*
> > > + * Remove a transport by writing it's transport name and port number
> > > + */
> > > + if (buf[0] == '-' && isalpha(buf[1])) {
> > > + struct svc_xprt *xprt;
> > > + int err = -EINVAL;
> > > + char transport[16];
> > > + int port;
> > > + if (sscanf(&buf[1], "%15s %4d", transport, &port) == 2) {
> > > + if (port == 0)
> > > + return -EINVAL;
> > > + lock_kernel();
> > > + if (nfsd_serv) {
> > > + xprt = svc_find_xprt(nfsd_serv, transport,
> > > + AF_UNSPEC, port);
> > > + if (xprt) {
> > > + svc_close_xprt(xprt);
> > > + err = 0;
> >
> > Hm, also: I'd have thought that svc_find_xprt should be incrementing the
> > reference count on the returned xprt, if we're expecting the caller to
> > do anything with it other than check it against NULL.
... there's still a race here, isn't there?
--b.