sunrpc: Add scope id to svc_addr_u for IPv6 LINKLOCAL address

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

This patch adds scope id to svc_addr_u for IPv6 address, and copy scope from xprt->xpt_local to rqstp->rq_daddr for use.

With this patch, lockd can callback to client success.

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/lockd/host.c            |    3 ++-
 fs/nfsd/nfs4state.c        |    7 ++++++-
 include/linux/sunrpc/svc.h |   12 ++++++++++--
 net/sunrpc/svc_xprt.c      |    5 ++++-
 net/sunrpc/svcsock.c       |    4 ++--
 5 files changed, 24 insertions(+), 7 deletions(-)

fs/lockd/host.c +2 -1

index b7c99bf..1fff87a 100644
@@ -346,7 +346,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 		src_sap = (struct sockaddr *)&sin;
 		break;
 	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
+		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6.sin6_addr);
+		sin6.sin6_scope_id = rqstp->rq_daddr.addr6.sin6_scope_id;
 		src_sap = (struct sockaddr *)&sin6;
 		break;
 	default:

fs/nfsd/nfs4state.c +6 -1

index 3787ec1..1169411 100644
@@ -1182,7 +1182,7 @@ static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, uni
 		return;
 	case AF_INET6:
 		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
+		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6.sin6_addr;
 		return;
 	}
 }

@@ -1219,6 +1219,11 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
 	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+
+	if (conn->cb_saddr.ss_family == AF_INET6)
+		((struct sockaddr_in6 *)&conn->cb_saddr)->sin6_scope_id = 
+			rqstp->rq_daddr.addr6.sin6_scope_id;
+
 	return;
 out_err:
 	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/svc.h +10 -2

index 223588a..cd611b5 100644
@@ -212,9 +212,17 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
+/* 
+ * Add scope id for LINKLOCAL address
+ */
+struct in6_addr_scopeid{
+	struct in6_addr	sin6_addr;
+	__u32		sin6_scope_id;
+};
+
 union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
+	struct in_addr		addr;
+	struct in6_addr_scopeid	addr6;
 };
 
 /*

net/sunrpc/svc_xprt.c +4 -1

index bd31208..8aada49 100644
@@ -269,7 +269,10 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
 		break;
 	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
+		rqstp->rq_daddr.addr6.sin6_addr = 
+				((struct sockaddr_in6 *)sin)->sin6_addr;
+		rqstp->rq_daddr.addr6.sin6_scope_id =
+				 ((struct sockaddr_in6 *)sin)->sin6_scope_id;
 		break;
 	}
 }

net/sunrpc/svcsock.c +2 -2

index 767d494..beb2575 100644
@@ -155,7 +155,7 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_type = IPV6_PKTINFO;
 			pki->ipi6_ifindex = 0;
 			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+					&rqstp->rq_daddr.addr6.sin6_addr);
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -513,7 +513,7 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+	ipv6_addr_copy(&rqstp->rq_daddr.addr6.sin6_addr, &pki->ipi6_addr);
 	return 1;
 }
 

Signature · 1 line

1.7.6

Quoted 18 lines · Mi Jinlong

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Quoted 4 lines

This patch adds scope id to svc_addr_u for IPv6 address, and copy scope from xprt->xpt_local to rqstp->rq_daddr for use.

With this patch, lockd can callback to client success.

I guess this makes sense to me, but someone who understands IPv6 better should comment... Chuck? Jeff?

Quoted 19 lines · On Mon, 22 Aug 2011 15:23:48 -0400 "J. Bruce Fields" <[email protected]>…

Quoted 18 lines · Mi Jinlong

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Quoted 8 lines

Quoted 4 lines

This patch adds scope id to svc_addr_u for IPv6 address, and copy scope from xprt->xpt_local to rqstp->rq_daddr for use.

With this patch, lockd can callback to client success.

I guess this makes sense to me, but someone who understands IPv6 better should comment... Chuck? Jeff?

Sounds like a reasonable explanation. Link-local addresses all have the same prefix, so we need a scopeid (aka interface ID# for linux) to know which interface we should send a call on.

When I did the patches to allow NFSv4 callbacks to go over IPv6, I did something similar, but used the rq_addr. From gen_callback:

       struct sockaddr *sa = svc_addr(rqstp);
       u32 scopeid = rpc_get_scope_id(sa);

...and the scopeid was used to populate the scopeid of the callback address. The rq_daddr should be equivalent though. The _addr and _daddr must have the same scopeid or something is very wrong...

That said, on a semi-related note...

I have to wonder about statd in conjunction with link-local addresses. I have a hard time understanding how you'd ever get reboot notifications in such a setup.

You might be able to make it work if /etc/hosts allowed you to put in a scopeid for an address, but even then it sounds sketchy...

Quoted 5 lines · Jeff Layton

That said, on a semi-related note...

I have to wonder about statd in conjunction with link-local addresses. I have a hard time understanding how you'd ever get reboot notifications in such a setup.

The only way that could work is if DNS was able to look at a local hostname, and provide the correct scope ID. Doubtful. I wonder if mDNS might provide such ability.

Quoted 38 lines · Jeff Layton

Quoted 19 lines · On Mon, 22 Aug 2011 15:23:48 -0400 "J. Bruce Fields" <[email protected]>…

Quoted 18 lines · Mi Jinlong

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Quoted 8 lines

Quoted 4 lines

This patch adds scope id to svc_addr_u for IPv6 address, and copy scope from xprt->xpt_local to rqstp->rq_daddr for use.

With this patch, lockd can callback to client success.

I guess this makes sense to me, but someone who understands IPv6 better should comment... Chuck? Jeff?

Sounds like a reasonable explanation. Link-local addresses all have the same prefix, so we need a scopeid (aka interface ID# for linux) to know which interface we should send a call on.

When I did the patches to allow NFSv4 callbacks to go over IPv6, I did something similar, but used the rq_addr. From gen_callback:

Quoted 6 lines

       struct sockaddr *sa = svc_addr(rqstp);
       u32 scopeid = rpc_get_scope_id(sa);

...and the scopeid was used to populate the scopeid of the callback address. The rq_daddr should be equivalent though. The _addr and _daddr must have the same scopeid or something is very wrong...

Sure, that would preclude the need to upgrade the field to a struct sockaddr_storage.

Quoted 5 lines

That said, on a semi-related note...

I have to wonder about statd in conjunction with link-local addresses. I have a hard time understanding how you'd ever get reboot notifications in such a setup.

The only way that could work is if DNS was able to look at a local hostname, and provide the correct scope ID. Doubtful. I wonder if mDNS might provide such ability.

Quoted 44 lines · On Mon, 22 Aug 2011 15:44:39 -0400 Chuck Lever <[email protected]> wrote:

Quoted 36 lines · Jeff Layton

Quoted 19 lines · On Mon, 22 Aug 2011 15:23:48 -0400 "J. Bruce Fields" <[email protected]>…

Quoted 18 lines · Mi Jinlong

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Quoted 7 lines

Quoted 4 lines

This patch adds scope id to svc_addr_u for IPv6 address, and copy scope from xprt->xpt_local to rqstp->rq_daddr for use.

With this patch, lockd can callback to client success.

I guess this makes sense to me, but someone who understands IPv6 better should comment... Chuck? Jeff?

Sounds like a reasonable explanation. Link-local addresses all have the same prefix, so we need a scopeid (aka interface ID# for linux) to know which interface we should send a call on.

When I did the patches to allow NFSv4 callbacks to go over IPv6, I did something similar, but used the rq_addr. From gen_callback:

Quoted 6 lines

       struct sockaddr *sa = svc_addr(rqstp);
       u32 scopeid = rpc_get_scope_id(sa);

...and the scopeid was used to populate the scopeid of the callback address. The rq_daddr should be equivalent though. The _addr and _daddr must have the same scopeid or something is very wrong...

Quoted 9 lines

Sure, that would preclude the need to upgrade the field to a struct sockaddr_storage.

Quoted 5 lines

That said, on a semi-related note...

I have to wonder about statd in conjunction with link-local addresses. I have a hard time understanding how you'd ever get reboot notifications in such a setup.

The only way that could work is if DNS was able to look at a local hostname, and provide the correct scope ID. Doubtful. I wonder if mDNS might provide such ability.

I haven't looked closely at the mDNS protocol, but regular DNS does not support scopeids on AAAA records.

It does seem somewhat plausible that this could work "accidentally". The reboot notifications could end up going via routable addresses instead of the link-local ones if you set nsm_use_hostnames.

NSM is a shaky protocol at best though, so I'd make sure to test this before relying on it. Mi, if you get this working (including reboot notifications), please let us know how you solved this...

Jeff Layton :

Quoted 64 lines

Quoted 44 lines · On Mon, 22 Aug 2011 15:44:39 -0400 Chuck Lever <[email protected]> wrote:

Quoted 36 lines · Jeff Layton

Quoted 19 lines · On Mon, 22 Aug 2011 15:23:48 -0400 "J. Bruce Fields" <[email protected]>…

Quoted 18 lines · Mi Jinlong

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Quoted 7 lines

Quoted 4 lines

This patch adds scope id to svc_addr_u for IPv6 address, and copy scope from xprt->xpt_local to rqstp->rq_daddr for use.

With this patch, lockd can callback to client success.

I guess this makes sense to me, but someone who understands IPv6 better should comment... Chuck? Jeff?

Sounds like a reasonable explanation. Link-local addresses all have the same prefix, so we need a scopeid (aka interface ID# for linux) to know which interface we should send a call on.

When I did the patches to allow NFSv4 callbacks to go over IPv6, I did something similar, but used the rq_addr. From gen_callback:

Quoted 6 lines

      struct sockaddr *sa = svc_addr(rqstp);
      u32 scopeid = rpc_get_scope_id(sa);

...and the scopeid was used to populate the scopeid of the callback address. The rq_daddr should be equivalent though. The _addr and _daddr must have the same scopeid or something is very wrong...

Quoted 9 lines

Sure, that would preclude the need to upgrade the field to a struct sockaddr_storage.

Quoted 5 lines

That said, on a semi-related note...

I have to wonder about statd in conjunction with link-local addresses. I have a hard time understanding how you'd ever get reboot notifications in such a setup.

The only way that could work is if DNS was able to look at a local hostname, and provide the correct scope ID. Doubtful. I wonder if mDNS might provide such ability.

I haven't looked closely at the mDNS protocol, but regular DNS does not support scopeids on AAAA records.

It does seem somewhat plausible that this could work "accidentally". The reboot notifications could end up going via routable addresses instead of the link-local ones if you set nsm_use_hostnames.

Quoted 3 lines

NSM is a shaky protocol at best though, so I'd make sure to test this before relying on it. Mi, if you get this working (including reboot notifications), please let us know how you solved this...

Hi Jeff,

I'm not working on NSM. It seems there are some problem of the latest statd from Steve's git tee.

If having time, I will check the latest statd and sm-notify.

Quoted 9 lines · Mi Jinlong

include/linux/sunrpc/svc.h +8 -0

+/* 
+ * Add scope id for LINKLOCAL address
+ */
+struct in6_addr_scopeid{
+	struct in6_addr	sin6_addr;
+	__u32		sin6_scope_id;
+};
+

