All of lore.kernel.org
 help / color / mirror / Atom feed
* mids and cifs sendrcv2
@ 2011-04-08 16:39 Steve French
       [not found] ` <BANLkTi=tUrieqB4jCo0GRX3Np10JxJUP9Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Steve French @ 2011-04-08 16:39 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel

Update on cifs vs. smb2 mids, and the smb2 sendrcv2.   Jeff had
suggested more closely matching the cifs and smb2 mids, in particular
extending the 16 bit cifs mid (multiplex identifier for inflight
network requests) to the 64 bit size needed for smb2 (and thus masking
the mid when used for cifs) and having cifs ignore the various smb2
unique fields in the mid (which makes the mid larger for cifs).
Since the smb2 code in cifs-2.6.git (put in February and early March)
has now been rereviewed, the next step in the smb2 merge is posting
and reviewing the transport routine for smb2 (smb2_sendrcv2 or reusing
cifs_sendrcv2) - the latter may make more sense if we go to a common
mid for cifs and smb2.   At the fs summit, Jeff and Jeremy and I
talked about this, but Pavel and others may have opinions on this
topic.   As soon as the cifs merge activity settles down for 2.6.39, I
plan to post sendrcv2 alternatives and then begin work with Pavel on
the superblock, file and inode routines and seeing whether for smb2
they should be smb2 unique (as we originally expected since smb2 is
handle based, and simpler) and look more like they did in the smb2.ko
work that Pavel did last summer or should be more common with the cifs
routines.

-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
       [not found] ` <BANLkTi=tUrieqB4jCo0GRX3Np10JxJUP9Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-04-15 10:45   ` Pavel Shilovsky
       [not found]     ` <BANLkTikLfy=4BPESJEmOe+mBhKSRtC=W6A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Pavel Shilovsky @ 2011-04-15 10:45 UTC (permalink / raw)
  To: Steve French; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel

2011/4/8 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Update on cifs vs. smb2 mids, and the smb2 sendrcv2.   Jeff had
> suggested more closely matching the cifs and smb2 mids, in particular
> extending the 16 bit cifs mid (multiplex identifier for inflight
> network requests) to the 64 bit size needed for smb2 (and thus masking
> the mid when used for cifs) and having cifs ignore the various smb2
> unique fields in the mid (which makes the mid larger for cifs).
> Since the smb2 code in cifs-2.6.git (put in February and early March)
> has now been rereviewed, the next step in the smb2 merge is posting
> and reviewing the transport routine for smb2 (smb2_sendrcv2 or reusing
> cifs_sendrcv2) - the latter may make more sense if we go to a common
> mid for cifs and smb2.   At the fs summit, Jeff and Jeremy and I
> talked about this, but Pavel and others may have opinions on this
> topic.   As soon as the cifs merge activity settles down for 2.6.39, I
> plan to post sendrcv2 alternatives and then begin work with Pavel on
> the superblock, file and inode routines and seeing whether for smb2
> they should be smb2 unique (as we originally expected since smb2 is
> handle based, and simpler) and look more like they did in the smb2.ko
> work that Pavel did last summer or should be more common with the cifs
> routines.
>
> --
> Thanks,
>
> Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

I suggest to make cifs and smb2 protocol mid structures use common
structure that has equals fields for both and then expand this
structure for protocol-dependent things.

It can look like this:

/* one of these for every pending CIFS request to the server */
struct mid_q_entry {
        __u8 protocol_id;
        struct list_head qhead; /* mids waiting on reply from this server */
        int midState;   /* wish this were enum but can not pass to wait_event */
        unsigned long when_alloc;  /* when mid was created */
#ifdef CONFIG_CIFS_STATS2
        unsigned long when_sent; /* time when smb send finished */
        unsigned long when_received; /* when demux complete (taken off wire) */
#endif
        bool largeBuf:1;        /* if valid response, is pointer to large buf */
        void *resp_buf;         /* response buffer */
        mid_callback_t *callback; /* call completion callback */
        void *callback_data;      /* general purpose pointer for callback */
};

