[PATCH v2 4/4] net: add NFSv1 support
Christian Gmeiner
christian.gmeiner at gmail.com
Sun Jun 11 17:00:54 CEST 2023
Am So., 11. Juni 2023 um 15:02 Uhr schrieb Peter Robinson
<pbrobinson at gmail.com>:
>
> On Sun, Jun 11, 2023 at 1:54 PM Pali Rohár <pali at kernel.org> wrote:
> >
> > Hello! I must admit that this patch is broken and does not add any NFSv1
> > support. Just look below....
> >
> > 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...)
>
> I was opposed to it, according to a number of resources on the
> internet NFSv1 was never publicly released, I suspect the NFS server
> that "works" with this isn't v1 at all but rather has bugs or
> differences in implementation that need to be worked around that this
> patch just so happens to by accident implement.
The patch is really about adding support for version one of the mount protocol.
Sadly I do not have access to the vxwork 5 source code anymore.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
More information about the U-Boot
mailing list