[U-Boot] [PATCH 0/9] Fix up issues with new NFSv3 implementation

[U-Boot] [PATCH 0/9] Fix up issues with new NFSv3 implementation

[Joe Hershberger]

This fixes most of the memory bloat that happened with the addition of NFSv3
Some checkpatch.pl issues are also fixed
Refactored some code to better share common snippets

These patches depend on the NFSv3 patch by Guillaume GARDET <guillaume.gardet@free.fr>
https://patchwork.ozlabs.org/patch/654061/


Joe Hershberger (9):
  net: nfs: Share the file handle buffer for v2 / v3
  net: nfs: Correct the reply data buffer size
  net: nfs: Fix lines that are too long
  net: nfs: Consolidate handling of NFSv3 attributes
  net: nfs: Correct a comment
  net: nfs: Use consistent names for the rpc_pkt
  net: nfs: Move some prints to debug statements
  net: nfs: Use the tx buffer to construct rpc msgs
  net: nfs: Simplify rpc_add_credentials()

 net/nfs.c | 330 +++++++++++++++++++++++++++-----------------------------------
 net/nfs.h |   2 +-
 2 files changed, 145 insertions(+), 187 deletions(-)

-- 
1.7.11.5

[U-Boot] [PATCH 1/9] net: nfs: Share the file handle buffer for v2 / v3

[Joe Hershberger]

The v3 handles can be larger than v2, but that doesn't mean we need a
separate buffer. Reuse the same (larger) buffer for both.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 net/nfs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index f61b96e..ac3cde4 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -52,10 +52,8 @@ static int nfs_len;
 static ulong nfs_timeout = NFS_TIMEOUT;
 
 static char dirfh[NFS_FHSIZE];	/* NFSv2 / NFSv3 file handle of directory */
-static char filefh[NFS_FHSIZE]; /* NFSv2 file handle */
-
-static char filefh3[NFS3_FHSIZE];	/* NFSv3 file handle  */
-static int filefh3_length;	/* (variable) length of filefh3 */
+static char filefh[NFS3_FHSIZE]; /* NFSv2 / NFSv3 file handle */
+static int filefh3_length;	/* (variable) length of filefh when NFSv3 */
 
 static enum net_loop_state nfs_download_state;
 static struct in_addr nfs_server_ip;
@@ -316,7 +314,7 @@ static void nfs_readlink_req(void)
 		p += (NFS_FHSIZE / 4);
 	} else { /* NFSV3_FLAG */
 		*p++ = htonl(filefh3_length);
-		memcpy(p, filefh3, filefh3_length);
+		memcpy(p, filefh, filefh3_length);
 		p += (filefh3_length / 4);
 	}
 
@@ -388,7 +386,7 @@ static void nfs_read_req(int offset, int readlen)
 		*p++ = 0;
 	} else { /* NFSV3_FLAG */
 		*p++ = htonl(filefh3_length);
-		memcpy(p, filefh3, filefh3_length);
+		memcpy(p, filefh, filefh3_length);
 		p += (filefh3_length / 4);
 		*p++ = htonl(0); /* offset is 64-bit long, so fill with 0 */
 		*p++ = htonl(offset);
@@ -582,7 +580,7 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
 		filefh3_length = ntohl(rpc_pkt.u.reply.data[1]);
 		if (filefh3_length > NFS3_FHSIZE)
 			filefh3_length  = NFS3_FHSIZE;
-		memcpy(filefh3, rpc_pkt.u.reply.data + 2, filefh3_length);
+		memcpy(filefh, rpc_pkt.u.reply.data + 2, filefh3_length);
 	}
 
 	return 0;
-- 
1.7.11.5

[U-Boot] [PATCH 2/9] net: nfs: Correct the reply data buffer size

[Joe Hershberger]

The type of the buffer is uint32_t, but the parameter used to size it
is referring to bytes. Divide by the size of the array elements.

Strictly speaking, this shouldn't be needed at all... It could just be 1
just like the request.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 net/nfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfs.h b/net/nfs.h
index 45da246..aa4e450 100644
--- a/net/nfs.h
+++ b/net/nfs.h
@@ -76,7 +76,7 @@ struct rpc_t {
 			uint32_t verifier;
 			uint32_t v2;
 			uint32_t astatus;
-			uint32_t data[NFS_READ_SIZE];
+			uint32_t data[NFS_READ_SIZE / sizeof(uint32_t)];
 		} reply;
 	} u;
 };
-- 
1.7.11.5

[U-Boot] [PATCH 3/9] net: nfs: Fix lines that are too long

[Joe Hershberger]

Fix complaints from checkpatch.pl.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 net/nfs.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index ac3cde4..14a0d2f 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -547,7 +547,8 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
 			/* Minimal supported NFS version */
 			case 3:
 				debug("*** Waring: NFS version not supported: Requested: V%d, accepted: min V%d - max V%d\n",
-				      (supported_nfs_versions & NFSV2_FLAG) ? 2 : 3,
+				      (supported_nfs_versions & NFSV2_FLAG) ?
+						2 : 3,
 				      ntohl(rpc_pkt.u.reply.data[0]),
 				      ntohl(rpc_pkt.u.reply.data[1]));
 				debug("Will retry with NFSv3\n");
@@ -557,7 +558,8 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
 			case 4:
 			default:
 				printf("*** ERROR: NFS version not supported: Requested: V%d, accepted: min V%d - max V%d\n",
-				       (supported_nfs_versions & NFSV2_FLAG) ? 2 : 3,
+				       (supported_nfs_versions & NFSV2_FLAG) ?
+						2 : 3,
 				       ntohl(rpc_pkt.u.reply.data[0]),
 				       ntohl(rpc_pkt.u.reply.data[1]));
 			}
@@ -828,7 +830,8 @@ static void nfs_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 			puts("*** ERROR: File lookup fail\n");
 			nfs_state = STATE_UMOUNT_REQ;
 			nfs_send();
-		} else if (reply == -NFS_RPC_PROG_MISMATCH && supported_nfs_versions != 0) {
+		} else if (reply == -NFS_RPC_PROG_MISMATCH &&
+			   supported_nfs_versions != 0) {
 			/* umount */
 			nfs_state = STATE_UMOUNT_REQ;
 			nfs_send();
-- 
1.7.11.5

[U-Boot] [PATCH 4/9] net: nfs: Consolidate handling of NFSv3 attributes

[Joe Hershberger]

Instead of repeating the same large snippet for dealing with attributes
it should be shared with a helper function.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 net/nfs.c | 151 ++++++++++++++++++++++++--------------------------------------
 1 file changed, 59 insertions(+), 92 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index 14a0d2f..08bdd92 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -588,10 +588,39 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
 	return 0;
 }
 
+static int nfs3_get_attributes_offset(uint32_t *data)
+{
+	if (ntohl(data[1]) != 0) {
+		/* 'attributes_follow' flag is TRUE,
+		 * so we have attributes on 21 bytes */
+		/* Skip unused values :
+			type;	32 bits value,
+			mode;	32 bits value,
+			nlink;	32 bits value,
+			uid;	32 bits value,
+			gid;	32 bits value,
+			size;	64 bits value,
+			used;	64 bits value,
+			rdev;	64 bits value,
+			fsid;	64 bits value,
+			fileid;	64 bits value,
+			atime;	64 bits value,
+			mtime;	64 bits value,
+			ctime;	64 bits value,
+		*/
+		return 22;
+	} else {
+		/* 'attributes_follow' flag is FALSE,
+		 * so we don't have any attributes */
+		return 1;
+	}
+}
+
 static int nfs_readlink_reply(uchar *pkt, unsigned len)
 {
 	struct rpc_t rpc_pkt;
 	int rlen;
+	int nfsv3_data_offset = 0;
 
 	debug("%s\n", __func__);
 
@@ -608,68 +637,28 @@ static int nfs_readlink_reply(uchar *pkt, unsigned len)
 	    rpc_pkt.u.reply.data[0])
 		return -1;
 
-	if (supported_nfs_versions & NFSV2_FLAG) {
+	if (!(supported_nfs_versions & NFSV2_FLAG)) { /* NFSV3_FLAG */
+		nfsv3_data_offset =
+			nfs3_get_attributes_offset(rpc_pkt.u.reply.data);
+	}
 
-		rlen = ntohl(rpc_pkt.u.reply.data[1]); /* new path length */
+	/* new path length */
+	rlen = ntohl(rpc_pkt.u.reply.data[1 + nfsv3_data_offset]);
 
-		if (*((char *)&(rpc_pkt.u.reply.data[2])) != '/') {
-			int pathlen;
-			strcat(nfs_path, "/");
-			pathlen = strlen(nfs_path);
-			memcpy(nfs_path + pathlen,
-			       (uchar *)&(rpc_pkt.u.reply.data[2]),
-			       rlen);
-			nfs_path[pathlen + rlen] = 0;
-		} else {
-			memcpy(nfs_path,
-			       (uchar *)&(rpc_pkt.u.reply.data[2]),
-			       rlen);
-			nfs_path[rlen] = 0;
-		}
-	} else {  /* NFSV3_FLAG */
-		int nfsv3_data_offset = 0;
-		if (ntohl(rpc_pkt.u.reply.data[1]) != 0) {
-			/* 'attributes_follow' flag is TRUE,
-			 * so we have attributes on 21 bytes */
-			/* Skip unused values :
-				type;	32 bits value,
-				mode;	32 bits value,
-				nlink;	32 bits value,
-				uid;	32 bits value,
-				gid;	32 bits value,
-				size;	64 bits value,
-				used;	64 bits value,
-				rdev;	64 bits value,
-				fsid;	64 bits value,
-				fileid;	64 bits value,
-				atime;	64 bits value,
-				mtime;	64 bits value,
-				ctime;	64 bits value,
-			*/
-			nfsv3_data_offset = 22;
-		} else {
-			/* 'attributes_follow' flag is FALSE,
-			 * so we don't have any attributes */
-			nfsv3_data_offset = 1;
-		}
+	if (*((char *)&(rpc_pkt.u.reply.data[2 + nfsv3_data_offset])) != '/') {
+		int pathlen;
 
-		/* new path length */
-		rlen = ntohl(rpc_pkt.u.reply.data[1+nfsv3_data_offset]);
-
-		if (*((char *)&(rpc_pkt.u.reply.data[2+nfsv3_data_offset])) != '/') {
-			int pathlen;
-			strcat(nfs_path, "/");
-			pathlen = strlen(nfs_path);
-			memcpy(nfs_path + pathlen,
-			       (uchar *)&(rpc_pkt.u.reply.data[2+nfsv3_data_offset]),
-			       rlen);
-			nfs_path[pathlen + rlen] = 0;
-		} else {
-			memcpy(nfs_path,
-			       (uchar *)&(rpc_pkt.u.reply.data[2+nfsv3_data_offset]),
-			       rlen);
-			nfs_path[rlen] = 0;
-		}
+		strcat(nfs_path, "/");
+		pathlen = strlen(nfs_path);
+		memcpy(nfs_path + pathlen,
+		       (uchar *)&(rpc_pkt.u.reply.data[2 + nfsv3_data_offset]),
+		       rlen);
+		nfs_path[pathlen + rlen] = 0;
+	} else {
+		memcpy(nfs_path,
+		       (uchar *)&(rpc_pkt.u.reply.data[2 + nfsv3_data_offset]),
+		       rlen);
+		nfs_path[rlen] = 0;
 	}
 	return 0;
 }