struct cifs_mid_q_entry {
        struct mid_q_entry mid_q;
        __u16 mid;              /* multiplex id */
        __u32 sequence_number;  /* for CIFS signing */
        __u8 command;           /* smb command code */
        __u16 pid;              /* process id */
        bool multiRsp:1;        /* multiple trans2 responses for one request  */
        bool multiEnd:1;        /* both received */
};

struct smb2_mid_q_entry {
        struct mid_q_entry mid_q;
        __u64 mid;              /* multiplex id(s), bigger for smb2 */
        __le16 command;         /* smb2 command code */
        __u32 pid;              /* process id - bigger for smb2 than cifs */
};


So, we always work with a pointer to common structure mid_q_entry and
then expand it according to protocol_id filed when we need it:

#define PROTOCOL_ID(mid) (*((__u8 *)mid))

process_mid(struct mid_q_entry *pmid)
{
        if (PROTOCOL_ID(pmid) == SMB2)
                process_smb2_mid((struct smb2_mid_q_entry *)pmid);
        else
                process_cifs_mid((struct cifs_mid_q_entry *)pmid);
}


-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
       [not found]     ` <BANLkTikLfy=4BPESJEmOe+mBhKSRtC=W6A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-04-15 13:38       ` Steve French
  2011-04-15 15:21         ` Jeff Layton
                           ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Steve French @ 2011-04-15 13:38 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel

On Fri, Apr 15, 2011 at 5:45 AM, Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2011/4/8 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> Update on cifs vs. smb2 mids, and the smb2 sendrcv2.   Jeff had
>> suggested more closely matching the cifs and smb2 mids, in particular
>> extending the 16 bit cifs mid (multiplex identifier for inflight
>> network requests) to the 64 bit size needed for smb2 (and thus masking
>> the mid when used for cifs) and having cifs ignore the various smb2
>> unique fields in the mid (which makes the mid larger for cifs).
>> Since the smb2 code in cifs-2.6.git (put in February and early March)
>> has now been rereviewed, the next step in the smb2 merge is posting
>> and reviewing the transport routine for smb2 (smb2_sendrcv2 or reusing
>> cifs_sendrcv2) - the latter may make more sense if we go to a common
>> mid for cifs and smb2.   At the fs summit, Jeff and Jeremy and I
>> talked about this, but Pavel and others may have opinions on this
>> topic.   As soon as the cifs merge activity settles down for 2.6.39, I
>> plan to post sendrcv2 alternatives and then begin work with Pavel on
>> the superblock, file and inode routines and seeing whether for smb2
>> they should be smb2 unique (as we originally expected since smb2 is
>> handle based, and simpler) and look more like they did in the smb2.ko
>> work that Pavel did last summer or should be more common with the cifs
>> routines.
>>
>> --
>> Thanks,
>>
>> Steve
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> I suggest to make cifs and smb2 protocol mid structures use common
> structure that has equals fields for both and then expand this
> structure for protocol-dependent things.
>
> It can look like this:
>
> /* one of these for every pending CIFS request to the server */
> struct mid_q_entry {
>        __u8 protocol_id;
>        struct list_head qhead; /* mids waiting on reply from this server */
>        int midState;   /* wish this were enum but can not pass to wait_event */
>        unsigned long when_alloc;  /* when mid was created */
> #ifdef CONFIG_CIFS_STATS2
>        unsigned long when_sent; /* time when smb send finished */
>        unsigned long when_received; /* when demux complete (taken off wire) */
> #endif
>        bool largeBuf:1;        /* if valid response, is pointer to large buf */
>        void *resp_buf;         /* response buffer */
>        mid_callback_t *callback; /* call completion callback */
>        void *callback_data;      /* general purpose pointer for callback */
> };
>
> struct cifs_mid_q_entry {
>        struct mid_q_entry mid_q;
>        __u16 mid;              /* multiplex id */
>        __u32 sequence_number;  /* for CIFS signing */
>        __u8 command;           /* smb command code */
>        __u16 pid;              /* process id */
>        bool multiRsp:1;        /* multiple trans2 responses for one request  */
>        bool multiEnd:1;        /* both received */
> };
>
> struct smb2_mid_q_entry {
>        struct mid_q_entry mid_q;
>        __u64 mid;              /* multiplex id(s), bigger for smb2 */
>        __le16 command;         /* smb2 command code */
>        __u32 pid;              /* process id - bigger for smb2 than cifs */
> };

I think nested mid structures (around a base of common mid fields)
like the above is going to be the easiest way to handle the differences.

I don't understand your PROTOCOL_ID #define though - we
identify smb2 vs. cifs via a bool in the tcp server info struct,
and presumably if we don't have access to the tcp server
info struct we would have to add a field in the base mid
(struct mid_q_entry) that indicates smb2 vs. cifs..


> So, we always work with a pointer to common structure mid_q_entry and
> then expand it according to protocol_id filed when we need it:
>
> #define PROTOCOL_ID(mid) (*((__u8 *)mid))
>
> process_mid(struct mid_q_entry *pmid)
> {
>        if (PROTOCOL_ID(pmid) == SMB2)
>                process_smb2_mid((struct smb2_mid_q_entry *)pmid);
>        else
>                process_cifs_mid((struct cifs_mid_q_entry *)pmid);
> }
>
>
> --
> Best regards,
> Pavel Shilovsky.
>



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
  2011-04-15 13:38       ` Steve French
