Re: [Nfs-ganesha-devel] ACL support

[Sagar M D]

Hi,

Kernel nfs reorders the ACE in ACL and i think it puts more restrictive
ACEs first in the list. But i see NFS Ganesha is not doing it,is reordering
the responsibility of  FSAL ?
Is there any plans to support reordering ?

Thanks,
Sagar.

On Mon, Feb 19, 2018 at 11:43 AM, Sagar M D  wrote:

> Sriram,
>
> Setting ATTR_ACL in supported_attrs helped. Now I could able to get the V4
> ACLs. Thanks!.
>
> Frank,
> Currently we are doing what you are suggesting i.e we are persistently
> saving the in-memory representation of ganesha NFSV4 ACL on disk.
> And I'm not sure whether we are ready to check in our fsal into ganesha
> yet. We will discuss this internally.
>
> Thanks!
>
> On Fri, Feb 16, 2018 at 9:21 PM, Sriram Patil  wrote:
>
>> Thank you for the correction, Frank.
>>
>>
>>
>> Sagar, there are a couple of more things that you have not mentioned yet,
>>
>>
>>
>>1. Have you set ATTR_ACL in supported_attrs field of your FSALs
>>static fsinfo? (check usage of function nfs4_Fattr_Supported to know why
>>this is required)
>>2. You may also want to take a look at ENABLE_RFC_ACL flag. This is
>>not for enabling ACLs but it is used for access checks in
>>fsal_check_access_acl.
>>
>>
>>
>> - Sriram
>>
>>
>>
>> *From: *Frank Filz 
>> *Date: *Friday, February 16, 2018 at 8:19 PM
>> *To: *Sriram Patil , 'Sagar M D' ,
>> 'Supriti Singh' 
>> *Cc: *"nfs-ganesha-devel@lists.sourceforge.net" <
>> nfs-ganesha-devel@lists.sourceforge.net>
>> *Subject: *RE: [Nfs-ganesha-devel] ACL support
>>
>>
>>
>> It isn’t quite true that NFS v4 ACLs are a superset of POSIX ACLs, but
>> that’s another detail.
>>
>>
>>
>> Sriram is right, Ganesha doesn’t support the NFS v3 sideband protocol for
>> POSIX ACLs. At this point Ganesha has the following support for ACLs:
>>
>>
>>
>> FSAL_GLUSTER has a translation from client side NFS v4 ACLs to server
>> side POSIX ACLs. In V2.7 we plan to move this support to the FSAL common
>> code so it is available to more FSALs (and we will hook it up for FSAL_VFS
>> at that point). Note that the conversion is not perfect due to NFS v4 ACLs
>> not actually being a superset of POSIX ACLs.
>>
>>
>>
>> FSAL_GPFS has native support for NFS v4 ACLs.
>>
>>
>>
>> At this time Ganesha is only set up to handle NFS v4 ACLs via the FSAL
>> API. If your file system can support NFS v4 ACLs natively, then all you
>> need to do is provide a mechanism to transfer between Ganesha’s in memory
>> representation of an NFS v4 ACL and your on-disk representation. If your
>> file system can only support POSIX ACLs, then you will need the translation
>> code from FSAL_GLUSTER (or write your own).
>>
>>
>>
>> I’d also like to add my usual plug, if you have an out of tree FSAL, we
>> encourage you to submit your FSAL into the tree. That allows us a better
>> understanding of how Ganesha is being used, and we are less likely to
>> change APIs in a way that breaks your FSAL (or we will change your FSAL
>> with the API change).
>>
>>
>>
>> Frank
>>
>>
>>
>> *From:* Sriram Patil [mailto:srir...@vmware.com]
>> *Sent:* Friday, February 16, 2018 2:51 AM
>> *To:* Sagar M D ; Supriti Singh <
>> supriti.si...@suse.com>
>> *Cc:* nfs-ganesha-devel@lists.sourceforge.net
>> *Subject:* Re: [Nfs-ganesha-devel] ACL support
>>
>>
>>
>> Hi Sagar,
>>
>>
>>
>> I see in your conf file that you are using NFSv4. POSIX acls do not work
>> on NFSv4. NFSv4 acls are a superset of POSIX acls. For using NFSv4 acls you
>> need to use nfs4_getfacl and nfs4_setfacl commands from the client. You can
>> find these commands in nfs4-acl-tools package.
>>
>>
>>
>> - Sriram
>>
>>
>>
>> *From: *Sagar M D 
>> *Date: *Friday, February 16, 2018 at 3:20 PM
>> *To: *Supriti Singh 
>> *Cc: *"nfs-ganesha-devel@lists.sourceforge.net" <
>> nfs-ganesha-devel@lists.sourceforge.net>
>> *Subject: *Re: [Nfs-ganesha-devel] ACL support
>>
>>
>>
>> I quickly checked on VFS FSAL using below EXPORT block. I see same issue
>> on vfs fsal also. Any suggestion here please ?
>>
>>
>>
>> *Operation to request attribute not supported. Failed to instantiate ACL.
>> *
>>
>> EXPORT
>> {
>> Export_Id = 77;
>>
>> # Exported path (mandatory)
>> Path = /home;
>>
>> # Pseudo Path (required for NFS v4)
>> Pseudo = /home;
>>
>> # Required for access (default is None)
>> # Could use CLIENT blocks instead
>> Access_Type = RW;
>> Disable_ACL = FALSE;
>> NFS_Protocols = 4;
>> Squash = no_root_squash;
>>
>> # Exporting FSAL
>> FSAL {
>> Name = VFS;
>> }
>> }
>>
>> Thanks,
>>
>> Sagar.
>>
>>
>>
>>
>>
>> On Fri, Feb 16, 2018 at 2:25 PM, Sagar M D  wrote:
>>
>> Supriti,
>>
>>
>>
>> We are testing our own FSAL.
>>
>> Thanks,
>>
>> Sagar.
>>
>>
>>
>>
>>
>> On Fri, Feb 16, 2018 at 2:15 PM, Supriti Singh 
>> wrote:
>>
>> Hi Sagar,
>>
>> Which FSAL are you using?
>>
>>
>>
>>
>>
>> --
>>
>> Supriti Singh
>>
>> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
>>
>> HRB 21284 (AG Nürn

Re: [Nfs-ganesha-devel] inconsistent '*' spacing

[William Allen Simpson]

On 2/21/18 11:35 AM, Frank Filz wrote:

There's a -n or --no-verify option that will bypass the commit hooks. I suggest 
trying to commit without that first to make sure the only checkpatch 
errors/warnings are for the spacing around * and then commit again with -n to 
bypass checkpatch to actually commit.


Boy oh boy, I wish I'd known about that!!!

So that's what I've done on the nfs41.h formatting.

I've also removed the K&R-style definitions that checkpatch hates.



I suppose now that it seems to complain either way, we can fix them
all to not have the space, and just force the commit and ignore the
checkpatch warnings (when I see ONLY warnings for things I know we
can't/won't fix in gerrithub, I ignore them and merge anyway). I'm not
sure how much of a potential merge headache changing them all would
cause though. I don't think nfsv41.h gets changed all that much so
even if we had to backport something, the potential manual merge wouldn't

be awful.



New -dev cycle, so it's time.


Yes, this is the right time to do it.


[...] > I sympathize with your grumbling about this one... I've chosen
the set of checkpatch checks to enable based on what seems to work
best for our code and keeps us as consistent style as possible. I
realize some folks have preferences opposite some of the checks, but
our code style is now what it is...


Or not, as in this case.

Anyway, I'll try to push a patch for those 2 files by tomorrow.


Ok, thanks


Turned out nfs23.h hadn't been updated into inlines for performance,
so doesn't seem to have these strange errors.  Hopefully nobody else
will want to patch nfs41.h this week.

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: MainNFSD: invert _NO_PORTMAPPER option

[Frank Filz]

> On Wed, 2018-02-21 at 13:40 -0800, Frank Filz wrote:
> > > On 2/21/18 1:59 PM, GerritHub wrote:
> > > > Jeff Layton has uploaded this change for *review*.
> > > >
> > > > View Change 
> > > >
> > > > MainNFSD: invert _NO_PORTMAPPER option
> > > >
> > > > The fact that this is a "negative" option is confusing. Change it
> > > > to a "PORTMAPPER" option, and have it default to ON.
> > > >
> > >
> > > While I vaguely agree with the former in principle, in this day and
> > > age we
> >
> > really
> > > should stop using the name PORTMAPPER.  Replaced by rpcbind a long
> > > time
> >
> > ago,
> > > and shouldn't be shipping with modern systems.
> > >
> > > In ntirpc, PORTMAP is as expected the old version 2 UDP-only call.
> > > We
> >
> > should
> > > kill it.
> > >
> > > We really shouldn't encourage folks to use a UDP system that has
> > > long had known DDoS attacks.
> > >
> > > And we really should be migrating from NFS 2 UDP to NFS 3 TCP, as a
> >
> > minimum
> > > supported version
> > >
> > > Also, we have talked about adding rpcbind itself to Ganesha or ntirpc.
> >
> > Well, the "NO_PORTMAPPER" option really controls rpcbind now...
> >
> > Though actually, I'm not sure Ganesha actually uses any of the
> > extension of rpcbind, it would probably talk to portmap just fine...
> >
> > And we no longer support NFS v2 (we have a couple NFS v2 stubs because
> > at one time in history, the Linux client issued NFS v2 requests even
> > though it was an NFS v3 mount - really just the UMOUNT was the NFS v2
> > version I think
> > - it's all there in the code). That code might indeed be safe to
> > remove, on the other hand, folks with appliance products tend to not
> > like to force particular client versions too much as long as the pain
> > point isn't too bad, and this fragment of V2 support isn't too bad.
> >
> > We could take this opportunity to change the option to RPCBIND...
> >
> 
> Fair enough.
> 
> I actually disagree with the "no udp" statement above too. UDP is great for
> single-shot request protocols like rpcbind, and the NFS client will use it. 
> DDoS is
> a possibility, but who exposes their rpcbind port to the Internet?
> 
> In any case, the real fix to this issue is to move to protocols that don't 
> require
> rpcbind at all. That means NFSv4.0 at a minimum (though obviously v4.1+ would
> be preferred).

The Linux kernel client can use NFS v3 without rpcbind, though not sure you can 
specify the NLM port, I know you can specify the NFS and MOUNT ports.

On the other hand, rpcbind is a nice way to check general reachability of the 
server and what's up (and you can ping services via rpcinfo...).

Frank


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] inconsistent '*' spacing

[Frank Filz]

Thanks for the efforts. In general I like the checks we have enabled. 
Occasionally one creates annoyance and my decision is to live with an 
occasional annoyance. This particular file shouldn't see too much changes and 
now that it's 99% fixed, we can easily move past the one or two annoyances that 
may crop up.

Frank

> -Original Message-
> From: William Allen Simpson [mailto:william.allen.simp...@gmail.com]
> Sent: Wednesday, February 21, 2018 2:29 PM
> To: Frank Filz ; 'NFS Ganesha Developers'  ganesha-de...@lists.sourceforge.net>
> Subject: Re: [Nfs-ganesha-devel] inconsistent '*' spacing
> 
> On 2/21/18 8:28 AM, William Allen Simpson wrote:
> > Anyway, I'll try to push a patch for those 2 files by tomorrow.
> 
> ERROR: "foo * bar" should be "foo *bar"
> #18656: FILE: src/include/nfsv41.h:9900:
> +static inline bool xdr_CB_COMPOUND4res(XDR * xdrs,
> 
> total: 450 errors, 14 warnings, 18658 lines checked
> 
> If those are fixed, we have remaining:
> 
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #4757: FILE: src/include/nfsv41.h:3696:
> +extern COMPOUND4res *nfsproc4_compound_4(COMPOUND4args *, CLIENT
> *);
>   ^
> 
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #4795: FILE: src/include/nfsv41.h:3719:
> +extern CB_COMPOUND4res *cb_compound_1(CB_COMPOUND4args *,
> CLIENT *);
>  ^
> 
> total: 7 errors, 14 warnings, 18659 lines checked
> 
> 
> And no way around many of them, since fixing this:
> 
> WARNING: Avoid multiple line dereference - prefer 'objp-
> >nad_new_entry_cookie.nad_new_entry_cookie_val'
> #13467: FILE: src/include/nfsv41.h:9151:
> +(char **)&objp->nad_new_entry_cookie.
> +nad_new_entry_cookie_val,
> 
> WARNING: Avoid multiple line dereference - prefer 'objp-
> >nad_new_entry_cookie.nad_new_entry_cookie_len'
> #13469: FILE: src/include/nfsv41.h:9153:
> +(u_int *) &objp->nad_new_entry_cookie.
> +nad_new_entry_cookie_len, 1, sizeof(nfs_cookie4),
> 
> changes to a complaint about too long line:
> 
> WARNING: line over 80 characters
> #13466: FILE: src/include/nfsv41.h:9150:
> +(char **)&objp-
> >nad_new_entry_cookie.nad_new_entry_cookie_val,
> 
> WARNING: line over 80 characters
> #13467: FILE: src/include/nfsv41.h:9151:
> +(u_int *)
> +&objp->nad_new_entry_cookie.nad_new_entry_cookie_len,
> 
> So un-indented the whole thing to make more room.
> 
> But even that won't fix:
> 
> WARNING: line over 80 characters
> #9587: FILE: src/include/nfsv41.h:6149:
> +  &objp-
> >open_none_delegation4_u.ond_server_will_signal_avail))
> 
> WARNING: line over 80 characters
> #11239: FILE: src/include/nfsv41.h:7261:
> +
> +&objp-
> >GET_DIR_DELEGATION4res_non_fatal_u.gddrnf_will_signal_deleg_avai
> +l))
> 
> 
> There are irredeemable errors, because checkpatch won't allow K&R C, but the
> #ifdef __STDC__ #else doesn't actually compile these lines:
> 
> ERROR: Bad function definition - int nfs4_program_4_freeresult() should
> probably be int nfs4_program_4_freeresult(void)
> #4773: FILE: src/include/nfsv41.h:3708:
> +extern int nfs4_program_4_freeresult();
> 
> ERROR: Bad function definition - int nfs4_callback_1_freeresult() should
> probably be int nfs4_callback_1_freeresult(void)
> #4812: FILE: src/include/nfsv41.h:3731:
> +extern int nfs4_callback_1_freeresult();
> 
> total: 4 errors, 2 warnings, 18577 lines checked
> 
> Would have been much easier to turn off this pointless check!!!


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: MainNFSD: invert _NO_PORTMAPPER option