union svc_addr_u {

include/linux/sunrpc/svc.h static inline void svc_putu32(struct kvec *iov, __be32 val) · line 225

Quoted 4 lines

include/linux/sunrpc/svc.h +2 -2

-    struct in_addr	addr;
-    struct in6_addr	addr6;
+	struct in_addr		addr;
+	struct in6_addr_scopeid	addr6;

By the way, is there any reason why nfsd really needs its own address structure? Shouldn't we use sockaddr_storage or something? I feel like we've got a little too much one-off address handling in nfsd.

Quoted 10 lines · J. Bruce Fields

Quoted 9 lines · Mi Jinlong

include/linux/sunrpc/svc.h +8 -0

+/* 
+ * Add scope id for LINKLOCAL address
+ */
+struct in6_addr_scopeid{
+	struct in6_addr	sin6_addr;
+	__u32		sin6_scope_id;
+};
+

union svc_addr_u {

Quoted 8 lines

include/linux/sunrpc/svc.h static inline void svc_putu32(struct kvec *iov, __be32 val) · line 225

Quoted 4 lines

include/linux/sunrpc/svc.h +2 -2

-    struct in_addr	addr;
-    struct in6_addr	addr6;
+	struct in_addr		addr;
+	struct in6_addr_scopeid	addr6;

By the way, is there any reason why nfsd really needs its own address structure? Shouldn't we use sockaddr_storage or something? I feel like we've got a little too much one-off address handling in nfsd.

That would be my only complaint about the patch.

I think we chose a smaller struct here to save space, and we could do that because we didn't need a port number or scope ID. If a scope ID is indeed required, then we should consider something larger like a struct sockaddr_storage, IMO.

Quoted 19 lines · J. Bruce Fields

Quoted 18 lines · Mi Jinlong

include/linux/sunrpc/svc.h static inline void svc_putu32(struct kvec *iov, __be32 val) · line 225

Quoted 13 lines

include/linux/sunrpc/svc.h +8 -0

+/* 
+ * Add scope id for LINKLOCAL address
+ */
+struct in6_addr_scopeid{
+	struct in6_addr	sin6_addr;
+	__u32		sin6_scope_id;
+};
+

union svc_addr_u {

include/linux/sunrpc/svc.h +2 -2

-    struct in_addr	addr;
-    struct in6_addr	addr6;
+	struct in_addr		addr;
+	struct in6_addr_scopeid	addr6;

By the way, is there any reason why nfsd really needs its own address structure? Shouldn't we use sockaddr_storage or something? I feel like we've got a little too much one-off address handling in nfsd.

Quoted 3 lines

That would be my only complaint about the patch.

I think we chose a smaller struct here to save space, and we could do that because we didn't need a port number or scope ID. If a scope ID is indeed required, then we should consider something larger like a struct sockaddr_storage, IMO.

If we care about the size of struct svc_rqst: rq_vec and rq_pages fields are 4k and 2k, respectively, and the rest is lost in the noise.

Mi Jinlong, would you have the time to try this?

- Replace svc_addr_u by sockaddr_storage
- See of that allows us to delete some lines of code in nfsd by using standard helper functions instead of copying things by hand.

You'd do that in one patch, then apply your scope-id fix on top of it as a second patch.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 19 lines · J. Bruce Fields

Quoted 10 lines · Mi Jinlong

Quoted 8 lines

(quoted diff) +8 -0

+/* 
+ * Add scope id for LINKLOCAL address
+ */
+struct in6_addr_scopeid{
+	struct in6_addr	sin6_addr;
+	__u32		sin6_scope_id;
+};
+

Quoted 8 lines

Review

Quoted 5 lines

union svc_addr_u {

(quoted diff) +2 -2

-    struct in_addr	addr;
-    struct in6_addr	addr6;
+	struct in_addr		addr;
+	struct in6_addr_scopeid	addr6;

By the way, is there any reason why nfsd really needs its own address structure? Shouldn't we use sockaddr_storage or something? I feel like we've got a little too much one-off address handling in nfsd.

Quoted 3 lines

That would be my only complaint about the patch.

I think we chose a smaller struct here to save space, and we could do that because we didn't need a port number or scope ID. If a scope ID is indeed required, then we should consider something larger like a struct sockaddr_storage, IMO.

If we care about the size of struct svc_rqst: rq_vec and rq_pages fields are 4k and 2k, respectively, and the rest is lost in the noise.

Mi Jinlong, would you have the time to try this?

- Replace svc_addr_u by sockaddr_storage
- See of that allows us to delete some lines of code in nfsd by using standard helper functions instead of copying things by hand.

You'd do that in one patch, then apply your scope-id fix on top of it as a second patch.

From 86bb57f2d7c29124ae73bd121bb6b97a72fe6f4d Mon Sep 17 00:00:00 2001

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/lockd/host.c            |   14 ++------------
 fs/nfsd/nfs4state.c        |   16 +---------------
 include/linux/sunrpc/svc.h |   30 +++++++++++++++++++++---------
 net/sunrpc/svc_xprt.c      |   13 ++-----------
 net/sunrpc/svcsock.c       |    9 +++++----
 5 files changed, 31 insertions(+), 51 deletions(-)

fs/lockd/host.c +2 -12

index b7c99bf..b23db05 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
 	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -342,12 +336,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 
 	switch (ni.sap->sa_family) {
 	case AF_INET:
-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;
 	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
+		src_sap = svc_daddr(rqstp);
 		break;
 	default:
 		dprintk("lockd: %s failed; unrecognized address family\n",

fs/nfsd/nfs4state.c +1 -15

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }
 
-static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, union svc_addr_u *svcaddr)
-{
-	switch (family) {
-	case AF_INET:
-		((struct sockaddr_in *)sa)->sin_family = AF_INET;
-		((struct sockaddr_in *)sa)->sin_addr = svcaddr->addr;
-		return;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
-		return;
-	}
-}
-
 static void
 gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_rqst *rqstp)
 {

@@ -1218,7 +1204,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
-	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+	memcpy(&conn->cb_saddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 	return;
 out_err:
 	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/svc.h +21 -9

index 223588a..3bc0dc9 100644
@@ -212,11 +212,6 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
-union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
-};
-
 /*
  * The context of a single thread, including the request currently being
  * processed.

@@ -225,8 +220,12 @@ struct svc_rqst {
 	struct list_head	rq_list;	/* idle list */
 	struct list_head	rq_all;		/* all threads list */
 	struct svc_xprt *	rq_xprt;	/* transport ptr */
+
 	struct sockaddr_storage	rq_addr;	/* peer address */
 	size_t			rq_addrlen;
+	struct sockaddr_storage	rq_daddr;	/* dest addr of request
+						 *  - reply from here */
+	size_t			rq_daddrlen;
 
 	struct svc_serv *	rq_server;	/* RPC service definition */
 	struct svc_pool *	rq_pool;	/* thread pool */

@@ -255,9 +254,6 @@ struct svc_rqst {
 	unsigned short
 				rq_secure  : 1;	/* secure port */
 
-	union svc_addr_u	rq_daddr;	/* dest addr of request
-						 *  - reply from here */
-
 	void *			rq_argp;	/* decoded arguments */
 	void *			rq_resp;	/* xdr'd results */
 	void *			rq_auth_data;	/* flavor-specific data */

@@ -300,6 +296,21 @@ static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 	return (struct sockaddr *) &rqst->rq_addr;
 }
 
+static inline struct sockaddr_in *svc_daddr_in(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr_in6 *svc_daddr_in6(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in6 *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr *svc_daddr(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr *) &rqst->rq_daddr;
+}
+
 /*
  * Check buffer bounds after decoding arguments
  */

@@ -340,7 +351,8 @@ struct svc_deferred_req {
 	struct svc_xprt		*xprt;
 	struct sockaddr_storage	addr;	/* where reply must go */
 	size_t			addrlen;
-	union svc_addr_u	daddr;	/* where reply must come from */
+	struct sockaddr_storage	daddr;	/* where reply must come from */
+	size_t			daddrlen;
 	struct cache_deferred_req handle;
 	size_t			xprt_hlen;
 	int			argslen;

net/sunrpc/svc_xprt.c +2 -11

index bd31208..d86bb67 100644
@@ -254,8 +254,6 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
  */
 void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
-	struct sockaddr *sin;
-
 	memcpy(&rqstp->rq_addr, &xprt->xpt_remote, xprt->xpt_remotelen);
 	rqstp->rq_addrlen = xprt->xpt_remotelen;
 

@@ -263,15 +261,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 	 * Destination address in request is needed for binding the
 	 * source address in RPC replies/callbacks later.
 	 */
-	sin = (struct sockaddr *)&xprt->xpt_local;
-	switch (sin->sa_family) {
-	case AF_INET:
-		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
-		break;
-	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
-		break;
-	}
+	memcpy(&rqstp->rq_daddr, &xprt->xpt_local, xprt->xpt_locallen);
+	rqstp->rq_daddrlen = xprt->xpt_locallen;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs);
 

net/sunrpc/svcsock.c +5 -4

index 767d494..b9f4ad4 100644
@@ -143,7 +143,8 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_level = SOL_IP;
 			cmh->cmsg_type = IP_PKTINFO;
 			pki->ipi_ifindex = 0;
-			pki->ipi_spec_dst.s_addr = rqstp->rq_daddr.addr.s_addr;
+			pki->ipi_spec_dst.s_addr =
+				 svc_daddr_in(rqstp)->sin_addr.s_addr;
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -155,7 +156,7 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_type = IPV6_PKTINFO;
 			pki->ipi6_ifindex = 0;
 			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+					&svc_daddr_in6(rqstp)->sin6_addr);
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -500,7 +501,7 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+	svc_daddr_in(rqstp)->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

@@ -513,7 +514,7 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+	ipv6_addr_copy(&svc_daddr_in6(rqstp)->sin6_addr, &pki->ipi6_addr);
 	return 1;
 }
 

Signature · 7 lines

1.7.6


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 21 lines

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Quoted 8 lines

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/lockd/host.c            |   14 ++------------
 fs/nfsd/nfs4state.c        |   16 +---------------
 include/linux/sunrpc/svc.h |   30 +++++++++++++++++++++---------
 net/sunrpc/svc_xprt.c      |   13 ++-----------
 net/sunrpc/svcsock.c       |    9 +++++----
 5 files changed, 31 insertions(+), 51 deletions(-)

Can't complain about that diffstat!

How are you testing this?

Quoted 220 lines

fs/lockd/host.c +2 -12

index b7c99bf..b23db05 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
 	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -342,12 +336,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 
 	switch (ni.sap->sa_family) {
 	case AF_INET:
-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;
 	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
+		src_sap = svc_daddr(rqstp);
 		break;
 	default:
 		dprintk("lockd: %s failed; unrecognized address family\n",

fs/nfsd/nfs4state.c +1 -15

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }
 
-static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, union svc_addr_u *svcaddr)
-{
-	switch (family) {
-	case AF_INET:
-		((struct sockaddr_in *)sa)->sin_family = AF_INET;
-		((struct sockaddr_in *)sa)->sin_addr = svcaddr->addr;
-		return;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
-		return;
-	}
-}
-
 static void
 gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_rqst *rqstp)
 {

@@ -1218,7 +1204,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
-	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+	memcpy(&conn->cb_saddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 	return;
 out_err:
 	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/svc.h +21 -9

index 223588a..3bc0dc9 100644
@@ -212,11 +212,6 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
-union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
-};
-
 /*
  * The context of a single thread, including the request currently being
  * processed.

@@ -225,8 +220,12 @@ struct svc_rqst {
 	struct list_head	rq_list;	/* idle list */
 	struct list_head	rq_all;		/* all threads list */
 	struct svc_xprt *	rq_xprt;	/* transport ptr */
+
 	struct sockaddr_storage	rq_addr;	/* peer address */
 	size_t			rq_addrlen;
+	struct sockaddr_storage	rq_daddr;	/* dest addr of request
+						 *  - reply from here */
+	size_t			rq_daddrlen;
 
 	struct svc_serv *	rq_server;	/* RPC service definition */
 	struct svc_pool *	rq_pool;	/* thread pool */

@@ -255,9 +254,6 @@ struct svc_rqst {
 	unsigned short
 				rq_secure  : 1;	/* secure port */
 
-	union svc_addr_u	rq_daddr;	/* dest addr of request
-						 *  - reply from here */
-
 	void *			rq_argp;	/* decoded arguments */
 	void *			rq_resp;	/* xdr'd results */
 	void *			rq_auth_data;	/* flavor-specific data */

@@ -300,6 +296,21 @@ static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 	return (struct sockaddr *) &rqst->rq_addr;
 }
 
+static inline struct sockaddr_in *svc_daddr_in(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr_in6 *svc_daddr_in6(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in6 *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr *svc_daddr(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr *) &rqst->rq_daddr;
+}
+
 /*
  * Check buffer bounds after decoding arguments
  */

@@ -340,7 +351,8 @@ struct svc_deferred_req {
 	struct svc_xprt		*xprt;
 	struct sockaddr_storage	addr;	/* where reply must go */
 	size_t			addrlen;
-	union svc_addr_u	daddr;	/* where reply must come from */
+	struct sockaddr_storage	daddr;	/* where reply must come from */
+	size_t			daddrlen;
 	struct cache_deferred_req handle;
 	size_t			xprt_hlen;
 	int			argslen;

net/sunrpc/svc_xprt.c +2 -11

index bd31208..d86bb67 100644
@@ -254,8 +254,6 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
  */
 void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
-	struct sockaddr *sin;
-
 	memcpy(&rqstp->rq_addr, &xprt->xpt_remote, xprt->xpt_remotelen);
 	rqstp->rq_addrlen = xprt->xpt_remotelen;
 

@@ -263,15 +261,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 	 * Destination address in request is needed for binding the
 	 * source address in RPC replies/callbacks later.
 	 */
-	sin = (struct sockaddr *)&xprt->xpt_local;
-	switch (sin->sa_family) {
-	case AF_INET:
-		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
-		break;
-	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
-		break;
-	}
+	memcpy(&rqstp->rq_daddr, &xprt->xpt_local, xprt->xpt_locallen);
+	rqstp->rq_daddrlen = xprt->xpt_locallen;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs);
 

net/sunrpc/svcsock.c +5 -4

index 767d494..b9f4ad4 100644
@@ -143,7 +143,8 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_level = SOL_IP;
 			cmh->cmsg_type = IP_PKTINFO;
 			pki->ipi_ifindex = 0;
-			pki->ipi_spec_dst.s_addr = rqstp->rq_daddr.addr.s_addr;
+			pki->ipi_spec_dst.s_addr =
+				 svc_daddr_in(rqstp)->sin_addr.s_addr;
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -155,7 +156,7 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_type = IPV6_PKTINFO;
 			pki->ipi6_ifindex = 0;
 			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+					&svc_daddr_in6(rqstp)->sin6_addr);
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -500,7 +501,7 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+	svc_daddr_in(rqstp)->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

@@ -513,7 +514,7 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+	ipv6_addr_copy(&svc_daddr_in6(rqstp)->sin6_addr, &pki->ipi6_addr);
 	return 1;
 }
 

Signature · 7 lines

1.7.6


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

J. Bruce Fields:

Quoted 29 lines · Chuck Lever

Quoted 19 lines · J. Bruce Fields

Quoted 10 lines · Mi Jinlong

Quoted 8 lines

(quoted diff) +8 -0

+/* 
+ * Add scope id for LINKLOCAL address
+ */
+struct in6_addr_scopeid{
+	struct in6_addr	sin6_addr;
+	__u32		sin6_scope_id;
+};
+

Quoted 8 lines

include/linux/sunrpc/svc.h static inline void svc_putu32(struct kvec *iov, __be32 val)

Quoted 5 lines

union svc_addr_u {

include/linux/sunrpc/svc.h +2 -2

-    struct in_addr	addr;
-    struct in6_addr	addr6;
+	struct in_addr		addr;
+	struct in6_addr_scopeid	addr6;

By the way, is there any reason why nfsd really needs its own address structure? Shouldn't we use sockaddr_storage or something? I feel like we've got a little too much one-off address handling in nfsd.

Quoted 3 lines

That would be my only complaint about the patch.

I think we chose a smaller struct here to save space, and we could do that because we didn't need a port number or scope ID. If a scope ID is indeed required, then we should consider something larger like a struct sockaddr_storage, IMO.

If we care about the size of struct svc_rqst: rq_vec and rq_pages fields are 4k and 2k, respectively, and the rest is lost in the noise.

Mi Jinlong, would you have the time to try this?

Quoted 7 lines

- Replace svc_addr_u by sockaddr_storage
- See of that allows us to delete some lines of code in nfsd by using standard helper functions instead of copying things by hand.

You'd do that in one patch, then apply your scope-id fix on top of it as a second patch.

After replace svc_addr_u by sockaddr_storage, the scope-id fix isn't needed.

thanks,
Mi Jinlong

From 86bb57f2d7c29124ae73bd121bb6b97a72fe6f4d Mon Sep 17 00:00:00 2001
From: Mi Jinlong <[email protected]>
Date: Mon, 24 Aug 2011 13:22:39 +0800
Subject: [PATCH] SUNRPC: Replace svc_addr_u by sockaddr_storage

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/lockd/host.c            |   14 ++------------
 fs/nfsd/nfs4state.c        |   16 +---------------
 include/linux/sunrpc/svc.h |   30 +++++++++++++++++++++---------
 net/sunrpc/svc_xprt.c      |   13 ++-----------
 net/sunrpc/svcsock.c       |    9 +++++----
 5 files changed, 31 insertions(+), 51 deletions(-)

fs/lockd/host.c +2 -12

index b7c99bf..b23db05 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
 	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -342,12 +336,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 
 	switch (ni.sap->sa_family) {
 	case AF_INET:
-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;
 	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
+		src_sap = svc_daddr(rqstp);
 		break;
 	default:
 		dprintk("lockd: %s failed; unrecognized address family\n",

fs/nfsd/nfs4state.c +1 -15

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }
 
-static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, union svc_addr_u *svcaddr)
-{
-	switch (family) {
-	case AF_INET:
-		((struct sockaddr_in *)sa)->sin_family = AF_INET;
-		((struct sockaddr_in *)sa)->sin_addr = svcaddr->addr;
-		return;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
-		return;
-	}
-}
-
 static void
 gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_rqst *rqstp)
 {

@@ -1218,7 +1204,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
-	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+	memcpy(&conn->cb_saddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 	return;
 out_err:
 	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/svc.h +21 -9

index 223588a..3bc0dc9 100644
@@ -212,11 +212,6 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
-union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
-};
-
 /*
  * The context of a single thread, including the request currently being
  * processed.

@@ -225,8 +220,12 @@ struct svc_rqst {
 	struct list_head	rq_list;	/* idle list */
 	struct list_head	rq_all;		/* all threads list */
 	struct svc_xprt *	rq_xprt;	/* transport ptr */
+
 	struct sockaddr_storage	rq_addr;	/* peer address */
 	size_t			rq_addrlen;
+	struct sockaddr_storage	rq_daddr;	/* dest addr of request
+						 *  - reply from here */
+	size_t			rq_daddrlen;
 
 	struct svc_serv *	rq_server;	/* RPC service definition */
 	struct svc_pool *	rq_pool;	/* thread pool */

@@ -255,9 +254,6 @@ struct svc_rqst {
 	unsigned short
 				rq_secure  : 1;	/* secure port */
 
-	union svc_addr_u	rq_daddr;	/* dest addr of request
-						 *  - reply from here */
-
 	void *			rq_argp;	/* decoded arguments */
 	void *			rq_resp;	/* xdr'd results */
 	void *			rq_auth_data;	/* flavor-specific data */

@@ -300,6 +296,21 @@ static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 	return (struct sockaddr *) &rqst->rq_addr;
 }
 
+static inline struct sockaddr_in *svc_daddr_in(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr_in6 *svc_daddr_in6(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in6 *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr *svc_daddr(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr *) &rqst->rq_daddr;
+}
+
 /*
  * Check buffer bounds after decoding arguments
  */

@@ -340,7 +351,8 @@ struct svc_deferred_req {
 	struct svc_xprt		*xprt;
 	struct sockaddr_storage	addr;	/* where reply must go */
 	size_t			addrlen;
-	union svc_addr_u	daddr;	/* where reply must come from */
+	struct sockaddr_storage	daddr;	/* where reply must come from */
+	size_t			daddrlen;
 	struct cache_deferred_req handle;
 	size_t			xprt_hlen;
 	int			argslen;

net/sunrpc/svc_xprt.c +2 -11

index bd31208..d86bb67 100644
@@ -254,8 +254,6 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
  */
 void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
-	struct sockaddr *sin;
-
 	memcpy(&rqstp->rq_addr, &xprt->xpt_remote, xprt->xpt_remotelen);
 	rqstp->rq_addrlen = xprt->xpt_remotelen;
 

@@ -263,15 +261,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 	 * Destination address in request is needed for binding the
 	 * source address in RPC replies/callbacks later.
 	 */
-	sin = (struct sockaddr *)&xprt->xpt_local;
-	switch (sin->sa_family) {
-	case AF_INET:
-		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
-		break;
-	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
-		break;
-	}
+	memcpy(&rqstp->rq_daddr, &xprt->xpt_local, xprt->xpt_locallen);
+	rqstp->rq_daddrlen = xprt->xpt_locallen;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs);
 

net/sunrpc/svcsock.c +4 -3

index 767d494..b9f4ad4 100644
@@ -143,7 +143,8 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_level = SOL_IP;
 			cmh->cmsg_type = IP_PKTINFO;
 			pki->ipi_ifindex = 0;
-			pki->ipi_spec_dst.s_addr = rqstp->rq_daddr.addr.s_addr;
+			pki->ipi_spec_dst.s_addr =
+				 svc_daddr_in(rqstp)->sin_addr.s_addr;
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -155,7 +156,7 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_type = IPV6_PKTINFO;
 			pki->ipi6_ifindex = 0;
 			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+					&svc_daddr_in6(rqstp)->sin6_addr);
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -500,7 +501,7 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+	svc_daddr_in(rqstp)->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

From: Mi Jinlong <[email protected]>
Date: Mon, 24 Aug 2011 13:22:39 +0800
Subject: [PATCH] SUNRPC: Replace svc_addr_u by sockaddr_storage

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---

fs/lockd/host.c            |   14 ++------------
fs/nfsd/nfs4state.c        |   16 +---------------
include/linux/sunrpc/svc.h |   30 +++++++++++++++++++++---------
net/sunrpc/svc_xprt.c      |   13 ++-----------
net/sunrpc/svcsock.c       |    9 +++++----
5 files changed, 31 insertions(+), 51 deletions(-)

fs/lockd/host.c +1 -7

index b7c99bf..b23db05 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
 	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -342,12 +336,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 
 	switch (ni.sap->sa_family) {
 	case AF_INET:

fs/nfsd/nfs4state.c +0 -14

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }
 
-static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, union svc_addr_u *svcaddr)
-{
-	switch (family) {
-	case AF_INET:
-		((struct sockaddr_in *)sa)->sin_family = AF_INET;
-		((struct sockaddr_in *)sa)->sin_addr = svcaddr->addr;
-		return;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
-		return;
-	}
-}
-

static void
gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_rqst *rqstp) {

fs/nfsd/nfs4state.c +1 -1

@@ -1218,7 +1204,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
-	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+	memcpy(&conn->cb_saddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 	return;

out_err:

	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/svc.h +21 -9

index 223588a..3bc0dc9 100644
@@ -212,11 +212,6 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
-union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
-};
-
 /*
 * The context of a single thread, including the request currently being
 * processed.

@@ -225,8 +220,12 @@ struct svc_rqst {
 	struct list_head	rq_list;	/* idle list */
 	struct list_head	rq_all;		/* all threads list */
 	struct svc_xprt *	rq_xprt;	/* transport ptr */
+
 	struct sockaddr_storage	rq_addr;	/* peer address */
 	size_t			rq_addrlen;
+	struct sockaddr_storage	rq_daddr;	/* dest addr of request
+						 *  - reply from here */
+	size_t			rq_daddrlen;
 
 	struct svc_serv *	rq_server;	/* RPC service definition */
 	struct svc_pool *	rq_pool;	/* thread pool */

@@ -255,9 +254,6 @@ struct svc_rqst {
 	unsigned short
 				rq_secure  : 1;	/* secure port */
 
-	union svc_addr_u	rq_daddr;	/* dest addr of request
-						 *  - reply from here */
-
 	void *			rq_argp;	/* decoded arguments */
 	void *			rq_resp;	/* xdr'd results */
 	void *			rq_auth_data;	/* flavor-specific data */

@@ -300,6 +296,21 @@ static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 	return (struct sockaddr *) &rqst->rq_addr;
 }
 
+static inline struct sockaddr_in *svc_daddr_in(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr_in6 *svc_daddr_in6(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in6 *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr *svc_daddr(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr *) &rqst->rq_daddr;
+}
+
 /*
 * Check buffer bounds after decoding arguments
 */

@@ -340,7 +351,8 @@ struct svc_deferred_req {
 	struct svc_xprt		*xprt;
 	struct sockaddr_storage	addr;	/* where reply must go */
 	size_t			addrlen;
-	union svc_addr_u	daddr;	/* where reply must come from */
+	struct sockaddr_storage	daddr;	/* where reply must come from */
+	size_t			daddrlen;
 	struct cache_deferred_req handle;
 	size_t			xprt_hlen;
 	int			argslen;

net/sunrpc/svc_xprt.c +2 -11

index bd31208..d86bb67 100644
@@ -254,8 +254,6 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
 */
 void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
-	struct sockaddr *sin;
-
 	memcpy(&rqstp->rq_addr, &xprt->xpt_remote, xprt->xpt_remotelen);
 	rqstp->rq_addrlen = xprt->xpt_remotelen;
 

@@ -263,15 +261,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 	 * Destination address in request is needed for binding the
 	 * source address in RPC replies/callbacks later.
 	 */
-	sin = (struct sockaddr *)&xprt->xpt_local;
-	switch (sin->sa_family) {
-	case AF_INET:
-		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
-		break;
-	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
-		break;
-	}
+	memcpy(&rqstp->rq_daddr, &xprt->xpt_local, xprt->xpt_locallen);
+	rqstp->rq_daddrlen = xprt->xpt_locallen;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs);
 

net/sunrpc/svcsock.c +2 -1

index 767d494..b9f4ad4 100644
@@ -143,7 +143,8 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_level = SOL_IP;
 			cmh->cmsg_type = IP_PKTINFO;
 			pki->ipi_ifindex = 0;
-			pki->ipi_spec_dst.s_addr = rqstp->rq_daddr.addr.s_addr;
+			pki->ipi_spec_dst.s_addr =
+				 svc_daddr_in(rqstp)->sin_addr.s_addr;
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

net/sunrpc/svcsock.c +1 -1

 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -500,7 +501,7 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+	svc_daddr_in(rqstp)->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

net/sunrpc/svcsock.c +0 -0

 	return 1;
 }
 

Signature · 7 lines

1.7.6


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

This seems like a good direction to go in. More comments below.

Quoted 58 lines · Mi Jinlong

From: Mi Jinlong <[email protected]>
Date: Mon, 24 Aug 2011 13:22:39 +0800
Subject: [PATCH] SUNRPC: Replace svc_addr_u by sockaddr_storage

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---

fs/lockd/host.c            |   14 ++------------
fs/nfsd/nfs4state.c        |   16 +---------------
include/linux/sunrpc/svc.h |   30 +++++++++++++++++++++---------
net/sunrpc/svc_xprt.c      |   13 ++-----------
net/sunrpc/svcsock.c       |    9 +++++----
5 files changed, 31 insertions(+), 51 deletions(-)

fs/lockd/host.c +1 -7

index b7c99bf..b23db05 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
 	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -342,12 +336,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 
 	switch (ni.sap->sa_family) {
 	case AF_INET:

fs/lockd/host.c struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp, · line 343

Quoted 10 lines

fs/lockd/host.c +1 -5

-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;
 	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
+		src_sap = svc_daddr(rqstp);
 		break;
 	default:
 		dprintk("lockd: %s failed; unrecognized address family\n",

With the destination address promoted to a full sockaddr, we probably do not need this switch any more.

I notice that the IPv6 fork of rpc_cmp_addr() does not consider the scope IDs of the passed-in addresses. Perhaps it should?

Quoted 151 lines

fs/nfsd/nfs4state.c +0 -14

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }
 
-static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, union svc_addr_u *svcaddr)
-{
-	switch (family) {
-	case AF_INET:
-		((struct sockaddr_in *)sa)->sin_family = AF_INET;
-		((struct sockaddr_in *)sa)->sin_addr = svcaddr->addr;
-		return;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
-		return;
-	}
-}
-

static void
gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_rqst *rqstp) {

fs/nfsd/nfs4state.c +1 -1

@@ -1218,7 +1204,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
-	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+	memcpy(&conn->cb_saddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 	return;

out_err:

	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/svc.h +21 -9

index 223588a..3bc0dc9 100644
@@ -212,11 +212,6 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
-union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
-};
-
 /*
 * The context of a single thread, including the request currently being
 * processed.

@@ -225,8 +220,12 @@ struct svc_rqst {
 	struct list_head	rq_list;	/* idle list */
 	struct list_head	rq_all;		/* all threads list */
 	struct svc_xprt *	rq_xprt;	/* transport ptr */
+
 	struct sockaddr_storage	rq_addr;	/* peer address */
 	size_t			rq_addrlen;
+	struct sockaddr_storage	rq_daddr;	/* dest addr of request
+						 *  - reply from here */
+	size_t			rq_daddrlen;
 
 	struct svc_serv *	rq_server;	/* RPC service definition */
 	struct svc_pool *	rq_pool;	/* thread pool */

@@ -255,9 +254,6 @@ struct svc_rqst {
 	unsigned short
 				rq_secure  : 1;	/* secure port */
 
-	union svc_addr_u	rq_daddr;	/* dest addr of request
-						 *  - reply from here */
-
 	void *			rq_argp;	/* decoded arguments */
 	void *			rq_resp;	/* xdr'd results */
 	void *			rq_auth_data;	/* flavor-specific data */

@@ -300,6 +296,21 @@ static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 	return (struct sockaddr *) &rqst->rq_addr;
 }
 
+static inline struct sockaddr_in *svc_daddr_in(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr_in6 *svc_daddr_in6(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in6 *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr *svc_daddr(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr *) &rqst->rq_daddr;
+}
+
 /*
 * Check buffer bounds after decoding arguments
 */

@@ -340,7 +351,8 @@ struct svc_deferred_req {
 	struct svc_xprt		*xprt;
 	struct sockaddr_storage	addr;	/* where reply must go */
 	size_t			addrlen;
-	union svc_addr_u	daddr;	/* where reply must come from */
+	struct sockaddr_storage	daddr;	/* where reply must come from */
+	size_t			daddrlen;
 	struct cache_deferred_req handle;
 	size_t			xprt_hlen;
 	int			argslen;

net/sunrpc/svc_xprt.c +2 -11

index bd31208..d86bb67 100644
@@ -254,8 +254,6 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
 */
 void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
-	struct sockaddr *sin;
-
 	memcpy(&rqstp->rq_addr, &xprt->xpt_remote, xprt->xpt_remotelen);
 	rqstp->rq_addrlen = xprt->xpt_remotelen;
 

@@ -263,15 +261,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 	 * Destination address in request is needed for binding the
 	 * source address in RPC replies/callbacks later.
 	 */
-	sin = (struct sockaddr *)&xprt->xpt_local;
-	switch (sin->sa_family) {
-	case AF_INET:
-		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
-		break;
-	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
-		break;
-	}
+	memcpy(&rqstp->rq_daddr, &xprt->xpt_local, xprt->xpt_locallen);
+	rqstp->rq_daddrlen = xprt->xpt_locallen;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs);
 

net/sunrpc/svcsock.c +2 -1

index 767d494..b9f4ad4 100644
@@ -143,7 +143,8 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_level = SOL_IP;
 			cmh->cmsg_type = IP_PKTINFO;
 			pki->ipi_ifindex = 0;
-			pki->ipi_spec_dst.s_addr = rqstp->rq_daddr.addr.s_addr;
+			pki->ipi_spec_dst.s_addr =
+				 svc_daddr_in(rqstp)->sin_addr.s_addr;
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

net/sunrpc/svcsock.c static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh) · line 159

Quoted 5 lines

net/sunrpc/svcsock.c +1 -1

@@ -155,7 +156,7 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_type = IPV6_PKTINFO;
 			pki->ipi6_ifindex = 0;
 			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+					&svc_daddr_in6(rqstp)->sin6_addr);

I kind of suspect more is needed here to deal with ipi6_ifindex.

Quoted 12 lines

net/sunrpc/svcsock.c +1 -1

 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -500,7 +501,7 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+	svc_daddr_in(rqstp)->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

net/sunrpc/svcsock.c static int svc_udp_get_dest_address6(struct svc_rqst *rqstp, · line 517

Quoted 6 lines

net/sunrpc/svcsock.c +1 -1

@@ -513,7 +514,7 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+	ipv6_addr_copy(&svc_daddr_in6(rqstp)->sin6_addr, &pki->ipi6_addr);

Likewise, is the incoming interface index ignored when constructing svc_daddr_in6(rqstp) ?

Quoted 6 lines

net/sunrpc/svcsock.c +0 -0

 	return 1;
 }
 

Signature · 1 line

1.7.6

Quoted 61 lines

This seems like a good direction to go in. More comments below.

Quoted 56 lines · Mi Jinlong

From: Mi Jinlong <[email protected]>
Date: Mon, 24 Aug 2011 13:22:39 +0800
Subject: [PATCH] SUNRPC: Replace svc_addr_u by sockaddr_storage

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---

fs/lockd/host.c            |   14 ++------------
fs/nfsd/nfs4state.c        |   16 +---------------
include/linux/sunrpc/svc.h |   30 +++++++++++++++++++++---------
net/sunrpc/svc_xprt.c      |   13 ++-----------
net/sunrpc/svcsock.c       |    9 +++++----
5 files changed, 31 insertions(+), 51 deletions(-)

fs/lockd/host.c +1 -7

index b7c99bf..b23db05 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
 	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -342,12 +336,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 

Quoted 5 lines

Quoted 5 lines

fs/lockd/host.c +0 -3

