* [PATCH 1/4] nfsd: convert nfs4_cb_conn struct to hold address in sockaddr_storage
2009-08-10 22:09 [PATCH 0/4] nfsd: add support for NFSv4 callbacks over IPv6 (try #3) Jeff Layton
@ 2009-08-10 22:09 ` Jeff Layton
2009-08-10 22:09 ` [PATCH 2/4] nfsd: make nfs4_client->cl_addr a struct sockaddr_storage Jeff Layton
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2009-08-10 22:09 UTC (permalink / raw)
To: bfields; +Cc: chuck.lever, linux-nfs, nfsv4
...rather than as a separate address and port fields. This will be
necessary for implementing callbacks over IPv6.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfsd/nfs4callback.c | 11 ++---------
fs/nfsd/nfs4state.c | 13 +++++++++----
include/linux/nfsd/state.h | 4 ++--
3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 3fd23f7..81d1c52 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -377,7 +377,6 @@ static int max_cb_time(void)
int setup_callback_client(struct nfs4_client *clp)
{
- struct sockaddr_in addr;
struct nfs4_cb_conn *cb = &clp->cl_cb_conn;
struct rpc_timeout timeparms = {
.to_initval = max_cb_time(),
@@ -385,8 +384,8 @@ int setup_callback_client(struct nfs4_client *clp)
};
struct rpc_create_args args = {
.protocol = IPPROTO_TCP,
- .address = (struct sockaddr *)&addr,
- .addrsize = sizeof(addr),
+ .address = (struct sockaddr *) &cb->cb_addr,
+ .addrsize = cb->cb_addrlen,
.timeout = &timeparms,
.program = &cb_program,
.prognumber = cb->cb_prog,
@@ -400,12 +399,6 @@ int setup_callback_client(struct nfs4_client *clp)
if (!clp->cl_principal && (clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
return -EINVAL;
- /* Initialize address */
- memset(&addr, 0, sizeof(addr));
- addr.sin_family = AF_INET;
- addr.sin_port = htons(cb->cb_port);
- addr.sin_addr.s_addr = htonl(cb->cb_addr);
-
/* Create RPC client */
client = rpc_create(&args);
if (IS_ERR(client)) {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9295c4b..95e2185 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -931,7 +931,7 @@ parse_octet(unsigned int *lenp, char **addrp)
/* parse and set the setclientid ipv4 callback address */
static int
-parse_ipv4(unsigned int addr_len, char *addr_val, unsigned int *cbaddrp, unsigned short *cbportp)
+parse_ipv4(unsigned int addr_len, char *addr_val, struct sockaddr_in *s4)
{
int temp = 0;
u32 cbaddr = 0;
@@ -940,6 +940,8 @@ parse_ipv4(unsigned int addr_len, char *addr_val, unsigned int *cbaddrp, unsigne
char *addr = addr_val;
int i, shift;
+ s4->sin_family = AF_INET;
+
/* ipaddress */
shift = 24;
for(i = 4; i > 0 ; i--) {
@@ -950,7 +952,7 @@ parse_ipv4(unsigned int addr_len, char *addr_val, unsigned int *cbaddrp, unsigne
if (shift > 0)
shift -= 8;
}
- *cbaddrp = cbaddr;
+ s4->sin_addr.s_addr = htonl(cbaddr);
/* port */
shift = 8;
@@ -962,7 +964,7 @@ parse_ipv4(unsigned int addr_len, char *addr_val, unsigned int *cbaddrp, unsigne
if (shift > 0)
shift -= 8;
}
- *cbportp = cbport;
+ s4->sin_port = htons(cbport);
return 1;
}
@@ -976,13 +978,16 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se)
goto out_err;
if ( !(parse_ipv4(se->se_callback_addr_len, se->se_callback_addr_val,
- &cb->cb_addr, &cb->cb_port)))
+ (struct sockaddr_in *) &cb->cb_addr)))
goto out_err;
+ cb->cb_addrlen = sizeof(struct sockaddr_in);
cb->cb_minorversion = 0;
cb->cb_prog = se->se_callback_prog;
cb->cb_ident = se->se_callback_ident;
return;
out_err:
+ cb->cb_addr.ss_family = AF_UNSPEC;
+ cb->cb_addrlen = 0;
dprintk(KERN_INFO "NFSD: this client (clientid %08x/%08x) "
"will not receive delegations\n",
clp->cl_clientid.cl_boot, clp->cl_clientid.cl_id);
diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
index 58bb197..d866368 100644
--- a/include/linux/nfsd/state.h
+++ b/include/linux/nfsd/state.h
@@ -81,8 +81,8 @@ struct nfs4_delegation {
/* client delegation callback info */
struct nfs4_cb_conn {
/* SETCLIENTID info */
- u32 cb_addr;
- unsigned short cb_port;
+ struct sockaddr_storage cb_addr;
+ size_t cb_addrlen;
u32 cb_prog;
u32 cb_minorversion;
u32 cb_ident; /* minorversion 0 only */
--
1.6.0.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/4] nfsd: make nfs4_client->cl_addr a struct sockaddr_storage
2009-08-10 22:09 [PATCH 0/4] nfsd: add support for NFSv4 callbacks over IPv6 (try #3) Jeff Layton
2009-08-10 22:09 ` [PATCH 1/4] nfsd: convert nfs4_cb_conn struct to hold address in sockaddr_storage Jeff Layton
@ 2009-08-10 22:09 ` Jeff Layton
2009-08-10 22:53 ` Chuck Lever
2009-08-10 22:09 ` [PATCH 3/4] nfsd: convert gen_callback to use rpc_uaddr2sockaddr Jeff Layton
2009-08-10 22:09 ` [PATCH 4/4] nfsd: add support for NFSv4 callbacks over IPv6 Jeff Layton
3 siblings, 1 reply; 7+ messages in thread
From: Jeff Layton @ 2009-08-10 22:09 UTC (permalink / raw)
To: bfields; +Cc: chuck.lever, linux-nfs, nfsv4
It's currently a __be32, which isn't big enough to hold an IPv6 address.
While we're at it, add some new routines for comparing and copying
cl_addr's.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfsd/nfs4state.c | 70 +++++++++++++++++++++++++++++++++++--------
include/linux/nfsd/state.h | 2 +-
2 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 95e2185..cbc1a05 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1216,6 +1216,45 @@ nfsd4_set_ex_flags(struct nfs4_client *new, struct nfsd4_exchange_id *clid)
clid->flags = new->cl_exchange_flags;
}
+/* Returns true if addresses are equal (ignoring port, scope, etc) */
+static bool
+cl_addr_equal(struct sockaddr *a, struct sockaddr *b)
+{
+ struct sockaddr_in *a4, *b4;
+
+ if (a->sa_family != b->sa_family)
+ return false;
+
+ switch (a->sa_family) {
+ case AF_INET:
+ a4 = (struct sockaddr_in *) a;
+ b4 = (struct sockaddr_in *) b;
+ return (a4->sin_addr.s_addr == b4->sin_addr.s_addr);
+ }
+
+ /* not sure how to compare, so just return false */
+ return false;
+}
+
+/* Given a sockaddr, fill in the cl_addr -- ignore port, scope, etc */
+static void
+cl_addr_copy(struct sockaddr *dst, struct sockaddr *src)
+{
+ struct sockaddr_in *s4, *d4;
+
+ dst->sa_family = src->sa_family;
+
+ switch (dst->sa_family) {
+ case AF_INET:
+ s4 = (struct sockaddr_in *) src;
+ d4 = (struct sockaddr_in *) dst;
+ d4->sin_addr.s_addr = s4->sin_addr.s_addr;
+ return;
+ default:
+ dst->sa_family = AF_UNSPEC;
+ }
+}
+
__be32
nfsd4_exchange_id(struct svc_rqst *rqstp,
struct nfsd4_compound_state *cstate,
@@ -1225,13 +1264,15 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
int status;
unsigned int strhashval;
char dname[HEXDIR_LEN];
+ char addr_str[INET_ADDRSTRLEN];
nfs4_verifier verf = exid->verifier;
- u32 ip_addr = svc_addr_in(rqstp)->sin_addr.s_addr;
+ struct sockaddr *sa = svc_addr(rqstp);
+ rpc_ntop(sa, addr_str, sizeof(addr_str));
dprintk("%s rqstp=%p exid=%p clname.len=%u clname.data=%p "
- " ip_addr=%u flags %x, spa_how %d\n",
+ "ip_addr=%s flags %x, spa_how %d\n",
__func__, rqstp, exid, exid->clname.len, exid->clname.data,
- ip_addr, exid->flags, exid->spa_how);
+ addr_str, exid->flags, exid->spa_how);
if (!check_name(exid->clname) || (exid->flags & ~EXCHGID4_FLAG_MASK_A))
return nfserr_inval;
@@ -1320,7 +1361,7 @@ out_new:
copy_verf(new, &verf);
copy_cred(&new->cl_cred, &rqstp->rq_cred);
- new->cl_addr = ip_addr;
+ cl_addr_copy((struct sockaddr *) &new->cl_addr, sa);
gen_clid(new);
gen_confirm(new);
add_to_unconfirmed(new, strhashval);
@@ -1394,7 +1435,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
struct nfsd4_compound_state *cstate,
struct nfsd4_create_session *cr_ses)
{
- u32 ip_addr = svc_addr_in(rqstp)->sin_addr.s_addr;
+ struct sockaddr *sa = svc_addr(rqstp);
struct nfs4_client *conf, *unconf;
struct nfsd4_clid_slot *cs_slot = NULL;
int status = 0;
@@ -1422,7 +1463,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
cs_slot->sl_seqid++;
} else if (unconf) {
if (!same_creds(&unconf->cl_cred, &rqstp->rq_cred) ||
- (ip_addr != unconf->cl_addr)) {
+ !cl_addr_equal(sa, (struct sockaddr *) &unconf->cl_addr)) {
status = nfserr_clid_inuse;
goto out;
}
@@ -1569,7 +1610,7 @@ __be32
nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_setclientid *setclid)
{
- struct sockaddr_in *sin = svc_addr_in(rqstp);
+ struct sockaddr *sa = svc_addr(rqstp);
struct xdr_netobj clname = {
.len = setclid->se_namelen,
.data = setclid->se_name,
@@ -1601,8 +1642,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
/* RFC 3530 14.2.33 CASE 0: */
status = nfserr_clid_inuse;
if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
- dprintk("NFSD: setclientid: string in use by client"
- " at %pI4\n", &conf->cl_addr);
+ char addr_str[INET_ADDRSTRLEN];
+ rpc_ntop((struct sockaddr *) &conf->cl_addr, addr_str,
+ sizeof(addr_str));
+ dprintk("NFSD: setclientid: string in use by client "
+ "at %s\n", addr_str);
goto out;
}
}
@@ -1664,7 +1708,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
gen_clid(new);
}
copy_verf(new, &clverifier);
- new->cl_addr = sin->sin_addr.s_addr;
+ cl_addr_copy((struct sockaddr *) &new->cl_addr, sa);
new->cl_flavor = rqstp->rq_flavor;
princ = svc_gss_principal(rqstp);
if (princ) {
@@ -1698,7 +1742,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
struct nfsd4_compound_state *cstate,
struct nfsd4_setclientid_confirm *setclientid_confirm)
{
- struct sockaddr_in *sin = svc_addr_in(rqstp);
+ struct sockaddr *sa = svc_addr(rqstp);
struct nfs4_client *conf, *unconf;
nfs4_verifier confirm = setclientid_confirm->sc_confirm;
clientid_t * clid = &setclientid_confirm->sc_clientid;
@@ -1717,9 +1761,9 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
unconf = find_unconfirmed_client(clid);
status = nfserr_clid_inuse;
- if (conf && conf->cl_addr != sin->sin_addr.s_addr)
+ if (conf && !cl_addr_equal((struct sockaddr *) &conf->cl_addr, sa))
goto out;
- if (unconf && unconf->cl_addr != sin->sin_addr.s_addr)
+ if (unconf && !cl_addr_equal((struct sockaddr *) &unconf->cl_addr, sa))
goto out;
/*
diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
index d866368..fb0c404 100644
--- a/include/linux/nfsd/state.h
+++ b/include/linux/nfsd/state.h
@@ -200,7 +200,7 @@ struct nfs4_client {
char cl_recdir[HEXDIR_LEN]; /* recovery dir */
nfs4_verifier cl_verifier; /* generated by client */
time_t cl_time; /* time of last lease renewal */
- __be32 cl_addr; /* client ipaddress */
+ struct sockaddr_storage cl_addr; /* client ipaddress */
u32 cl_flavor; /* setclientid pseudoflavor */
char *cl_principal; /* setclientid principal name */
struct svc_cred cl_cred; /* setclientid principal */
--
1.6.0.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/4] nfsd: make nfs4_client->cl_addr a struct sockaddr_storage
2009-08-10 22:09 ` [PATCH 2/4] nfsd: make nfs4_client->cl_addr a struct sockaddr_storage Jeff Layton
@ 2009-08-10 22:53 ` Chuck Lever
2009-08-11 11:02 ` Jeff Layton
0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever @ 2009-08-10 22:53 UTC (permalink / raw)
To: Jeff Layton, Bruce Fields; +Cc: NFS list, Linux NFSv4 mailing list
On Aug 10, 2009, at 6:09 PM, Jeff Layton wrote:
> It's currently a __be32, which isn't big enough to hold an IPv6
> address.
> While we're at it, add some new routines for comparing and copying
> cl_addr's.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfsd/nfs4state.c | 70 ++++++++++++++++++++++++++++++++++
> +--------
> include/linux/nfsd/state.h | 2 +-
> 2 files changed, 58 insertions(+), 14 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 95e2185..cbc1a05 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1216,6 +1216,45 @@ nfsd4_set_ex_flags(struct nfs4_client *new,
> struct nfsd4_exchange_id *clid)
> clid->flags = new->cl_exchange_flags;
> }
>
> +/* Returns true if addresses are equal (ignoring port, scope, etc) */
> +static bool
> +cl_addr_equal(struct sockaddr *a, struct sockaddr *b)
> +{
> + struct sockaddr_in *a4, *b4;
> +
> + if (a->sa_family != b->sa_family)
> + return false;
> +
> + switch (a->sa_family) {
> + case AF_INET:
> + a4 = (struct sockaddr_in *) a;
> + b4 = (struct sockaddr_in *) b;
> + return (a4->sin_addr.s_addr == b4->sin_addr.s_addr);
> + }
> +
> + /* not sure how to compare, so just return false */
> + return false;
> +}
lockd has a similar requirement, iirc. I wonder if it makes sense to
add this functionality instead as a generic routine in include/linux/
sunrpc/clnt.h alongside rpc_{get,set}_port.
Also, I wonder if we need to compile out the IPv6-related code
throughout if CONFIG_IPV6 is not set...? Glancing around, I don't see
any strong dependency on CONFIG_IPV6, but we may want to reduce the
resulting object size in that case. Did you happen to try a kernel
build test with CONFIG_IPV6 disabled?
> +
> +/* Given a sockaddr, fill in the cl_addr -- ignore port, scope, etc
> */
It's probably worth mentioning somewhere (patch description and/or
block comment) that this IPv6 support does not include link local
support. It's obvious to those of us who work closely with this code,
but others may assume IPv6 support means link local works too.
Otherwise, I'm pretty sure you can just copy the scope ID of the
destination address (the server's receiving address) for the
SETCLIENTID RPC in order to get the right scope ID for callbacks, in
which case no extra comment is needed.
> +static void
> +cl_addr_copy(struct sockaddr *dst, struct sockaddr *src)
> +{
> + struct sockaddr_in *s4, *d4;
> +
> + dst->sa_family = src->sa_family;
> +
> + switch (dst->sa_family) {
> + case AF_INET:
> + s4 = (struct sockaddr_in *) src;
> + d4 = (struct sockaddr_in *) dst;
> + d4->sin_addr.s_addr = s4->sin_addr.s_addr;
> + return;
> + default:
> + dst->sa_family = AF_UNSPEC;
> + }
> +}
This is probably also useful functionality to make generic. I seem to
recall both lockd and the client need this kind of address copy
function.
> +
> __be32
> nfsd4_exchange_id(struct svc_rqst *rqstp,
> struct nfsd4_compound_state *cstate,
> @@ -1225,13 +1264,15 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
> int status;
> unsigned int strhashval;
> char dname[HEXDIR_LEN];
> + char addr_str[INET_ADDRSTRLEN];
> nfs4_verifier verf = exid->verifier;
> - u32 ip_addr = svc_addr_in(rqstp)->sin_addr.s_addr;
> + struct sockaddr *sa = svc_addr(rqstp);
>
> + rpc_ntop(sa, addr_str, sizeof(addr_str));
> dprintk("%s rqstp=%p exid=%p clname.len=%u clname.data=%p "
> - " ip_addr=%u flags %x, spa_how %d\n",
> + "ip_addr=%s flags %x, spa_how %d\n",
> __func__, rqstp, exid, exid->clname.len, exid->clname.data,
> - ip_addr, exid->flags, exid->spa_how);
> + addr_str, exid->flags, exid->spa_how);
>
> if (!check_name(exid->clname) || (exid->flags &
> ~EXCHGID4_FLAG_MASK_A))
> return nfserr_inval;
> @@ -1320,7 +1361,7 @@ out_new:
>
> copy_verf(new, &verf);
> copy_cred(&new->cl_cred, &rqstp->rq_cred);
> - new->cl_addr = ip_addr;
> + cl_addr_copy((struct sockaddr *) &new->cl_addr, sa);
> gen_clid(new);
> gen_confirm(new);
> add_to_unconfirmed(new, strhashval);
> @@ -1394,7 +1435,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
> struct nfsd4_compound_state *cstate,
> struct nfsd4_create_session *cr_ses)
> {
> - u32 ip_addr = svc_addr_in(rqstp)->sin_addr.s_addr;
> + struct sockaddr *sa = svc_addr(rqstp);
> struct nfs4_client *conf, *unconf;
> struct nfsd4_clid_slot *cs_slot = NULL;
> int status = 0;
> @@ -1422,7 +1463,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
> cs_slot->sl_seqid++;
> } else if (unconf) {
> if (!same_creds(&unconf->cl_cred, &rqstp->rq_cred) ||
> - (ip_addr != unconf->cl_addr)) {
> + !cl_addr_equal(sa, (struct sockaddr *) &unconf->cl_addr)) {
> status = nfserr_clid_inuse;
> goto out;
> }
> @@ -1569,7 +1610,7 @@ __be32
> nfsd4_setclientid(struct svc_rqst *rqstp, struct
> nfsd4_compound_state *cstate,
> struct nfsd4_setclientid *setclid)
> {
> - struct sockaddr_in *sin = svc_addr_in(rqstp);
> + struct sockaddr *sa = svc_addr(rqstp);
> struct xdr_netobj clname = {
> .len = setclid->se_namelen,
> .data = setclid->se_name,
> @@ -1601,8 +1642,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp,
> struct nfsd4_compound_state *cstate,
> /* RFC 3530 14.2.33 CASE 0: */
> status = nfserr_clid_inuse;
> if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
> - dprintk("NFSD: setclientid: string in use by client"
> - " at %pI4\n", &conf->cl_addr);
> + char addr_str[INET_ADDRSTRLEN];
Nit: It's kind of odd that you are taking pains to switch from
svc_addr_in() to svc_addr() and use sockaddr_storage and so on, but
then right here you add a buffer that is only big enough for an IPv4
address. I know you bump its size in a subsequent patch, but it might
be more consistent if you made this buffer larger right when you
introduce it.
> + rpc_ntop((struct sockaddr *) &conf->cl_addr, addr_str,
> + sizeof(addr_str));
> + dprintk("NFSD: setclientid: string in use by client "
> + "at %s\n", addr_str);
> goto out;
> }
> }
> @@ -1664,7 +1708,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp,
> struct nfsd4_compound_state *cstate,
> gen_clid(new);
> }
> copy_verf(new, &clverifier);
> - new->cl_addr = sin->sin_addr.s_addr;
> + cl_addr_copy((struct sockaddr *) &new->cl_addr, sa);
> new->cl_flavor = rqstp->rq_flavor;
> princ = svc_gss_principal(rqstp);
> if (princ) {
> @@ -1698,7 +1742,7 @@ nfsd4_setclientid_confirm(struct svc_rqst
> *rqstp,
> struct nfsd4_compound_state *cstate,
> struct nfsd4_setclientid_confirm *setclientid_confirm)
> {
> - struct sockaddr_in *sin = svc_addr_in(rqstp);
> + struct sockaddr *sa = svc_addr(rqstp);
> struct nfs4_client *conf, *unconf;
> nfs4_verifier confirm = setclientid_confirm->sc_confirm;
> clientid_t * clid = &setclientid_confirm->sc_clientid;
> @@ -1717,9 +1761,9 @@ nfsd4_setclientid_confirm(struct svc_rqst
> *rqstp,
> unconf = find_unconfirmed_client(clid);
>
> status = nfserr_clid_inuse;
> - if (conf && conf->cl_addr != sin->sin_addr.s_addr)
> + if (conf && !cl_addr_equal((struct sockaddr *) &conf->cl_addr, sa))
> goto out;
> - if (unconf && unconf->cl_addr != sin->sin_addr.s_addr)
> + if (unconf && !cl_addr_equal((struct sockaddr *) &unconf->cl_addr,
> sa))
> goto out;
>
> /*
> diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
> index d866368..fb0c404 100644
> --- a/include/linux/nfsd/state.h
> +++ b/include/linux/nfsd/state.h
> @@ -200,7 +200,7 @@ struct nfs4_client {
> char cl_recdir[HEXDIR_LEN]; /* recovery dir */
> nfs4_verifier cl_verifier; /* generated by client */
> time_t cl_time; /* time of last lease
> renewal */
> - __be32 cl_addr; /* client ipaddress */
> + struct sockaddr_storage cl_addr; /* client ipaddress */
> u32 cl_flavor; /* setclientid pseudoflavor */
> char *cl_principal; /* setclientid principal name */
> struct svc_cred cl_cred; /* setclientid principal */
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/4] nfsd: make nfs4_client->cl_addr a struct sockaddr_storage
2009-08-10 22:53 ` Chuck Lever
@ 2009-08-11 11:02 ` Jeff Layton
0 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2009-08-11 11:02 UTC (permalink / raw)
To: Chuck Lever; +Cc: NFS list, Linux NFSv4 mailing list
On Mon, 10 Aug 2009 18:53:56 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
> On Aug 10, 2009, at 6:09 PM, Jeff Layton wrote:
> > It's currently a __be32, which isn't big enough to hold an IPv6
> > address.
> > While we're at it, add some new routines for comparing and copying
> > cl_addr's.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfsd/nfs4state.c | 70 ++++++++++++++++++++++++++++++++++
> > +--------
> > include/linux/nfsd/state.h | 2 +-
> > 2 files changed, 58 insertions(+), 14 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 95e2185..cbc1a05 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -1216,6 +1216,45 @@ nfsd4_set_ex_flags(struct nfs4_client *new,
> > struct nfsd4_exchange_id *clid)
> > clid->flags = new->cl_exchange_flags;
> > }
> >
> > +/* Returns true if addresses are equal (ignoring port, scope, etc) */
> > +static bool
> > +cl_addr_equal(struct sockaddr *a, struct sockaddr *b)
> > +{
> > + struct sockaddr_in *a4, *b4;
> > +
> > + if (a->sa_family != b->sa_family)
> > + return false;
> > +
> > + switch (a->sa_family) {
> > + case AF_INET:
> > + a4 = (struct sockaddr_in *) a;
> > + b4 = (struct sockaddr_in *) b;
> > + return (a4->sin_addr.s_addr == b4->sin_addr.s_addr);
> > + }
> > +
> > + /* not sure how to compare, so just return false */
> > + return false;
> > +}
>
> lockd has a similar requirement, iirc. I wonder if it makes sense to
> add this functionality instead as a generic routine in include/linux/
> sunrpc/clnt.h alongside rpc_{get,set}_port.
>
> Also, I wonder if we need to compile out the IPv6-related code
> throughout if CONFIG_IPV6 is not set...? Glancing around, I don't see
> any strong dependency on CONFIG_IPV6, but we may want to reduce the
> resulting object size in that case. Did you happen to try a kernel
> build test with CONFIG_IPV6 disabled?
>
Yeah lockd does have similar routines for this. I'll see about adding a
common set of routines, probably more closely based on the lockd ones
since they wrap the IPv6 parts in CONFIG_IPV6, and convert lockd to use
those too.
> > +
> > +/* Given a sockaddr, fill in the cl_addr -- ignore port, scope, etc
> > */
>
> It's probably worth mentioning somewhere (patch description and/or
> block comment) that this IPv6 support does not include link local
> support. It's obvious to those of us who work closely with this code,
> but others may assume IPv6 support means link local works too.
>
> Otherwise, I'm pretty sure you can just copy the scope ID of the
> destination address (the server's receiving address) for the
> SETCLIENTID RPC in order to get the right scope ID for callbacks, in
> which case no extra comment is needed.
>
Yeah, that's doable. I'll add that to the next respin.
> > +static void
> > +cl_addr_copy(struct sockaddr *dst, struct sockaddr *src)
> > +{
> > + struct sockaddr_in *s4, *d4;
> > +
> > + dst->sa_family = src->sa_family;
> > +
> > + switch (dst->sa_family) {
> > + case AF_INET:
> > + s4 = (struct sockaddr_in *) src;
> > + d4 = (struct sockaddr_in *) dst;
> > + d4->sin_addr.s_addr = s4->sin_addr.s_addr;
> > + return;
> > + default:
> > + dst->sa_family = AF_UNSPEC;
> > + }
> > +}
>
> This is probably also useful functionality to make generic. I seem to
> recall both lockd and the client need this kind of address copy
> function.
>
Ok, I'll look into that too.
> > +
> > __be32
> > nfsd4_exchange_id(struct svc_rqst *rqstp,
> > struct nfsd4_compound_state *cstate,
> > @@ -1225,13 +1264,15 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
> > int status;
> > unsigned int strhashval;
> > char dname[HEXDIR_LEN];
> > + char addr_str[INET_ADDRSTRLEN];
> > nfs4_verifier verf = exid->verifier;
> > - u32 ip_addr = svc_addr_in(rqstp)->sin_addr.s_addr;
> > + struct sockaddr *sa = svc_addr(rqstp);
> >
> > + rpc_ntop(sa, addr_str, sizeof(addr_str));
> > dprintk("%s rqstp=%p exid=%p clname.len=%u clname.data=%p "
> > - " ip_addr=%u flags %x, spa_how %d\n",
> > + "ip_addr=%s flags %x, spa_how %d\n",
> > __func__, rqstp, exid, exid->clname.len, exid->clname.data,
> > - ip_addr, exid->flags, exid->spa_how);
> > + addr_str, exid->flags, exid->spa_how);
> >
> > if (!check_name(exid->clname) || (exid->flags &
> > ~EXCHGID4_FLAG_MASK_A))
> > return nfserr_inval;
> > @@ -1320,7 +1361,7 @@ out_new:
> >
> > copy_verf(new, &verf);
> > copy_cred(&new->cl_cred, &rqstp->rq_cred);
> > - new->cl_addr = ip_addr;
> > + cl_addr_copy((struct sockaddr *) &new->cl_addr, sa);
> > gen_clid(new);
> > gen_confirm(new);
> > add_to_unconfirmed(new, strhashval);
> > @@ -1394,7 +1435,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
> > struct nfsd4_compound_state *cstate,
> > struct nfsd4_create_session *cr_ses)
> > {
> > - u32 ip_addr = svc_addr_in(rqstp)->sin_addr.s_addr;
> > + struct sockaddr *sa = svc_addr(rqstp);
> > struct nfs4_client *conf, *unconf;
> > struct nfsd4_clid_slot *cs_slot = NULL;
> > int status = 0;
> > @@ -1422,7 +1463,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
> > cs_slot->sl_seqid++;
> > } else if (unconf) {
> > if (!same_creds(&unconf->cl_cred, &rqstp->rq_cred) ||
> > - (ip_addr != unconf->cl_addr)) {
> > + !cl_addr_equal(sa, (struct sockaddr *) &unconf->cl_addr)) {
> > status = nfserr_clid_inuse;
> > goto out;
> > }
> > @@ -1569,7 +1610,7 @@ __be32
> > nfsd4_setclientid(struct svc_rqst *rqstp, struct
> > nfsd4_compound_state *cstate,
> > struct nfsd4_setclientid *setclid)
> > {
> > - struct sockaddr_in *sin = svc_addr_in(rqstp);
> > + struct sockaddr *sa = svc_addr(rqstp);
> > struct xdr_netobj clname = {
> > .len = setclid->se_namelen,
> > .data = setclid->se_name,
> > @@ -1601,8 +1642,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp,
> > struct nfsd4_compound_state *cstate,
> > /* RFC 3530 14.2.33 CASE 0: */
> > status = nfserr_clid_inuse;
> > if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
> > - dprintk("NFSD: setclientid: string in use by client"
> > - " at %pI4\n", &conf->cl_addr);
> > + char addr_str[INET_ADDRSTRLEN];
>
> Nit: It's kind of odd that you are taking pains to switch from
> svc_addr_in() to svc_addr() and use sockaddr_storage and so on, but
> then right here you add a buffer that is only big enough for an IPv4
> address. I know you bump its size in a subsequent patch, but it might
> be more consistent if you made this buffer larger right when you
> introduce it.
>
Simple enough to change. Maybe I'll even make the size switchable based
on CONFIG_IPV6 setting...
> > + rpc_ntop((struct sockaddr *) &conf->cl_addr, addr_str,
> > + sizeof(addr_str));
> > + dprintk("NFSD: setclientid: string in use by client "
> > + "at %s\n", addr_str);
> > goto out;
> > }
> > }
> > @@ -1664,7 +1708,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp,
> > struct nfsd4_compound_state *cstate,
> > gen_clid(new);
> > }
> > copy_verf(new, &clverifier);
> > - new->cl_addr = sin->sin_addr.s_addr;
> > + cl_addr_copy((struct sockaddr *) &new->cl_addr, sa);
> > new->cl_flavor = rqstp->rq_flavor;
> > princ = svc_gss_principal(rqstp);
> > if (princ) {
> > @@ -1698,7 +1742,7 @@ nfsd4_setclientid_confirm(struct svc_rqst
> > *rqstp,
> > struct nfsd4_compound_state *cstate,
> > struct nfsd4_setclientid_confirm *setclientid_confirm)
> > {
> > - struct sockaddr_in *sin = svc_addr_in(rqstp);
> > + struct sockaddr *sa = svc_addr(rqstp);
> > struct nfs4_client *conf, *unconf;
> > nfs4_verifier confirm = setclientid_confirm->sc_confirm;
> > clientid_t * clid = &setclientid_confirm->sc_clientid;
> > @@ -1717,9 +1761,9 @@ nfsd4_setclientid_confirm(struct svc_rqst
> > *rqstp,
> > unconf = find_unconfirmed_client(clid);
> >
> > status = nfserr_clid_inuse;
> > - if (conf && conf->cl_addr != sin->sin_addr.s_addr)
> > + if (conf && !cl_addr_equal((struct sockaddr *) &conf->cl_addr, sa))
> > goto out;
> > - if (unconf && unconf->cl_addr != sin->sin_addr.s_addr)
> > + if (unconf && !cl_addr_equal((struct sockaddr *) &unconf->cl_addr,
> > sa))
> > goto out;
> >
> > /*
> > diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
> > index d866368..fb0c404 100644
> > --- a/include/linux/nfsd/state.h
> > +++ b/include/linux/nfsd/state.h
> > @@ -200,7 +200,7 @@ struct nfs4_client {
> > char cl_recdir[HEXDIR_LEN]; /* recovery dir */
> > nfs4_verifier cl_verifier; /* generated by client */
> > time_t cl_time; /* time of last lease
> > renewal */
> > - __be32 cl_addr; /* client ipaddress */
> > + struct sockaddr_storage cl_addr; /* client ipaddress */
> > u32 cl_flavor; /* setclientid pseudoflavor */
> > char *cl_principal; /* setclientid principal name */
> > struct svc_cred cl_cred; /* setclientid principal */
>
Thanks for the comments so far!
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/4] nfsd: convert gen_callback to use rpc_uaddr2sockaddr
2009-08-10 22:09 [PATCH 0/4] nfsd: add support for NFSv4 callbacks over IPv6 (try #3) Jeff Layton
2009-08-10 22:09 ` [PATCH 1/4] nfsd: convert nfs4_cb_conn struct to hold address in sockaddr_storage Jeff Layton
2009-08-10 22:09 ` [PATCH 2/4] nfsd: make nfs4_client->cl_addr a struct sockaddr_storage Jeff Layton
@ 2009-08-10 22:09 ` Jeff Layton
2009-08-10 22:09 ` [PATCH 4/4] nfsd: add support for NFSv4 callbacks over IPv6 Jeff Layton
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2009-08-10 22:09 UTC (permalink / raw)
To: bfields; +Cc: chuck.lever, linux-nfs, nfsv4
No need for a separate routine here. Use the standard one provided in
sunrpc module.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfsd/nfs4state.c | 82 ++++----------------------------------------------
1 files changed, 7 insertions(+), 75 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cbc1a05..d294b1d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -55,6 +55,7 @@
#include <linux/lockd/bind.h>
#include <linux/module.h>
#include <linux/sunrpc/svcauth_gss.h>
+#include <linux/sunrpc/clnt.h>
#define NFSDDBG_FACILITY NFSDDBG_PROC
@@ -896,78 +897,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval,
return NULL;
}
-/* a helper function for parse_callback */
-static int
-parse_octet(unsigned int *lenp, char **addrp)
-{
- unsigned int len = *lenp;
- char *p = *addrp;
- int n = -1;
- char c;
-
- for (;;) {
- if (!len)
- break;
- len--;
- c = *p++;
- if (c == '.')
- break;
- if ((c < '0') || (c > '9')) {
- n = -1;
- break;
- }
- if (n < 0)
- n = 0;
- n = (n * 10) + (c - '0');
- if (n > 255) {
- n = -1;
- break;
- }
- }
- *lenp = len;
- *addrp = p;
- return n;
-}
-
-/* parse and set the setclientid ipv4 callback address */
-static int
-parse_ipv4(unsigned int addr_len, char *addr_val, struct sockaddr_in *s4)
-{
- int temp = 0;
- u32 cbaddr = 0;
- u16 cbport = 0;
- u32 addrlen = addr_len;
- char *addr = addr_val;
- int i, shift;
-
- s4->sin_family = AF_INET;
-
- /* ipaddress */
- shift = 24;
- for(i = 4; i > 0 ; i--) {
- if ((temp = parse_octet(&addrlen, &addr)) < 0) {
- return 0;
- }
- cbaddr |= (temp << shift);
- if (shift > 0)
- shift -= 8;
- }
- s4->sin_addr.s_addr = htonl(cbaddr);
-
- /* port */
- shift = 8;
- for(i = 2; i > 0 ; i--) {
- if ((temp = parse_octet(&addrlen, &addr)) < 0) {
- return 0;
- }
- cbport |= (temp << shift);
- if (shift > 0)
- shift -= 8;
- }
- s4->sin_port = htons(cbport);
- return 1;
-}
-
static void
gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se)
{
@@ -977,10 +906,13 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se)
if ((se->se_callback_netid_len != 3) || memcmp((char *)se->se_callback_netid_val, "tcp", 3))
goto out_err;
- if ( !(parse_ipv4(se->se_callback_addr_len, se->se_callback_addr_val,
- (struct sockaddr_in *) &cb->cb_addr)))
+ cb->cb_addrlen = rpc_uaddr2sockaddr(se->se_callback_addr_val,
+ se->se_callback_addr_len,
+ (struct sockaddr *) &cb->cb_addr,
+ sizeof(cb->cb_addr));
+
+ if (cb->cb_addr.ss_family != AF_INET)
goto out_err;
- cb->cb_addrlen = sizeof(struct sockaddr_in);
cb->cb_minorversion = 0;
cb->cb_prog = se->se_callback_prog;
cb->cb_ident = se->se_callback_ident;
--
1.6.0.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/4] nfsd: add support for NFSv4 callbacks over IPv6
2009-08-10 22:09 [PATCH 0/4] nfsd: add support for NFSv4 callbacks over IPv6 (try #3) Jeff Layton
` (2 preceding siblings ...)
2009-08-10 22:09 ` [PATCH 3/4] nfsd: convert gen_callback to use rpc_uaddr2sockaddr Jeff Layton
@ 2009-08-10 22:09 ` Jeff Layton
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2009-08-10 22:09 UTC (permalink / raw)
To: bfields; +Cc: chuck.lever, linux-nfs, nfsv4
The framework to add this is all in place. Now, add the code to allow
support for establishing a callback channel on an IPv6 socket.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++++------
1 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d294b1d..578e48c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -56,6 +56,7 @@
#include <linux/module.h>
#include <linux/sunrpc/svcauth_gss.h>
#include <linux/sunrpc/clnt.h>
+#include <net/ipv6.h>
#define NFSDDBG_FACILITY NFSDDBG_PROC
@@ -901,9 +902,16 @@ static void
gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se)
{
struct nfs4_cb_conn *cb = &clp->cl_cb_conn;
-
- /* Currently, we only support tcp for the callback channel */
- if ((se->se_callback_netid_len != 3) || memcmp((char *)se->se_callback_netid_val, "tcp", 3))
+ unsigned short expected_family;
+
+ /* Currently, we only support tcp and tcp6 for the callback channel */
+ if (se->se_callback_netid_len == 3 &&
+ !memcmp(se->se_callback_netid_val, "tcp", 3))
+ expected_family = AF_INET;
+ else if (se->se_callback_netid_len == 4 &&
+ !memcmp(se->se_callback_netid_val, "tcp6", 4))
+ expected_family = AF_INET6;
+ else
goto out_err;
cb->cb_addrlen = rpc_uaddr2sockaddr(se->se_callback_addr_val,
@@ -911,7 +919,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se)
(struct sockaddr *) &cb->cb_addr,
sizeof(cb->cb_addr));
- if (cb->cb_addr.ss_family != AF_INET)
+ if (cb->cb_addr.ss_family != expected_family)
goto out_err;
cb->cb_minorversion = 0;
cb->cb_prog = se->se_callback_prog;
@@ -1153,6 +1161,7 @@ static bool
cl_addr_equal(struct sockaddr *a, struct sockaddr *b)
{
struct sockaddr_in *a4, *b4;
+ struct sockaddr_in6 *a6, *b6;
if (a->sa_family != b->sa_family)
return false;
@@ -1162,6 +1171,10 @@ cl_addr_equal(struct sockaddr *a, struct sockaddr *b)
a4 = (struct sockaddr_in *) a;
b4 = (struct sockaddr_in *) b;
return (a4->sin_addr.s_addr == b4->sin_addr.s_addr);
+ case AF_INET6:
+ a6 = (struct sockaddr_in6 *) a;
+ b6 = (struct sockaddr_in6 *) b;
+ return ipv6_addr_equal(&a6->sin6_addr, &b6->sin6_addr);
}
/* not sure how to compare, so just return false */
@@ -1173,6 +1186,7 @@ static void
cl_addr_copy(struct sockaddr *dst, struct sockaddr *src)
{
struct sockaddr_in *s4, *d4;
+ struct sockaddr_in6 *s6, *d6;
dst->sa_family = src->sa_family;
@@ -1182,6 +1196,11 @@ cl_addr_copy(struct sockaddr *dst, struct sockaddr *src)
d4 = (struct sockaddr_in *) dst;
d4->sin_addr.s_addr = s4->sin_addr.s_addr;
return;
+ case AF_INET6:
+ s6 = (struct sockaddr_in6 *) src;
+ d6 = (struct sockaddr_in6 *) dst;
+ ipv6_addr_copy(&d6->sin6_addr, &s6->sin6_addr);
+ return;
default:
dst->sa_family = AF_UNSPEC;
}
@@ -1196,7 +1215,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
int status;
unsigned int strhashval;
char dname[HEXDIR_LEN];
- char addr_str[INET_ADDRSTRLEN];
+ char addr_str[INET6_ADDRSTRLEN];
nfs4_verifier verf = exid->verifier;
struct sockaddr *sa = svc_addr(rqstp);
@@ -1574,7 +1593,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
/* RFC 3530 14.2.33 CASE 0: */
status = nfserr_clid_inuse;
if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
- char addr_str[INET_ADDRSTRLEN];
+ char addr_str[INET6_ADDRSTRLEN];
rpc_ntop((struct sockaddr *) &conf->cl_addr, addr_str,
sizeof(addr_str));
dprintk("NFSD: setclientid: string in use by client "
--
1.6.0.6
^ permalink raw reply related [flat|nested] 7+ messages in thread