[Jeff Layton]

On Wed, 2018-02-21 at 18:24 -0500, William Allen Simpson wrote:
> On 2/21/18 4:51 PM, Jeff Layton wrote:
> > On Wed, 2018-02-21 at 13:40 -0800, Frank Filz wrote:
> > > We could take this opportunity to change the option to RPCBIND...
> > > 
> > 
> > Fair enough.
> > 
> 
> I'd support this.
> 

Cool, I pushed out an updated patchset that is along these lines.

> 
> > I actually disagree with the "no udp" statement above too. UDP is great
> > for single-shot request protocols like rpcbind, and the NFS client will
> > use it. DDoS is a possibility, but who exposes their rpcbind port to the
> > Internet?
> > 
> 
> Unfortunately, millions of websites.  At one time, portmapper was a
> leading method of DDoS.
> 

That's just malpractice. rpcbind really shouldn't be an Internet service
in this day and age.

> Actually, it's *NOT* great.  When Ganesha/ntirpc cannot find something,
> it drops back from TCP to UDP.  And then tries over and over into the
> void.  There's no return signal from UDP.
> 
> When the TCP service isn't available, you get a nice RST flag.  No need
> for all these retry timeouts that UDP requires.
> 

True, but it's a lot less overhead for the cases where it _does_ work.
One roundtrip and that's it.