 	switch (ni.sap->sa_family) {
 	case AF_INET:
-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;

Quoted 8 lines

fs/lockd/host.c struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp, · line 343

Quoted 7 lines

fs/lockd/host.c +1 -2

 	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
+		src_sap = svc_daddr(rqstp);
 		break;
 	default:
 		dprintk("lockd: %s failed; unrecognized address family\n",

With the destination address promoted to a full sockaddr, we probably do not need this switch any more.

Quoted 2 lines

I notice that the IPv6 fork of rpc_cmp_addr() does not consider the scope IDs of the passed-in addresses. Perhaps it should?

Quoted 8 lines

Quoted 8 lines

fs/nfsd/nfs4state.c +0 -0

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }   

Quoted 8 lines

fs/nfsd/nfs4state.c static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh) · line 159

Quoted 7 lines

		break;

fs/nfsd/nfs4state.c +1 -1

@@ -155,7 +156,7 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_type = IPV6_PKTINFO;
 			pki->ipi6_ifindex = 0;
 			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+					&svc_daddr_in6(rqstp)->sin6_addr);

I kind of suspect more is needed here to deal with ipi6_ifindex.

Quoted 13 lines

Quoted 13 lines

fs/nfsd/nfs4state.c +1 -1

 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -500,7 +501,7 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+	svc_daddr_in(rqstp)->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