@ 2011-04-15 15:21         ` Jeff Layton
       [not found]           ` <20110415112123.0a3c2996-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
       [not found]         ` <BANLkTiknBBtzjEjXK5N=SpUaU=b4GAos_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2011-04-22 10:33         ` Pavel Shilovsky
  2 siblings, 1 reply; 10+ messages in thread
From: Jeff Layton @ 2011-04-15 15:21 UTC (permalink / raw)
  To: Steve French; +Cc: Pavel Shilovsky, linux-cifs, linux-fsdevel

On Fri, 15 Apr 2011 08:38:59 -0500
Steve French <smfrench@gmail.com> wrote:

> On Fri, Apr 15, 2011 at 5:45 AM, Pavel Shilovsky <piastryyy@gmail.com> wrote:
> > 2011/4/8 Steve French <smfrench@gmail.com>:
> >> Update on cifs vs. smb2 mids, and the smb2 sendrcv2.   Jeff had
> >> suggested more closely matching the cifs and smb2 mids, in particular
> >> extending the 16 bit cifs mid (multiplex identifier for inflight
> >> network requests) to the 64 bit size needed for smb2 (and thus masking
> >> the mid when used for cifs) and having cifs ignore the various smb2
> >> unique fields in the mid (which makes the mid larger for cifs).
> >> Since the smb2 code in cifs-2.6.git (put in February and early March)
> >> has now been rereviewed, the next step in the smb2 merge is posting
> >> and reviewing the transport routine for smb2 (smb2_sendrcv2 or reusing
> >> cifs_sendrcv2) - the latter may make more sense if we go to a common
> >> mid for cifs and smb2.   At the fs summit, Jeff and Jeremy and I
> >> talked about this, but Pavel and others may have opinions on this
> >> topic.   As soon as the cifs merge activity settles down for 2.6.39, I
> >> plan to post sendrcv2 alternatives and then begin work with Pavel on
> >> the superblock, file and inode routines and seeing whether for smb2
> >> they should be smb2 unique (as we originally expected since smb2 is
> >> handle based, and simpler) and look more like they did in the smb2.ko
> >> work that Pavel did last summer or should be more common with the cifs
> >> routines.
> >>
> >> --
> >> Thanks,
> >>
> >> Steve
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >
> > I suggest to make cifs and smb2 protocol mid structures use common
> > structure that has equals fields for both and then expand this
> > structure for protocol-dependent things.
> >
> > It can look like this:
> >
> > /* one of these for every pending CIFS request to the server */
> > struct mid_q_entry {
> >        __u8 protocol_id;
> >        struct list_head qhead; /* mids waiting on reply from this server */
> >        int midState;   /* wish this were enum but can not pass to wait_event */
> >        unsigned long when_alloc;  /* when mid was created */
> > #ifdef CONFIG_CIFS_STATS2
> >        unsigned long when_sent; /* time when smb send finished */
> >        unsigned long when_received; /* when demux complete (taken off wire) */
> > #endif
> >        bool largeBuf:1;        /* if valid response, is pointer to large buf */
> >        void *resp_buf;         /* response buffer */
> >        mid_callback_t *callback; /* call completion callback */
> >        void *callback_data;      /* general purpose pointer for callback */
> > };
> >
> > struct cifs_mid_q_entry {
> >        struct mid_q_entry mid_q;
> >        __u16 mid;              /* multiplex id */
> >        __u32 sequence_number;  /* for CIFS signing */
> >        __u8 command;           /* smb command code */
> >        __u16 pid;              /* process id */
> >        bool multiRsp:1;        /* multiple trans2 responses for one request  */
> >        bool multiEnd:1;        /* both received */
> > };
> >
> > struct smb2_mid_q_entry {
> >        struct mid_q_entry mid_q;
> >        __u64 mid;              /* multiplex id(s), bigger for smb2 */
> >        __le16 command;         /* smb2 command code */
> >        __u32 pid;              /* process id - bigger for smb2 than cifs */
> > };
> 
> I think nested mid structures (around a base of common mid fields)
> like the above is going to be the easiest way to handle the differences.
> 
> I don't understand your PROTOCOL_ID #define though - we
> identify smb2 vs. cifs via a bool in the tcp server info struct,
> and presumably if we don't have access to the tcp server
> info struct we would have to add a field in the base mid
> (struct mid_q_entry) that indicates smb2 vs. cifs..
> 

