[PATCH v2 01/12] smb: smbdirect: add smbdirect_pdu.h with protocol definitions

[Steve French]

> Can you explain why he has split it into smbdirect_socket.h?

The three header names seem plausible, but would be useful to have
Metze's clarification/explanation:
- the "protocol" related header info for smbdirect goes in
smb/common/smbdirect/smbdirect_pdu.h   (we use similar name smb2pdu.h
for the smb2/smb3 protocol related wire definitions)
- smbdirect.h for internal smbdirect structure definitions
- smbdirect_socket.h for things related to exporting it as a socket
(since one of the goals is to make smbdirect useable by Samba
userspace tools)

On Sun, Jun 1, 2025 at 12:00 AM Namjae Jeon <linkinjeon at kernel.org> wrote:
>
> On Sun, Jun 1, 2025 at 12:49 PM Steve French <smfrench at gmail.com> wrote:
> >
> > Moving to use common headers is something I did multiple times with ksmbd and cifs.ko already, not a bad first step
> This is not simply moved to the common header. He splits the header
> into smaller pieces for some unknown reason.
> Can you explain why he has split it into smbdirect_socket.h?
> We doesn't need to do it now if he's thinking of creating a
> smbdirect_socket.c file later.
> >
> > Thanks,
> >
> > Steve
> >
> > On Sat, May 31, 2025, 7:01 PM Namjae Jeon <linkinjeon at kernel.org> wrote:
> >>
> >> On Sun, Jun 1, 2025 at 8:23 AM Steve French <smfrench at gmail.com> wrote:
> >> >
> >> > I do like the small, relatively safe steps he is doing these in.
> >> Small is okay, but I wonder when he will send the rest.
> >> What if he just separates it like this and doesn't send the rest of
> >> patches later?
> >> I've never seen a case where the headers are separated first,
> >> And send the main if it's implemented later. This is not a personal repository.
> >>
> >> Thanks.
> >> >
> >> > Thanks,
> >> >
> >> > Steve
> >> >
> >> > On Fri, May 30, 2025, 5:29 PM Namjae Jeon <linkinjeon at kernel.org> wrote:
> >> >>
> >> >> On Sat, May 31, 2025 at 4:03 AM Stefan Metzmacher <metze at samba.org> wrote:
> >> >> >
> >> >> > Am 29.05.25 um 01:28 schrieb Namjae Jeon:
> >> >> > > On Thu, May 29, 2025 at 1:02 AM Stefan Metzmacher <metze at samba.org> wrote:
> >> >> > >>
> >> >> > >> This is just a start moving into a common smbdirect layer.
> >> >> > >>
> >> >> > >> It will be used in the next commits...
> >> >> > >>
> >> >> > >> Cc: Steve French <smfrench at gmail.com>
> >> >> > >> Cc: Tom Talpey <tom at talpey.com>
> >> >> > >> Cc: Long Li <longli at microsoft.com>
> >> >> > >> Cc: Namjae Jeon <linkinjeon at kernel.org>
> >> >> > >> Cc: Hyunchul Lee <hyc.lee at gmail.com>
> >> >> > >> Cc: Meetakshi Setiya <meetakshisetiyaoss at gmail.com>
> >> >> > >> Cc: linux-cifs at vger.kernel.org
> >> >> > >> Cc: samba-technical at lists.samba.org
> >> >> > >> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> >> >> > >> ---
> >> >> > >>   fs/smb/common/smbdirect/smbdirect_pdu.h | 55 +++++++++++++++++++++++++
> >> >> > >>   1 file changed, 55 insertions(+)
> >> >> > >>   create mode 100644 fs/smb/common/smbdirect/smbdirect_pdu.h
> >> >> > >>
> >> >> > >> diff --git a/fs/smb/common/smbdirect/smbdirect_pdu.h b/fs/smb/common/smbdirect/smbdirect_pdu.h
> >> >> > >> new file mode 100644
> >> >> > >> index 000000000000..ae9fdb05ce23
> >> >> > >> --- /dev/null
> >> >> > >> +++ b/fs/smb/common/smbdirect/smbdirect_pdu.h
> >> >> > >> @@ -0,0 +1,55 @@
> >> >> > >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> >> > >> +/*
> >> >> > >> + *   Copyright (c) 2017 Stefan Metzmacher
> >> >> > > Isn't it 2025? It looks like a typo.
> >> >> >
> >> >> > I took it from here:
> >> >> > https://git.samba.org/?p=metze/linux/smbdirect.git;a=blob;f=smbdirect_private.h;hb=284ad8ea768c06e3cc70d6f2754929a6abbd2719
> >> >> >
> >> >> > > And why do you split the existing one into multiple header
> >> >> > > files(smbdirect_pdu.h, smbdirect_socket.h, smbdirect.h)?
> >> >> >
> >> >> > In the end smbdirect.h will be the only header used outside
> >> >> > of fs/smb/common/smbdirect, it will be the public api, to be used
> >> >> > by the smb layer.
> >> >> >
> >> >> > smbdirect_pdu.h holds protocol definitions, while smbdirect_socket.h
> >> >> > will be some kind of internal header that holds structures shared between multiple .c files.
> >> >> >
> >> >> > But we'll see I think this is a start in the correct direction.
> >> >> When will you send the patches for multiple .c files?
> >> >> I'm not sure if this is the right direction when I check only this patch-set.
> >> >> I don't prefer to change the headers like this in advance without a body.
> >> >> When you're ready, how about sending the patches including the body all at once?
> >> >> >
> >> >> > I try to focus on doing tiny steps avoiding doing to much at the same time
> >> >> > or even try to avoid thinking about the next step already...
> >> >> >
> >> >> > metze



-- 
Thanks,

Steve