[PATCH] smb3: simplify code by removing CONFIG_CIFS_SMB311

Steve French smfrench at gmail.com
Sat Jun 30 18:03:32 UTC 2018


On Sat, Jun 30, 2018 at 1:46 AM Pavel Shilovsky <piastryyy at gmail.com> wrote:
>
> пт, 29 июн. 2018 г. в 12:44, Steve French <smfrench at gmail.com>:
> >
> > An additional piece of information on this - in CIFS POSIX was turned
> > on by default, where in SMB3.1.1 currently - not only is the dialect
> > off by default, the POSIX mount option is not the default for SMB3.1.1
> > and it warns that it is experimental if you did choose BOTH mount
> > options (vers=3.1.1 and posix), and it is fairly low risk.   If others
> > feel strongly that we want a third layer of protection (ie a CONFIG
> > option for it) that is ok - but my instinct is that since the user has
> > to do multiple things to turn this on, and it is fairly safe even if
> > for some strange non-developer/non-testing reason they were able to
> > turn it on to a Samba server which included this.
> > On Fri, Jun 29, 2018 at 2:40 PM Steve French <smfrench at gmail.com> wrote:
> > >
> > > On Fri, Jun 29, 2018 at 8:52 AM Pavel Shilovsky <piastryyy at gmail.com> wrote:
> > > >
> > > > чт, 28 июн. 2018 г. в 23:54, Aurélien Aptel via samba-technical
> > > > <samba-technical at lists.samba.org>:
> > > > >
> > > > > Steve French <smfrench at gmail.com> writes:
> > > > > > Ronnie,
> > > > > > What about the attached wording, slightly updated patch - I made minor
> > > > > > changes to remove the second mention
> > > > > > of SMB3.1.1 and to correct a spelling mistake (and to fix the missing
> > > > > > word "Windows 2016" as
> > > > > > "Windows Server 2016").
> > > > >
> > > > > +1
> > > > >
> > > > > I fully support removing that config option :)
> > > > >
> > > > > Cheers,
> > > > > --
> > > > > Aurélien Aptel / SUSE Labs Samba Team
> > > > > GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> > > > > SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> > > > > GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> > > > >
> > > >
> > > > I like the idea to remove SMB311 config option too but I think we
> > > > should put all the things related to POSIX ext under
> > > > CONFIG_CIFS_POSIX, e.g.:
> > > >
> > > > -#ifdef CONFIG_CIFS_SMB311
> > > > +#ifdef CONFIG_CIFS_POSIX
> > > >          if (server->posix_ext_supported)
> > > >              seq_printf(m, " posix");
> > > > -#endif /* 3.1.1 */
> > > > +#ifdef /* POSIX */
> > > >
> > > > and the same for other places mentioning POSIX ext. Thoughts?
> > >
> > > My gut reaction on that is that since the 3.1.1 POSIX changes are
> > > pretty small except a couple functions which are very distinct, and
> > > don't intersect much with complex code (as the CIFS POSIX extensions
> > > did), simply marking the mount option experimental is less likely to
> > > lead to weird build breaks, and problems testing.  We need to continue
> > > testing and finishup of the SMB3.1.1 POSIX Extensions as soon as
> > > possible due to real problems with CIFS (and thus CIFS/POSIX)
> > > deprecation.
> > >
> > > Don't have a strong feeling on this though one way or the other (about
> > > whether adding and additional #ifdef would help - I am a little
> > > worried that nesting a 3.1.1 feature ifdef in an ifdef for an SMB1
> > > (CIFS POSIX) feature would get confusing.
>
> In a case we decide to keep POSIX related things without ifdefs, the
> following code from patch "cifs: add missing debug entries for kconfig
> options" should be fixed as well:
>
>  #ifdef CONFIG_CIFS_POSIX
> -    seq_printf(m, " posix");
> +    seq_printf(m, ",CIFS_POSIX");
>  #endif

I figured that there is little point in touching the old
CONFIG_CIFS_POSIX stuff (for SMB1), but could remove that ifdef as
well if others want to - maybe it will make it less confusing.


-- 
Thanks,

Steve



More information about the samba-technical mailing list