Why do even that? Why not a common mid struct that's simply a little larger? Sure it means slightly more memory consumption, but we never have that many in flight. Something like:

struct mid_q_entry {
        __u8 protocol_id;
        struct list_head qhead; /* mids waiting on reply from this server */
        int midState;   /* wish this were enum but can not pass to wait_event */
        unsigned long when_alloc;  /* when mid was created */
#ifdef CONFIG_CIFS_STATS2
        unsigned long when_sent; /* time when smb send finished */
        unsigned long when_received; /* when demux complete (taken off wire) */
#endif
        bool largeBuf:1;        /* if valid response, is pointer to large buf */
        void *resp_buf;         /* response buffer */
        mid_callback_t *callback; /* call completion callback */
        void *callback_data;      /* general purpose pointer for callback */
 	__u64 mid;              /* multiplex id */
        __le16 command;         /* command code */
        __u32 pid;              /* process id */
	__u32 sequence_number;  /* for CIFS signing */
        bool multiRsp:1;        /* multiple trans2 responses for one request  */
        bool multiEnd:1;        /* both received */
};

Using different structs here means that you need an entirely different
set of code to deal with them. IMO, the memory savings (if any) isn't
worth the duplicate code that'll be necessary.

-- 
Jeff Layton <jlayton@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
       [not found]         ` <BANLkTiknBBtzjEjXK5N=SpUaU=b4GAos_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-04-15 18:41           ` Pavel Shilovsky
  0 siblings, 0 replies; 10+ messages in thread
From: Pavel Shilovsky @ 2011-04-15 18:41 UTC (permalink / raw)
  To: Steve French; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel

2011/4/15 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> I think nested mid structures (around a base of common mid fields)
> like the above is going to be the easiest way to handle the differences.
>
> I don't understand your PROTOCOL_ID #define though - we
> identify smb2 vs. cifs via a bool in the tcp server info struct,
> and presumably if we don't have access to the tcp server
> info struct we would have to add a field in the base mid
> (struct mid_q_entry) that indicates smb2 vs. cifs..
>
>
Good variant too. I don't it really matters how to get protocol
version - I just propose an idea how to organize structures.


-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
       [not found]           ` <20110415112123.0a3c2996-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
@ 2011-04-22 10:26             ` Pavel Shilovsky
  0 siblings, 0 replies; 10+ messages in thread
From: Pavel Shilovsky @ 2011-04-22 10:26 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel

2011/4/15 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> On Fri, 15 Apr 2011 08:38:59 -0500
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Fri, Apr 15, 2011 at 5:45 AM, Pavel Shilovsky <piastryyy-Re5JQEeQqe8@public.gmane.orgm> wrote:
>> > 2011/4/8 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> >> Update on cifs vs. smb2 mids, and the smb2 sendrcv2.   Jeff had
>> >> suggested more closely matching the cifs and smb2 mids, in particular
>> >> extending the 16 bit cifs mid (multiplex identifier for inflight
>> >> network requests) to the 64 bit size needed for smb2 (and thus masking
>> >> the mid when used for cifs) and having cifs ignore the various smb2
>> >> unique fields in the mid (which makes the mid larger for cifs).
>> >> Since the smb2 code in cifs-2.6.git (put in February and early March)
>> >> has now been rereviewed, the next step in the smb2 merge is posting
>> >> and reviewing the transport routine for smb2 (smb2_sendrcv2 or reusing
>> >> cifs_sendrcv2) - the latter may make more sense if we go to a common
>> >> mid for cifs and smb2.   At the fs summit, Jeff and Jeremy and I
>> >> talked about this, but Pavel and others may have opinions on this
>> >> topic.   As soon as the cifs merge activity settles down for 2.6.39, I
>> >> plan to post sendrcv2 alternatives and then begin work with Pavel on
>> >> the superblock, file and inode routines and seeing whether for smb2
>> >> they should be smb2 unique (as we originally expected since smb2 is
>> >> handle based, and simpler) and look more like they did in the smb2.ko
>> >> work that Pavel did last summer or should be more common with the cifs
>> >> routines.
>> >>
>> >> --
>> >> Thanks,
>> >>
>> >> Steve
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> >> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >>
>> >
>> > I suggest to make cifs and smb2 protocol mid structures use common
>> > structure that has equals fields for both and then expand this
>> > structure for protocol-dependent things.
>> >
>> > It can look like this:
>> >
>> > /* one of these for every pending CIFS request to the server */
>> > struct mid_q_entry {
>> >        __u8 protocol_id;
>> >        struct list_head qhead; /* mids waiting on reply from this server */
>> >        int midState;   /* wish this were enum but can not pass to wait_event */
>> >        unsigned long when_alloc;  /* when mid was created */
>> > #ifdef CONFIG_CIFS_STATS2
>> >        unsigned long when_sent; /* time when smb send finished */
>> >        unsigned long when_received; /* when demux complete (taken off wire) */
>> > #endif
>> >        bool largeBuf:1;        /* if valid response, is pointer to large buf */
>> >        void *resp_buf;         /* response buffer */
>> >        mid_callback_t *callback; /* call completion callback */
>> >        void *callback_data;      /* general purpose pointer for callback */
>> > };
>> >
>> > struct cifs_mid_q_entry {
>> >        struct mid_q_entry mid_q;
>> >        __u16 mid;              /* multiplex id */
>> >        __u32 sequence_number;  /* for CIFS signing */
>> >        __u8 command;           /* smb command code */
>> >        __u16 pid;              /* process id */
>> >        bool multiRsp:1;        /* multiple trans2 responses for one request  */
>> >        bool multiEnd:1;        /* both received */
>> > };
>> >
>> > struct smb2_mid_q_entry {
>> >        struct mid_q_entry mid_q;
>> >        __u64 mid;              /* multiplex id(s), bigger for smb2 */
>> >        __le16 command;         /* smb2 command code */
>> >        __u32 pid;              /* process id - bigger for smb2 than cifs */
>> > };
>>
>> I think nested mid structures (around a base of common mid fields)
>> like the above is going to be the easiest way to handle the differences.
>>
>> I don't understand your PROTOCOL_ID #define though - we
>> identify smb2 vs. cifs via a bool in the tcp server info struct,
>> and presumably if we don't have access to the tcp server
>> info struct we would have to add a field in the base mid
>> (struct mid_q_entry) that indicates smb2 vs. cifs..
>>
>
> Why do even that? Why not a common mid struct that's simply a little larger? Sure it means slightly more memory consumption, but we never have that many in flight. Something like:
>
> struct mid_q_entry {
>        __u8 protocol_id;
>        struct list_head qhead; /* mids waiting on reply from this server */
>        int midState;   /* wish this were enum but can not pass to wait_event */
>        unsigned long when_alloc;  /* when mid was created */
> #ifdef CONFIG_CIFS_STATS2
>        unsigned long when_sent; /* time when smb send finished */
>        unsigned long when_received; /* when demux complete (taken off wire) */
> #endif
>        bool largeBuf:1;        /* if valid response, is pointer to large buf */
>        void *resp_buf;         /* response buffer */
>        mid_callback_t *callback; /* call completion callback */
>        void *callback_data;      /* general purpose pointer for callback */
>        __u64 mid;              /* multiplex id */
>        __le16 command;         /* command code */
>        __u32 pid;              /* process id */
>        __u32 sequence_number;  /* for CIFS signing */
>        bool multiRsp:1;        /* multiple trans2 responses for one request  */
>        bool multiEnd:1;        /* both received */
> };
>
> Using different structs here means that you need an entirely different
> set of code to deal with them. IMO, the memory savings (if any) isn't
> worth the duplicate code that'll be necessary.
>

