Skip to content

Add more info to iscsi dump info #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Add more info to iscsi dump info #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b2be43
Add more info to iscsi dump info
gulams Jan 7, 2025
c070415
Correct the name of test function
gulams Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
243 changes: 175 additions & 68 deletions drgn_tools/iscsi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@
"""
import argparse
from typing import Iterator
from typing import Dict
from typing import Any

import drgn
import socket
from drgn import cast

from drgn import Object
from drgn import Program
Expand All @@ -21,8 +27,83 @@
from drgn_tools.table import print_dictionary
from drgn_tools.util import enum_name_get

ISCSI_SESSION_STATES = ["LOGGED_IN", "FAILED", "FREE"]
# Available Hardware and Software iSCSI Transports
iscsi_transports = ["bnx2i",
"cxgb3i",
"cxgb4i",
"iser",
"qedi",
"bfin dpmc",
"h1940-bt",
"tcp"]
Comment on lines +30 to +38
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constant is unused


# iSCSI Session States
iscsi_session_states = ["UNKNOWN",
"FREE",
"LOGGED_IN",
"FAILED",
"TERMINATE",
"IN_RECOVERY",
"RECOVERY_FAILED",
"LOGGING_OUT"
]
Comment on lines +41 to +49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First: please make all global variables named in ALL_CAPS, since it makes reading the code that uses them much easier. This should apply for the rest of the globals here.

Second: These states appear to be an enumeration from libiscsi.h. For enums, the best thing to do is rely on the kernel's debuginfo to contain the enumeration constants. Sadly, libiscsi.h makes these enumerations anonymous, so it's slightly difficult to get them. Here's an example:

from typing import Dict, Type
from drgn import Program
from drgn.helpers.common import enum_type_to_class

def iscsi_constants(prog: Program) -> Dict[str, Type]:
    return {
        "ISCSI_STATE": enum_type_to_class(
            prog.constant("ISCSI_STATE_FREE").type_,
            "IscsiState",
            prefix="ISCSI_STATE",
        ),
        ...
    }

You can do this for each enum: choose the most common variant (the one whose name is least likely to change), and then create the Python class with enum_type_to_class().

Once you have the python enum class corresponding to it, it will be quite easy to just lookup the name. For instance:

state = 5  # in reality, this would come from the program
consts = iscsi_constants(prog)
value = consts(state)
print(str(value))

That goes for all these lists of constants below - look them up from the program and create them dynamically. That way if any constants change in any kernel version, you'll get the right values.


# iSCSI Connection Stages
iscsi_connection_stages = ["INITIAL_STAGE",
"STARTED",
"STOPPED",
"CLEANUP_WAIT"
]
# iSCSI Connection States
iscsi_connection_states = ["UP",
"DOWN",
"FAILED",
"BOUND"
]

# SCSI device States
scsi_device_states = ["unknown",
"created",
"running",
"cancel",
"del",
"quiesce",
"offline",
"transport offline",
"blocked",
"created block"
]

# Scsi_Host states
scsi_host_states = ["UNKNOWN",
"CREATED",
"RUNNING",
"CANCEL",
"DEL",
"RECOVERY",
"CANCEL_RECOVERY",
"DEL_RECOVERY"
]
# Sub-Headings
sub_headings = ["Interface",
"CHAP",
"Timeouts",
"Negotiated iSCSI params",
"Attached SCSI devices"
]

def print_iscsi_info(info: Dict[str, Any]) -> None:
for line in info:
if "Target IQN" in line:
print(f"{line}: {info[line]}")
elif line == "Current Portal" or line == "Persistent Portal":
print(f"\t{line}: {info[line]}")
elif any(line in s for s in sub_headings):
print(f"\t\t"+"*" * (len(line) + 1))
print(f"\t\t{line}: {info[line]}")
print(f"\t\t"+"*" * (len(line) + 1))
else:
print(f"\t\t{line}: {info[line]}")
Comment on lines +88 to +106
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like seeing a separation between helpers that get information, and those that print information. That makes it easier for others to write new code based on your helpers. So I do like what you're trying to do here.

But I'm not certain that it's really helping to have a print_iscsi_info() function and a get_iscsi_info() here, for a few reasons.

  1. The dictionary returned by get_iscsi_info() is just a bunch of strings. It seems to me that if a developer wanted to get the timeout associated with an iscsi_session, they would want it as a numeric value, not a string that they would need to interpret. So if you're going to make a get_iscsi_info() function, it should return data in a form that a developer would like, not in the form that you want to print it.
  2. The use of blank "header" keys is a confusing design choice, that can easily go out-of-date if somebody updates the get_iscsi_info() function, but forgets to update the list of sub_headings.

I think there are two ways to go about this. Either change get_iscsi_info() to just be print_iscsi_info(), so that it directly prints the data without sticking it into a dict first, or else change get_iscsi_info() to return the data in a developer-friendly format (e.g. a dataclass or named tuple) and then format it dircetly in the print_iscsi_info() function.

For an example of the latter, see get_kmem_cache_slub_info() in drgn_tools/slabinfo.py. It returns a named tuple that is later formatted (as part of a table).


def for_each_iscsi_host(prog: Program) -> Iterator[Object]:
"""
Expand Down Expand Up @@ -62,8 +143,95 @@ def for_each_iscsi_session(prog: Program) -> Iterator[Object]:
address=cls_session.dd_data,
)

