Re: [PATCH 4/4] cifs: Reformat DebugData and index connections by conn_id.

From: Steve French <smfrench@gmail.com>
To: Shyam Prasad N <nspmangalore@gmail.com>
Cc: "Tom Talpey" <tom@talpey.com>,
	"Pavel Shilovsky" <piastryyy@gmail.com>,
	"Aurélien Aptel" <aaptel@suse.com>,
	CIFS <linux-cifs@vger.kernel.org>,
	"ronnie sahlberg" <ronniesahlberg@gmail.com>
Subject: Re: [PATCH 4/4] cifs: Reformat DebugData and index connections by conn_id.
Date: Tue, 16 Feb 2021 13:56:26 -0600	[thread overview]
Message-ID: <CAH2r5muAhJaVTkGnzgTKXhLKaEXocgSk-WOguA4KkZWb5rMraQ@mail.gmail.com> (raw)
In-Reply-To: <CANT5p=o4=uy8HV4L_nXfPUJ=bUO5Oyf8p6=Y7dY5PxsabHxJYQ@mail.gmail.com>

On Tue, Feb 16, 2021 at 6:29 AM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> Hi Pavel,
>
> Thanks for the review.
> As Tom pointed out, the server name is currently a field in
> TCP_Session_Info struct.
> We do store the struct sockaddr_storage, which I'm guessing holds the
> IP address in binary format, and we could use that. And may need to
> consider IPv4 vs IPv6 when we do it.
> I'll submit that as a new patch later on.
>
> Hi Tom,
>
> > Including the transport type (TCP, RDMA...) and multichannel attributes
> > (link speed, RSS count, ...) would be useful too.
> Can you please clarify this for me?
> From what I can see from the code, RDMA connection DebugData is a
> superset of TCP connection. The RDMA specific details get printed only
> when server->rdma is set.
>
> Regards,
> Shyam
>
> On Thu, Feb 11, 2021 at 11:41 AM Tom Talpey <tom@talpey.com> wrote:
> >
> > On 2/11/2021 12:12 PM, Pavel Shilovsky wrote:
> > > Hi Shyam,
> > >
> > > The output looks very informative! I have one comment:
> > >
> > > Servers:
> > > 1) ConnectionId: 0x1
> > > Number of credits: 326 Dialect 0x311
> > > TCP status: 1 Instance: 1
> > > Local Users To Server: 1 SecMode: 0x1 Req On Wire: 0
> > > In Send: 0 In MaxReq Wait: 0
> > >
> > > Sessions:
> > > 1) Name: 10.229.158.38 Uses: 1 Capability: 0x300077 Session Status: 1
> > >                       ^^^^
> > > Isn't this name (or hostname) a property of the connection? I would
> > > expect an IP or a hostname to be printed in the connection settings
> > > above.
> >
> > The servername is a property of the session, in this case since the
> > mount specified a dotted quad, it would correctly appear as the
> > servername at this level.
> >
> > However, I definitely agree that an IP address is important in the
> > per-connection (channel) stanzas. Multichannel, multihoming, witness
> > redirects, and any number of things can vary among them. It would
> > be useful indeed to display them.
> >
> > Including the transport type (TCP, RDMA...) and multichannel attributes
> > (link speed, RSS count, ...) would be useful too.

It does show whether interface supports RSS or RDMA in the channel
list for every session
(and whether that interface is 'CONNECTED' for that session).  See
below example from his
sample /proc/fs/cifs/DebugData output (although this part did not
change with his patch)

4) Speed: 1000000000 bps
Capabilities: rss
IPv4: 10.229.158.38
[CONNECTED]

Thanks,

Steve