diff mbox series

[v4,1/8] NFSD fill-in netloc4 structure

Message ID 20190708192352.12614-2-olga.kornievskaia@gmail.com (mailing list archive)
State New, archived
Headers show
Series server-side support for "inter" SSC copy | expand

Commit Message

Olga Kornievskaia July 8, 2019, 7:23 p.m. UTC 
From: Olga Kornievskaia <kolga@netapp.com>

nfs.4 defines nfs42_netaddr structure that represents netloc4.

Populate needed fields from the sockaddr structure.

This will be used by flexfiles and 4.2 inter copy

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfsd.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

J. Bruce Fields July 17, 2019, 9:13 p.m. UTC | #1 
On Mon, Jul 08, 2019 at 03:23:45PM -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> nfs.4 defines nfs42_netaddr structure that represents netloc4.
> 
> Populate needed fields from the sockaddr structure.
> 
> This will be used by flexfiles and 4.2 inter copy
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfsd/nfsd.h | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 24187b5..8f4fc50 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -19,6 +19,7 @@
>  #include <linux/sunrpc/svc.h>
>  #include <linux/sunrpc/svc_xprt.h>
>  #include <linux/sunrpc/msg_prot.h>
> +#include <linux/sunrpc/addr.h>
>  
>  #include <uapi/linux/nfsd/debug.h>
>  
> @@ -375,6 +376,37 @@ static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
>  
>  extern const u32 nfsd_suppattrs[3][3];
>  
> +static inline u32 nfsd4_set_netaddr(struct sockaddr *addr,
> +				    struct nfs42_netaddr *netaddr)
> +{
> +	struct sockaddr_in *sin = (struct sockaddr_in *)addr;
> +	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)addr;
> +	unsigned int port;
> +	size_t ret_addr, ret_port;
> +
> +	switch (addr->sa_family) {
> +	case AF_INET:
> +		port = ntohs(sin->sin_port);
> +		sprintf(netaddr->netid, "tcp");
> +		netaddr->netid_len = 3;
> +		break;
> +	case AF_INET6:
> +		port = ntohs(sin6->sin6_port);
> +		sprintf(netaddr->netid, "tcp6");
> +		netaddr->netid_len = 4;
> +		break;
> +	default:
> +		return nfserr_inval;
> +	}
> +	ret_addr = rpc_ntop(addr, netaddr->addr, sizeof(netaddr->addr));
> +	ret_port = snprintf(netaddr->addr + ret_addr,
> +			    RPCBIND_MAXUADDRLEN + 1 - ret_addr,
> +			    ".%u.%u", port >> 8, port & 0xff);
> +	WARN_ON(ret_port >= RPCBIND_MAXUADDRLEN + 1 - ret_addr);
> +	netaddr->addr_len = ret_addr + ret_port;
> +	return 0;
> +}

Kinda surprised we don't already do something like this elsewhere, but I
don't see anything exactly the same.  Might be possible to put this in
net/sunrpc/addr.c and share some code with rpc_sockaddr2uaddr?  I'll
leave it to you whether that looks worth it.

--b.