> UDP turned out to be a security nightmare for NFS.  We all remember the
> IP fragmentation DDoS?
> 
> That's why we tried (circa 1992) to eliminate IP fragmentation in IPv6.
> Steve Deering was all over this.  DNS and NFS were the big culprits,
> and NFS over UDP yields far bigger IP fragment chains than DNS
> 

No question that TCP is generally superior for NFS. You sort of expect
there to be a long-lived association between a NFS client and server
though.

For rpcbind, that's not generally the case. You query it to get the port
and never talk to it again until you need to reconnect a socket (and
often not even then).

> 
> > In any case, the real fix to this issue is to move to protocols that
> > don't require rpcbind at all. That means NFSv4.0 at a minimum (though
> > obviously v4.1+ would be preferred).
> > 
> 
> Ah, you're speaking to my heart.  But we apparently still have a lot
> of UDP downstream, and now FSAL_PROXY.
> 
> When will we ever get away from the sins of our fathers, unto the 7th
> generation?

Yeccchh. I do wonder about the use cases that are driving these
configurations.

The real question is who really requires v3 these days?
-- 
Jeff Layton 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: MainNFSD: invert _NO_PORTMAPPER option

[William Allen Simpson]

On 2/21/18 4:51 PM, Jeff Layton wrote:

On Wed, 2018-02-21 at 13:40 -0800, Frank Filz wrote:

We could take this opportunity to change the option to RPCBIND...



Fair enough.


I'd support this.



I actually disagree with the "no udp" statement above too. UDP is great
for single-shot request protocols like rpcbind, and the NFS client will
use it. DDoS is a possibility, but who exposes their rpcbind port to the
Internet?


Unfortunately, millions of websites.  At one time, portmapper was a
leading method of DDoS.

Actually, it's *NOT* great.  When Ganesha/ntirpc cannot find something,
it drops back from TCP to UDP.  And then tries over and over into the
void.  There's no return signal from UDP.

When the TCP service isn't available, you get a nice RST flag.  No need
for all these retry timeouts that UDP requires.

UDP turned out to be a security nightmare for NFS.  We all remember the
IP fragmentation DDoS?

That's why we tried (circa 1992) to eliminate IP fragmentation in IPv6.
Steve Deering was all over this.  DNS and NFS were the big culprits,
and NFS over UDP yields far bigger IP fragment chains than DNS



In any case, the real fix to this issue is to move to protocols that
don't require rpcbind at all. That means NFSv4.0 at a minimum (though
obviously v4.1+ would be preferred).


Ah, you're speaking to my heart.  But we apparently still have a lot
of UDP downstream, and now FSAL_PROXY.

When will we ever get away from the sins of our fathers, unto the 7th
generation?

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: (nfs41.h) unindent

[GerritHub]

>From :