@@ -710,39 +699,17 @@ static int nfs_read_reply(uchar *pkt, unsigned len)
 		rlen = ntohl(rpc_pkt.u.reply.data[18]);
 		data_ptr = (uchar *)&(rpc_pkt.u.reply.data[19]);
 	} else {  /* NFSV3_FLAG */
-		if (ntohl(rpc_pkt.u.reply.data[1]) != 0) {
-			/* 'attributes_follow' is TRUE,
-			 * so we have attributes on 21 bytes */
-			/* Skip unused values :
-				type;	32 bits value,
-				mode;	32 bits value,
-				nlink;	32 bits value,
-				uid;	32 bits value,
-				gid;	32 bits value,
-				size;	64 bits value,
-				used;	64 bits value,
-				rdev;	64 bits value,
-				fsid;	64 bits value,
-				fileid;	64 bits value,
-				atime;	64 bits value,
-				mtime;	64 bits value,
-				ctime;	64 bits value,
-			*/
-			rlen = ntohl(rpc_pkt.u.reply.data[23]); /* count value */
-			/* Skip unused values :
-				EOF:		32 bits value,
-				data_size:	32 bits value,
-			*/
-			data_ptr = (uchar *)&(rpc_pkt.u.reply.data[26]);
-		} else {
-			/* attributes_follow is FALSE, so we don't have any attributes */
-			rlen = ntohl(rpc_pkt.u.reply.data[2]); /* count value */
-			/* Skip unused values :
-				EOF:		32 bits value,
-				data_size:	32 bits value,
-			*/
-			data_ptr = (uchar *)&(rpc_pkt.u.reply.data[5]);
-		}
+		int nfsv3_data_offset =
+			nfs3_get_attributes_offset(rpc_pkt.u.reply.data);
+
+		/* count value */
+		rlen = ntohl(rpc_pkt.u.reply.data[1 + nfsv3_data_offset]);
+		/* Skip unused values :
+			EOF:		32 bits value,
+			data_size:	32 bits value,
+		*/
+		data_ptr = (uchar *)
+			&(rpc_pkt.u.reply.data[4 + nfsv3_data_offset]);
 	}
 
 	if (store_block(data_ptr, nfs_offset, rlen))
-- 
1.7.11.5

[U-Boot] [PATCH 5/9] net: nfs: Correct a comment

[Joe Hershberger]

The buffer is of 32-bit elements, not bytes.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 net/nfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfs.c b/net/nfs.c
index 08bdd92..ade589c 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -592,7 +592,7 @@ static int nfs3_get_attributes_offset(uint32_t *data)
 {
 	if (ntohl(data[1]) != 0) {
 		/* 'attributes_follow' flag is TRUE,
-		 * so we have attributes on 21 bytes */
+		 * so we have attributes on 21 dwords */
 		/* Skip unused values :
 			type;	32 bits value,
 			mode;	32 bits value,
-- 
1.7.11.5

[U-Boot] [PATCH 6/9] net: nfs: Use consistent names for the rpc_pkt

[Joe Hershberger]

Use the same name throughout the nfs code and use the same member of the
union to avoid casts.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 net/nfs.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index ade589c..bdbdc26 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -185,39 +185,39 @@ RPC_LOOKUP - Lookup RPC Port numbers
 **************************************************************************/
 static void rpc_req(int rpc_prog, int rpc_proc, uint32_t *data, int datalen)
 {
-	struct rpc_t pkt;
+	struct rpc_t rpc_pkt;
 	unsigned long id;
 	uint32_t *p;
 	int pktlen;
 	int sport;
 
 	id = ++rpc_id;
-	pkt.u.call.id = htonl(id);
-	pkt.u.call.type = htonl(MSG_CALL);
-	pkt.u.call.rpcvers = htonl(2);	/* use RPC version 2 */
-	pkt.u.call.prog = htonl(rpc_prog);
+	rpc_pkt.u.call.id = htonl(id);
+	rpc_pkt.u.call.type = htonl(MSG_CALL);
+	rpc_pkt.u.call.rpcvers = htonl(2);	/* use RPC version 2 */
+	rpc_pkt.u.call.prog = htonl(rpc_prog);
 	switch (rpc_prog) {
 	case PROG_NFS:
 		if (supported_nfs_versions & NFSV2_FLAG)
-			pkt.u.call.vers = htonl(2);	/* NFS v2 */
+			rpc_pkt.u.call.vers = htonl(2);	/* NFS v2 */
 		else /* NFSV3_FLAG */
-			pkt.u.call.vers = htonl(3);	/* NFS v3 */
+			rpc_pkt.u.call.vers = htonl(3);	/* NFS v3 */
 		break;
 	case PROG_PORTMAP:
 	case PROG_MOUNT:
 	default:
-		pkt.u.call.vers = htonl(2);	/* portmapper is version 2 */
+		rpc_pkt.u.call.vers = htonl(2);	/* portmapper is version 2 */
 	}
-	pkt.u.call.proc = htonl(rpc_proc);
-	p = (uint32_t *)&(pkt.u.call.data);
+	rpc_pkt.u.call.proc = htonl(rpc_proc);
+	p = (uint32_t *)&(rpc_pkt.u.call.data);
 
 	if (datalen)
 		memcpy((char *)p, (char *)data, datalen*sizeof(uint32_t));
 
-	pktlen = (char *)p + datalen*sizeof(uint32_t) - (char *)&pkt;
+	pktlen = (char *)p + datalen * sizeof(uint32_t) - (char *)&rpc_pkt;
 
 	memcpy((char *)net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE,
-	       (char *)&pkt, pktlen);
+	       &rpc_pkt.u.data[0], pktlen);
 
 	if (rpc_prog == PROG_PORTMAP)
 		sport = SUNRPC_PORT;
@@ -445,7 +445,7 @@ static int rpc_lookup_reply(int prog, uchar *pkt, unsigned len)
 {
 	struct rpc_t rpc_pkt;
 
-	memcpy((unsigned char *)&rpc_pkt, pkt, len);
+	memcpy(&rpc_pkt.u.data[0], pkt, len);
 
 	debug("%s\n", __func__);
 
@@ -477,7 +477,7 @@ static int nfs_mount_reply(uchar *pkt, unsigned len)
 
 	debug("%s\n", __func__);
 
-	memcpy((unsigned char *)&rpc_pkt, pkt, len);
+	memcpy(&rpc_pkt.u.data[0], pkt, len);
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
 		return -NFS_RPC_ERR;
@@ -503,7 +503,7 @@ static int nfs_umountall_reply(uchar *pkt, unsigned len)
 
 	debug("%s\n", __func__);
 
-	memcpy((unsigned char *)&rpc_pkt, pkt, len);
+	memcpy(&rpc_pkt.u.data[0], pkt, len);
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
 		return -NFS_RPC_ERR;
@@ -527,7 +527,7 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
 
 	debug("%s\n", __func__);
 
-	memcpy((unsigned char *)&rpc_pkt, pkt, len);
+	memcpy(&rpc_pkt.u.data[0], pkt, len);
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
 		return -NFS_RPC_ERR;
@@ -671,7 +671,7 @@ static int nfs_read_reply(uchar *pkt, unsigned len)
 
 	debug("%s\n", __func__);
 
-	memcpy((uchar *)&rpc_pkt, pkt, sizeof(rpc_pkt.u.reply));
+	memcpy(&rpc_pkt.u.data[0], pkt, sizeof(rpc_pkt.u.reply));
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
 		return -NFS_RPC_ERR;
-- 
1.7.11.5

[U-Boot] [PATCH 7/9] net: nfs: Move some prints to debug statements

[Joe Hershberger]

Much of the information is verbose and derived directly from the
environment. Only output in debug mode. This also saves about 300 bytes
from the code size.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 net/nfs.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index bdbdc26..31047c2 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -557,11 +557,13 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
 				return -NFS_RPC_PROG_MISMATCH;
 			case 4:
 			default:
-				printf("*** ERROR: NFS version not supported: Requested: V%d, accepted: min V%d - max V%d\n",
-				       (supported_nfs_versions & NFSV2_FLAG) ?
+				puts("*** ERROR: NFS version not supported");
+				debug(": Requested: V%d, accepted: min V%d - max V%d\n",
+				      (supported_nfs_versions & NFSV2_FLAG) ?
 						2 : 3,
-				       ntohl(rpc_pkt.u.reply.data[0]),
-				       ntohl(rpc_pkt.u.reply.data[1]));
+				      ntohl(rpc_pkt.u.reply.data[0]),
+				      ntohl(rpc_pkt.u.reply.data[1]));
+				puts("\n");
 			}
 			break;
 		case NFS_RPC_PROG_UNAVAIL:
@@ -569,8 +571,8 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
 		case NFS_RPC_GARBAGE_ARGS:
 		case NFS_RPC_SYSTEM_ERR:
 		default: /* Unknown error on 'accept state' flag */
-			printf("*** ERROR: accept state error (%d)\n",
-			       ntohl(rpc_pkt.u.reply.astatus));
+			debug("*** ERROR: accept state error (%d)\n",
+			      ntohl(rpc_pkt.u.reply.astatus));
 			break;
 		}
 		return -1;
@@ -781,7 +783,7 @@ static void nfs_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 		if (reply == -NFS_RPC_DROP) {
 			break;
 		} else if (reply == -NFS_RPC_ERR) {
-			puts("*** ERROR: Cannot umount\n");
+			debug("*** ERROR: Cannot umount\n");
 			net_set_state(NETLOOP_FAIL);
 		} else {
 			puts("\ndone\n");
@@ -845,7 +847,7 @@ static void nfs_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 			if (!rlen)
 				nfs_download_state = NETLOOP_SUCCESS;
 			if (rlen < 0)
-				printf("NFS READ error (%d)\n", rlen);
+				debug("NFS READ error (%d)\n", rlen);
 			nfs_state = STATE_UMOUNT_REQ;
 			nfs_send();
 		}
@@ -864,7 +866,7 @@ void nfs_start(void)
 
 	if (nfs_path == NULL) {
 		net_set_state(NETLOOP_FAIL);
-		puts("*** ERROR: Fail allocate memory\n");
+		debug("*** ERROR: Fail allocate memory\n");
 		return;
 	}
 
@@ -875,8 +877,8 @@ void nfs_start(void)
 			(net_ip.s_addr >> 16) & 0xFF,
 			(net_ip.s_addr >> 24) & 0xFF);
 
-		printf("*** Warning: no boot file name; using '%s'\n",
-		       nfs_path);
+		debug("*** Warning: no boot file name; using '%s'\n",
+		      nfs_path);
 	} else {
 		char *p = net_boot_file_name;
 
@@ -894,10 +896,10 @@ void nfs_start(void)
 	nfs_filename = basename(nfs_path);
 	nfs_path     = dirname(nfs_path);
 
-	printf("Using %s device\n", eth_get_name());
+	debug("Using %s device\n", eth_get_name());
 
-	printf("File transfer via NFS from server %pI4; our IP address is %pI4",
-	       &nfs_server_ip, &net_ip);
+	debug("File transfer via NFS from server %pI4; our IP address is %pI4",
+	      &nfs_server_ip, &net_ip);
 
 	/* Check if we need to send across this subnet */
 	if (net_gateway.s_addr && net_netmask.s_addr) {
@@ -907,18 +909,17 @@ void nfs_start(void)
 		our_net.s_addr = net_ip.s_addr & net_netmask.s_addr;
 		server_net.s_addr = net_server_ip.s_addr & net_netmask.s_addr;
 		if (our_net.s_addr != server_net.s_addr)
-			printf("; sending through gateway %pI4",
-			       &net_gateway);
+			debug("; sending through gateway %pI4",
+			      &net_gateway);
 	}
-	printf("\nFilename '%s/%s'.", nfs_path, nfs_filename);
+	debug("\nFilename '%s/%s'.", nfs_path, nfs_filename);
 
 	if (net_boot_file_expected_size_in_blocks) {
-		printf(" Size is 0x%x Bytes = ",
-		       net_boot_file_expected_size_in_blocks << 9);
+		debug(" Size is 0x%x Bytes = ",
+		      net_boot_file_expected_size_in_blocks << 9);
 		print_size(net_boot_file_expected_size_in_blocks << 9, "");
 	}
-	printf("\nLoad address: 0x%lx\n"
-		"Loading: *\b", load_addr);
+	debug("\nLoad address: 0x%lx\nLoading: *\b", load_addr);
 
 	net_set_timeout_handler(nfs_timeout, nfs_timeout_handler);
 	net_set_udp_handler(nfs_handler);
-- 
1.7.11.5

[U-Boot] [PATCH 8/9] net: nfs: Use the tx buffer to construct rpc msgs

[Joe Hershberger]

Instead of always allocating a huge temporary buffer on the stack and
then memcpy()ing the result into the transmit buffer, simply figure out
where in the transmit buffer the bytes will belong and write them there
directly as each message is built.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 net/nfs.c | 88 ++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 45 insertions(+), 43 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index 31047c2..3fb253b 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -183,41 +183,41 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
 /**************************************************************************
 RPC_LOOKUP - Lookup RPC Port numbers
 **************************************************************************/
-static void rpc_req(int rpc_prog, int rpc_proc, uint32_t *data, int datalen)
+static struct rpc_t *rpc_req_prep(void)
+{
+	return (struct rpc_t *)(net_tx_packet + net_eth_hdr_size() +
+				IP_UDP_HDR_SIZE);
+}
+
+static void rpc_req(int rpc_prog, int rpc_proc, struct rpc_t *rpc_pkt,
+		    int datalen)
 {
-	struct rpc_t rpc_pkt;
 	unsigned long id;
-	uint32_t *p;
 	int pktlen;
 	int sport;
 
 	id = ++rpc_id;
-	rpc_pkt.u.call.id = htonl(id);
-	rpc_pkt.u.call.type = htonl(MSG_CALL);
-	rpc_pkt.u.call.rpcvers = htonl(2);	/* use RPC version 2 */
-	rpc_pkt.u.call.prog = htonl(rpc_prog);
+	rpc_pkt->u.call.id = htonl(id);
+	rpc_pkt->u.call.type = htonl(MSG_CALL);
+	rpc_pkt->u.call.rpcvers = htonl(2);	/* use RPC version 2 */
+	rpc_pkt->u.call.prog = htonl(rpc_prog);
 	switch (rpc_prog) {
 	case PROG_NFS:
 		if (supported_nfs_versions & NFSV2_FLAG)
-			rpc_pkt.u.call.vers = htonl(2);	/* NFS v2 */
+			rpc_pkt->u.call.vers = htonl(2);	/* NFS v2 */
 		else /* NFSV3_FLAG */
-			rpc_pkt.u.call.vers = htonl(3);	/* NFS v3 */
+			rpc_pkt->u.call.vers = htonl(3);	/* NFS v3 */
 		break;
 	case PROG_PORTMAP:
 	case PROG_MOUNT:
 	default:
-		rpc_pkt.u.call.vers = htonl(2);	/* portmapper is version 2 */
+		/* portmapper is version 2 */
+		rpc_pkt->u.call.vers = htonl(2);
 	}
-	rpc_pkt.u.call.proc = htonl(rpc_proc);
-	p = (uint32_t *)&(rpc_pkt.u.call.data);
-
-	if (datalen)
-		memcpy((char *)p, (char *)data, datalen*sizeof(uint32_t));
-
-	pktlen = (char *)p + datalen * sizeof(uint32_t) - (char *)&rpc_pkt;
+	rpc_pkt->u.call.proc = htonl(rpc_proc);
 
-	memcpy((char *)net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE,
-	       &rpc_pkt.u.data[0], pktlen);
+	pktlen = ((char *)&rpc_pkt->u.call.data - (char *)&rpc_pkt) +
+		datalen * sizeof(uint32_t);
 
 	if (rpc_prog == PROG_PORTMAP)
 		sport = SUNRPC_PORT;
@@ -235,15 +235,17 @@ RPC_LOOKUP - Lookup RPC Port numbers
 **************************************************************************/
 static void rpc_lookup_req(int prog, int ver)
 {
-	uint32_t data[16];
+	uint32_t *data;
+	struct rpc_t *rpc_pkt = rpc_req_prep();
 
+	data = rpc_pkt->u.call.data;
 	data[0] = 0; data[1] = 0;	/* auth credential */
 	data[2] = 0; data[3] = 0;	/* auth verifier */
 	data[4] = htonl(prog);
 	data[5] = htonl(ver);
 	data[6] = htonl(17);	/* IP_UDP */
 	data[7] = 0;
-	rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, data, 8);
+	rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, rpc_pkt, 8);
 }
 
 /**************************************************************************
@@ -251,14 +253,14 @@ NFS_MOUNT - Mount an NFS Filesystem
 **************************************************************************/
 static void nfs_mount_req(char *path)
 {
-	uint32_t data[1024];
 	uint32_t *p;
 	int len;
 	int pathlen;
+	struct rpc_t *rpc_pkt = rpc_req_prep();
 
 	pathlen = strlen(path);
 
-	p = &(data[0]);
+	p = rpc_pkt->u.call.data;
 	p = rpc_add_credentials(p);
 
 	*p++ = htonl(pathlen);
@@ -267,9 +269,9 @@ static void nfs_mount_req(char *path)
 	memcpy(p, path, pathlen);
 	p += (pathlen + 3) / 4;
 
-	len = (uint32_t *)p - (uint32_t *)&(data[0]);
+	len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
 
-	rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, data, len);
+	rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, rpc_pkt, len);
 }
 
 /**************************************************************************
@@ -277,20 +279,20 @@ NFS_UMOUNTALL - Unmount all our NFS Filesystems on the Server
 **************************************************************************/
 static void nfs_umountall_req(void)
 {
-	uint32_t data[1024];
 	uint32_t *p;
 	int len;
+	struct rpc_t *rpc_pkt = rpc_req_prep();
 
 	if ((nfs_server_mount_port == -1) || (!fs_mounted))
 		/* Nothing mounted, nothing to umount */
 		return;
 
-	p = &(data[0]);
+	p = rpc_pkt->u.call.data;
 	p = rpc_add_credentials(p);
 
-	len = (uint32_t *)p - (uint32_t *)&(data[0]);
+	len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
 
-	rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, data, len);
+	rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, rpc_pkt, len);
 }
 
 /***************************************************************************
@@ -302,11 +304,11 @@ static void nfs_umountall_req(void)
  **************************************************************************/
 static void nfs_readlink_req(void)
 {
-	uint32_t data[1024];
 	uint32_t *p;
 	int len;
+	struct rpc_t *rpc_pkt = rpc_req_prep();
 
-	p = &(data[0]);
+	p = rpc_pkt->u.call.data;
 	p = rpc_add_credentials(p);
 
 	if (supported_nfs_versions & NFSV2_FLAG) {
@@ -318,9 +320,9 @@ static void nfs_readlink_req(void)
 		p += (filefh3_length / 4);
 	}
 
-	len = (uint32_t *)p - (uint32_t *)&(data[0]);
+	len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
 
-	rpc_req(PROG_NFS, NFS_READLINK, data, len);
+	rpc_req(PROG_NFS, NFS_READLINK, rpc_pkt, len);
 }
 
 /**************************************************************************
@@ -328,14 +330,14 @@ NFS_LOOKUP - Lookup Pathname
 **************************************************************************/
 static void nfs_lookup_req(char *fname)
 {
-	uint32_t data[1024];
 	uint32_t *p;
 	int len;
 	int fnamelen;
+	struct rpc_t *rpc_pkt = rpc_req_prep();
 
 	fnamelen = strlen(fname);
 
-	p = &(data[0]);
+	p = rpc_pkt->u.call.data;
 	p = rpc_add_credentials(p);
 
 	if (supported_nfs_versions & NFSV2_FLAG) {
@@ -347,9 +349,9 @@ static void nfs_lookup_req(char *fname)
 		memcpy(p, fname, fnamelen);
 		p += (fnamelen + 3) / 4;
 
-		len = (uint32_t *)p - (uint32_t *)&(data[0]);
+		len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
 
-		rpc_req(PROG_NFS, NFS_LOOKUP, data, len);
+		rpc_req(PROG_NFS, NFS_LOOKUP, rpc_pkt, len);
 	} else {  /* NFSV3_FLAG */
 		*p++ = htonl(NFS_FHSIZE);	/* Dir handle length */
 		memcpy(p, dirfh, NFS_FHSIZE);
@@ -360,9 +362,9 @@ static void nfs_lookup_req(char *fname)
 		memcpy(p, fname, fnamelen);
 		p += (fnamelen + 3) / 4;
 
-		len = (uint32_t *)p - (uint32_t *)&(data[0]);
+		len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
 
-		rpc_req(PROG_NFS, NFS3PROC_LOOKUP, data, len);
+		rpc_req(PROG_NFS, NFS3PROC_LOOKUP, rpc_pkt, len);
 	}
 }
 
