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

Joe Hershberger joe.hershberger at gmail.com
Tue Sep 6 21:09:02 CEST 2016


On Tue, Sep 6, 2016 at 8:09 AM, Guillaume Gardet
<guillaume.gardet at 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 at 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


More information about the U-Boot mailing list