william.allen.simp...@gmail.com has uploaded this change for review. ( 
https://review.gerrithub.io/400889


Change subject: (nfs41.h) unindent
..

(nfs41.h) unindent

Change-Id: I0c4504ed3cc1c5478dac7dcdc825a1257164880d
Signed-off-by: William Allen Simpson 
---
M src/include/nfsv41.h
1 file changed, 8,413 insertions(+), 8,527 deletions(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/89/400889/1
-- 
To view, visit https://review.gerrithub.io/400889
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c4504ed3cc1c5478dac7dcdc825a1257164880d
Gerrit-Change-Number: 400889
Gerrit-PatchSet: 1
Gerrit-Owner: william.allen.simp...@gmail.com
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] inconsistent '*' spacing

[William Allen Simpson]

On 2/21/18 8:28 AM, William Allen Simpson wrote:

Anyway, I'll try to push a patch for those 2 files by tomorrow.


ERROR: "foo * bar" should be "foo *bar"
#18656: FILE: src/include/nfsv41.h:9900:
+static inline bool xdr_CB_COMPOUND4res(XDR * xdrs,

total: 450 errors, 14 warnings, 18658 lines checked

If those are fixed, we have remaining:

ERROR: need consistent spacing around '*' (ctx:WxV)
#4757: FILE: src/include/nfsv41.h:3696:
+extern COMPOUND4res *nfsproc4_compound_4(COMPOUND4args *, CLIENT *);
 ^

ERROR: need consistent spacing around '*' (ctx:WxV)
#4795: FILE: src/include/nfsv41.h:3719:
+extern CB_COMPOUND4res *cb_compound_1(CB_COMPOUND4args *, CLIENT *);
^

total: 7 errors, 14 warnings, 18659 lines checked


And no way around many of them, since fixing this:

WARNING: Avoid multiple line dereference - prefer 
'objp->nad_new_entry_cookie.nad_new_entry_cookie_val'
#13467: FILE: src/include/nfsv41.h:9151:
+  (char **)&objp->nad_new_entry_cookie.
+  nad_new_entry_cookie_val,

WARNING: Avoid multiple line dereference - prefer 
'objp->nad_new_entry_cookie.nad_new_entry_cookie_len'
#13469: FILE: src/include/nfsv41.h:9153:
+  (u_int *) &objp->nad_new_entry_cookie.
+  nad_new_entry_cookie_len, 1, sizeof(nfs_cookie4),

changes to a complaint about too long line:

WARNING: line over 80 characters
#13466: FILE: src/include/nfsv41.h:9150:
+  (char 
**)&objp->nad_new_entry_cookie.nad_new_entry_cookie_val,

WARNING: line over 80 characters
#13467: FILE: src/include/nfsv41.h:9151:
+  (u_int *) 
&objp->nad_new_entry_cookie.nad_new_entry_cookie_len,

So un-indented the whole thing to make more room.

But even that won't fix:

WARNING: line over 80 characters
#9587: FILE: src/include/nfsv41.h:6149:
+
&objp->open_none_delegation4_u.ond_server_will_signal_avail))

WARNING: line over 80 characters
#11239: FILE: src/include/nfsv41.h:7261:
+
&objp->GET_DIR_DELEGATION4res_non_fatal_u.gddrnf_will_signal_deleg_avail))


There are irredeemable errors, because checkpatch won't allow K&R C,
but the #ifdef __STDC__ #else doesn't actually compile these lines:

ERROR: Bad function definition - int nfs4_program_4_freeresult() should 
probably be int nfs4_program_4_freeresult(void)
#4773: FILE: src/include/nfsv41.h:3708:
+extern int nfs4_program_4_freeresult();

ERROR: Bad function definition - int nfs4_callback_1_freeresult() should 
probably be int nfs4_callback_1_freeresult(void)
#4812: FILE: src/include/nfsv41.h:3731:
+extern int nfs4_callback_1_freeresult();

total: 4 errors, 2 warnings, 18577 lines checked

Would have been much easier to turn off this pointless check!!!

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: MainNFSD: invert _NO_PORTMAPPER option

[Jeff Layton]

On Wed, 2018-02-21 at 13:40 -0800, Frank Filz wrote:
> > On 2/21/18 1:59 PM, GerritHub wrote:
> > > Jeff Layton has uploaded this change for *review*.
> > > 
> > > View Change 
> > > 
> > > MainNFSD: invert _NO_PORTMAPPER option
> > > 
> > > The fact that this is a "negative" option is confusing. Change it to a
> > > "PORTMAPPER" option, and have it default to ON.
> > > 
> > 
> > While I vaguely agree with the former in principle, in this day and age we
> 
> really
> > should stop using the name PORTMAPPER.  Replaced by rpcbind a long time
> 
> ago,
> > and shouldn't be shipping with modern systems.
> > 
> > In ntirpc, PORTMAP is as expected the old version 2 UDP-only call.  We
> 
> should
> > kill it.
> > 
> > We really shouldn't encourage folks to use a UDP system that has long had
> > known DDoS attacks.
> > 
> > And we really should be migrating from NFS 2 UDP to NFS 3 TCP, as a
> 
> minimum
> > supported version
> > 
> > Also, we have talked about adding rpcbind itself to Ganesha or ntirpc.
> 
> Well, the "NO_PORTMAPPER" option really controls rpcbind now...
> 
> Though actually, I'm not sure Ganesha actually uses any of the extension of
> rpcbind, it would probably talk to portmap just fine...
> 
> And we no longer support NFS v2 (we have a couple NFS v2 stubs because at
> one time in history, the Linux client issued NFS v2 requests even though it
> was an NFS v3 mount - really just the UMOUNT was the NFS v2 version I think
> - it's all there in the code). That code might indeed be safe to remove, on
> the other hand, folks with appliance products tend to not like to force
> particular client versions too much as long as the pain point isn't too bad,
> and this fragment of V2 support isn't too bad.
> 
> We could take this opportunity to change the option to RPCBIND...
> 

Fair enough.

I actually disagree with the "no udp" statement above too. UDP is great
for single-shot request protocols like rpcbind, and the NFS client will
use it. DDoS is a possibility, but who exposes their rpcbind port to the
Internet?

In any case, the real fix to this issue is to move to protocols that
don't require rpcbind at all. That means NFSv4.0 at a minimum (though
obviously v4.1+ would be preferred).
-- 
Jeff Layton 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: MainNFSD: invert _NO_PORTMAPPER option

[Frank Filz]

> On 2/21/18 1:59 PM, GerritHub wrote:
> > Jeff Layton has uploaded this change for *review*.
> >
> > View Change 
> >
> > MainNFSD: invert _NO_PORTMAPPER option
> >
> > The fact that this is a "negative" option is confusing. Change it to a
> > "PORTMAPPER" option, and have it default to ON.
> >
> While I vaguely agree with the former in principle, in this day and age we
really
> should stop using the name PORTMAPPER.  Replaced by rpcbind a long time
ago,
> and shouldn't be shipping with modern systems.
> 
> In ntirpc, PORTMAP is as expected the old version 2 UDP-only call.  We
should
> kill it.
> 
> We really shouldn't encourage folks to use a UDP system that has long had
> known DDoS attacks.
> 
> And we really should be migrating from NFS 2 UDP to NFS 3 TCP, as a
minimum
> supported version
> 
> Also, we have talked about adding rpcbind itself to Ganesha or ntirpc.

Well, the "NO_PORTMAPPER" option really controls rpcbind now...

Though actually, I'm not sure Ganesha actually uses any of the extension of
rpcbind, it would probably talk to portmap just fine...

And we no longer support NFS v2 (we have a couple NFS v2 stubs because at
one time in history, the Linux client issued NFS v2 requests even though it
was an NFS v3 mount - really just the UMOUNT was the NFS v2 version I think
- it's all there in the code). That code might indeed be safe to remove, on
the other hand, folks with appliance products tend to not like to force
particular client versions too much as long as the pain point isn't too bad,
and this fragment of V2 support isn't too bad.

We could take this opportunity to change the option to RPCBIND...

Frank


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: MainNFSD: invert _NO_PORTMAPPER option

[Jeff Layton]

On Wed, 2018-02-21 at 16:22 -0500, William Allen Simpson wrote:
> On 2/21/18 1:59 PM, GerritHub wrote:
> > Jeff Layton has uploaded this change for *review*.
> > 
> > View Change 
> > 
> > MainNFSD: invert _NO_PORTMAPPER option
> > 
> > The fact that this is a "negative" option is confusing. Change it
> > to a "PORTMAPPER" option, and have it default to ON.
> > 
> 
> While I vaguely agree with the former in principle, in this day and age
> we really should stop using the name PORTMAPPER.  Replaced by rpcbind a
> long time ago, and shouldn't be shipping with modern systems.
> 
> In ntirpc, PORTMAP is as expected the old version 2 UDP-only call.  We
> should kill it.
> 
> We really shouldn't encourage folks to use a UDP system that has long
> had known DDoS attacks.
> 
> And we really should be migrating from NFS 2 UDP to NFS 3 TCP, as a
> minimum supported version
> 
> Also, we have talked about adding rpcbind itself to Ganesha or ntirpc.

FWIW, my main interest is in being able to just remove this support
wholesale for v4.x-only configs. NFSv4 does not require any rpcbind
registration.

I don't feel too strongly about the naming, and I doubt this would
encourage anyone to go use the ancient portmapper code. I can rename it
if you like, but this seems like bikeshedding.

Note that the rpm specfile has the ability to build ganesha for really
old distros (pre-RHEL6). If you feel strongly about this then we should
remove that support as well.

-- 
Jeff Layton 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: MainNFSD: invert _NO_PORTMAPPER option

[William Allen Simpson]

On 2/21/18 1:59 PM, GerritHub wrote:

Jeff Layton has uploaded this change for *review*.

View Change 

MainNFSD: invert _NO_PORTMAPPER option

The fact that this is a "negative" option is confusing. Change it
to a "PORTMAPPER" option, and have it default to ON.


While I vaguely agree with the former in principle, in this day and age
we really should stop using the name PORTMAPPER.  Replaced by rpcbind a
long time ago, and shouldn't be shipping with modern systems.

In ntirpc, PORTMAP is as expected the old version 2 UDP-only call.  We
should kill it.

We really shouldn't encourage folks to use a UDP system that has long
had known DDoS attacks.

And we really should be migrating from NFS 2 UDP to NFS 3 TCP, as a
minimum supported version

Also, we have talked about adding rpcbind itself to Ganesha or ntirpc.

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: Pullup NTIRPC through #106

[GerritHub]

>From :

william.allen.simp...@gmail.com has uploaded this change for review. ( 
https://review.gerrithub.io/400875


Change subject: Pullup NTIRPC through #106
..

Pullup NTIRPC through #106

(was16backport)
 * Remove xdr_array.c
 * inline xdr_array and xdr_vector
 * Replace xdr_[u_]int
 * Replace xdr_[u_]long
 * Remove unused xdr_[u_]quad
 * Remove unused xdr_[u_]hyper
 * Remove unused xdr_[u_]short
 * Remove unused xdr_[u_]char
 * Remove unused inline xdr_free and xdr_void
 * Merge inline xdr_union
 * Merge inline xdr_bool
 * Merge inline xdr_enum
 * Merge inline xdr_[u_]int.._t functions

Change-Id: Iaf1c6952f244468d92318def768f88b9370784ef
Signed-off-by: William Allen Simpson 
---
M src/libntirpc
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/75/400875/1
--
To view, visit https://review.gerrithub.io/400875
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaf1c6952f244468d92318def768f88b9370784ef
Gerrit-Change-Number: 400875
Gerrit-PatchSet: 1
Gerrit-Owner: william.allen.simp...@gmail.com
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: MainNFSD: invert _NO_PORTMAPPER option

[GerritHub]

>From Jeff Layton :

Jeff Layton has uploaded this change for review. ( 
https://review.gerrithub.io/400871


Change subject: MainNFSD: invert _NO_PORTMAPPER option
..

MainNFSD: invert _NO_PORTMAPPER option

The fact that this is a "negative" option is confusing. Change it
to a "PORTMAPPER" option, and have it default to ON.

Change-Id: If789273d5310addca7bfa125bcf0e789df161964
Signed-off-by: Jeff Layton 
---
M src/CMakeLists.txt
M src/MainNFSD/nfs_rpc_dispatcher_thread.c
2 files changed, 4 insertions(+), 4 deletions(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/71/400871/1
--
To view, visit https://review.gerrithub.io/400871
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: If789273d5310addca7bfa125bcf0e789df161964
Gerrit-Change-Number: 400871
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Layton 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: spec: allow packagers to remove dependency on rpcbind

[GerritHub]

>From Jeff Layton :

Jeff Layton has uploaded this change for review. ( 
https://review.gerrithub.io/400860


Change subject: spec: allow packagers to remove dependency on rpcbind
..

spec: allow packagers to remove dependency on rpcbind

Allow package builders to fully disable rpcbind support. This allows us
to avoid pulling in rpcbind as a dependency. While we're in there, fix
a couple of warnings that show up due to macro expansion in comments.

Change-Id: I79b8285ce771cbff571b94fbee9f1471ff07f386
Signed-off-by: Jeff Layton 
---
M src/CMakeLists.txt
M src/nfs-ganesha.spec-in.cmake
2 files changed, 22 insertions(+), 4 deletions(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/60/400860/1
--
To view, visit https://review.gerrithub.io/400860
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: I79b8285ce771cbff571b94fbee9f1471ff07f386
Gerrit-Change-Number: 400860
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Layton 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: Fixing the fslocations tests failures as seen in pynfs

[GerritHub]

>From Sriram Patil :

Sriram Patil has uploaded this change for review. ( 
https://review.gerrithub.io/400805


Change subject: Fixing the fslocations tests failures as seen in pynfs
..

Fixing the fslocations tests failures as seen in pynfs

More than 50% of the pynfs fslocations tests were failing. These are tests
arounf fs_locations and rdattr_error attributes, when requested in getattr
and readdir. Fixing these calls to make sure it returns rdattr_error along
with other restricted attributes when requested.

Also changed the way procol methods figure out if the current handle is a
referral point. Added a new method in objected ops, named is_referral. The
default implementation always returns false. Implementing this method
allows FSALs to take control of how referral points are identified.

Change-Id: I596ff88dcbb7ad01e36c8e0d990931ef3210486c
Signed-off-by: Sriram Patil 
---
M src/FSAL/FSAL_VFS/handle.c
M src/FSAL/FSAL_VFS/vfs/main.c
M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_handle.c
M src/FSAL/default_methods.c
M src/Protocols/NFS/nfs4_op_getattr.c
M src/Protocols/NFS/nfs4_op_getfh.c
M src/Protocols/NFS/nfs4_op_readdir.c
M src/Protocols/NFS/nfs_proto_tools.c
M src/include/fsal_api.h
M src/include/nfs_proto_tools.h
M src/include/nfsv41.h
11 files changed, 203 insertions(+), 64 deletions(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/05/400805/1
--
To view, visit https://review.gerrithub.io/400805
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: I596ff88dcbb7ad01e36c8e0d990931ef3210486c
Gerrit-Change-Number: 400805
Gerrit-PatchSet: 1
Gerrit-Owner: Sriram Patil 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: build: replace archaic caddr_t with modern void *

[GerritHub]

>From :

ka...@redhat.com has uploaded this change for review. ( 
https://review.gerrithub.io/400806


Change subject: build: replace archaic caddr_t with modern void *
..

build: replace archaic caddr_t with modern void *

Build fails on prerelease of Ubuntu 18.04 (bionic) because caddr_t is
no longer implicitly defined when  is #included.

Trying to untangle the #ifdefs and #ifndefs in  and
 (which is /usr/include/sys/types.h on redhatish linux
and /usr/include/{x86_64-linux-gnu,...}/sys/types on debianish linux)
it is revealed that caddr_t is defined when __USE_MISC #defined.

And __USE_MISC is #defined when either _DEFAULT_SOURCE is defined or
_GNU_SOURCE is defined. (_DEFAULT_SOURCE is/was a replacement for
_BSD_SOURCE and/or SVID_SOURCE which were deprecated in g(nu)libc some
time ago.)

Some combination of distributions implicitly or explicitly enabling
newer and stricter ANSI C compilation has resulted, in the case of
Ubuntu 18.04 anyway, caddr_t not being defined.

While it's possible that this is a transitory bug in Ubuntu 18.04's
headers (it hasn't reared its head on other bleeding edge linux
distributions like Fedora 28/rawhide, or Debian 20/Buster yet) it
seems marginally better to use void * instead; a) it's modern C, and
b) it's a built in type that won't or can't be broken by churn in
headers and feature tests.

Change-Id: Ie8eb79656fc5c3c48073b42f73bdcb92614b7c7c
Signed-off-by: Kaleb S. KEITHLEY 
---
M src/include/config_parsing.h
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/06/400806/1
--
To view, visit https://review.gerrithub.io/400806
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie8eb79656fc5c3c48073b42f73bdcb92614b7c7c
Gerrit-Change-Number: 400806
Gerrit-PatchSet: 1
Gerrit-Owner: ka...@redhat.com
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: RADOS_URLS: enable them by default

[GerritHub]

>From Jeff Layton :

Jeff Layton has uploaded this change for review. ( 
https://review.gerrithub.io/400818


Change subject: RADOS_URLS: enable them by default
..

RADOS_URLS: enable them by default

We'll already enable RADOS_KV recovery backends by default, so we might
as well turn this on too.

Change-Id: Iebda3ea3a5a80e2bbb6191172ef42f1c15e929f1
Signed-off-by: Jeff Layton 
---
M src/CMakeLists.txt
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/18/400818/1
--
To view, visit https://review.gerrithub.io/400818
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iebda3ea3a5a80e2bbb6191172ef42f1c15e929f1
Gerrit-Change-Number: 400818
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Layton 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: RADOS_URLS: allow the RADOS_URLS config block to be optional

[GerritHub]

>From Jeff Layton :

Jeff Layton has uploaded this change for review. ( 
https://review.gerrithub.io/400817


Change subject: RADOS_URLS: allow the RADOS_URLS config block to be optional
..

RADOS_URLS: allow the RADOS_URLS config block to be optional

Just warn if it doesn't exist and assume we're ok with NULLs for
the parameters.

Change-Id: I4357dd8e2246083d407dd86dfed257241d81aed6
Signed-off-by: Jeff Layton 
---
M src/config_parsing/conf_url_rados.c
1 file changed, 8 insertions(+), 7 deletions(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/17/400817/1
--
To view, visit https://review.gerrithub.io/400817
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4357dd8e2246083d407dd86dfed257241d81aed6
Gerrit-Change-Number: 400817
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Layton 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: Add a new field "fsal_name" in fsal_staticfsinfo_t. In case of multip...

[GerritHub]

>From :

supriti.si...@suse.com has uploaded this change for review. ( 
https://review.gerrithub.io/400832


Change subject: Add a new field "fsal_name" in fsal_staticfsinfo_t. In case of 
multiple exports this should help in identifying the right fsal. Also, call 
display_fsinfo for FSAL_RGW and FSAL_CEPH
..

Add a new field "fsal_name" in fsal_staticfsinfo_t.
In case of multiple exports this should help in identifying the right fsal.
Also, call display_fsinfo for FSAL_RGW and FSAL_CEPH

Change-Id: Ibf0add7629502a133113b1176802e0c6d0bbfe58
Signed-off-by: Supriti Singh 
---
M src/FSAL/FSAL_CEPH/main.c
M src/FSAL/FSAL_GPFS/main.c
M src/FSAL/FSAL_MEM/mem_main.c
M src/FSAL/FSAL_PSEUDO/main.c
M src/FSAL/FSAL_RGW/main.c
M src/FSAL/FSAL_VFS/panfs/main.c
M src/FSAL/FSAL_VFS/vfs/main.c
M src/FSAL/FSAL_VFS/xfs/main.c
M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_main.c
M src/FSAL/Stackable_FSALs/FSAL_NULL/main.c
M src/FSAL/commonlib.c
M src/include/fsal_types.h
12 files changed, 16 insertions(+), 1 deletion(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/32/400832/1
--
To view, visit https://review.gerrithub.io/400832
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibf0add7629502a133113b1176802e0c6d0bbfe58
Gerrit-Change-Number: 400832
Gerrit-PatchSet: 1
Gerrit-Owner: supriti.si...@suse.com
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] inconsistent '*' spacing

[William Allen Simpson]

On 2/20/18 1:06 PM, Frank Filz wrote:

As I'm trying to update nfs41.h, I've run into the problem that the commit

check

is complaining that the pointer '*' on parameters is sometimes " * v" and

others

" *v" -- usually the same function definition.

Presumably the generator made these.  They are cosmetic.

Why oh why are we checking this now, after all these years?

Do I need to make a pass fixing all these header files before doing real

coding?


Or can we turn off this silly cosmetic check?


The problem is that checkpatch gets confused between multiplication and
pointer. Shame on C for overloading *...

The problem is that checkpatch doesn't recognize, for example, SVCXPRT as a
type, so it thinks SVCXPRT *xprt is a multiplication rather than a
declaration.

The number of places this causes problems is tiny (and mostly confined to
nfsv41.h and nfs23.h) that I have just decided to live with SVCXPRT * xprt.


Well, I cannot, because it won't let me commit anything in those files.



[...]
I suppose now that it seems to complain either way, we can fix them all to
not have the space, and just force the commit and ignore the checkpatch
warnings (when I see ONLY warnings for things I know we can't/won't fix in
gerrithub, I ignore them and merge anyway). I'm not sure how much of a
potential merge headache changing them all would cause though. I don't think
nfsv41.h gets changed all that much so even if we had to backport something,
the potential manual merge wouldn't be awful.


New -dev cycle, so it's time.



[...] > I sympathize with your grumbling about this one... I've chosen the set 
of
checkpatch checks to enable based on what seems to work best for our code
and keeps us as consistent style as possible. I realize some folks have
preferences opposite some of the checks, but our code style is now what it
is...


Or not, as in this case.

Anyway, I'll try to push a patch for those 2 files by tomorrow.

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] nfs ganesha vs nfs kernel performance

[Matt Benjamin]

Hi Bill,

Let's just talk about the rpc-ping-mt/request minimal latency.

On Wed, Feb 21, 2018 at 8:24 AM, William Allen Simpson
 wrote:

>>
>> It measures minimal request latency all the way to nfs-ganesha's, or
>> the Linux kernel's, rpcnull handler--the "top" of the request handling
>> stack, in the given client/server/network configuration.  Scaled up,
>> it can report the max such calls/s, which is a kind of best-possible
>> value for iops, taking FSAL ops to have 0 latency.
>>
> As I've mentioned elsewhere, this should be entirely dominated by the
> link speed and protocol.  We should see UDP as slowest, TCP in the
> middle, and RDMA as fastest.

I think the point is, the "should" assumes perfect behavior from the
dispatch and (minimal) request execution path within nfs-ganesha, does
it not?  In other words, profiling of rpc null is effectively
profiling the first stage of the request pipeline, and if it's not
fast, more interesting ops won't be.

>
> OTOH, the "max such calls/s" would be reported by using XDR_RAW, which
> is currently not working.
>

My intended meaning, and Daniel's when he articulated the same point
Friday, is that "max such calls/s" -is- meaningful for transports NFS
actually supports, and those are the ones we can compare with other
implementations.  I think the raw transport opens up some interesting
paths, but it seems like they would involve more development.

Matt


-- 

Matt Benjamin
Red Hat, Inc.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://www.redhat.com/en/technologies/storage

tel.  734-821-5101
fax.  734-769-8938
cel.  734-216-5309

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] nfs ganesha vs nfs kernel performance

[William Allen Simpson]

I really don't have time today to respond to every one-liner
throw-away comment here, so I'll try to stick to the most cogent.

On 2/20/18 8:33 AM, Matt Benjamin wrote:

On Tue, Feb 20, 2018 at 8:12 AM, William Allen Simpson
 wrote:

On 2/18/18 2:47 PM, Matt Benjamin wrote:


On Fri, Feb 16, 2018 at 11:23 AM, William Allen Simpson


But the planned 2.7 improvements are mostly throughput related, not IOPS.


Not at all, though I am trying to ensure that we get async FSAL ops
in.  There are people working on IOPs too.


async FSAL ops are not likely to further improve IOPS.

As I've pointed out many times in the past, async only allows
the same number of threads to handle more concurrent operations.

But it's actually slower.  It basically doubles the number of
system calls.  System calls are one of the reasons that Ganesha is
slower than knfsd.  It also ruins CPU cache coherency.

If we're CPU bound or network bandwidth constrained, it won't help.


Not everything being worked on is a direct answer to this problem.


Anything that isn't about protocol correctness or performance
should be lower priority.

Async FSAL ops could -- in a few cases where network or disk spinning
dominate the thread timing -- improve thread utilization.  But it
will never improve IOPS.



The effort that I've been waiting for *3* years -- add io vectors to
the FSAL interface, so that we can zero-copy buffers -- is more
likely to improve throughput.


Which, as you've noted, also is happening.


Finally.  Again, I've been asking for it now 3 years.  For V2.7, this is
what I anticipate will result in the most performance increase.



Moreover, going through the code and removing locking other such
bottlenecks is more likely to improve IOPS.


No one is disputing this.  It is not a new discovery, however.


And yet we seem to have to keep reminding folks.  I remember that
Malahal did a survey of lock timing/congestion a couple of years ago.

Perhaps he's willing to run it again?



If Ganesha is adding 6 ms to every read operation, we have a serious
problem, and need to profile immediately!



That's kind of what our team is doing.  I look forward to your work
with rpc-ping-mt.


Well, you were able to send me a copy after 6 pm on a Friday, so I'm
now taking a look at it.  Hopefully I'll have something by Friday.


It came up in a meeting on Friday, that's why I sent it Friday.  I got
busy in meetings and issues, that's why after 6 pm.



But I really wish you'd posted it 3 years ago.  It doesn't really test
IOPS, other than whatever bandwidth limit is imposed by the interface,
but it does test the client call interface.


It measures minimal request latency all the way to nfs-ganesha's, or
the Linux kernel's, rpcnull handler--the "top" of the request handling
stack, in the given client/server/network configuration.  Scaled up,
it can report the max such calls/s, which is a kind of best-possible
value for iops, taking FSAL ops to have 0 latency.


As I've mentioned elsewhere, this should be entirely dominated by the
link speed and protocol.  We should see UDP as slowest, TCP in the
middle, and RDMA as fastest.

OTOH, the "max such calls/s" would be reported by using XDR_RAW, which
is currently not working.



It was posted to this list by Tigran, iirc, in 2011 or 2012.


In which case, I really wish Tigran had put it in the tests folder,
so we'd have been continuously using and updating it.  The gtests
are a great improvement.  Now all we need to do is run them every
week for each new -dev release to track improvements/regressions.

I didn't know about this old test, because my archives only go back
to late 2014.  I wasn't involved before then.

Moreover, I didn't appreciate being criticized for not running an
old test that wasn't in the tree and wasn't maintained.



We've been using Jeff Layton's delegation callback work to test, and
this test would have been better and easier.

But a unit test is not what we need.  I wrote "profile".  We need to
know where the CPU bottlenecks are in Ganesha itself.


You also wrote unit test.


Looking back over this thread, I don't see those words.

I wrote "profile".

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: specfile: fix libcephfs-devel and librgw-devel BuildRequires

[GerritHub]

>From Jeff Layton :

Jeff Layton has uploaded this change for review. ( 
https://review.gerrithub.io/400827


Change subject: specfile: fix libcephfs-devel and librgw-devel BuildRequires
..

specfile: fix libcephfs-devel and librgw-devel BuildRequires

These were changed in the upstream packages around a year ago. The
one for librados-devel is already fixed, but we need to remove the
embedded version numbers in these -devel packages as well.

Change-Id: Ib1e482c3233309891ded6f172a79aec2eb143f5b
Signed-off-by: Jeff Layton 
---
M src/nfs-ganesha.spec-in.cmake
1 file changed, 2 insertions(+), 2 deletions(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/27/400827/1
--
To view, visit https://review.gerrithub.io/400827
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib1e482c3233309891ded6f172a79aec2eb143f5b
Gerrit-Change-Number: 400827
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Layton 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] inconsistent '*' spacing

[Frank Filz]

> On 2/20/18 1:06 PM, Frank Filz wrote:
> >> As I'm trying to update nfs41.h, I've run into the problem that the
> >> commit
> > check
> >> is complaining that the pointer '*' on parameters is sometimes " * v"
> >> and
> > others
> >> " *v" -- usually the same function definition.
> >>
> >> Presumably the generator made these.  They are cosmetic.
> >>
> >> Why oh why are we checking this now, after all these years?
> >>
> >> Do I need to make a pass fixing all these header files before doing
> >> real
> > coding?
> >>
> >> Or can we turn off this silly cosmetic check?
> >
> > The problem is that checkpatch gets confused between multiplication
> > and pointer. Shame on C for overloading *...
> >
> > The problem is that checkpatch doesn't recognize, for example, SVCXPRT
> > as a type, so it thinks SVCXPRT *xprt is a multiplication rather than
> > a declaration.
> >
> > The number of places this causes problems is tiny (and mostly confined
> > to nfsv41.h and nfs23.h) that I have just decided to live with SVCXPRT * 
> > xprt.
> >
> Well, I cannot, because it won't let me commit anything in those files.

There's a -n or --no-verify option that will bypass the commit hooks. I suggest 
trying to commit without that first to make sure the only checkpatch 
errors/warnings are for the spacing around * and then commit again with -n to 
bypass checkpatch to actually commit.

> > I suppose now that it seems to complain either way, we can fix them
> > all to not have the space, and just force the commit and ignore the
> > checkpatch warnings (when I see ONLY warnings for things I know we
> > can't/won't fix in gerrithub, I ignore them and merge anyway). I'm not
> > sure how much of a potential merge headache changing them all would
> > cause though. I don't think nfsv41.h gets changed all that much so
> > even if we had to backport something, the potential manual merge wouldn't
> be awful.
> >
> New -dev cycle, so it's time.

Yes, this is the right time to do it.

> > [...] > I sympathize with your grumbling about this one... I've chosen
> > the set of checkpatch checks to enable based on what seems to work
> > best for our code and keeps us as consistent style as possible. I
> > realize some folks have preferences opposite some of the checks, but
> > our code style is now what it is...
> >
> Or not, as in this case.
> 
> Anyway, I'll try to push a patch for those 2 files by tomorrow.

Ok, thanks

Frank


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: doc: fix typo in ganesha-config manpage

[GerritHub]

>From Jeff Layton :

Jeff Layton has uploaded this change for review. ( 
https://review.gerrithub.io/400826


Change subject: doc: fix typo in ganesha-config manpage
..

doc: fix typo in ganesha-config manpage

Change-Id: I3c4ee0cf3d49532a59274bd8323edbb92315f842
Signed-off-by: Jeff Layton 
---
M src/doc/man/ganesha-config.rst
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/26/400826/1
--
To view, visit https://review.gerrithub.io/400826
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c4ee0cf3d49532a59274bd8323edbb92315f842
Gerrit-Change-Number: 400826
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Layton 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: Set op_ctx when reverting exports

[GerritHub]

>From Daniel Gryniewicz :

Daniel Gryniewicz has uploaded this change for review. ( 
https://review.gerrithub.io/400846


Change subject: Set op_ctx when reverting exports
..

Set op_ctx when reverting exports

Change-Id: Id50f1d2cd15f22696a2a339f494884331b7da548
Signed-off-by: Daniel Gryniewicz 
---
M src/support/export_mgr.c
1 file changed, 6 insertions(+), 0 deletions(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/46/400846/1
--
To view, visit https://review.gerrithub.io/400846
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id50f1d2cd15f22696a2a339f494884331b7da548
Gerrit-Change-Number: 400846
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Gryniewicz 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel