| Message ID | 20240912225320.24178-1-pali@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | lockd: Fix comment about NLMv3 backwards compatibility | expand |
On Fri, 13 Sep 2024, Pali Rohár wrote: > NLMv2 is completely different protocol than NLMv1 and NLMv3, and in > original Sun implementation is used for RPC loopback callbacks from statd > to lockd services. Linux does not use nor does not implement NLMv2. > > Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward > compatible with NLMv1. Fix comment. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > fs/lockd/clntxdr.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c > index a3e97278b997..81ffa521f945 100644 > --- a/fs/lockd/clntxdr.c > +++ b/fs/lockd/clntxdr.c > @@ -3,7 +3,9 @@ > * linux/fs/lockd/clntxdr.c > * > * XDR functions to encode/decode NLM version 3 RPC arguments and results. > - * NLM version 3 is backwards compatible with NLM versions 1 and 2. > + * NLM version 3 is backwards compatible with NLM version 1. > + * NLM version 2 is different protocol used only for RPC loopback callbacks > + * from statd to lockd and is not implemented on Linux. > * > * NLM client-side only. > * Reviewed-by: NeilBrown <neilb@suse.de> Do you have a reference for that info about v2? I hadn't heard of it before. NeilBrown
On Friday 13 September 2024 09:10:45 NeilBrown wrote: > On Fri, 13 Sep 2024, Pali Rohár wrote: > > NLMv2 is completely different protocol than NLMv1 and NLMv3, and in > > original Sun implementation is used for RPC loopback callbacks from statd > > to lockd services. Linux does not use nor does not implement NLMv2. > > > > Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward > > compatible with NLMv1. Fix comment. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > fs/lockd/clntxdr.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c > > index a3e97278b997..81ffa521f945 100644 > > --- a/fs/lockd/clntxdr.c > > +++ b/fs/lockd/clntxdr.c > > @@ -3,7 +3,9 @@ > > * linux/fs/lockd/clntxdr.c > > * > > * XDR functions to encode/decode NLM version 3 RPC arguments and results. > > - * NLM version 3 is backwards compatible with NLM versions 1 and 2. > > + * NLM version 3 is backwards compatible with NLM version 1. > > + * NLM version 2 is different protocol used only for RPC loopback callbacks > > + * from statd to lockd and is not implemented on Linux. > > * > > * NLM client-side only. > > * > > Reviewed-by: NeilBrown <neilb@suse.de> > > Do you have a reference for that info about v2? I hadn't heard of it > before. > > NeilBrown I have just this information in my notes. I guess it should be possible to gather more information about v2 from released Sun/Solaris source code via OpenSolaris / Illumos projects.
On Friday 13 September 2024 01:22:07 Pali Rohár wrote: > On Friday 13 September 2024 09:10:45 NeilBrown wrote: > > On Fri, 13 Sep 2024, Pali Rohár wrote: > > > NLMv2 is completely different protocol than NLMv1 and NLMv3, and in > > > original Sun implementation is used for RPC loopback callbacks from statd > > > to lockd services. Linux does not use nor does not implement NLMv2. > > > > > > Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward > > > compatible with NLMv1. Fix comment. > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > --- > > > fs/lockd/clntxdr.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c > > > index a3e97278b997..81ffa521f945 100644 > > > --- a/fs/lockd/clntxdr.c > > > +++ b/fs/lockd/clntxdr.c > > > @@ -3,7 +3,9 @@ > > > * linux/fs/lockd/clntxdr.c > > > * > > > * XDR functions to encode/decode NLM version 3 RPC arguments and results. > > > - * NLM version 3 is backwards compatible with NLM versions 1 and 2. > > > + * NLM version 3 is backwards compatible with NLM version 1. > > > + * NLM version 2 is different protocol used only for RPC loopback callbacks > > > + * from statd to lockd and is not implemented on Linux. > > > * > > > * NLM client-side only. > > > * > > > > Reviewed-by: NeilBrown <neilb@suse.de> > > > > Do you have a reference for that info about v2? I hadn't heard of it > > before. > > > > NeilBrown > > I have just this information in my notes. I guess it should be possible > to gather more information about v2 from released Sun/Solaris source > code via OpenSolaris / Illumos projects. Just very quickly I found this Illumos XDR file for NLM: https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/rpcsvc/nlm_prot.x And it defines NLMv2 with two procedures numbered 17 and 18, plus there is a comment in file header about v2. So probably the best reference would be the Illumos source code.
On Fri, Sep 13, 2024 at 01:28:20AM +0200, Pali Rohár wrote: > On Friday 13 September 2024 01:22:07 Pali Rohár wrote: > > On Friday 13 September 2024 09:10:45 NeilBrown wrote: > > > On Fri, 13 Sep 2024, Pali Rohár wrote: > > > > NLMv2 is completely different protocol than NLMv1 and NLMv3, and in > > > > original Sun implementation is used for RPC loopback callbacks from statd > > > > to lockd services. Linux does not use nor does not implement NLMv2. > > > > > > > > Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward > > > > compatible with NLMv1. Fix comment. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > --- > > > > fs/lockd/clntxdr.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c > > > > index a3e97278b997..81ffa521f945 100644 > > > > --- a/fs/lockd/clntxdr.c > > > > +++ b/fs/lockd/clntxdr.c > > > > @@ -3,7 +3,9 @@ > > > > * linux/fs/lockd/clntxdr.c > > > > * > > > > * XDR functions to encode/decode NLM version 3 RPC arguments and results. > > > > - * NLM version 3 is backwards compatible with NLM versions 1 and 2. > > > > + * NLM version 3 is backwards compatible with NLM version 1. > > > > + * NLM version 2 is different protocol used only for RPC loopback callbacks > > > > + * from statd to lockd and is not implemented on Linux. > > > > * > > > > * NLM client-side only. > > > > * > > > > > > Reviewed-by: NeilBrown <neilb@suse.de> > > > > > > Do you have a reference for that info about v2? I hadn't heard of it > > > before. > > > > > > NeilBrown > > > > I have just this information in my notes. I guess it should be possible > > to gather more information about v2 from released Sun/Solaris source > > code via OpenSolaris / Illumos projects. > > Just very quickly I found this Illumos XDR file for NLM: > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/rpcsvc/nlm_prot.x > > And it defines NLMv2 with two procedures numbered 17 and 18, plus there > is a comment in file header about v2. > > So probably the best reference would be the Illumos source code. What you see in the Illumos code is not something that is part of the standard NLM protocol, but rather a private upcall protocol between the kernel and user space that is special sauce added by each implementation of NLM/NSM. Also note the way NLMv3 is defined in this file: it defines only a handful of new operations. The other operations are inherited from NLMv1. IMO the comment is accurate and does not warrant a change.
On Thursday 12 September 2024 19:34:02 Chuck Lever wrote: > On Fri, Sep 13, 2024 at 01:28:20AM +0200, Pali Rohár wrote: > > On Friday 13 September 2024 01:22:07 Pali Rohár wrote: > > > On Friday 13 September 2024 09:10:45 NeilBrown wrote: > > > > On Fri, 13 Sep 2024, Pali Rohár wrote: > > > > > NLMv2 is completely different protocol than NLMv1 and NLMv3, and in > > > > > original Sun implementation is used for RPC loopback callbacks from statd > > > > > to lockd services. Linux does not use nor does not implement NLMv2. > > > > > > > > > > Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward > > > > > compatible with NLMv1. Fix comment. > > > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > > --- > > > > > fs/lockd/clntxdr.c | 4 +++- > > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c > > > > > index a3e97278b997..81ffa521f945 100644 > > > > > --- a/fs/lockd/clntxdr.c > > > > > +++ b/fs/lockd/clntxdr.c > > > > > @@ -3,7 +3,9 @@ > > > > > * linux/fs/lockd/clntxdr.c > > > > > * > > > > > * XDR functions to encode/decode NLM version 3 RPC arguments and results. > > > > > - * NLM version 3 is backwards compatible with NLM versions 1 and 2. > > > > > + * NLM version 3 is backwards compatible with NLM version 1. > > > > > + * NLM version 2 is different protocol used only for RPC loopback callbacks > > > > > + * from statd to lockd and is not implemented on Linux. > > > > > * > > > > > * NLM client-side only. > > > > > * > > > > > > > > Reviewed-by: NeilBrown <neilb@suse.de> > > > > > > > > Do you have a reference for that info about v2? I hadn't heard of it > > > > before. > > > > > > > > NeilBrown > > > > > > I have just this information in my notes. I guess it should be possible > > > to gather more information about v2 from released Sun/Solaris source > > > code via OpenSolaris / Illumos projects. > > > > Just very quickly I found this Illumos XDR file for NLM: > > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/rpcsvc/nlm_prot.x > > > > And it defines NLMv2 with two procedures numbered 17 and 18, plus there > > is a comment in file header about v2. > > > > So probably the best reference would be the Illumos source code. > > What you see in the Illumos code is not something that is part > of the standard NLM protocol, but rather a private upcall protocol > between the kernel and user space that is special sauce added > by each implementation of NLM/NSM. Ok. But this applies for v2, no? > Also note the way NLMv3 is defined in this file: it defines only > a handful of new operations. The other operations are inherited > from NLMv1. Yes, v3 is there and is inherited from v1. This is also what I pointed in the comment. That v3 inherits from v1, not v2. In header file of that nlm_prot.x is written: * There are currently 3 versions of the protocol in use. Versions 1 * and 3 are used with NFS version 2. Version 4 is used with NFS * version 3. * * (Note: there is also a version 2, but it defines an orthogonal set of * procedures that the status monitor uses to notify the lock manager of * changes in monitored systems.) Which sounds like version 3 has nothing with version 2. My understanding of that comment is that version 2 contains only those private upcall protocol between kernel and userspace about which you wrote, and therefore version 3 is not backward compatible with version 2. > IMO the comment is accurate and does not warrant a change.
> On Oct 5, 2024, at 12:51 PM, Pali Rohár <pali@kernel.org> wrote: > > On Thursday 12 September 2024 19:34:02 Chuck Lever wrote: >> On Fri, Sep 13, 2024 at 01:28:20AM +0200, Pali Rohár wrote: >>> On Friday 13 September 2024 01:22:07 Pali Rohár wrote: >>>> On Friday 13 September 2024 09:10:45 NeilBrown wrote: >>>>> On Fri, 13 Sep 2024, Pali Rohár wrote: >>>>>> NLMv2 is completely different protocol than NLMv1 and NLMv3, and in >>>>>> original Sun implementation is used for RPC loopback callbacks from statd >>>>>> to lockd services. Linux does not use nor does not implement NLMv2. >>>>>> >>>>>> Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward >>>>>> compatible with NLMv1. Fix comment. >>>>>> >>>>>> Signed-off-by: Pali Rohár <pali@kernel.org> >>>>>> --- >>>>>> fs/lockd/clntxdr.c | 4 +++- >>>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c >>>>>> index a3e97278b997..81ffa521f945 100644 >>>>>> --- a/fs/lockd/clntxdr.c >>>>>> +++ b/fs/lockd/clntxdr.c >>>>>> @@ -3,7 +3,9 @@ >>>>>> * linux/fs/lockd/clntxdr.c >>>>>> * >>>>>> * XDR functions to encode/decode NLM version 3 RPC arguments and results. >>>>>> - * NLM version 3 is backwards compatible with NLM versions 1 and 2. >>>>>> + * NLM version 3 is backwards compatible with NLM version 1. >>>>>> + * NLM version 2 is different protocol used only for RPC loopback callbacks >>>>>> + * from statd to lockd and is not implemented on Linux. >>>>>> * >>>>>> * NLM client-side only. >>>>>> * >>>>> >>>>> Reviewed-by: NeilBrown <neilb@suse.de> >>>>> >>>>> Do you have a reference for that info about v2? I hadn't heard of it >>>>> before. >>>>> >>>>> NeilBrown >>>> >>>> I have just this information in my notes. I guess it should be possible >>>> to gather more information about v2 from released Sun/Solaris source >>>> code via OpenSolaris / Illumos projects. >>> >>> Just very quickly I found this Illumos XDR file for NLM: >>> https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/rpcsvc/nlm_prot.x >>> >>> And it defines NLMv2 with two procedures numbered 17 and 18, plus there >>> is a comment in file header about v2. >>> >>> So probably the best reference would be the Illumos source code. >> >> What you see in the Illumos code is not something that is part >> of the standard NLM protocol, but rather a private upcall protocol >> between the kernel and user space that is special sauce added >> by each implementation of NLM/NSM. > > Ok. But this applies for v2, no? On Linux, those operations are part of the NLMv1/3/4 protocol implementation, so essentially the NLM v2 functionality is a part of all NLM versions on Linux. >> Also note the way NLMv3 is defined in this file: it defines only >> a handful of new operations. The other operations are inherited >> from NLMv1. > > Yes, v3 is there and is inherited from v1. This is also what I pointed > in the comment. That v3 inherits from v1, not v2. Generally this is an abuse of the purpose of the RPC program versioning mechanism. Linux has a very similar upcall mechanism, but uses NLM procedure numbers that are set aside for this purpose instead of abusing a moribund protocol version. > In header file of that nlm_prot.x is written: > > * There are currently 3 versions of the protocol in use. Versions 1 > * and 3 are used with NFS version 2. Version 4 is used with NFS > * version 3. > * > * (Note: there is also a version 2, but it defines an orthogonal set of > * procedures that the status monitor uses to notify the lock manager of > * changes in monitored systems.) > > Which sounds like version 3 has nothing with version 2. > > My understanding of that comment is that version 2 contains only those > private upcall protocol between kernel and userspace about which you > wrote, and therefore version 3 is not backward compatible with version 2. > >> IMO the comment is accurate and does not warrant a change. How about this replacement: * XDR functions to encode/decode NLM version 1 and 3 RPC * arguments and results. NLM version 2 is not specified * by a standard, thus it is not implemented. -- Chuck Lever
On Saturday 05 October 2024 17:52:13 Chuck Lever III wrote: > > On Oct 5, 2024, at 12:51 PM, Pali Rohár <pali@kernel.org> wrote: > > > > On Thursday 12 September 2024 19:34:02 Chuck Lever wrote: > >> On Fri, Sep 13, 2024 at 01:28:20AM +0200, Pali Rohár wrote: > >>> On Friday 13 September 2024 01:22:07 Pali Rohár wrote: > >>>> On Friday 13 September 2024 09:10:45 NeilBrown wrote: > >>>>> On Fri, 13 Sep 2024, Pali Rohár wrote: > >>>>>> NLMv2 is completely different protocol than NLMv1 and NLMv3, and in > >>>>>> original Sun implementation is used for RPC loopback callbacks from statd > >>>>>> to lockd services. Linux does not use nor does not implement NLMv2. > >>>>>> > >>>>>> Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward > >>>>>> compatible with NLMv1. Fix comment. > >>>>>> > >>>>>> Signed-off-by: Pali Rohár <pali@kernel.org> > >>>>>> --- > >>>>>> fs/lockd/clntxdr.c | 4 +++- > >>>>>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c > >>>>>> index a3e97278b997..81ffa521f945 100644 > >>>>>> --- a/fs/lockd/clntxdr.c > >>>>>> +++ b/fs/lockd/clntxdr.c > >>>>>> @@ -3,7 +3,9 @@ > >>>>>> * linux/fs/lockd/clntxdr.c > >>>>>> * > >>>>>> * XDR functions to encode/decode NLM version 3 RPC arguments and results. > >>>>>> - * NLM version 3 is backwards compatible with NLM versions 1 and 2. > >>>>>> + * NLM version 3 is backwards compatible with NLM version 1. > >>>>>> + * NLM version 2 is different protocol used only for RPC loopback callbacks > >>>>>> + * from statd to lockd and is not implemented on Linux. > >>>>>> * > >>>>>> * NLM client-side only. > >>>>>> * > >>>>> > >>>>> Reviewed-by: NeilBrown <neilb@suse.de> > >>>>> > >>>>> Do you have a reference for that info about v2? I hadn't heard of it > >>>>> before. > >>>>> > >>>>> NeilBrown > >>>> > >>>> I have just this information in my notes. I guess it should be possible > >>>> to gather more information about v2 from released Sun/Solaris source > >>>> code via OpenSolaris / Illumos projects. > >>> > >>> Just very quickly I found this Illumos XDR file for NLM: > >>> https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/rpcsvc/nlm_prot.x > >>> > >>> And it defines NLMv2 with two procedures numbered 17 and 18, plus there > >>> is a comment in file header about v2. > >>> > >>> So probably the best reference would be the Illumos source code. > >> > >> What you see in the Illumos code is not something that is part > >> of the standard NLM protocol, but rather a private upcall protocol > >> between the kernel and user space that is special sauce added > >> by each implementation of NLM/NSM. > > > > Ok. But this applies for v2, no? > > On Linux, those operations are part of the NLMv1/3/4 > protocol implementation, so essentially the NLM v2 > functionality is a part of all NLM versions on Linux. > > > >> Also note the way NLMv3 is defined in this file: it defines only > >> a handful of new operations. The other operations are inherited > >> from NLMv1. > > > > Yes, v3 is there and is inherited from v1. This is also what I pointed > > in the comment. That v3 inherits from v1, not v2. > > Generally this is an abuse of the purpose of the RPC > program versioning mechanism. Linux has a very similar > upcall mechanism, but uses NLM procedure numbers that > are set aside for this purpose instead of abusing a > moribund protocol version. I agree that this abuse of the versioning scheme. But it is there and used in this way for a very long time. > > > In header file of that nlm_prot.x is written: > > > > * There are currently 3 versions of the protocol in use. Versions 1 > > * and 3 are used with NFS version 2. Version 4 is used with NFS > > * version 3. > > * > > * (Note: there is also a version 2, but it defines an orthogonal set of > > * procedures that the status monitor uses to notify the lock manager of > > * changes in monitored systems.) > > > > Which sounds like version 3 has nothing with version 2. > > > > My understanding of that comment is that version 2 contains only those > > private upcall protocol between kernel and userspace about which you > > wrote, and therefore version 3 is not backward compatible with version 2. > > > >> IMO the comment is accurate and does not warrant a change. > > How about this replacement: > > * XDR functions to encode/decode NLM version 1 and 3 RPC > * arguments and results. NLM version 2 is not specified > * by a standard, thus it is not implemented. That is perfect! Covers everything.
From: Chuck Lever <chuck.lever@oracle.com> On Fri, 13 Sep 2024 00:53:20 +0200, Pali Rohár wrote: > NLMv2 is completely different protocol than NLMv1 and NLMv3, and in > original Sun implementation is used for RPC loopback callbacks from statd > to lockd services. Linux does not use nor does not implement NLMv2. > > Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward > compatible with NLMv1. Fix comment. > > [...] Applied to nfsd-next for v6.13, thanks! [1/1] lockd: Fix comment about NLMv3 backwards compatibility commit: 6c64686e6bb8b5ec1e3cca4f495ba38341666745 -- Chuck Lever
diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c index a3e97278b997..81ffa521f945 100644 --- a/fs/lockd/clntxdr.c +++ b/fs/lockd/clntxdr.c @@ -3,7 +3,9 @@ * linux/fs/lockd/clntxdr.c * * XDR functions to encode/decode NLM version 3 RPC arguments and results. - * NLM version 3 is backwards compatible with NLM versions 1 and 2. + * NLM version 3 is backwards compatible with NLM version 1. + * NLM version 2 is different protocol used only for RPC loopback callbacks + * from statd to lockd and is not implemented on Linux. * * NLM client-side only. *
NLMv2 is completely different protocol than NLMv1 and NLMv3, and in original Sun implementation is used for RPC loopback callbacks from statd to lockd services. Linux does not use nor does not implement NLMv2. Hence, NLMv3 is not backward compatible with NLMv2. But NLMv3 is backward compatible with NLMv1. Fix comment. Signed-off-by: Pali Rohár <pali@kernel.org> --- fs/lockd/clntxdr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)