Quoted 7 lines

fs/nfsd/nfs4state.c static int svc_udp_get_dest_address6(struct svc_rqst *rqstp, · line 517

Quoted 6 lines

fs/nfsd/nfs4state.c +1 -1

@@ -513,7 +514,7 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+	ipv6_addr_copy(&svc_daddr_in6(rqstp)->sin6_addr, &pki->ipi6_addr);

Likewise, is the incoming interface index ignored when constructing svc_daddr_in6(rqstp) ?

Also, I can't seem to find where the address family field of svc_daddr() is being set.

From 84420475509ccc39ac8899f6f63140879a5278b6 Mon Sep 17 00:00:00 2001

Quoted 1 line

From 84420475509ccc39ac8899f6f63140879a5278b6 Mon Sep 17 00:00:00 2001

From: Mi Jinlong <[email protected]>
Date: Mon, 26 Aug 2011 13:22:39 +0800
Subject: [PATCH v2] SUNRPC: Replace svc_addr_u by sockaddr_storage

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/lockd/host.c             |   25 ++-----------------------
 fs/nfsd/nfs4state.c         |   16 +---------------
 include/linux/sunrpc/clnt.h |    3 ++-
 include/linux/sunrpc/svc.h  |   30 +++++++++++++++++++++---------
 net/sunrpc/svc_xprt.c       |   13 ++-----------
 net/sunrpc/svcsock.c        |   23 +++++++++++++++++------
 6 files changed, 45 insertions(+), 65 deletions(-)

fs/lockd/host.c +2 -23

index b7c99bf..6f29836 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
-	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	struct sockaddr *src_sap = svc_daddr(rqstp);
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -340,21 +334,6 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 
 	mutex_lock(&nlm_host_mutex);
 
-	switch (ni.sap->sa_family) {
-	case AF_INET:
-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;
-	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
-		break;
-	default:
-		dprintk("lockd: %s failed; unrecognized address family\n",
-			__func__);
-		goto out;
-	}
-
 	if (time_after_eq(jiffies, next_gc))
 		nlm_gc_hosts();
 

fs/nfsd/nfs4state.c +1 -15

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }
 