def get_iscsi_info(iscsi_conn: Object, prog: Program ) -> Dict[str, Any]:

def print_iscsi_sessions(prog: Program) -> None:
iscsi_session = iscsi_conn.session
iscsi_cls_conn = iscsi_conn.cls_conn
iscsi_cls_session = iscsi_session.cls_session
iscsi_tcp_conn = cast("struct iscsi_tcp_conn *", iscsi_conn.dd_data)
iscsi_sw_tcp_conn = cast("struct iscsi_sw_tcp_conn *", iscsi_tcp_conn.dd_data)
iscsi_socket = iscsi_sw_tcp_conn.sock
iscsi_sock = iscsi_socket.sk
scsi_host = iscsi_session.host
iscsi_host_data = scsi_host.hostdata
iscsi_host = cast("struct iscsi_host *", iscsi_host_data)

try:
if iscsi_sock.__sk_common.skc_family == 2:
current_address = iscsi_sock.__sk_common.skc_daddr
current_address = prog.read_u32(current_address.address_of_())
current_ip = socket.inet_ntoa(bytes([(current_address) & 0xFF,
(current_address >> 8) & 0xFF,
(current_address >> 16) & 0xFF,
(current_address >> 24) & 0xFF]))
current_port = iscsi_sock.__sk_common.skc_dport
current_port = prog.read_u16(current_port.address_of_())
current_port = socket.ntohs(current_port)
local_address = iscsi_sock.__sk_common.skc_rcv_saddr
local_address = prog.read_u32(local_address.address_of_())
local_ip = socket.inet_ntoa(bytes([(local_address) & 0xFF,
(local_address >> 8) & 0xFF,
(local_address >> 16) & 0xFF,
(local_address >> 24) & 0xFF]))
Comment on lines +161 to +175
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For IP addresses, you should use get_ipv4() which is defined in drgn_tools/crash_net.py, see its use there. The kernel always stores the IP addresses in network byte order, so as long as you use that function, you don't need to do byte swapping.

For the port, only do the byte swap if the program is little endian. (which is true for all the architectures we use, but we really should do it correctly). For example, see these lines in the crash_net.py file:

            dport = int(skc.skc_dport)
            if prog.platform.flags & PlatformFlags.IS_LITTLE_ENDIAN:
                dport = socketlib.ntohs(dport)

except drgn.FaultError:
current_ip = "[default]"
local_ip = "[default]"
current_port = ""
return {
"Target IQN":escape_ascii_string(iscsi_session.targetname.string_()),
"Current Portal":(current_ip+":"+str(current_port)),
"Persistent Portal": escape_ascii_string(iscsi_conn.persistent_address.string_())+
":"+str(iscsi_conn.persistent_port.value_())+","+
str(iscsi_session.tpgt.value_()),
"Interface":"",
"Iface Name":escape_ascii_string(iscsi_session.ifacename.string_()),
"Iface Transport":escape_ascii_string(iscsi_cls_session.transport.name.string_()),
"Iface Initiatorname": escape_ascii_string(iscsi_session.initiatorname.string_()),
"Iface IPaddress": local_ip,
"Iface HWaddress": escape_ascii_string(iscsi_host.hwaddress.string_()) if iscsi_host.hwaddress else "default",
"Iface Netdev": escape_ascii_string(iscsi_host.netdev.string_()) if iscsi_host.netdev else "default",
"SID": iscsi_cls_session.sid.value_(),
"iSCSI Connection State": iscsi_connection_states[iscsi_cls_conn.state.value_()],
"iSCSI Session State": iscsi_session_states[iscsi_session.state.value_()],
"Internal iscsid Session State": iscsi_session_states[iscsi_session.state.value_()],
"Timeouts":"",
"Recovery Timeout": iscsi_cls_session.recovery_tmo.value_(),
"Target Reset Timeout": iscsi_session.tgt_reset_timeout.value_(),
"LUN Reset Timeout": iscsi_session.lu_reset_timeout.value_(),
"Abort Timeout": iscsi_session.abort_timeout.value_(),
"CHAP":"",
"username": (escape_ascii_string(iscsi_session.username.string_()) if iscsi_session.username else "<empty>"),
"password": (escape_ascii_string(iscsi_session.password.string_()) if iscsi_session.password else "********"),
"username_in": (escape_ascii_string(iscsi_session.username_in.string_()) if iscsi_session.username_in else "<empty>"),
"password_in": (escape_ascii_string(iscsi_session.password_in.string_()) if iscsi_session.password_in else "********"),
Comment on lines +203 to +206
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, what's the security implication of this? We should not print and include username/password combinations in corelens reports just because they are there. Do we have a valid debugging use case for including them? I would strongly discourage this unless absolutely necessary.

"Negotiated iSCSI params":"",
"HeaderDigest": (iscsi_conn.hdrdgst_en.value_() if iscsi_conn.hdrdgst_en else "None"),
"DataDigest": (iscsi_conn.datadgst_en.value_() if iscsi_conn.datadgst_en else "None"),
"MaxRecvDataSegmentLength": iscsi_conn.max_recv_dlength.value_(),
"MaxXmitDataSegmentLength": iscsi_conn.max_xmit_dlength.value_(),
"FirstBurstLength": iscsi_session.first_burst.value_(),
"MaxBurstLength": iscsi_session.max_burst.value_(),
"ImmediateData": ("Yes" if iscsi_session.imm_data_en.value_() else "No"),
"InitialR2T": ("Yes" if iscsi_session.initial_r2t_en.value_() else "No"),
"MaxOutstandingR2T": iscsi_session.max_r2t.value_(),
"Attached SCSI devices":"",
f"Host Number: {scsi_host.host_no.value_()} State": scsi_host_states[scsi_host.shost_state.value_()]
}