When we need to work only with common field we can always assign it to
a common mid_q_entry pointer and work with it. But when we need to
work with protocol dependent fields we will do check like (in both
variants - yours and mine)

if (protocol == SMB2)
    smb2_func
else
    cifs_func

So, we don't need entirely different set of code. We will have common
routines as well as cifs and smb2 specific ones. It seems that you
suggest the same - I understand so. Please, sorry me, if I am
mistaken.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
  2011-04-15 13:38       ` Steve French
  2011-04-15 15:21         ` Jeff Layton
       [not found]         ` <BANLkTiknBBtzjEjXK5N=SpUaU=b4GAos_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-04-22 10:33         ` Pavel Shilovsky
  2011-04-22 11:26           ` Jeff Layton
  2011-04-22 11:45           ` Christoph Hellwig
  2 siblings, 2 replies; 10+ messages in thread
From: Pavel Shilovsky @ 2011-04-22 10:33 UTC (permalink / raw)
  To: Steve French; +Cc: linux-cifs, linux-fsdevel

2011/4/15 Steve French <smfrench@gmail.com>:
> I think nested mid structures (around a base of common mid fields)
> like the above is going to be the easiest way to handle the differences.
>
> I don't understand your PROTOCOL_ID #define though - we
> identify smb2 vs. cifs via a bool in the tcp server info struct,
> and presumably if we don't have access to the tcp server
> info struct we would have to add a field in the base mid
> (struct mid_q_entry) that indicates smb2 vs. cifs..