-static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, union svc_addr_u *svcaddr)
-{
-	switch (family) {
-	case AF_INET:
-		((struct sockaddr_in *)sa)->sin_family = AF_INET;
-		((struct sockaddr_in *)sa)->sin_addr = svcaddr->addr;
-		return;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
-		return;
-	}
-}
-
 static void
 gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_rqst *rqstp)
 {

@@ -1218,7 +1204,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
-	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+	memcpy(&conn->cb_saddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 	return;
 out_err:
 	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/clnt.h +2 -1

index db7bcaf..1529a80 100644
@@ -218,7 +218,8 @@ static inline bool __rpc_cmp_addr6(const struct sockaddr *sap1,
 {
 	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sap1;
 	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sap2;
-	return ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr);
+	return ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr)
+		&& (sin1->sin6_scope_id == sin2->sin6_scope_id);
 }
 
 static inline bool __rpc_copy_addr6(struct sockaddr *dst,

include/linux/sunrpc/svc.h +21 -9

index 223588a..3bc0dc9 100644
@@ -212,11 +212,6 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
-union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
-};
-
 /*
  * The context of a single thread, including the request currently being
  * processed.

@@ -225,8 +220,12 @@ struct svc_rqst {
 	struct list_head	rq_list;	/* idle list */
 	struct list_head	rq_all;		/* all threads list */
 	struct svc_xprt *	rq_xprt;	/* transport ptr */
+
 	struct sockaddr_storage	rq_addr;	/* peer address */
 	size_t			rq_addrlen;
+	struct sockaddr_storage	rq_daddr;	/* dest addr of request
+						 *  - reply from here */
+	size_t			rq_daddrlen;
 
 	struct svc_serv *	rq_server;	/* RPC service definition */
 	struct svc_pool *	rq_pool;	/* thread pool */

@@ -255,9 +254,6 @@ struct svc_rqst {
 	unsigned short
 				rq_secure  : 1;	/* secure port */
 
-	union svc_addr_u	rq_daddr;	/* dest addr of request
-						 *  - reply from here */
-
 	void *			rq_argp;	/* decoded arguments */
 	void *			rq_resp;	/* xdr'd results */
 	void *			rq_auth_data;	/* flavor-specific data */

@@ -300,6 +296,21 @@ static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 	return (struct sockaddr *) &rqst->rq_addr;
 }
 
+static inline struct sockaddr_in *svc_daddr_in(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr_in6 *svc_daddr_in6(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in6 *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr *svc_daddr(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr *) &rqst->rq_daddr;
+}
+
 /*
  * Check buffer bounds after decoding arguments
  */

@@ -340,7 +351,8 @@ struct svc_deferred_req {
 	struct svc_xprt		*xprt;
 	struct sockaddr_storage	addr;	/* where reply must go */
 	size_t			addrlen;
-	union svc_addr_u	daddr;	/* where reply must come from */
+	struct sockaddr_storage	daddr;	/* where reply must come from */
+	size_t			daddrlen;
 	struct cache_deferred_req handle;
 	size_t			xprt_hlen;
 	int			argslen;

net/sunrpc/svc_xprt.c +2 -11

index bd31208..d86bb67 100644
@@ -254,8 +254,6 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
  */
 void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
-	struct sockaddr *sin;
-
 	memcpy(&rqstp->rq_addr, &xprt->xpt_remote, xprt->xpt_remotelen);
 	rqstp->rq_addrlen = xprt->xpt_remotelen;
 

@@ -263,15 +261,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 	 * Destination address in request is needed for binding the
 	 * source address in RPC replies/callbacks later.
 	 */
-	sin = (struct sockaddr *)&xprt->xpt_local;
-	switch (sin->sa_family) {
-	case AF_INET:
-		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
-		break;
-	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
-		break;
-	}
+	memcpy(&rqstp->rq_daddr, &xprt->xpt_local, xprt->xpt_locallen);
+	rqstp->rq_daddrlen = xprt->xpt_locallen;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs);
 

net/sunrpc/svcsock.c +17 -6

index 767d494..dfd686e 100644
@@ -143,19 +143,20 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_level = SOL_IP;
 			cmh->cmsg_type = IP_PKTINFO;
 			pki->ipi_ifindex = 0;
-			pki->ipi_spec_dst.s_addr = rqstp->rq_daddr.addr.s_addr;
+			pki->ipi_spec_dst.s_addr =
+				 svc_daddr_in(rqstp)->sin_addr.s_addr;
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;
 
 	case AF_INET6: {
 			struct in6_pktinfo *pki = CMSG_DATA(cmh);
+			struct sockaddr_in6 *daddr = svc_daddr_in6(rqstp);
 
 			cmh->cmsg_level = SOL_IPV6;
 			cmh->cmsg_type = IPV6_PKTINFO;
-			pki->ipi6_ifindex = 0;
-			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+			pki->ipi6_ifindex = daddr->sin6_scope_id;
+			ipv6_addr_copy(&pki->ipi6_addr,	&daddr->sin6_addr);
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -498,9 +499,13 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 				     struct cmsghdr *cmh)
 {
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
+	struct sockaddr_in *daddr = svc_daddr_in(rqstp);
+
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+
+	daddr->sin_family = AF_INET;
+	daddr->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

@@ -511,9 +516,14 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 				     struct cmsghdr *cmh)
 {
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
+	struct sockaddr_in6 *daddr = svc_daddr_in6(rqstp);
+
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+
+	daddr->sin6_family = AF_INET6;
+	ipv6_addr_copy(&daddr->sin6_addr, &pki->ipi6_addr);
+	daddr->sin6_scope_id = pki->ipi6_ifindex;
 	return 1;
 }
 

@@ -614,6 +624,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
 		skb_free_datagram_locked(svsk->sk_sk, skb);
 		return 0;
 	}
+	rqstp->rq_daddrlen = svc_addr_len(svc_daddr(rqstp));
 
 	if (skb_is_nonlinear(skb)) {
 		/* we have to copy */

Signature · 1 line

1.7.6

Jeff and Chuck, you're OK with the revised version?

--b.

Quoted 302 lines · Mi Jinlong

Quoted 1 line

From 84420475509ccc39ac8899f6f63140879a5278b6 Mon Sep 17 00:00:00 2001

From: Mi Jinlong <[email protected]>
Date: Mon, 26 Aug 2011 13:22:39 +0800
Subject: [PATCH v2] SUNRPC: Replace svc_addr_u by sockaddr_storage

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---

fs/lockd/host.c             |   25 ++-----------------------
fs/nfsd/nfs4state.c         |   16 +---------------
include/linux/sunrpc/clnt.h |    3 ++-
include/linux/sunrpc/svc.h  |   30 +++++++++++++++++++++---------
net/sunrpc/svc_xprt.c       |   13 ++-----------
net/sunrpc/svcsock.c        |   23 +++++++++++++++++------
6 files changed, 45 insertions(+), 65 deletions(-)

fs/lockd/host.c +2 -23

index b7c99bf..6f29836 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
-	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	struct sockaddr *src_sap = svc_daddr(rqstp);
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -340,21 +334,6 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 
 	mutex_lock(&nlm_host_mutex);
 
-	switch (ni.sap->sa_family) {
-	case AF_INET:
-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;
-	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
-		break;
-	default:
-		dprintk("lockd: %s failed; unrecognized address family\n",
-			__func__);
-		goto out;
-	}
-
 	if (time_after_eq(jiffies, next_gc))
 		nlm_gc_hosts();
 

fs/nfsd/nfs4state.c +0 -14

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }
 
-static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, union svc_addr_u *svcaddr)
-{
-	switch (family) {
-	case AF_INET:
-		((struct sockaddr_in *)sa)->sin_family = AF_INET;
-		((struct sockaddr_in *)sa)->sin_addr = svcaddr->addr;
-		return;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
-		return;
-	}
-}
-