def get_iscsi_disks_info(scsi_device: Object, prog: Program) -> Dict[str, Any]:
scsi_host = scsi_device.host
lun = scsi_device.lun.value_()
host_no = scsi_host.host_no.value_()
Id = scsi_device.id.value_()
channel = scsi_device.channel.value_()
sdev_state = scsi_device_states[scsi_device.sdev_state.value_()]
sdev_name = scsi_device_name(prog, scsi_device)
return {
f"scsi{host_no} channel {channel} id {Id} Lun": lun,
f"\tAttached scsi disk {sdev_name}\tState": sdev_state
}

def dump_iscsi_sessions(prog: Program) -> None:
"""
Dump iscsi sessions.

Expand All @@ -73,76 +241,15 @@ def print_iscsi_sessions(prog: Program) -> None:
print(msg)
return

output = {}

for session in reversed(list(for_each_iscsi_session(prog))):
print("**********")
conn = session.leadconn

persistent_address = escape_ascii_string(
conn.persistent_address.string_()
)
persistent_port = int(conn.persistent_port)
output[
"Scsi_Host"
] = f"host{session.host.host_no.value_()} ({hex(session.host.value_())})"
output["Session"] = hex(session.address_of_())
output["SID"] = str(int(session.cls_session.sid))
output["Persistent Portal"] = f"{persistent_address}:{persistent_port}"
output["Iface Name"] = escape_ascii_string(session.ifacename.string_())
output["Session State"] = ISCSI_SESSION_STATES[
session.cls_session.state
]
connstate = enum_type_to_class(
prog.type("enum iscsi_connection_state"), "connstate"
)
output["Connection State"] = str(
enum_name_get(
connstate,
conn.cls_conn.state,
"UNKNOWN",
)
)
output["Initiatorname"] = escape_ascii_string(
session.initiatorname.string_()
)
output["Targetname"] = escape_ascii_string(
session.targetname.string_()
)

print_dictionary(output)

print("Attached SCSI devices: ")
print("**********")
print(
"Host Number: {} STATE: {}".format(
session.host.host_no.value_(),
session.host.shost_state.format_(type_name=False),
)
)
iscsi_info = get_iscsi_info(conn, prog)
print_iscsi_info(iscsi_info)

for scsi_dev in for_each_scsi_host_device(prog, session.host):
name = scsi_device_name(prog, scsi_dev)
print(
"scsi{} Channel {} Id {} Lun: {}".format(
session.host.host_no.value_(),
int(scsi_dev.channel),
int(scsi_dev.id),
int(scsi_dev.lun),
)
)
sdev_state = enum_type_to_class(
prog.type("enum scsi_device_state"), "sdev_state"
)
state = enum_name_get(
sdev_state,
scsi_dev.sdev_state,
"UNKNOWN",
)
print(" Attached iscsi disk: {} State: {}".format(name, state))

print()

iscsi_disks_info = get_iscsi_disks_info(scsi_dev, prog)
print_iscsi_info(iscsi_disks_info)
Comment on lines 250 to +252
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a table instead? It would probably make it easier to read. See the drgn_tools/table.py.


class Iscsi(CorelensModule):
"""
Expand All @@ -153,4 +260,4 @@ class Iscsi(CorelensModule):
skip_unless_have_kmod = ["libiscsi", "scsi_transport_iscsi"]

def run(self, prog: Program, args: argparse.Namespace) -> None:
print_iscsi_sessions(prog)
dump_iscsi_sessions(prog)
2 changes: 1 addition & 1 deletion tests/test_iscsi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@


def test_iscsi(prog):
iscsi.print_iscsi_sessions(prog)
iscsi.dump_iscsi_sessions(prog)
Loading