> +
>  static inline bool bmval_is_subset(const u32 *bm1, const u32 *bm2)
>  {
>  	return !((bm1[0] & ~bm2[0]) ||
> -- 
> 1.8.3.1
Olga Kornievskaia July 22, 2019, 7:59 p.m. UTC | #2 
On Wed, Jul 17, 2019 at 5:13 PM J. Bruce Fields <bfields@fieldses.org> wrote:
>
> On Mon, Jul 08, 2019 at 03:23:45PM -0400, Olga Kornievskaia wrote:
> > From: Olga Kornievskaia <kolga@netapp.com>
> >
> > nfs.4 defines nfs42_netaddr structure that represents netloc4.
> >
> > Populate needed fields from the sockaddr structure.
> >
> > This will be used by flexfiles and 4.2 inter copy
> >
> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > ---
> >  fs/nfsd/nfsd.h | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index 24187b5..8f4fc50 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -19,6 +19,7 @@
> >  #include <linux/sunrpc/svc.h>
> >  #include <linux/sunrpc/svc_xprt.h>
> >  #include <linux/sunrpc/msg_prot.h>
> > +#include <linux/sunrpc/addr.h>
> >
> >  #include <uapi/linux/nfsd/debug.h>
> >
> > @@ -375,6 +376,37 @@ static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
> >
> >  extern const u32 nfsd_suppattrs[3][3];
> >
> > +static inline u32 nfsd4_set_netaddr(struct sockaddr *addr,
> > +                                 struct nfs42_netaddr *netaddr)
> > +{
> > +     struct sockaddr_in *sin = (struct sockaddr_in *)addr;
> > +     struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)addr;
> > +     unsigned int port;
> > +     size_t ret_addr, ret_port;
> > +
> > +     switch (addr->sa_family) {
> > +     case AF_INET:
> > +             port = ntohs(sin->sin_port);
> > +             sprintf(netaddr->netid, "tcp");
> > +             netaddr->netid_len = 3;
> > +             break;
> > +     case AF_INET6:
> > +             port = ntohs(sin6->sin6_port);
> > +             sprintf(netaddr->netid, "tcp6");
> > +             netaddr->netid_len = 4;
> > +             break;
> > +     default:
> > +             return nfserr_inval;
> > +     }
> > +     ret_addr = rpc_ntop(addr, netaddr->addr, sizeof(netaddr->addr));
> > +     ret_port = snprintf(netaddr->addr + ret_addr,
> > +                         RPCBIND_MAXUADDRLEN + 1 - ret_addr,
> > +                         ".%u.%u", port >> 8, port & 0xff);
> > +     WARN_ON(ret_port >= RPCBIND_MAXUADDRLEN + 1 - ret_addr);
> > +     netaddr->addr_len = ret_addr + ret_port;
> > +     return 0;
> > +}
>
> Kinda surprised we don't already do something like this elsewhere, but I
> don't see anything exactly the same.  Might be possible to put this in
> net/sunrpc/addr.c and share some code with rpc_sockaddr2uaddr?  I'll
> leave it to you whether that looks worth it.

I'll investigate how to move this into the sunrpc. Client also
populates nfs42_netaddr structure but it's slightly different. I need
to go back and see if what's shareable.

>
> --b.
>
> > +
> >  static inline bool bmval_is_subset(const u32 *bm1, const u32 *bm2)
> >  {
> >       return !((bm1[0] & ~bm2[0]) ||
> > --
> > 1.8.3.1
Olga Kornievskaia July 30, 2019, 3:48 p.m. UTC | #3 
On Mon, Jul 22, 2019 at 3:59 PM Olga Kornievskaia
<olga.kornievskaia@gmail.com> wrote:
>
> On Wed, Jul 17, 2019 at 5:13 PM J. Bruce Fields <bfields@fieldses.org> wrote:
> >
> > On Mon, Jul 08, 2019 at 03:23:45PM -0400, Olga Kornievskaia wrote:
> > > From: Olga Kornievskaia <kolga@netapp.com>
> > >
> > > nfs.4 defines nfs42_netaddr structure that represents netloc4.
> > >
> > > Populate needed fields from the sockaddr structure.
> > >
> > > This will be used by flexfiles and 4.2 inter copy
> > >
> > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > > ---
> > >  fs/nfsd/nfsd.h | 32 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >
> > > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > > index 24187b5..8f4fc50 100644
> > > --- a/fs/nfsd/nfsd.h
> > > +++ b/fs/nfsd/nfsd.h
> > > @@ -19,6 +19,7 @@
> > >  #include <linux/sunrpc/svc.h>
> > >  #include <linux/sunrpc/svc_xprt.h>
> > >  #include <linux/sunrpc/msg_prot.h>
> > > +#include <linux/sunrpc/addr.h>
> > >
> > >  #include <uapi/linux/nfsd/debug.h>
> > >
> > > @@ -375,6 +376,37 @@ static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
> > >
> > >  extern const u32 nfsd_suppattrs[3][3];
> > >
> > > +static inline u32 nfsd4_set_netaddr(struct sockaddr *addr,
> > > +                                 struct nfs42_netaddr *netaddr)
> > > +{
> > > +     struct sockaddr_in *sin = (struct sockaddr_in *)addr;
> > > +     struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)addr;
> > > +     unsigned int port;
> > > +     size_t ret_addr, ret_port;
> > > +
> > > +     switch (addr->sa_family) {
> > > +     case AF_INET:
> > > +             port = ntohs(sin->sin_port);
> > > +             sprintf(netaddr->netid, "tcp");
> > > +             netaddr->netid_len = 3;
> > > +             break;
> > > +     case AF_INET6:
> > > +             port = ntohs(sin6->sin6_port);
> > > +             sprintf(netaddr->netid, "tcp6");
> > > +             netaddr->netid_len = 4;
> > > +             break;
> > > +     default:
> > > +             return nfserr_inval;
> > > +     }
> > > +     ret_addr = rpc_ntop(addr, netaddr->addr, sizeof(netaddr->addr));
> > > +     ret_port = snprintf(netaddr->addr + ret_addr,
> > > +                         RPCBIND_MAXUADDRLEN + 1 - ret_addr,
> > > +                         ".%u.%u", port >> 8, port & 0xff);
> > > +     WARN_ON(ret_port >= RPCBIND_MAXUADDRLEN + 1 - ret_addr);
> > > +     netaddr->addr_len = ret_addr + ret_port;
> > > +     return 0;
> > > +}
> >
> > Kinda surprised we don't already do something like this elsewhere, but I
> > don't see anything exactly the same.  Might be possible to put this in
> > net/sunrpc/addr.c and share some code with rpc_sockaddr2uaddr?  I'll
> > leave it to you whether that looks worth it.
>
> I'll investigate how to move this into the sunrpc. Client also
> populates nfs42_netaddr structure but it's slightly different. I need
> to go back and see if what's shareable.

Ok I'd like argue for the code to stay as is because
1. can't move the whole function into addr.c because it created a data
structure (nfs42_netaddr) that rpc knows nothing about
2. While the nfs42_netaddr->addr is the output of the rpc_sock2uaddr()
but we still need the switch to populate the netid . Also since
rpc_sock2uaddr returns memory than the nfs42_netaddr data structure
needs to change to store pointers (and that's shared with the client).
Thus client and server would need to add other code to free the
created netaddr.
3. this function as is can be used by the flexfile layout as well
(they also decided not to share code with rpc_sockaddr2uaddr but use
same content). that function also doesn't want the memory to be
allocated.

Maybe I'm wrong about all of it and it all needs to be re-written to
take dynamic memory. But to use as is I don't want to call it and then
memcpy into existing static buffers and freeing what
rpc_sockaddr2uaddr has allocated.



>
> >
> > --b.
> >
> > > +
> > >  static inline bool bmval_is_subset(const u32 *bm1, const u32 *bm2)
> > >  {
> > >       return !((bm1[0] & ~bm2[0]) ||
> > > --
> > > 1.8.3.1
J. Bruce Fields July 30, 2019, 3:51 p.m. UTC | #4 
On Tue, Jul 30, 2019 at 11:48:40AM -0400, Olga Kornievskaia wrote:
> Ok I'd like argue for the code to stay as is because
> 1. can't move the whole function into addr.c because it created a data
> structure (nfs42_netaddr) that rpc knows nothing about
> 2. While the nfs42_netaddr->addr is the output of the rpc_sock2uaddr()
> but we still need the switch to populate the netid . Also since
> rpc_sock2uaddr returns memory than the nfs42_netaddr data structure
> needs to change to store pointers (and that's shared with the client).
> Thus client and server would need to add other code to free the
> created netaddr.
> 3. this function as is can be used by the flexfile layout as well
> (they also decided not to share code with rpc_sockaddr2uaddr but use
> same content). that function also doesn't want the memory to be
> allocated.
> 
> Maybe I'm wrong about all of it and it all needs to be re-written to
> take dynamic memory. But to use as is I don't want to call it and then
> memcpy into existing static buffers and freeing what
> rpc_sockaddr2uaddr has allocated.

OK, that's fine.

--b.
diff mbox series

Patch

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 24187b5..8f4fc50 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -19,6 +19,7 @@ 
 #include <linux/sunrpc/svc.h>
 #include <linux/sunrpc/svc_xprt.h>
 #include <linux/sunrpc/msg_prot.h>
+#include <linux/sunrpc/addr.h>
 
 #include <uapi/linux/nfsd/debug.h>
 
@@ -375,6 +376,37 @@  static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
 
 extern const u32 nfsd_suppattrs[3][3];
 
+static inline u32 nfsd4_set_netaddr(struct sockaddr *addr,
+				    struct nfs42_netaddr *netaddr)
+{
+	struct sockaddr_in *sin = (struct sockaddr_in *)addr;
+	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)addr;
+	unsigned int port;
+	size_t ret_addr, ret_port;
+
+	switch (addr->sa_family) {
+	case AF_INET:
+		port = ntohs(sin->sin_port);
+		sprintf(netaddr->netid, "tcp");
+		netaddr->netid_len = 3;
+		break;
+	case AF_INET6:
+		port = ntohs(sin6->sin6_port);
+		sprintf(netaddr->netid, "tcp6");
+		netaddr->netid_len = 4;
+		break;
+	default:
+		return nfserr_inval;
+	}
+	ret_addr = rpc_ntop(addr, netaddr->addr, sizeof(netaddr->addr));
+	ret_port = snprintf(netaddr->addr + ret_addr,
+			    RPCBIND_MAXUADDRLEN + 1 - ret_addr,
+			    ".%u.%u", port >> 8, port & 0xff);
+	WARN_ON(ret_port >= RPCBIND_MAXUADDRLEN + 1 - ret_addr);
+	netaddr->addr_len = ret_addr + ret_port;
+	return 0;
+}
+
 static inline bool bmval_is_subset(const u32 *bm1, const u32 *bm2)
 {
 	return !((bm1[0] & ~bm2[0]) ||