Date:   Wed, 3 Aug 2022 00:38:46 -0500
From:   Steve French <smfrench@...il.com>
To:     Tom Talpey <tom@...pey.com>
Cc:     Jeff Layton <jlayton@...nel.org>,
        Enzo Matsumiya <ematsumiya@...e.de>,
        CIFS <linux-cifs@...r.kernel.org>, Paulo Alcantara <pc@....nz>,
        ronnie sahlberg <ronniesahlberg@...il.com>,
        Shyam Prasad N <nspmangalore@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        samba-technical <samba-technical@...ts.samba.org>,
        Pavel Shilovsky <pshilovsky@...ba.org>
Subject: Re: [RFC PATCH 0/3] Rename "cifs" module to "smbfs"

On Tue, Aug 2, 2022 at 8:32 PM Tom Talpey <tom@...pey.com> wrote:
>
> On 8/2/2022 4:07 PM, Jeff Layton wrote:
> > On Tue, 2022-08-02 at 16:36 -0300, Enzo Matsumiya wrote:
> >> On 08/02, Jeff Layton wrote:
> >>> On Mon, 2022-08-01 at 16:09 -0300, Enzo Matsumiya wrote:
> >>>> Hi,
> >>>>
> >>>> As part of the ongoing effort to remove the "cifs" nomenclature from the
> >>>> Linux SMB client, I'm proposing the rename of the module to "smbfs".
> >>>>
> >>>> As it's widely known, CIFS is associated to SMB1.0, which, in turn, is
> >>>> associated with the security issues it presented in the past. Using
> >>>> "SMBFS" makes clear what's the protocol in use for outsiders, but also
> >>>> unties it from any particular protocol version. It also fits in the
> >>>> already existing "fs/smbfs_common" and "fs/ksmbd" naming scheme.
> >>>>
> >>>> This short patch series only changes directory names and includes/ifdefs in
> >>>> headers and source code, and updates docs to reflect the rename. Other
> >>>> than that, no source code/functionality is modified (WIP though).
> >>>>
> >>>> Patch 1/3: effectively changes the module name to "smbfs" and create a
> >>>>       "cifs" module alias to maintain compatibility (a warning
> >>>>       should be added to indicate the complete removal/isolation of
> >>>>       CIFS/SMB1.0 code).
> >>>> Patch 2/3: rename the source-code directory to align with the new module
> >>>>       name
> >>>> Patch 3/3: update documentation references to "fs/cifs" or "cifs.ko" or
> >>>>       "cifs module" to use the new name
> >>>>
> >>>> Enzo Matsumiya (3):
> >>>>    cifs: change module name to "smbfs.ko"
> >>>>    smbfs: rename directory "fs/cifs" -> "fs/smbfs"
> >>>>    smbfs: update doc references
> >>>> ...
> >>>
> >>> Why do this? My inclination is to say NAK here.
> >>>
> >>> This seems like a lot of change for not a lot of benefit. Renaming the
> >>> directory like this pretty much guarantees that backporting patches
> >>> after this change to kernels that existed before it will be very
> >>> difficult.
> >>
> >> Hi Jeff, yes that's a big concern that I've discussed internally with my
> >> team as well, since we'll also suffer from those future backports.
> >>
> >> But, as stated in the commit message, and from what I gathered from
> >> Steve, it has been an ongoing wish to have the "cifs" name no longer
> >> associated with a module handling SMB2.0 and SMB3.0, as the name brings
> >> back old bad memories for several users.
> >>
> >> There really is no functional benefit for this change, and I have no
> >> argument against that.
> >>
> >
> > If the concern is "branding" then I don't see how this really helps.
> > Very few users interact with the kernel modules directly.
> >
> > FWIW, I just called "modprobe smb3" on my workstation and got this:
> >
> > [ 1223.581583] Key type cifs.spnego registered
> > [ 1223.582523] Key type cifs.idmap registered
> > [ 1230.411422] Key type cifs.idmap unregistered
> > [ 1230.412542] Key type cifs.spnego unregistered
> >
> > Are you going to rename the keyrings too? That will have implications
> > for userland helper programs like cifs.upcall. There's also
> > /proc/fs/cifs/*.
> >
> > These are a "stable interfaces" that you can't just rename at will. If
> > you want to change these interfaces then you need to do a formal
> > deprecation announcement, and probably a period with /proc/fs/smbfs and
> > /proc/fs/cifs coexisting.
> >
> > There are also a ton of printk's and such that have "CIFS" in them that
> > will need to be changed.
> >
> > These costs do not seem worth the perceived benefit to me. You could
> > probably hide a lot of what users see by just renaming (or symlinking)
> > mount.cifs to mount.smb3.
> >
> > I think if you guys are serious about this, you should probably start
> > somewhere else besides renaming the directory and module. This is going
> > to impact developers (and people who make their living doing backports)
> > far more than it will users.
>
> The initial goal is to modularize the SMB1 code, so it can be completely
> removed from a running system. The extensive refactoring logically leads
> to this directory renaming, but renaming is basically a side effect.
>
> Stamping out the four-letter word C-I-F-S is a secondary goal. At this
> point, the industry has stopped using it. You make a good point that
> it's still visible outside the kernel source though.
>
> It makes good sense to do the refactoring in place, at first. Splitting
> the {smb1,cifs}*.[ch] files will be more complex, but maybe easier to
> review and merge, without folding in a new directory tree and git rm/mv.
> Either way, there will be at least two modules, maybe three if we split
> out generic subroutines.

Yes, Tom's points make sense.  The initial goal is to modularize the
smb1 (cifs) code,
and first goal is to do the refactoring in place without creating a
new directory
tree, allowing more and more of the smb1 code to be split out (currently
we can save about 10% on the module size when built with legacy disabled, but
I suspect that it will be about double that as more smb1 code is moved into
ifdefs or the smb1 specific c files).


-- 
Thanks,

Steve

Powered by blists - more mailing lists