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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/16/2021 2:56 PM, Steve French wrote:
On Tue, Feb 16, 2021 at 6:29 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> 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.

I would hope that the client would walk its list of connections and
report the local attributes such as source+destination addresses and
ports, along with the transport such as whether it's a TCP or RDMA
connection.

Here, for example (from your patch):

...cifs_dump_channel(struct seq_file *m, int i, struct cifs_chan *chan)
...
+	seq_printf(m, "\n\n\t\tChannel: %d ConnectionId: 0x%llx"
+		   "\n\t\tNumber of credits: %d Dialect 0x%x"
+		   "\n\t\tTCP status: %d Instance: %d"
+		   "\n\t\tLocal Users To Server: %d SecMode: 0x%x Req On Wire: %d"
+		   "\n\t\tIn Send: %d In MaxReq Wait: %d",
+		   i+1, server->conn_id,

I'd strongly suggest digging into the cifs->chan  and adding at least
some of the connection data.

In the second hunk, I believe that's only invoked if CONFIG_CIFS_SMB_DIRECT
is configured, and it doesn't have any of the actual connection
information either, although it does dump the rdma flag.

Bottom line, it seems to me that a more complete picture of
the endpoints would help a lot, especially when multichannel
allows multiple sockets, NICs, and protocols to be involved.

Regards,
Shyam

On Thu, Feb 11, 2021 at 11:41 AM Tom Talpey <tom@xxxxxxxxxx> 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]

Steve, I believe this list is just the server's response to the
multichannel FSCTL_QUERY_NETWORK_INTERFACE_INFO, which is an
enumeration of *available* server-side interfaces? This is useful,
but isn't helpful to determine which interfaces the client session
is actually using to each one, and from what client NIC ports,
protocols, etc. All it gives is a "CONNECTED" indication to a
single server port. To figure anything else out means groveling
through netstat and guessing at port 445's.

Tom.


Thanks,

Steve




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux