| Message ID | 4E4F8B5D.6060404@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: > 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. I guess this makes sense to me, but someone who understands IPv6 better should comment... Chuck? Jeff? --b. > > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > --- > 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(-) > > diff --git a/fs/lockd/host.c b/fs/lockd/host.c > index b7c99bf..1fff87a 100644 > --- a/fs/lockd/host.c > +++ b/fs/lockd/host.c > @@ -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: > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 3787ec1..1169411 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -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; > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h > index 223588a..cd611b5 100644 > --- a/include/linux/sunrpc/svc.h > +++ b/include/linux/sunrpc/svc.h > @@ -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; > }; > > /* > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c > index bd31208..8aada49 100644 > --- a/net/sunrpc/svc_xprt.c > +++ b/net/sunrpc/svc_xprt.c > @@ -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; > } > } > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c > index 767d494..beb2575 100644 > --- a/net/sunrpc/svcsock.c > +++ b/net/sunrpc/svcsock.c > @@ -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; > } > > -- > 1.7.6 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: > +/* > + * 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; 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. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 22, 2011, at 3:26 PM, J. Bruce Fields wrote: > On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: >> +/* >> + * 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; > > 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.
On Mon, 22 Aug 2011 15:23:48 -0400 "J. Bruce Fields" <bfields@fieldses.org> wrote: > On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: > > 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. > > 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...
On Aug 22, 2011, at 3:39 PM, Jeff Layton wrote: > On Mon, 22 Aug 2011 15:23:48 -0400 > "J. Bruce Fields" <bfields@fieldses.org> wrote: > >> On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: >>> 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. >> >> 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... Sure, that would preclude the need to upgrade the field to a struct sockaddr_storage. > 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. > 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...
On Mon, Aug 22, 2011 at 03:32:13PM -0400, Chuck Lever wrote: > > On Aug 22, 2011, at 3:26 PM, J. Bruce Fields wrote: > > > On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: > >> +/* > >> + * 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; > > > > 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. 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 22, 2011 at 03:44:39PM -0400, Chuck Lever wrote: > On Aug 22, 2011, at 3:39 PM, Jeff Layton wrote: > > 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. OK. Mi Jinlong, if you don't have a use case in mind for this, let's not bother. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
J. Bruce Fields: > On Mon, Aug 22, 2011 at 03:32:13PM -0400, Chuck Lever wrote: >> On Aug 22, 2011, at 3:26 PM, J. Bruce Fields wrote: >> >>> On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: >>>> +/* >>>> + * 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; >>> 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. > > 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? OK, I will do this. Thanks for all comment.
On Mon, 22 Aug 2011 15:44:39 -0400 Chuck Lever <chuck.lever@oracle.com> wrote: > > On Aug 22, 2011, at 3:39 PM, Jeff Layton wrote: > > > On Mon, 22 Aug 2011 15:23:48 -0400 > > "J. Bruce Fields" <bfields@fieldses.org> wrote: > > > >> On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: > >>> 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. > >> > >> 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... > > Sure, that would preclude the need to upgrade the field to a struct sockaddr_storage. > > > 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 : > On Mon, 22 Aug 2011 15:44:39 -0400 > Chuck Lever <chuck.lever@oracle.com> wrote: > >> On Aug 22, 2011, at 3:39 PM, Jeff Layton wrote: >> >>> On Mon, 22 Aug 2011 15:23:48 -0400 >>> "J. Bruce Fields" <bfields@fieldses.org> wrote: >>> >>>> On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: >>>>> 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. >>>> 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... >> Sure, that would preclude the need to upgrade the field to a struct sockaddr_storage. >> >>> 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... 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. thanks, Mi Jinlong -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 24, 2011, at 4:00 AM, Mi Jinlong wrote: > Jeff Layton : >> On Mon, 22 Aug 2011 15:44:39 -0400 >> Chuck Lever <chuck.lever@oracle.com> wrote: >> >>> On Aug 22, 2011, at 3:39 PM, Jeff Layton wrote: >>> >>>> On Mon, 22 Aug 2011 15:23:48 -0400 >>>> "J. Bruce Fields" <bfields@fieldses.org> wrote: >>>> >>>>> On Sat, Aug 20, 2011 at 06:24:29PM +0800, Mi Jinlong wrote: >>>>>> 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. >>>>> 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... >>> Sure, that would preclude the need to upgrade the field to a struct sockaddr_storage. >>> >>>> 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... > > Hi Jeff, > > I'm not working on NSM. It seems there are some problem of the > latest statd from Steve's git tee. I kind of agree that NSM can't work with link-local IPv6 because of its dependence on DNS, but NFSv4 probably should work. > If having time, I will check the latest statd and sm-notify. It's a reasonable regression test for your patch :-)
diff --git a/fs/lockd/host.c b/fs/lockd/host.c index b7c99bf..1fff87a 100644 --- a/fs/lockd/host.c +++ b/fs/lockd/host.c @@ -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: diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 3787ec1..1169411 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -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; diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h index 223588a..cd611b5 100644 --- a/include/linux/sunrpc/svc.h +++ b/include/linux/sunrpc/svc.h @@ -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; }; /* diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index bd31208..8aada49 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -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; } } diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index 767d494..beb2575 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -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; }
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 <mijinlong@cn.fujitsu.com> --- 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(-)