@@ -371,11 +373,11 @@ NFS_READ - Read File on NFS Server
 **************************************************************************/
 static void nfs_read_req(int offset, int readlen)
 {
-	uint32_t data[1024];
 	uint32_t *p;
 	int len;
+	struct rpc_t *rpc_pkt = rpc_req_prep();
 
-	p = &(data[0]);
+	p = rpc_pkt->u.call.data;
 	p = rpc_add_credentials(p);
 
 	if (supported_nfs_versions & NFSV2_FLAG) {
@@ -394,9 +396,9 @@ static void nfs_read_req(int offset, int readlen)
 		*p++ = 0;
 	}
 
-	len = (uint32_t *)p - (uint32_t *)&(data[0]);
+	len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
 
-	rpc_req(PROG_NFS, NFS_READ, data, len);
+	rpc_req(PROG_NFS, NFS_READ, rpc_pkt, len);
 }
 
 /**************************************************************************
-- 
1.7.11.5

[U-Boot] [PATCH 9/9] net: nfs: Simplify rpc_add_credentials()

[Joe Hershberger]

We use an empty hostname, so remove all the "processing" of the
known-to-be-empty hostname and just write 0's where needed.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

---

 net/nfs.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index 3fb253b..814751b 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -142,13 +142,6 @@ RPC_ADD_CREDENTIALS - Add RPC authentication/verifier entries
 **************************************************************************/
 static uint32_t *rpc_add_credentials(uint32_t *p)
 {
-	int hl;
-	int hostnamelen;
-	char hostname[256];
-
-	strcpy(hostname, "");
-	hostnamelen = strlen(hostname);
-
 	/* Here's the executive summary on authentication requirements of the
 	 * various NFS server implementations:	Linux accepts both AUTH_NONE
 	 * and AUTH_UNIX authentication (also accepts an empty hostname field
@@ -158,17 +151,11 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
 	 * it (if the BOOTP/DHCP reply didn't give one, just use an empty
 	 * hostname).  */
 
-	hl = (hostnamelen + 3) & ~3;
-
 	/* Provide an AUTH_UNIX credential.  */
 	*p++ = htonl(1);		/* AUTH_UNIX */
-	*p++ = htonl(hl+20);		/* auth length */
-	*p++ = htonl(0);		/* stamp */
-	*p++ = htonl(hostnamelen);	/* hostname string */
-	if (hostnamelen & 3)
-		*(p + hostnamelen / 4) = 0; /* add zero padding */
-	memcpy(p, hostname, hostnamelen);
-	p += hl / 4;
+	*p++ = htonl(20);		/* auth length */
+	*p++ = 0;			/* stamp */
+	*p++ = 0;			/* hostname string */
 	*p++ = 0;			/* uid */
 	*p++ = 0;			/* gid */
 	*p++ = 0;			/* auxiliary gid list */
-- 
1.7.11.5

[U-Boot] [PATCH 0/9] Fix up issues with new NFSv3 implementation

[Joe Hershberger]

On Mon, Aug 15, 2016 at 3:03 PM, Joe Hershberger <joe.hershberger@ni.com> wrote:
> This fixes most of the memory bloat that happened with the addition of NFSv3
> Some checkpatch.pl issues are also fixed
> Refactored some code to better share common snippets
>
> These patches depend on the NFSv3 patch by Guillaume GARDET <guillaume.gardet@free.fr>
> https://patchwork.ozlabs.org/patch/654061/
>
>
> Joe Hershberger (9):
>   net: nfs: Share the file handle buffer for v2 / v3
>   net: nfs: Correct the reply data buffer size
>   net: nfs: Fix lines that are too long
>   net: nfs: Consolidate handling of NFSv3 attributes
>   net: nfs: Correct a comment
>   net: nfs: Use consistent names for the rpc_pkt
>   net: nfs: Move some prints to debug statements
>   net: nfs: Use the tx buffer to construct rpc msgs
>   net: nfs: Simplify rpc_add_credentials()

Barring any comments on this, I'm going to pull these in next week.

Thanks,
-Joe

[U-Boot] net: nfs: Share the file handle buffer for v2 / v3

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659407/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] net: nfs: Correct the reply data buffer size

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659403/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] net: nfs: Fix lines that are too long

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659410/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] net: nfs: Consolidate handling of NFSv3 attributes

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659404/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] net: nfs: Correct a comment

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659408/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] net: nfs: Use consistent names for the rpc_pkt

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659405/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] net: nfs: Move some prints to debug statements

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659409/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] net: nfs: Use the tx buffer to construct rpc msgs

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659411/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] net: nfs: Simplify rpc_add_credentials()

[Joe Hershberger]

Hi Joe,

https://patchwork.ozlabs.org/patch/659406/ was applied to u-boot-net.git.

Thanks!
-Joe

[U-Boot] [PATCH 8/9] net: nfs: Use the tx buffer to construct rpc msgs

[Guillaume Gardet]

Hi Joe,

I tested 2016.09-rc2 on a beagleboard xM and NFS does not work anymore!
When I call the nfs command, I get a "data abort" error with a reboot of the board!

A git bisect point to this patch: "net: nfs: Use the tx buffer to construct rpc msgs"

Could you have a look and fix it for the release, please? At least revert it for now.


Guillaume



Le 15/08/2016 ? 22:03, Joe Hershberger a ?crit :
> Instead of always allocating a huge temporary buffer on the stack and
> then memcpy()ing the result into the transmit buffer, simply figure out
> where in the transmit buffer the bytes will belong and write them there
> directly as each message is built.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---
>
>   net/nfs.c | 88 ++++++++++++++++++++++++++++++++-------------------------------
>   1 file changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/net/nfs.c b/net/nfs.c
> index 31047c2..3fb253b 100644
> --- a/net/nfs.c
> +++ b/net/nfs.c
> @@ -183,41 +183,41 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
>   /**************************************************************************
>   RPC_LOOKUP - Lookup RPC Port numbers
>   **************************************************************************/
> -static void rpc_req(int rpc_prog, int rpc_proc, uint32_t *data, int datalen)
> +static struct rpc_t *rpc_req_prep(void)
> +{
> +	return (struct rpc_t *)(net_tx_packet + net_eth_hdr_size() +
> +				IP_UDP_HDR_SIZE);
> +}
> +
> +static void rpc_req(int rpc_prog, int rpc_proc, struct rpc_t *rpc_pkt,
> +		    int datalen)
>   {
> -	struct rpc_t rpc_pkt;
>   	unsigned long id;
> -	uint32_t *p;
>   	int pktlen;
>   	int sport;
>   
>   	id = ++rpc_id;
> -	rpc_pkt.u.call.id = htonl(id);
> -	rpc_pkt.u.call.type = htonl(MSG_CALL);
> -	rpc_pkt.u.call.rpcvers = htonl(2);	/* use RPC version 2 */
> -	rpc_pkt.u.call.prog = htonl(rpc_prog);
> +	rpc_pkt->u.call.id = htonl(id);
> +	rpc_pkt->u.call.type = htonl(MSG_CALL);
> +	rpc_pkt->u.call.rpcvers = htonl(2);	/* use RPC version 2 */
> +	rpc_pkt->u.call.prog = htonl(rpc_prog);
>   	switch (rpc_prog) {
>   	case PROG_NFS:
>   		if (supported_nfs_versions & NFSV2_FLAG)
> -			rpc_pkt.u.call.vers = htonl(2);	/* NFS v2 */
> +			rpc_pkt->u.call.vers = htonl(2);	/* NFS v2 */
>   		else /* NFSV3_FLAG */
> -			rpc_pkt.u.call.vers = htonl(3);	/* NFS v3 */
> +			rpc_pkt->u.call.vers = htonl(3);	/* NFS v3 */
>   		break;
>   	case PROG_PORTMAP:
>   	case PROG_MOUNT:
>   	default:
> -		rpc_pkt.u.call.vers = htonl(2);	/* portmapper is version 2 */
> +		/* portmapper is version 2 */
> +		rpc_pkt->u.call.vers = htonl(2);
>   	}
> -	rpc_pkt.u.call.proc = htonl(rpc_proc);
> -	p = (uint32_t *)&(rpc_pkt.u.call.data);
> -
> -	if (datalen)
> -		memcpy((char *)p, (char *)data, datalen*sizeof(uint32_t));
> -
> -	pktlen = (char *)p + datalen * sizeof(uint32_t) - (char *)&rpc_pkt;
> +	rpc_pkt->u.call.proc = htonl(rpc_proc);
>   
> -	memcpy((char *)net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE,
> -	       &rpc_pkt.u.data[0], pktlen);
> +	pktlen = ((char *)&rpc_pkt->u.call.data - (char *)&rpc_pkt) +
> +		datalen * sizeof(uint32_t);
>   
>   	if (rpc_prog == PROG_PORTMAP)
>   		sport = SUNRPC_PORT;
> @@ -235,15 +235,17 @@ RPC_LOOKUP - Lookup RPC Port numbers
>   **************************************************************************/
>   static void rpc_lookup_req(int prog, int ver)
>   {
> -	uint32_t data[16];
> +	uint32_t *data;
> +	struct rpc_t *rpc_pkt = rpc_req_prep();
>   
> +	data = rpc_pkt->u.call.data;
>   	data[0] = 0; data[1] = 0;	/* auth credential */
>   	data[2] = 0; data[3] = 0;	/* auth verifier */
>   	data[4] = htonl(prog);
>   	data[5] = htonl(ver);
>   	data[6] = htonl(17);	/* IP_UDP */
>   	data[7] = 0;
> -	rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, data, 8);
> +	rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, rpc_pkt, 8);
>   }
>   
>   /**************************************************************************
> @@ -251,14 +253,14 @@ NFS_MOUNT - Mount an NFS Filesystem
>   **************************************************************************/
>   static void nfs_mount_req(char *path)
>   {
> -	uint32_t data[1024];
>   	uint32_t *p;
>   	int len;
>   	int pathlen;
> +	struct rpc_t *rpc_pkt = rpc_req_prep();
>   
>   	pathlen = strlen(path);
>   
> -	p = &(data[0]);
> +	p = rpc_pkt->u.call.data;
>   	p = rpc_add_credentials(p);
>   
>   	*p++ = htonl(pathlen);
> @@ -267,9 +269,9 @@ static void nfs_mount_req(char *path)
>   	memcpy(p, path, pathlen);
>   	p += (pathlen + 3) / 4;
>   
> -	len = (uint32_t *)p - (uint32_t *)&(data[0]);
> +	len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>   
> -	rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, data, len);
> +	rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, rpc_pkt, len);
>   }
>   
>   /**************************************************************************
> @@ -277,20 +279,20 @@ NFS_UMOUNTALL - Unmount all our NFS Filesystems on the Server
>   **************************************************************************/
>   static void nfs_umountall_req(void)
>   {
> -	uint32_t data[1024];
>   	uint32_t *p;
>   	int len;
> +	struct rpc_t *rpc_pkt = rpc_req_prep();
>   
>   	if ((nfs_server_mount_port == -1) || (!fs_mounted))
>   		/* Nothing mounted, nothing to umount */
>   		return;
>   
> -	p = &(data[0]);
> +	p = rpc_pkt->u.call.data;
>   	p = rpc_add_credentials(p);
>   
> -	len = (uint32_t *)p - (uint32_t *)&(data[0]);
> +	len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>   
> -	rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, data, len);
> +	rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, rpc_pkt, len);
>   }
>   
>   /***************************************************************************
> @@ -302,11 +304,11 @@ static void nfs_umountall_req(void)
>    **************************************************************************/
>   static void nfs_readlink_req(void)
>   {
> -	uint32_t data[1024];
>   	uint32_t *p;
>   	int len;
> +	struct rpc_t *rpc_pkt = rpc_req_prep();
>   
> -	p = &(data[0]);
> +	p = rpc_pkt->u.call.data;
>   	p = rpc_add_credentials(p);
>   
>   	if (supported_nfs_versions & NFSV2_FLAG) {
> @@ -318,9 +320,9 @@ static void nfs_readlink_req(void)
>   		p += (filefh3_length / 4);
>   	}
>   
> -	len = (uint32_t *)p - (uint32_t *)&(data[0]);
> +	len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>   
> -	rpc_req(PROG_NFS, NFS_READLINK, data, len);
> +	rpc_req(PROG_NFS, NFS_READLINK, rpc_pkt, len);
>   }
>   
>   /**************************************************************************
> @@ -328,14 +330,14 @@ NFS_LOOKUP - Lookup Pathname
>   **************************************************************************/
>   static void nfs_lookup_req(char *fname)
>   {
> -	uint32_t data[1024];
>   	uint32_t *p;
>   	int len;
>   	int fnamelen;
> +	struct rpc_t *rpc_pkt = rpc_req_prep();
>   
>   	fnamelen = strlen(fname);
>   
> -	p = &(data[0]);
> +	p = rpc_pkt->u.call.data;
>   	p = rpc_add_credentials(p);
>   
>   	if (supported_nfs_versions & NFSV2_FLAG) {
> @@ -347,9 +349,9 @@ static void nfs_lookup_req(char *fname)
>   		memcpy(p, fname, fnamelen);
>   		p += (fnamelen + 3) / 4;
>   
> -		len = (uint32_t *)p - (uint32_t *)&(data[0]);
> +		len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>   
> -		rpc_req(PROG_NFS, NFS_LOOKUP, data, len);
> +		rpc_req(PROG_NFS, NFS_LOOKUP, rpc_pkt, len);
>   	} else {  /* NFSV3_FLAG */
>   		*p++ = htonl(NFS_FHSIZE);	/* Dir handle length */
>   		memcpy(p, dirfh, NFS_FHSIZE);
> @@ -360,9 +362,9 @@ static void nfs_lookup_req(char *fname)
>   		memcpy(p, fname, fnamelen);
>   		p += (fnamelen + 3) / 4;
>   
> -		len = (uint32_t *)p - (uint32_t *)&(data[0]);
> +		len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>   
> -		rpc_req(PROG_NFS, NFS3PROC_LOOKUP, data, len);
> +		rpc_req(PROG_NFS, NFS3PROC_LOOKUP, rpc_pkt, len);
>   	}
>   }
>   
> @@ -371,11 +373,11 @@ NFS_READ - Read File on NFS Server
>   **************************************************************************/
>   static void nfs_read_req(int offset, int readlen)
>   {
> -	uint32_t data[1024];
>   	uint32_t *p;
>   	int len;
> +	struct rpc_t *rpc_pkt = rpc_req_prep();
>   
> -	p = &(data[0]);
> +	p = rpc_pkt->u.call.data;
>   	p = rpc_add_credentials(p);
>   
>   	if (supported_nfs_versions & NFSV2_FLAG) {
> @@ -394,9 +396,9 @@ static void nfs_read_req(int offset, int readlen)
>   		*p++ = 0;
>   	}
>   
> -	len = (uint32_t *)p - (uint32_t *)&(data[0]);
> +	len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>   
> -	rpc_req(PROG_NFS, NFS_READ, data, len);
> +	rpc_req(PROG_NFS, NFS_READ, rpc_pkt, len);
>   }
>   
>   /**************************************************************************

[U-Boot] [PATCH 8/9] net: nfs: Use the tx buffer to construct rpc msgs

[Joe Hershberger]

On Tue, Sep 6, 2016 at 8:09 AM, Guillaume Gardet
<guillaume.gardet@free.fr> wrote:
> Hi Joe,
>
> I tested 2016.09-rc2 on a beagleboard xM and NFS does not work anymore!
> When I call the nfs command, I get a "data abort" error with a reboot of the
> board!

Yikes!

> A git bisect point to this patch: "net: nfs: Use the tx buffer to construct
> rpc msgs"
>
> Could you have a look and fix it for the release, please? At least revert it
> for now.

In your testing does reverting just this patch fix things for you? If
so, I can send a revert of just that for the short term.

Thanks,
-Joe

>
> Guillaume
>
>
>
>
> Le 15/08/2016 ? 22:03, Joe Hershberger a ?crit :
>>
>> Instead of always allocating a huge temporary buffer on the stack and
>> then memcpy()ing the result into the transmit buffer, simply figure out
>> where in the transmit buffer the bytes will belong and write them there
>> directly as each message is built.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>> ---
>>
>>   net/nfs.c | 88
>> ++++++++++++++++++++++++++++++++-------------------------------
>>   1 file changed, 45 insertions(+), 43 deletions(-)
>>
>> diff --git a/net/nfs.c b/net/nfs.c
>> index 31047c2..3fb253b 100644
>> --- a/net/nfs.c
>> +++ b/net/nfs.c
>> @@ -183,41 +183,41 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
>>
>> /**************************************************************************
>>   RPC_LOOKUP - Lookup RPC Port numbers
>>
>> **************************************************************************/
>> -static void rpc_req(int rpc_prog, int rpc_proc, uint32_t *data, int
>> datalen)
>> +static struct rpc_t *rpc_req_prep(void)
>> +{
>> +       return (struct rpc_t *)(net_tx_packet + net_eth_hdr_size() +
>> +                               IP_UDP_HDR_SIZE);
>> +}
>> +
>> +static void rpc_req(int rpc_prog, int rpc_proc, struct rpc_t *rpc_pkt,
>> +                   int datalen)
>>   {
>> -       struct rpc_t rpc_pkt;
>>         unsigned long id;
>> -       uint32_t *p;
>>         int pktlen;
>>         int sport;
>>         id = ++rpc_id;
>> -       rpc_pkt.u.call.id = htonl(id);
>> -       rpc_pkt.u.call.type = htonl(MSG_CALL);
>> -       rpc_pkt.u.call.rpcvers = htonl(2);      /* use RPC version 2 */
>> -       rpc_pkt.u.call.prog = htonl(rpc_prog);
>> +       rpc_pkt->u.call.id = htonl(id);
>> +       rpc_pkt->u.call.type = htonl(MSG_CALL);
>> +       rpc_pkt->u.call.rpcvers = htonl(2);     /* use RPC version 2 */
>> +       rpc_pkt->u.call.prog = htonl(rpc_prog);
>>         switch (rpc_prog) {
>>         case PROG_NFS:
>>                 if (supported_nfs_versions & NFSV2_FLAG)
>> -                       rpc_pkt.u.call.vers = htonl(2); /* NFS v2 */
>> +                       rpc_pkt->u.call.vers = htonl(2);        /* NFS v2
>> */
>>                 else /* NFSV3_FLAG */
>> -                       rpc_pkt.u.call.vers = htonl(3); /* NFS v3 */
>> +                       rpc_pkt->u.call.vers = htonl(3);        /* NFS v3
>> */
>>                 break;
>>         case PROG_PORTMAP:
>>         case PROG_MOUNT:
>>         default:
>> -               rpc_pkt.u.call.vers = htonl(2); /* portmapper is version 2
>> */
>> +               /* portmapper is version 2 */
>> +               rpc_pkt->u.call.vers = htonl(2);
>>         }
>> -       rpc_pkt.u.call.proc = htonl(rpc_proc);
>> -       p = (uint32_t *)&(rpc_pkt.u.call.data);
>> -
>> -       if (datalen)
>> -               memcpy((char *)p, (char *)data, datalen*sizeof(uint32_t));
>> -
>> -       pktlen = (char *)p + datalen * sizeof(uint32_t) - (char
>> *)&rpc_pkt;
>> +       rpc_pkt->u.call.proc = htonl(rpc_proc);
>>   -     memcpy((char *)net_tx_packet + net_eth_hdr_size() +
>> IP_UDP_HDR_SIZE,
>> -              &rpc_pkt.u.data[0], pktlen);
>> +       pktlen = ((char *)&rpc_pkt->u.call.data - (char *)&rpc_pkt) +
>> +               datalen * sizeof(uint32_t);
>>         if (rpc_prog == PROG_PORTMAP)
>>                 sport = SUNRPC_PORT;
>> @@ -235,15 +235,17 @@ RPC_LOOKUP - Lookup RPC Port numbers
>>
>> **************************************************************************/
>>   static void rpc_lookup_req(int prog, int ver)
>>   {
>> -       uint32_t data[16];
>> +       uint32_t *data;
>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>   +     data = rpc_pkt->u.call.data;
>>         data[0] = 0; data[1] = 0;       /* auth credential */
>>         data[2] = 0; data[3] = 0;       /* auth verifier */
>>         data[4] = htonl(prog);
>>         data[5] = htonl(ver);
>>         data[6] = htonl(17);    /* IP_UDP */
>>         data[7] = 0;
>> -       rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, data, 8);
>> +       rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, rpc_pkt, 8);
>>   }
>>
>> /**************************************************************************
>> @@ -251,14 +253,14 @@ NFS_MOUNT - Mount an NFS Filesystem
>>
>> **************************************************************************/
>>   static void nfs_mount_req(char *path)
>>   {
>> -       uint32_t data[1024];
>>         uint32_t *p;
>>         int len;
>>         int pathlen;
>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>         pathlen = strlen(path);
>>   -     p = &(data[0]);
>> +       p = rpc_pkt->u.call.data;
>>         p = rpc_add_credentials(p);
>>         *p++ = htonl(pathlen);
>> @@ -267,9 +269,9 @@ static void nfs_mount_req(char *path)
>>         memcpy(p, path, pathlen);
>>         p += (pathlen + 3) / 4;
>>   -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>   -     rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, data, len);
>> +       rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, rpc_pkt, len);
>>   }
>>
>> /**************************************************************************
>> @@ -277,20 +279,20 @@ NFS_UMOUNTALL - Unmount all our NFS Filesystems on
>> the Server
>>
>> **************************************************************************/
>>   static void nfs_umountall_req(void)
>>   {
>> -       uint32_t data[1024];
>>         uint32_t *p;
>>         int len;
>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>         if ((nfs_server_mount_port == -1) || (!fs_mounted))
>>                 /* Nothing mounted, nothing to umount */
>>                 return;
>>   -     p = &(data[0]);
>> +       p = rpc_pkt->u.call.data;
>>         p = rpc_add_credentials(p);
>>   -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>   -     rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, data, len);
>> +       rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, rpc_pkt, len);
>>   }
>>
>> /***************************************************************************
>> @@ -302,11 +304,11 @@ static void nfs_umountall_req(void)
>>
>> **************************************************************************/
>>   static void nfs_readlink_req(void)
>>   {
>> -       uint32_t data[1024];
>>         uint32_t *p;
>>         int len;
>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>   -     p = &(data[0]);
>> +       p = rpc_pkt->u.call.data;
>>         p = rpc_add_credentials(p);
>>         if (supported_nfs_versions & NFSV2_FLAG) {
>> @@ -318,9 +320,9 @@ static void nfs_readlink_req(void)
>>                 p += (filefh3_length / 4);
>>         }
>>   -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>   -     rpc_req(PROG_NFS, NFS_READLINK, data, len);
>> +       rpc_req(PROG_NFS, NFS_READLINK, rpc_pkt, len);
>>   }
>>
>> /**************************************************************************
>> @@ -328,14 +330,14 @@ NFS_LOOKUP - Lookup Pathname
>>
>> **************************************************************************/
>>   static void nfs_lookup_req(char *fname)
>>   {
>> -       uint32_t data[1024];
>>         uint32_t *p;
>>         int len;
>>         int fnamelen;
>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>         fnamelen = strlen(fname);
>>   -     p = &(data[0]);
>> +       p = rpc_pkt->u.call.data;
>>         p = rpc_add_credentials(p);
>>         if (supported_nfs_versions & NFSV2_FLAG) {
>> @@ -347,9 +349,9 @@ static void nfs_lookup_req(char *fname)
>>                 memcpy(p, fname, fnamelen);
>>                 p += (fnamelen + 3) / 4;
>>   -             len = (uint32_t *)p - (uint32_t *)&(data[0]);
>> +               len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>   -             rpc_req(PROG_NFS, NFS_LOOKUP, data, len);
>> +               rpc_req(PROG_NFS, NFS_LOOKUP, rpc_pkt, len);
>>         } else {  /* NFSV3_FLAG */
>>                 *p++ = htonl(NFS_FHSIZE);       /* Dir handle length */
>>                 memcpy(p, dirfh, NFS_FHSIZE);
>> @@ -360,9 +362,9 @@ static void nfs_lookup_req(char *fname)
>>                 memcpy(p, fname, fnamelen);
>>                 p += (fnamelen + 3) / 4;
>>   -             len = (uint32_t *)p - (uint32_t *)&(data[0]);
>> +               len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>   -             rpc_req(PROG_NFS, NFS3PROC_LOOKUP, data, len);
>> +               rpc_req(PROG_NFS, NFS3PROC_LOOKUP, rpc_pkt, len);
>>         }
>>   }
>>   @@ -371,11 +373,11 @@ NFS_READ - Read File on NFS Server
>>
>> **************************************************************************/
>>   static void nfs_read_req(int offset, int readlen)
>>   {
>> -       uint32_t data[1024];
>>         uint32_t *p;
>>         int len;
>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>   -     p = &(data[0]);
>> +       p = rpc_pkt->u.call.data;
>>         p = rpc_add_credentials(p);
>>         if (supported_nfs_versions & NFSV2_FLAG) {
>> @@ -394,9 +396,9 @@ static void nfs_read_req(int offset, int readlen)
>>                 *p++ = 0;
>>         }
>>   -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>   -     rpc_req(PROG_NFS, NFS_READ, data, len);
>> +       rpc_req(PROG_NFS, NFS_READ, rpc_pkt, len);
>>   }
>>
>> /**************************************************************************
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 8/9] net: nfs: Use the tx buffer to construct rpc msgs

[Guillaume Gardet]

Le 06/09/2016 ? 21:09, Joe Hershberger a ?crit :
> On Tue, Sep 6, 2016 at 8:09 AM, Guillaume Gardet
> <guillaume.gardet@free.fr> wrote:
>> Hi Joe,
>>
>> I tested 2016.09-rc2 on a beagleboard xM and NFS does not work anymore!
>> When I call the nfs command, I get a "data abort" error with a reboot of the
>> board!
> Yikes!
>
>> A git bisect point to this patch: "net: nfs: Use the tx buffer to construct
>> rpc msgs"
>>
>> Could you have a look and fix it for the release, please? At least revert it
>> for now.
> In your testing does reverting just this patch fix things for you? If
> so, I can send a revert of just that for the short term.

If I revert this patch only, data abort disappear but NFS is not working fine since when I download a boot.scr file, I get the folowing error while sourcing it:
     ## Executing script at 80200000
     Bad data crc

I will try to find the problem.


Guillaume

>
> Thanks,
> -Joe
>
>> Guillaume
>>
>>
>>
>>
>> Le 15/08/2016 ? 22:03, Joe Hershberger a ?crit :
>>> Instead of always allocating a huge temporary buffer on the stack and
>>> then memcpy()ing the result into the transmit buffer, simply figure out
>>> where in the transmit buffer the bytes will belong and write them there
>>> directly as each message is built.
>>>
>>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>>> ---
>>>
>>>    net/nfs.c | 88
>>> ++++++++++++++++++++++++++++++++-------------------------------
>>>    1 file changed, 45 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/net/nfs.c b/net/nfs.c
>>> index 31047c2..3fb253b 100644
>>> --- a/net/nfs.c
>>> +++ b/net/nfs.c
>>> @@ -183,41 +183,41 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
>>>
>>> /**************************************************************************
>>>    RPC_LOOKUP - Lookup RPC Port numbers
>>>
>>> **************************************************************************/
>>> -static void rpc_req(int rpc_prog, int rpc_proc, uint32_t *data, int
>>> datalen)
>>> +static struct rpc_t *rpc_req_prep(void)
>>> +{
>>> +       return (struct rpc_t *)(net_tx_packet + net_eth_hdr_size() +
>>> +                               IP_UDP_HDR_SIZE);
>>> +}
>>> +
>>> +static void rpc_req(int rpc_prog, int rpc_proc, struct rpc_t *rpc_pkt,
>>> +                   int datalen)
>>>    {
>>> -       struct rpc_t rpc_pkt;
>>>          unsigned long id;
>>> -       uint32_t *p;
>>>          int pktlen;
>>>          int sport;
>>>          id = ++rpc_id;
>>> -       rpc_pkt.u.call.id = htonl(id);
>>> -       rpc_pkt.u.call.type = htonl(MSG_CALL);
>>> -       rpc_pkt.u.call.rpcvers = htonl(2);      /* use RPC version 2 */
>>> -       rpc_pkt.u.call.prog = htonl(rpc_prog);
>>> +       rpc_pkt->u.call.id = htonl(id);
>>> +       rpc_pkt->u.call.type = htonl(MSG_CALL);
>>> +       rpc_pkt->u.call.rpcvers = htonl(2);     /* use RPC version 2 */
>>> +       rpc_pkt->u.call.prog = htonl(rpc_prog);
>>>          switch (rpc_prog) {
>>>          case PROG_NFS:
>>>                  if (supported_nfs_versions & NFSV2_FLAG)
>>> -                       rpc_pkt.u.call.vers = htonl(2); /* NFS v2 */
>>> +                       rpc_pkt->u.call.vers = htonl(2);        /* NFS v2
>>> */
>>>                  else /* NFSV3_FLAG */
>>> -                       rpc_pkt.u.call.vers = htonl(3); /* NFS v3 */
>>> +                       rpc_pkt->u.call.vers = htonl(3);        /* NFS v3
>>> */
>>>                  break;
>>>          case PROG_PORTMAP:
>>>          case PROG_MOUNT:
>>>          default:
>>> -               rpc_pkt.u.call.vers = htonl(2); /* portmapper is version 2
>>> */
>>> +               /* portmapper is version 2 */
>>> +               rpc_pkt->u.call.vers = htonl(2);
>>>          }
>>> -       rpc_pkt.u.call.proc = htonl(rpc_proc);
>>> -       p = (uint32_t *)&(rpc_pkt.u.call.data);
>>> -
>>> -       if (datalen)
>>> -               memcpy((char *)p, (char *)data, datalen*sizeof(uint32_t));
>>> -
>>> -       pktlen = (char *)p + datalen * sizeof(uint32_t) - (char
>>> *)&rpc_pkt;
>>> +       rpc_pkt->u.call.proc = htonl(rpc_proc);
>>>    -     memcpy((char *)net_tx_packet + net_eth_hdr_size() +
>>> IP_UDP_HDR_SIZE,
>>> -              &rpc_pkt.u.data[0], pktlen);
>>> +       pktlen = ((char *)&rpc_pkt->u.call.data - (char *)&rpc_pkt) +
>>> +               datalen * sizeof(uint32_t);
>>>          if (rpc_prog == PROG_PORTMAP)
>>>                  sport = SUNRPC_PORT;
>>> @@ -235,15 +235,17 @@ RPC_LOOKUP - Lookup RPC Port numbers
>>>
>>> **************************************************************************/
>>>    static void rpc_lookup_req(int prog, int ver)
>>>    {
>>> -       uint32_t data[16];
>>> +       uint32_t *data;
>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>    +     data = rpc_pkt->u.call.data;
>>>          data[0] = 0; data[1] = 0;       /* auth credential */
>>>          data[2] = 0; data[3] = 0;       /* auth verifier */
>>>          data[4] = htonl(prog);
>>>          data[5] = htonl(ver);
>>>          data[6] = htonl(17);    /* IP_UDP */
>>>          data[7] = 0;
>>> -       rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, data, 8);
>>> +       rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, rpc_pkt, 8);
>>>    }
>>>
>>> /**************************************************************************
>>> @@ -251,14 +253,14 @@ NFS_MOUNT - Mount an NFS Filesystem
>>>
>>> **************************************************************************/
>>>    static void nfs_mount_req(char *path)
>>>    {
>>> -       uint32_t data[1024];
>>>          uint32_t *p;
>>>          int len;
>>>          int pathlen;
>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>          pathlen = strlen(path);
>>>    -     p = &(data[0]);
>>> +       p = rpc_pkt->u.call.data;
>>>          p = rpc_add_credentials(p);
>>>          *p++ = htonl(pathlen);
>>> @@ -267,9 +269,9 @@ static void nfs_mount_req(char *path)
>>>          memcpy(p, path, pathlen);
>>>          p += (pathlen + 3) / 4;
>>>    -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>    -     rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, data, len);
>>> +       rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, rpc_pkt, len);
>>>    }
>>>
>>> /**************************************************************************
>>> @@ -277,20 +279,20 @@ NFS_UMOUNTALL - Unmount all our NFS Filesystems on
>>> the Server
>>>
>>> **************************************************************************/
>>>    static void nfs_umountall_req(void)
>>>    {
>>> -       uint32_t data[1024];
>>>          uint32_t *p;
>>>          int len;
>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>          if ((nfs_server_mount_port == -1) || (!fs_mounted))
>>>                  /* Nothing mounted, nothing to umount */
>>>                  return;
>>>    -     p = &(data[0]);
>>> +       p = rpc_pkt->u.call.data;
>>>          p = rpc_add_credentials(p);
>>>    -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>    -     rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, data, len);
>>> +       rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, rpc_pkt, len);
>>>    }
>>>
>>> /***************************************************************************
>>> @@ -302,11 +304,11 @@ static void nfs_umountall_req(void)
>>>
>>> **************************************************************************/
>>>    static void nfs_readlink_req(void)
>>>    {
>>> -       uint32_t data[1024];
>>>          uint32_t *p;
>>>          int len;
>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>    -     p = &(data[0]);
>>> +       p = rpc_pkt->u.call.data;
>>>          p = rpc_add_credentials(p);
>>>          if (supported_nfs_versions & NFSV2_FLAG) {
>>> @@ -318,9 +320,9 @@ static void nfs_readlink_req(void)
>>>                  p += (filefh3_length / 4);
>>>          }
>>>    -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>    -     rpc_req(PROG_NFS, NFS_READLINK, data, len);
>>> +       rpc_req(PROG_NFS, NFS_READLINK, rpc_pkt, len);
>>>    }
>>>
>>> /**************************************************************************
>>> @@ -328,14 +330,14 @@ NFS_LOOKUP - Lookup Pathname
>>>
>>> **************************************************************************/
>>>    static void nfs_lookup_req(char *fname)
>>>    {
>>> -       uint32_t data[1024];
>>>          uint32_t *p;
>>>          int len;
>>>          int fnamelen;
>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>          fnamelen = strlen(fname);
>>>    -     p = &(data[0]);
>>> +       p = rpc_pkt->u.call.data;
>>>          p = rpc_add_credentials(p);
>>>          if (supported_nfs_versions & NFSV2_FLAG) {
>>> @@ -347,9 +349,9 @@ static void nfs_lookup_req(char *fname)
>>>                  memcpy(p, fname, fnamelen);
>>>                  p += (fnamelen + 3) / 4;
>>>    -             len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>> +               len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>    -             rpc_req(PROG_NFS, NFS_LOOKUP, data, len);
>>> +               rpc_req(PROG_NFS, NFS_LOOKUP, rpc_pkt, len);
>>>          } else {  /* NFSV3_FLAG */
>>>                  *p++ = htonl(NFS_FHSIZE);       /* Dir handle length */
>>>                  memcpy(p, dirfh, NFS_FHSIZE);
>>> @@ -360,9 +362,9 @@ static void nfs_lookup_req(char *fname)
>>>                  memcpy(p, fname, fnamelen);
>>>                  p += (fnamelen + 3) / 4;
>>>    -             len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>> +               len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>    -             rpc_req(PROG_NFS, NFS3PROC_LOOKUP, data, len);
>>> +               rpc_req(PROG_NFS, NFS3PROC_LOOKUP, rpc_pkt, len);
>>>          }
>>>    }
>>>    @@ -371,11 +373,11 @@ NFS_READ - Read File on NFS Server
>>>
>>> **************************************************************************/
>>>    static void nfs_read_req(int offset, int readlen)
>>>    {
>>> -       uint32_t data[1024];
>>>          uint32_t *p;
>>>          int len;
>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>    -     p = &(data[0]);
>>> +       p = rpc_pkt->u.call.data;
>>>          p = rpc_add_credentials(p);
>>>          if (supported_nfs_versions & NFSV2_FLAG) {
>>> @@ -394,9 +396,9 @@ static void nfs_read_req(int offset, int readlen)
>>>                  *p++ = 0;
>>>          }
>>>    -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>    -     rpc_req(PROG_NFS, NFS_READ, data, len);
>>> +       rpc_req(PROG_NFS, NFS_READ, rpc_pkt, len);
>>>    }
>>>
>>> /**************************************************************************
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 8/9] net: nfs: Use the tx buffer to construct rpc msgs

[Guillaume Gardet]

Le 07/09/2016 ? 09:33, Guillaume Gardet a ?crit :
>
>
> Le 06/09/2016 ? 21:09, Joe Hershberger a ?crit :
>> On Tue, Sep 6, 2016 at 8:09 AM, Guillaume Gardet
>> <guillaume.gardet@free.fr> wrote:
>>> Hi Joe,
>>>
>>> I tested 2016.09-rc2 on a beagleboard xM and NFS does not work anymore!
>>> When I call the nfs command, I get a "data abort" error with a reboot of the
>>> board!
>> Yikes!
>>
>>> A git bisect point to this patch: "net: nfs: Use the tx buffer to construct
>>> rpc msgs"
>>>
>>> Could you have a look and fix it for the release, please? At least revert it
>>> for now.
>> In your testing does reverting just this patch fix things for you? If
>> so, I can send a revert of just that for the short term.
>
> If I revert this patch only, data abort disappear but NFS is not working fine since when I download a boot.scr file, I get the folowing error while sourcing it:
>     ## Executing script at 80200000
>     Bad data crc
>
> I will try to find the problem.

Found. You also need to revert: commit 6279b49e6c2fdaf8665355d1777bc90cd41fcf90: "net: nfs: Correct the reply data buffer size"

Reverting those 2 commits makes the NFS working again.


Guillaume

>
>
> Guillaume
>
>>
>> Thanks,
>> -Joe
>>
>>> Guillaume
>>>
>>>
>>>
>>>
>>> Le 15/08/2016 ? 22:03, Joe Hershberger a ?crit :
>>>> Instead of always allocating a huge temporary buffer on the stack and
>>>> then memcpy()ing the result into the transmit buffer, simply figure out
>>>> where in the transmit buffer the bytes will belong and write them there
>>>> directly as each message is built.
>>>>
>>>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>>>> ---
>>>>
>>>>    net/nfs.c | 88
>>>> ++++++++++++++++++++++++++++++++-------------------------------
>>>>    1 file changed, 45 insertions(+), 43 deletions(-)
>>>>
>>>> diff --git a/net/nfs.c b/net/nfs.c
>>>> index 31047c2..3fb253b 100644
>>>> --- a/net/nfs.c
>>>> +++ b/net/nfs.c
>>>> @@ -183,41 +183,41 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
>>>>
>>>> /**************************************************************************
>>>>    RPC_LOOKUP - Lookup RPC Port numbers
>>>>
>>>> **************************************************************************/
>>>> -static void rpc_req(int rpc_prog, int rpc_proc, uint32_t *data, int
>>>> datalen)
>>>> +static struct rpc_t *rpc_req_prep(void)
>>>> +{
>>>> +       return (struct rpc_t *)(net_tx_packet + net_eth_hdr_size() +
>>>> +                               IP_UDP_HDR_SIZE);
>>>> +}
>>>> +
>>>> +static void rpc_req(int rpc_prog, int rpc_proc, struct rpc_t *rpc_pkt,
>>>> +                   int datalen)
>>>>    {
>>>> -       struct rpc_t rpc_pkt;
>>>>          unsigned long id;
>>>> -       uint32_t *p;
>>>>          int pktlen;
>>>>          int sport;
>>>>          id = ++rpc_id;
>>>> -       rpc_pkt.u.call.id = htonl(id);
>>>> -       rpc_pkt.u.call.type = htonl(MSG_CALL);
>>>> -       rpc_pkt.u.call.rpcvers = htonl(2);      /* use RPC version 2 */
>>>> -       rpc_pkt.u.call.prog = htonl(rpc_prog);
>>>> +       rpc_pkt->u.call.id = htonl(id);
>>>> +       rpc_pkt->u.call.type = htonl(MSG_CALL);
>>>> +       rpc_pkt->u.call.rpcvers = htonl(2);     /* use RPC version 2 */
>>>> +       rpc_pkt->u.call.prog = htonl(rpc_prog);
>>>>          switch (rpc_prog) {
>>>>          case PROG_NFS:
>>>>                  if (supported_nfs_versions & NFSV2_FLAG)
>>>> -                       rpc_pkt.u.call.vers = htonl(2); /* NFS v2 */
>>>> +                       rpc_pkt->u.call.vers = htonl(2);        /* NFS v2
>>>> */
>>>>                  else /* NFSV3_FLAG */
>>>> -                       rpc_pkt.u.call.vers = htonl(3); /* NFS v3 */
>>>> +                       rpc_pkt->u.call.vers = htonl(3);        /* NFS v3
>>>> */
>>>>                  break;
>>>>          case PROG_PORTMAP:
>>>>          case PROG_MOUNT:
>>>>          default:
>>>> -               rpc_pkt.u.call.vers = htonl(2); /* portmapper is version 2
>>>> */
>>>> +               /* portmapper is version 2 */
>>>> +               rpc_pkt->u.call.vers = htonl(2);
>>>>          }
>>>> -       rpc_pkt.u.call.proc = htonl(rpc_proc);
>>>> -       p = (uint32_t *)&(rpc_pkt.u.call.data);
>>>> -
>>>> -       if (datalen)
>>>> -               memcpy((char *)p, (char *)data, datalen*sizeof(uint32_t));
>>>> -
>>>> -       pktlen = (char *)p + datalen * sizeof(uint32_t) - (char
>>>> *)&rpc_pkt;
>>>> +       rpc_pkt->u.call.proc = htonl(rpc_proc);
>>>>    -     memcpy((char *)net_tx_packet + net_eth_hdr_size() +
>>>> IP_UDP_HDR_SIZE,
>>>> -              &rpc_pkt.u.data[0], pktlen);
>>>> +       pktlen = ((char *)&rpc_pkt->u.call.data - (char *)&rpc_pkt) +
>>>> +               datalen * sizeof(uint32_t);
>>>>          if (rpc_prog == PROG_PORTMAP)
>>>>                  sport = SUNRPC_PORT;
>>>> @@ -235,15 +235,17 @@ RPC_LOOKUP - Lookup RPC Port numbers
>>>>
>>>> **************************************************************************/
>>>>    static void rpc_lookup_req(int prog, int ver)
>>>>    {
>>>> -       uint32_t data[16];
>>>> +       uint32_t *data;
>>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>>    +     data = rpc_pkt->u.call.data;
>>>>          data[0] = 0; data[1] = 0;       /* auth credential */
>>>>          data[2] = 0; data[3] = 0;       /* auth verifier */
>>>>          data[4] = htonl(prog);
>>>>          data[5] = htonl(ver);
>>>>          data[6] = htonl(17);    /* IP_UDP */
>>>>          data[7] = 0;
>>>> -       rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, data, 8);
>>>> +       rpc_req(PROG_PORTMAP, PORTMAP_GETPORT, rpc_pkt, 8);
>>>>    }
>>>>
>>>> /**************************************************************************
>>>> @@ -251,14 +253,14 @@ NFS_MOUNT - Mount an NFS Filesystem
>>>>
>>>> **************************************************************************/
>>>>    static void nfs_mount_req(char *path)
>>>>    {
>>>> -       uint32_t data[1024];
>>>>          uint32_t *p;
>>>>          int len;
>>>>          int pathlen;
>>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>>          pathlen = strlen(path);
>>>>    -     p = &(data[0]);
>>>> +       p = rpc_pkt->u.call.data;
>>>>          p = rpc_add_credentials(p);
>>>>          *p++ = htonl(pathlen);
>>>> @@ -267,9 +269,9 @@ static void nfs_mount_req(char *path)
>>>>          memcpy(p, path, pathlen);
>>>>          p += (pathlen + 3) / 4;
>>>>    -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>>    -     rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, data, len);
>>>> +       rpc_req(PROG_MOUNT, MOUNT_ADDENTRY, rpc_pkt, len);
>>>>    }
>>>>
>>>> /**************************************************************************
>>>> @@ -277,20 +279,20 @@ NFS_UMOUNTALL - Unmount all our NFS Filesystems on
>>>> the Server
>>>>
>>>> **************************************************************************/
>>>>    static void nfs_umountall_req(void)
>>>>    {
>>>> -       uint32_t data[1024];
>>>>          uint32_t *p;
>>>>          int len;
>>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>>          if ((nfs_server_mount_port == -1) || (!fs_mounted))
>>>>                  /* Nothing mounted, nothing to umount */
>>>>                  return;
>>>>    -     p = &(data[0]);
>>>> +       p = rpc_pkt->u.call.data;
>>>>          p = rpc_add_credentials(p);
>>>>    -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>>    -     rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, data, len);
>>>> +       rpc_req(PROG_MOUNT, MOUNT_UMOUNTALL, rpc_pkt, len);
>>>>    }
>>>>
>>>> /***************************************************************************
>>>> @@ -302,11 +304,11 @@ static void nfs_umountall_req(void)
>>>>
>>>> **************************************************************************/
>>>>    static void nfs_readlink_req(void)
>>>>    {
>>>> -       uint32_t data[1024];
>>>>          uint32_t *p;
>>>>          int len;
>>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>>    -     p = &(data[0]);
>>>> +       p = rpc_pkt->u.call.data;
>>>>          p = rpc_add_credentials(p);
>>>>          if (supported_nfs_versions & NFSV2_FLAG) {
>>>> @@ -318,9 +320,9 @@ static void nfs_readlink_req(void)
>>>>                  p += (filefh3_length / 4);
>>>>          }
>>>>    -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>>    -     rpc_req(PROG_NFS, NFS_READLINK, data, len);
>>>> +       rpc_req(PROG_NFS, NFS_READLINK, rpc_pkt, len);
>>>>    }
>>>>
>>>> /**************************************************************************
>>>> @@ -328,14 +330,14 @@ NFS_LOOKUP - Lookup Pathname
>>>>
>>>> **************************************************************************/
>>>>    static void nfs_lookup_req(char *fname)
>>>>    {
>>>> -       uint32_t data[1024];
>>>>          uint32_t *p;
>>>>          int len;
>>>>          int fnamelen;
>>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>>          fnamelen = strlen(fname);
>>>>    -     p = &(data[0]);
>>>> +       p = rpc_pkt->u.call.data;
>>>>          p = rpc_add_credentials(p);
>>>>          if (supported_nfs_versions & NFSV2_FLAG) {
>>>> @@ -347,9 +349,9 @@ static void nfs_lookup_req(char *fname)
>>>>                  memcpy(p, fname, fnamelen);
>>>>                  p += (fnamelen + 3) / 4;
>>>>    -             len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>>> +               len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>>    -             rpc_req(PROG_NFS, NFS_LOOKUP, data, len);
>>>> +               rpc_req(PROG_NFS, NFS_LOOKUP, rpc_pkt, len);
>>>>          } else {  /* NFSV3_FLAG */
>>>>                  *p++ = htonl(NFS_FHSIZE);       /* Dir handle length */
>>>>                  memcpy(p, dirfh, NFS_FHSIZE);
>>>> @@ -360,9 +362,9 @@ static void nfs_lookup_req(char *fname)
>>>>                  memcpy(p, fname, fnamelen);
>>>>                  p += (fnamelen + 3) / 4;
>>>>    -             len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>>> +               len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>>    -             rpc_req(PROG_NFS, NFS3PROC_LOOKUP, data, len);
>>>> +               rpc_req(PROG_NFS, NFS3PROC_LOOKUP, rpc_pkt, len);
>>>>          }
>>>>    }
>>>>    @@ -371,11 +373,11 @@ NFS_READ - Read File on NFS Server
>>>>
>>>> **************************************************************************/
>>>>    static void nfs_read_req(int offset, int readlen)
>>>>    {
>>>> -       uint32_t data[1024];
>>>>          uint32_t *p;
>>>>          int len;
>>>> +       struct rpc_t *rpc_pkt = rpc_req_prep();
>>>>    -     p = &(data[0]);
>>>> +       p = rpc_pkt->u.call.data;
>>>>          p = rpc_add_credentials(p);
>>>>          if (supported_nfs_versions & NFSV2_FLAG) {
>>>> @@ -394,9 +396,9 @@ static void nfs_read_req(int offset, int readlen)
>>>>                  *p++ = 0;
>>>>          }
>>>>    -     len = (uint32_t *)p - (uint32_t *)&(data[0]);
>>>> +       len = (uint32_t *)p - (uint32_t *)&(rpc_pkt->u.call.data);
>>>>    -     rpc_req(PROG_NFS, NFS_READ, data, len);
>>>> +       rpc_req(PROG_NFS, NFS_READ, rpc_pkt, len);
>>>>    }
>>>>
>>>> /**************************************************************************
>>>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>