static void
gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_rqst *rqstp) {

fs/nfsd/nfs4state.c +1 -1

@@ -1218,7 +1204,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
-	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+	memcpy(&conn->cb_saddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 	return;

out_err:

	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/clnt.h +2 -1

index db7bcaf..1529a80 100644
@@ -218,7 +218,8 @@ static inline bool __rpc_cmp_addr6(const struct sockaddr *sap1,
 {
 	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sap1;
 	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sap2;
-	return ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr);
+	return ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr)
+		&& (sin1->sin6_scope_id == sin2->sin6_scope_id);
 }
 
 static inline bool __rpc_copy_addr6(struct sockaddr *dst,

include/linux/sunrpc/svc.h +21 -9

index 223588a..3bc0dc9 100644
@@ -212,11 +212,6 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
-union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
-};
-
 /*
 * The context of a single thread, including the request currently being
 * processed.

@@ -225,8 +220,12 @@ struct svc_rqst {
 	struct list_head	rq_list;	/* idle list */
 	struct list_head	rq_all;		/* all threads list */
 	struct svc_xprt *	rq_xprt;	/* transport ptr */
+
 	struct sockaddr_storage	rq_addr;	/* peer address */
 	size_t			rq_addrlen;
+	struct sockaddr_storage	rq_daddr;	/* dest addr of request
+						 *  - reply from here */
+	size_t			rq_daddrlen;
 
 	struct svc_serv *	rq_server;	/* RPC service definition */
 	struct svc_pool *	rq_pool;	/* thread pool */

@@ -255,9 +254,6 @@ struct svc_rqst {
 	unsigned short
 				rq_secure  : 1;	/* secure port */
 
-	union svc_addr_u	rq_daddr;	/* dest addr of request
-						 *  - reply from here */
-
 	void *			rq_argp;	/* decoded arguments */
 	void *			rq_resp;	/* xdr'd results */
 	void *			rq_auth_data;	/* flavor-specific data */

@@ -300,6 +296,21 @@ static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 	return (struct sockaddr *) &rqst->rq_addr;
 }
 
+static inline struct sockaddr_in *svc_daddr_in(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr_in6 *svc_daddr_in6(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in6 *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr *svc_daddr(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr *) &rqst->rq_daddr;
+}
+
 /*
 * Check buffer bounds after decoding arguments
 */

@@ -340,7 +351,8 @@ struct svc_deferred_req {
 	struct svc_xprt		*xprt;
 	struct sockaddr_storage	addr;	/* where reply must go */
 	size_t			addrlen;
-	union svc_addr_u	daddr;	/* where reply must come from */
+	struct sockaddr_storage	daddr;	/* where reply must come from */
+	size_t			daddrlen;
 	struct cache_deferred_req handle;
 	size_t			xprt_hlen;
 	int			argslen;

net/sunrpc/svc_xprt.c +2 -11

index bd31208..d86bb67 100644
@@ -254,8 +254,6 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
 */
 void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
-	struct sockaddr *sin;
-
 	memcpy(&rqstp->rq_addr, &xprt->xpt_remote, xprt->xpt_remotelen);
 	rqstp->rq_addrlen = xprt->xpt_remotelen;
 

@@ -263,15 +261,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 	 * Destination address in request is needed for binding the
 	 * source address in RPC replies/callbacks later.
 	 */
-	sin = (struct sockaddr *)&xprt->xpt_local;
-	switch (sin->sa_family) {
-	case AF_INET:
-		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
-		break;
-	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
-		break;
-	}
+	memcpy(&rqstp->rq_daddr, &xprt->xpt_local, xprt->xpt_locallen);
+	rqstp->rq_daddrlen = xprt->xpt_locallen;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs);
 

net/sunrpc/svcsock.c +17 -6

index 767d494..dfd686e 100644
@@ -143,19 +143,20 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_level = SOL_IP;
 			cmh->cmsg_type = IP_PKTINFO;
 			pki->ipi_ifindex = 0;
-			pki->ipi_spec_dst.s_addr = rqstp->rq_daddr.addr.s_addr;
+			pki->ipi_spec_dst.s_addr =
+				 svc_daddr_in(rqstp)->sin_addr.s_addr;
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;
 
 	case AF_INET6: {
 			struct in6_pktinfo *pki = CMSG_DATA(cmh);
+			struct sockaddr_in6 *daddr = svc_daddr_in6(rqstp);
 
 			cmh->cmsg_level = SOL_IPV6;
 			cmh->cmsg_type = IPV6_PKTINFO;
-			pki->ipi6_ifindex = 0;
-			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+			pki->ipi6_ifindex = daddr->sin6_scope_id;
+			ipv6_addr_copy(&pki->ipi6_addr,	&daddr->sin6_addr);
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -498,9 +499,13 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 				     struct cmsghdr *cmh)
 {
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
+	struct sockaddr_in *daddr = svc_daddr_in(rqstp);
+
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+
+	daddr->sin_family = AF_INET;
+	daddr->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

@@ -511,9 +516,14 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 				     struct cmsghdr *cmh)
 {
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
+	struct sockaddr_in6 *daddr = svc_daddr_in6(rqstp);
+
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+
+	daddr->sin6_family = AF_INET6;
+	ipv6_addr_copy(&daddr->sin6_addr, &pki->ipi6_addr);
+	daddr->sin6_scope_id = pki->ipi6_ifindex;
 	return 1;
 }
 

@@ -614,6 +624,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
 		skb_free_datagram_locked(svsk->sk_sk, skb);
 		return 0;
 	}
+	rqstp->rq_daddrlen = svc_addr_len(svc_daddr(rqstp));
 
 	if (skb_is_nonlinear(skb)) {
 		/* we have to copy */

Signature · 1 line

1.7.6

Thanks for you comment!

Chuck Lever:

Quoted 62 lines · Chuck Lever

Quoted 61 lines

This seems like a good direction to go in. More comments below.

Quoted 56 lines · Mi Jinlong

From: Mi Jinlong <[email protected]>
Date: Mon, 24 Aug 2011 13:22:39 +0800
Subject: [PATCH] SUNRPC: Replace svc_addr_u by sockaddr_storage

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---

fs/lockd/host.c            |   14 ++------------
fs/nfsd/nfs4state.c        |   16 +---------------
include/linux/sunrpc/svc.h |   30 +++++++++++++++++++++---------
net/sunrpc/svc_xprt.c      |   13 ++-----------
net/sunrpc/svcsock.c       |    9 +++++----
5 files changed, 31 insertions(+), 51 deletions(-)

fs/lockd/host.c +1 -7

index b7c99bf..b23db05 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
 	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -342,12 +336,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 

Quoted 13 lines

Quoted 5 lines

Quoted 5 lines

fs/lockd/host.c +0 -3

 	switch (ni.sap->sa_family) {
 	case AF_INET:
-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;

Quoted 8 lines

fs/lockd/host.c struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp, · line 343

Quoted 7 lines

fs/lockd/host.c +1 -2

 	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
+		src_sap = svc_daddr(rqstp);
 		break;
 	default:
 		dprintk("lockd: %s failed; unrecognized address family\n",

With the destination address promoted to a full sockaddr, we probably do not need this switch any more.

Got it.

Quoted 2 lines

Quoted 2 lines

I notice that the IPv6 fork of rpc_cmp_addr() does not consider the scope IDs of the passed-in addresses. Perhaps it should?

Have add scope ID compare at __rpc_cmp_addr6 as following patch.

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

fs/nfsd/nfs4state.c +0 -0

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }   

... snip ...

Quoted 8 lines

Quoted 8 lines

fs/nfsd/nfs4state.c static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh) · line 159

Quoted 7 lines

		break;

fs/nfsd/nfs4state.c +1 -1

@@ -155,7 +156,7 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_type = IPV6_PKTINFO;
 			pki->ipi6_ifindex = 0;
 			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+					&svc_daddr_in6(rqstp)->sin6_addr);

I kind of suspect more is needed here to deal with ipi6_ifindex.

Got it.

Quoted 21 lines

Quoted 13 lines

Quoted 13 lines

fs/nfsd/nfs4state.c +1 -1

 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -500,7 +501,7 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+	svc_daddr_in(rqstp)->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

Quoted 7 lines

fs/nfsd/nfs4state.c static int svc_udp_get_dest_address6(struct svc_rqst *rqstp, · line 517

Quoted 6 lines

fs/nfsd/nfs4state.c +1 -1

@@ -513,7 +514,7 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+	ipv6_addr_copy(&svc_daddr_in6(rqstp)->sin6_addr, &pki->ipi6_addr);

Likewise, is the incoming interface index ignored when constructing svc_daddr_in6(rqstp) ?

Quoted 2 lines

Also, I can't seem to find where the address family field of svc_daddr() is being set.

Have complete it.

thanks,
Mi Jinlong

From 84420475509ccc39ac8899f6f63140879a5278b6 Mon Sep 17 00:00:00 2001
From: Mi Jinlong <[email protected]>
Date: Mon, 26 Aug 2011 13:22:39 +0800
Subject: [PATCH v2] SUNRPC: Replace svc_addr_u by sockaddr_storage

For IPv6 local address, lockd can not callback to client for missing scope id when binding address at inet6_bind:

324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
325 if (addr_len >= sizeof(struct sockaddr_in6) &&
326 addr->sin6_scope_id) {
327 /* Override any existing binding, if another one
328 * is supplied by user.
329 */
330 sk->sk_bound_dev_if = addr->sin6_scope_id;
331 }
332
333 /* Binding to link-local address requires an interface */
334 if (!sk->sk_bound_dev_if) {
335 err = -EINVAL;
336 goto out_unlock;
337 }

Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info besides address.

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/lockd/host.c             |   25 ++-----------------------
 fs/nfsd/nfs4state.c         |   16 +---------------
 include/linux/sunrpc/clnt.h |    3 ++-
 include/linux/sunrpc/svc.h  |   30 +++++++++++++++++++++---------
 net/sunrpc/svc_xprt.c       |   13 ++-----------
 net/sunrpc/svcsock.c        |   23 +++++++++++++++++------
 6 files changed, 45 insertions(+), 65 deletions(-)

fs/lockd/host.c +2 -23

index b7c99bf..6f29836 100644
@@ -316,14 +316,8 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 	struct hlist_node *pos;
 	struct nlm_host	*host = NULL;
 	struct nsm_handle *nsm = NULL;
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	struct sockaddr_in6 sin6 = {
-		.sin6_family	= AF_INET6,
-	};
-	struct sockaddr *src_sap;
-	size_t src_len = rqstp->rq_addrlen;
+	struct sockaddr *src_sap = svc_daddr(rqstp);
+	size_t src_len = rqstp->rq_daddrlen;
 	struct nlm_lookup_host_info ni = {
 		.server		= 1,
 		.sap		= svc_addr(rqstp),

@@ -340,21 +334,6 @@ struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
 
 	mutex_lock(&nlm_host_mutex);
 
-	switch (ni.sap->sa_family) {
-	case AF_INET:
-		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
-		src_sap = (struct sockaddr *)&sin;
-		break;
-	case AF_INET6:
-		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
-		src_sap = (struct sockaddr *)&sin6;
-		break;
-	default:
-		dprintk("lockd: %s failed; unrecognized address family\n",
-			__func__);
-		goto out;
-	}
-
 	if (time_after_eq(jiffies, next_gc))
 		nlm_gc_hosts();
 

fs/nfsd/nfs4state.c +1 -15

index 3787ec1..07a18a5 100644
@@ -1173,20 +1173,6 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
 	return NULL;
 }
 
-static void rpc_svcaddr2sockaddr(struct sockaddr *sa, unsigned short family, union svc_addr_u *svcaddr)
-{
-	switch (family) {
-	case AF_INET:
-		((struct sockaddr_in *)sa)->sin_family = AF_INET;
-		((struct sockaddr_in *)sa)->sin_addr = svcaddr->addr;
-		return;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sa)->sin6_family = AF_INET6;
-		((struct sockaddr_in6 *)sa)->sin6_addr = svcaddr->addr6;
-		return;
-	}
-}
-
 static void
 gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_rqst *rqstp)
 {

@@ -1218,7 +1204,7 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, struct svc_r
 
 	conn->cb_prog = se->se_callback_prog;
 	conn->cb_ident = se->se_callback_ident;
-	rpc_svcaddr2sockaddr((struct sockaddr *)&conn->cb_saddr, expected_family, &rqstp->rq_daddr);
+	memcpy(&conn->cb_saddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 	return;
 out_err:
 	conn->cb_addr.ss_family = AF_UNSPEC;

include/linux/sunrpc/clnt.h +0 -0

index db7bcaf..1529a80 100644

include/linux/sunrpc/clnt.h +0 -0

 }
 
 static inline bool __rpc_copy_addr6(struct sockaddr *dst,

include/linux/sunrpc/svc.h +21 -9

index 223588a..3bc0dc9 100644
@@ -212,11 +212,6 @@ static inline void svc_putu32(struct kvec *iov, __be32 val)
 	iov->iov_len += sizeof(__be32);
 }
 
-union svc_addr_u {
-    struct in_addr	addr;
-    struct in6_addr	addr6;
-};
-
 /*
  * The context of a single thread, including the request currently being
  * processed.

@@ -225,8 +220,12 @@ struct svc_rqst {
 	struct list_head	rq_list;	/* idle list */
 	struct list_head	rq_all;		/* all threads list */
 	struct svc_xprt *	rq_xprt;	/* transport ptr */
+
 	struct sockaddr_storage	rq_addr;	/* peer address */
 	size_t			rq_addrlen;
+	struct sockaddr_storage	rq_daddr;	/* dest addr of request
+						 *  - reply from here */
+	size_t			rq_daddrlen;
 
 	struct svc_serv *	rq_server;	/* RPC service definition */
 	struct svc_pool *	rq_pool;	/* thread pool */

@@ -255,9 +254,6 @@ struct svc_rqst {
 	unsigned short
 				rq_secure  : 1;	/* secure port */
 
-	union svc_addr_u	rq_daddr;	/* dest addr of request
-						 *  - reply from here */
-
 	void *			rq_argp;	/* decoded arguments */
 	void *			rq_resp;	/* xdr'd results */
 	void *			rq_auth_data;	/* flavor-specific data */

@@ -300,6 +296,21 @@ static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 	return (struct sockaddr *) &rqst->rq_addr;
 }
 
+static inline struct sockaddr_in *svc_daddr_in(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr_in6 *svc_daddr_in6(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr_in6 *) &rqst->rq_daddr;
+}
+
+static inline struct sockaddr *svc_daddr(const struct svc_rqst *rqst)
+{
+	return (struct sockaddr *) &rqst->rq_daddr;
+}
+
 /*
  * Check buffer bounds after decoding arguments
  */

@@ -340,7 +351,8 @@ struct svc_deferred_req {
 	struct svc_xprt		*xprt;
 	struct sockaddr_storage	addr;	/* where reply must go */
 	size_t			addrlen;
-	union svc_addr_u	daddr;	/* where reply must come from */
+	struct sockaddr_storage	daddr;	/* where reply must come from */
+	size_t			daddrlen;
 	struct cache_deferred_req handle;
 	size_t			xprt_hlen;
 	int			argslen;

net/sunrpc/svc_xprt.c +2 -11

index bd31208..d86bb67 100644
@@ -254,8 +254,6 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
  */
 void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
-	struct sockaddr *sin;
-
 	memcpy(&rqstp->rq_addr, &xprt->xpt_remote, xprt->xpt_remotelen);
 	rqstp->rq_addrlen = xprt->xpt_remotelen;
 

@@ -263,15 +261,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 	 * Destination address in request is needed for binding the
 	 * source address in RPC replies/callbacks later.
 	 */
-	sin = (struct sockaddr *)&xprt->xpt_local;
-	switch (sin->sa_family) {
-	case AF_INET:
-		rqstp->rq_daddr.addr = ((struct sockaddr_in *)sin)->sin_addr;
-		break;
-	case AF_INET6:
-		rqstp->rq_daddr.addr6 = ((struct sockaddr_in6 *)sin)->sin6_addr;
-		break;
-	}
+	memcpy(&rqstp->rq_daddr, &xprt->xpt_local, xprt->xpt_locallen);
+	rqstp->rq_daddrlen = xprt->xpt_locallen;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_copy_addrs);
 

net/sunrpc/svcsock.c +17 -6

index 767d494..dfd686e 100644
@@ -143,19 +143,20 @@ static void svc_set_cmsg_data(struct svc_rqst *rqstp, struct cmsghdr *cmh)
 			cmh->cmsg_level = SOL_IP;
 			cmh->cmsg_type = IP_PKTINFO;
 			pki->ipi_ifindex = 0;
-			pki->ipi_spec_dst.s_addr = rqstp->rq_daddr.addr.s_addr;
+			pki->ipi_spec_dst.s_addr =
+				 svc_daddr_in(rqstp)->sin_addr.s_addr;
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;
 
 	case AF_INET6: {
 			struct in6_pktinfo *pki = CMSG_DATA(cmh);
+			struct sockaddr_in6 *daddr = svc_daddr_in6(rqstp);
 
 			cmh->cmsg_level = SOL_IPV6;
 			cmh->cmsg_type = IPV6_PKTINFO;
-			pki->ipi6_ifindex = 0;
-			ipv6_addr_copy(&pki->ipi6_addr,
-					&rqstp->rq_daddr.addr6);
+			pki->ipi6_ifindex = daddr->sin6_scope_id;
+			ipv6_addr_copy(&pki->ipi6_addr,	&daddr->sin6_addr);
 			cmh->cmsg_len = CMSG_LEN(sizeof(*pki));
 		}
 		break;

@@ -498,9 +499,13 @@ static int svc_udp_get_dest_address4(struct svc_rqst *rqstp,
 				     struct cmsghdr *cmh)
 {
 	struct in_pktinfo *pki = CMSG_DATA(cmh);
+	struct sockaddr_in *daddr = svc_daddr_in(rqstp);
+
 	if (cmh->cmsg_type != IP_PKTINFO)
 		return 0;
-	rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;
+
+	daddr->sin_family = AF_INET;
+	daddr->sin_addr.s_addr = pki->ipi_spec_dst.s_addr;
 	return 1;
 }
 

@@ -511,9 +516,14 @@ static int svc_udp_get_dest_address6(struct svc_rqst *rqstp,
 				     struct cmsghdr *cmh)
 {
 	struct in6_pktinfo *pki = CMSG_DATA(cmh);
+	struct sockaddr_in6 *daddr = svc_daddr_in6(rqstp);
+
 	if (cmh->cmsg_type != IPV6_PKTINFO)
 		return 0;
-	ipv6_addr_copy(&rqstp->rq_daddr.addr6, &pki->ipi6_addr);
+
+	daddr->sin6_family = AF_INET6;
+	ipv6_addr_copy(&daddr->sin6_addr, &pki->ipi6_addr);
+	daddr->sin6_scope_id = pki->ipi6_ifindex;
 	return 1;
 }
 

@@ -614,6 +624,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
 		skb_free_datagram_locked(svsk->sk_sk, skb);
 		return 0;
 	}
+	rqstp->rq_daddrlen = svc_addr_len(svc_daddr(rqstp));
 
 	if (skb_is_nonlinear(skb)) {
 		/* we have to copy */