[PATCH v2 4/4] net: add NFSv1 support
Christian Gmeiner
christian.gmeiner at gmail.com
Sun Jun 11 16:57:07 CEST 2023
Hello
>
> Hello! I must admit that this patch is broken and does not add any NFSv1
> support. Just look below....
>
So .. let see what happend here.
> On Friday 10 March 2023 10:51:55 Christian Gmeiner wrote:
> > From: Thomas RIENOESSL <thomas.rienoessl at bachmann.info>
> >
> > NFSv1 support added by Christian Gmeiner, Thomas Rienoessl,
> > September 27, 2018. As of now, NFSv3 is the default choice.
> > if the server does not support NFSv3, we fall back to
> > versions 2 or 1.
> >
> > Signed-off-by: Thomas RIENOESSL <thomas.rienoessl at bachmann.info>
> > ---
> > net/nfs.c | 42 +++++++++++++++++++++++++++++++++---------
> > 1 file changed, 33 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/nfs.c b/net/nfs.c
> > index 21cae52f35..7a8887ef23 100644
> > --- a/net/nfs.c
> > +++ b/net/nfs.c
> > @@ -26,6 +26,10 @@
> > * NFSv2 is still used by default. But if server does not support NFSv2, then
> > * NFSv3 is used, if available on NFS server. */
> >
> > +/* NOTE 5: NFSv1 support added by Christian Gmeiner, Thomas Rienoessl,
> > + * September 27, 2018. As of now, NFSv3 is the default choice. If the server
> > + * does not support NFSv3, we fall back to versions 2 or 1. */
> > +
> > #include <common.h>
> > #include <command.h>
> > #include <display_options.h>
> > @@ -78,6 +82,7 @@ static char nfs_path_buff[2048];
> >
> > enum nfs_version {
> > NFS_UNKOWN = 0,
> > + NFS_V1 = 1,
> > NFS_V2 = 2,
> > NFS_V3 = 3,
> > };
> > @@ -192,6 +197,7 @@ static void rpc_req(int rpc_prog, int rpc_proc, uint32_t *data, int datalen)
> > switch (rpc_prog) {
> > case PROG_NFS:
> > switch (choosen_nfs_version) {
> > + case NFS_V1:
> > case NFS_V2:
> > rpc_pkt.u.call.vers = htonl(2);
>
> So if NFSv1 is chosen then this code uses NFSv2. This is either rebasing
> problem or just prove that this patch does not add any NFSv1 support.
>
> > break;
> > @@ -205,8 +211,26 @@ static void rpc_req(int rpc_prog, int rpc_proc, uint32_t *data, int datalen)
> > break;
> > }
> > break;
> > - case PROG_PORTMAP:
> > case PROG_MOUNT:
> > + switch (choosen_nfs_version) {
> > + case NFS_V1:
> > + rpc_pkt.u.call.vers = htonl(1);
> > + break;
>
> And later here for NFSv1 we are trying to use Mount Server, which NFSv1
> did not use at all. So this patch really does not have to work with old
> NFSv1 servers.
>
> Instead NFSv1 uses NFSPROC_ROOT RPC call exported by NFS server.
> (See that this RPC call is deprecated in NFSv2 and MNT server is used
> in NFSv2 instead.)
>
> MNTv1 is service used by the NFSv2 and it returns NFSv2 file handles
> (not NFSv1 file handles). MNTv2 is also used by NFSv2 and as addition to
> MNTv1, it adds DIRPATH rpc call. So if NFSv2 does not need to use
> DIRPATH then it is fine to use just MNTv1 in NFSv2.
>
> > +
> > + case NFS_V2:
> > + rpc_pkt.u.call.vers = htonl(2);
> > + break;
> > +
> > + case NFS_V3:
> > + rpc_pkt.u.call.vers = htonl(3);
> > + break;
> > +
> > + case NFS_UNKOWN:
> > + /* nothing to do */
> > + break;
> > + }
> > + break;
> > + case PROG_PORTMAP:
> > default:
> > rpc_pkt.u.call.vers = htonl(2); /* portmapper is version 2 */
> > }
> > @@ -311,7 +335,7 @@ static void nfs_readlink_req(void)
> > p = &(data[0]);
> > p = rpc_add_credentials(p);
> >
> > - if (choosen_nfs_version == NFS_V2) {
> > + if (choosen_nfs_version != NFS_V3) {
> > memcpy(p, filefh, NFS_FHSIZE);
> > p += (NFS_FHSIZE / 4);
> > } else { /* NFS_V3 */
> > @@ -340,7 +364,7 @@ static void nfs_lookup_req(char *fname)
> > p = &(data[0]);
> > p = rpc_add_credentials(p);
> >
> > - if (choosen_nfs_version == NFS_V2) {
> > + if (choosen_nfs_version != NFS_V3) {
> > memcpy(p, dirfh, NFS_FHSIZE);
> > p += (NFS_FHSIZE / 4);
> > *p++ = htonl(fnamelen);
> > @@ -380,7 +404,7 @@ static void nfs_read_req(int offset, int readlen)
> > p = &(data[0]);
> > p = rpc_add_credentials(p);
> >
> > - if (choosen_nfs_version == NFS_V2) {
> > + if (choosen_nfs_version != NFS_V3) {
> > memcpy(p, filefh, NFS_FHSIZE);
> > p += (NFS_FHSIZE / 4);
> > *p++ = htonl(offset);
> > @@ -410,13 +434,13 @@ static void nfs_send(void)
> >
> > switch (nfs_state) {
> > case STATE_PRCLOOKUP_PROG_MOUNT_REQ:
> > - if (choosen_nfs_version == NFS_V2)
> > + if (choosen_nfs_version != NFS_V3)
> > rpc_lookup_req(PROG_MOUNT, 1);
> > else /* NFS_V3 */
> > rpc_lookup_req(PROG_MOUNT, 3);
> > break;
> > case STATE_PRCLOOKUP_PROG_NFS_REQ:
> > - if (choosen_nfs_version == NFS_V2)
> > + if (choosen_nfs_version != NFS_V3)
> > rpc_lookup_req(PROG_NFS, 2);
> > else /* NFS_V3 */
> > rpc_lookup_req(PROG_NFS, 3);
> > @@ -457,7 +481,7 @@ static int rpc_handle_error(struct rpc_t *rpc_pkt)
> > const int min = ntohl(rpc_pkt->u.reply.data[0]);
> > const int max = ntohl(rpc_pkt->u.reply.data[1]);
> >
> > - if (max < NFS_V2 || max > NFS_V3 || min > NFS_V3) {
> > + if (max < NFS_V1 || max > NFS_V3 || min > NFS_V3) {
> > puts("*** ERROR: NFS version not supported");
> > debug(": Requested: V%d, accepted: min V%d - max V%d\n",
> > choosen_nfs_version,
> > @@ -588,7 +612,7 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
> > if (ret)
> > return ret;
> >
> > - if (choosen_nfs_version == NFS_V2) {
> > + if (choosen_nfs_version != NFS_V3) {
> > if (((uchar *)&(rpc_pkt.u.reply.data[0]) - (uchar *)(&rpc_pkt) + NFS_FHSIZE) > len)
> > return -NFS_RPC_DROP;
> > memcpy(filefh, rpc_pkt.u.reply.data + 1, NFS_FHSIZE);
> > @@ -712,7 +736,7 @@ static int nfs_read_reply(uchar *pkt, unsigned len)
> > if (!(nfs_offset % ((NFS_READ_SIZE / 2) * 10)))
> > putc('#');
> >
> > - if (choosen_nfs_version == NFS_V2) {
> > + if (choosen_nfs_version != NFS_V3) {
> > rlen = ntohl(rpc_pkt.u.reply.data[18]);
> > data_ptr = (uchar *)&(rpc_pkt.u.reply.data[19]);
> > } else { /* NFS_V3 */
> > --
> > 2.39.2
> >
>
> And looking at the other changes here, there is really _no_ code which
> adds NFSv1 support.
>
> So what is this patch doing? The only thing which it does is that for
> NFSv1 requests it does NFSv2 calls. On every place is just check that
> choosen_nfs_version is not NFS_V3.
>
> Which just basically duplicates NFSv2 to be used two times.
>
> I would suggest to revisit this patch (who reviewed it at all?) and
> either fix it or revert it. And of course properly test it. (And I
> really curious where you find NFSv1 server because Linux has already
> removed also NFSv2 support from userspace...)
Soo. I had a look at RFC 1094 and this patch adds version one of the
mount protocol.
I am quite unhappy that we got into this state, but the company I
worked for uses the
term NFSv1 for this in all their configuration tools etc.
What would you suggest to improve this situation?
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
More information about the U-Boot
mailing list