From: Steve French <smfrench@gmail.com>
To: Tom Talpey <tom@talpey.com>
Cc: Jeff Layton <jlayton@kernel.org>,
Enzo Matsumiya <ematsumiya@suse.de>,
CIFS <linux-cifs@vger.kernel.org>, Paulo Alcantara <pc@cjr.nz>,
ronnie sahlberg <ronniesahlberg@gmail.com>,
Shyam Prasad N <nspmangalore@gmail.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
samba-technical <samba-technical@lists.samba.org>,
Pavel Shilovsky <pshilovsky@samba.org>
Subject: Re: [RFC PATCH 0/3] Rename "cifs" module to "smbfs"
Date: Wed, 3 Aug 2022 00:38:46 -0500 [thread overview]
Message-ID: <CAH2r5mtWn85=RknxfE2p=Zo24H2ynin2ReLV1jijSG-yLN9J4w@mail.gmail.com> (raw)
In-Reply-To: <1c2e8880-3efe-b55d-ee50-87d57efc3130@talpey.com>
On Tue, Aug 2, 2022 at 8:32 PM Tom Talpey <tom@talpey.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
next prev parent reply other threads:[~2022-08-03 5:39 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-01 19:09 [RFC PATCH 0/3] Rename "cifs" module to "smbfs" Enzo Matsumiya
2022-08-01 19:09 ` [RFC PATCH 1/3] cifs: change module name to "smbfs.ko" Enzo Matsumiya
2023-07-26 20:28 ` Steve French
2023-07-26 20:29 ` Steve French
2023-07-26 20:31 ` Steve French
2023-07-27 0:24 ` Enzo Matsumiya
2022-08-01 19:09 ` [RFC PATCH 2/3] smbfs: rename directory "fs/cifs" -> "fs/smbfs" Enzo Matsumiya
2022-08-01 19:09 ` [RFC PATCH 3/3] smbfs: update doc references Enzo Matsumiya
2022-08-02 14:42 ` [RFC PATCH 0/3] Rename "cifs" module to "smbfs" Jeff Layton
2022-08-02 19:36 ` Enzo Matsumiya
2022-08-02 20:07 ` Jeff Layton
2022-08-03 1:32 ` Tom Talpey
2022-08-03 1:56 ` Enzo Matsumiya
2022-08-04 19:03 ` Jeff Layton
2022-08-04 20:23 ` Matthew Wilcox
2022-08-04 20:48 ` Jeff Layton
2022-08-03 5:38 ` Steve French [this message]
2022-08-03 14:45 ` Enzo Matsumiya
2022-08-03 17:50 ` Paulo Alcantara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAH2r5mtWn85=RknxfE2p=Zo24H2ynin2ReLV1jijSG-yLN9J4w@mail.gmail.com' \
--to=smfrench@gmail.com \
--cc=ematsumiya@suse.de \
--cc=jlayton@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nspmangalore@gmail.com \
--cc=pc@cjr.nz \
--cc=pshilovsky@samba.org \
--cc=ronniesahlberg@gmail.com \
--cc=samba-technical@lists.samba.org \
--cc=tom@talpey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).