I've just thought about to share tcp_server_info struct between
different protocol connections. So, we can use cifs and smb2
connection on the same server and don't keep two socket connections.
What do you think about to store protocol_id in ses_info rather that
tcp_server_info?

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
  2011-04-22 10:33         ` Pavel Shilovsky
@ 2011-04-22 11:26           ` Jeff Layton
  2011-04-22 11:45           ` Christoph Hellwig
  1 sibling, 0 replies; 10+ messages in thread
From: Jeff Layton @ 2011-04-22 11:26 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: Steve French, linux-cifs, linux-fsdevel

On Fri, 22 Apr 2011 14:33:08 +0400
Pavel Shilovsky <piastryyy@gmail.com> wrote:

> 2011/4/15 Steve French <smfrench@gmail.com>:
> > I think nested mid structures (around a base of common mid fields)
> > like the above is going to be the easiest way to handle the differences.
> >
> > I don't understand your PROTOCOL_ID #define though - we
> > identify smb2 vs. cifs via a bool in the tcp server info struct,
> > and presumably if we don't have access to the tcp server
> > info struct we would have to add a field in the base mid
> > (struct mid_q_entry) that indicates smb2 vs. cifs..
> 
> I've just thought about to share tcp_server_info struct between
> different protocol connections. So, we can use cifs and smb2
> connection on the same server and don't keep two socket connections.
> What do you think about to store protocol_id in ses_info rather that
> tcp_server_info?
> 

I don't think that's actually allowed by the protocol. Once a socket
has had a NEGOTIATE done on it, you can't go back and re-do another one.

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
  2011-04-22 10:33         ` Pavel Shilovsky
  2011-04-22 11:26           ` Jeff Layton
@ 2011-04-22 11:45           ` Christoph Hellwig
       [not found]             ` <20110422114526.GA31546-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
  1 sibling, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2011-04-22 11:45 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: Steve French, linux-cifs, linux-fsdevel

On Fri, Apr 22, 2011 at 02:33:08PM +0400, Pavel Shilovsky wrote:
> I've just thought about to share tcp_server_info struct between
> different protocol connections. So, we can use cifs and smb2
> connection on the same server and don't keep two socket connections.
> What do you think about to store protocol_id in ses_info rather that
> tcp_server_info?

What about prototyping it either first, and then review an actually
complete smb2 implementation on a branch / as a patchkit do decide
what makes most sense?


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mids and cifs sendrcv2
       [not found]             ` <20110422114526.GA31546-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2011-04-26 19:40               ` Pavel Shilovsky
  0 siblings, 0 replies; 10+ messages in thread
From: Pavel Shilovsky @ 2011-04-26 19:40 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel

2011/4/22 Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>:
> On Fri, Apr 22, 2011 at 02:33:08PM +0400, Pavel Shilovsky wrote:
>> I've just thought about to share tcp_server_info struct between
>> different protocol connections. So, we can use cifs and smb2
>> connection on the same server and don't keep two socket connections.
>> What do you think about to store protocol_id in ses_info rather that
>> tcp_server_info?
>
> What about prototyping it either first, and then review an actually
> complete smb2 implementation on a branch / as a patchkit do decide
> what makes most sense?

Yes, you are right. I am going to create such a branch and start to
merge smb2 code there.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-04-26 19:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-08 16:39 mids and cifs sendrcv2 Steve French
     [not found] ` <BANLkTi=tUrieqB4jCo0GRX3Np10JxJUP9Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-04-15 10:45   ` Pavel Shilovsky
     [not found]     ` <BANLkTikLfy=4BPESJEmOe+mBhKSRtC=W6A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-04-15 13:38       ` Steve French
2011-04-15 15:21         ` Jeff Layton
     [not found]           ` <20110415112123.0a3c2996-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2011-04-22 10:26             ` Pavel Shilovsky
     [not found]         ` <BANLkTiknBBtzjEjXK5N=SpUaU=b4GAos_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-04-15 18:41           ` Pavel Shilovsky
2011-04-22 10:33         ` Pavel Shilovsky
2011-04-22 11:26           ` Jeff Layton
2011-04-22 11:45           ` Christoph Hellwig
     [not found]             ` <20110422114526.GA31546-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2011-04-26 19:40               ` Pavel Shilovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.