On Sat, Oct 29, 2022 at 6:18 PM Tom Talpey <tom@xxxxxxxxxx> wrote: > > On 10/28/2022 10:29 PM, Steve French wrote: > > thx for testing this - Shyam's fix for it looks promising (needs > > review though and some testing) > > Good, because the original fix confuses me deeply. A tcon is not tied > to a connection, or at least, it never should be. It's a property of > the session, which is shared among multichannel connections. So if we > have to poke at other connections to find a tcon, that's a fundamental > issue. > > Shyam's fix always simply looks on the primary channel, which works, > but it's weird relative to the protocol. The real fix would be to hang > the tcon off the proper object. The reference to the primary channel there is to just get to the session (and from there, the tcon) lists, which only hang off the primary channel today. I'm not sure why this choice was made by Aurelien originally, but that is how it can be fixed with minimal changes today. Ideally, each channel should point to the session list (taking the necessary reference), and hence the tcons. Also, we should be able to share connections between sessions. Today, the secondary channels are used exclusively by one session. I plan to get to that next. > > Ronnie asked: > > > What does MS-SMB2.pdf say about channels and oplocks? > > Oplocks are not tied to channels, they're tied to sessions, tree > connects and files. An oplock can be granted on one channel and be > valid client-wide. And broken on any channel too btw. > > Tom. > > > On Fri, Oct 28, 2022 at 5:30 AM Namjae Jeon <linkinjeon@xxxxxxxxxx> wrote: > >> > >> 2022-10-28 18:19 GMT+09:00, ronnie sahlberg <ronniesahlberg@xxxxxxxxx>: > >>> On Fri, 28 Oct 2022 at 16:55, ronnie sahlberg <ronniesahlberg@xxxxxxxxx> > >>> wrote: > >>>> > >>>> On Fri, 28 Oct 2022 at 16:53, Steve French <smfrench@xxxxxxxxx> wrote: > >>>>> > >>>>> I haven't tried a scenario to windows where we turn off leases and force > >>>>> server to use oplocks but with ksmbd that is the default. > >>>>> Worth also investigating how primary vs secondary works for finding > >>>>> leases for windows case > >>>> > >>>> Yes. Until we know what/how windows does things and what ms-smb2.pdf > >>>> says we can not know if this is a cifs client bug or a ksmbd bug. > >>> > >>> So lets wait with this patch until we know where the bug is. > >>> I will review it later for locking correctness, but lets make sure it > >>> is not a ksmbd bug first. > >> We can reproduce it against samba with the following parameters. > >> > >> server multi channel support = yes > >> oplock break wait time = 35000 > >> smb2 leases = no > >> > >>> > >>> > >>>> > >>>> > >>>>> > >>>>> On Fri, Oct 28, 2022, 01:48 ronnie sahlberg <ronniesahlberg@xxxxxxxxx> > >>>>> wrote: > >>>>>> > >>>>>> On Fri, 28 Oct 2022 at 16:25, Steve French <smfrench@xxxxxxxxx> wrote: > >>>>>>> > >>>>>>> If a mount to a server is using multichannel, an oplock break > >>>>>>> arriving > >>>>>>> on a secondary channel won't find the open file (since it won't find > >>>>>>> the > >>>>>>> tcon for it), and this will cause each oplock break on secondary > >>>>>>> channels > >>>>>>> to time out, slowing performance drastically. > >>>>>>> > >>>>>>> Fix smb2_is_valid_oplock_break so that if it is a secondary channel > >>>>>>> and > >>>>>>> an oplock break was not found, check for tcons (and the files > >>>>>>> hanging > >>>>>>> off the tcons) on the primary channel. > >>>>>> > >>>>>> Does this also happen against windows or is is only against ksmbd this > >>>>>> triggers? > >>>>>> What does MS-SMB2.pdf say about channels and oplocks? > >>>>>> > >>>>>>> > >>>>>>> Fixes xfstest generic/013 to ksmbd > >>>>>>> > >>>>>>> Cc: <stable@xxxxxxxxxxxxxxx> > >>>>>>> > >>>>>>> > >>>>>>> -- > >>>>>>> Thanks, > >>>>>>> > >>>>>>> Steve > >>> > > > > > > -- Regards, Shyam
