[PATCH] iscsi: Report connection state on sysfs

[Gabriel Krisman Bertazi]

If an iSCSI connection happens to fail while the daemon isn't
running (due to a crash or for another reason), the kernel failure
report is not received. When the daemon restarts, there is insufficient
kernel state in sysfs for it to know that this happened. open-iscsi
tries to reopen every connection, but on different initiators, we'd like
to know which connections have failed.

There is session->state, but that has a different lifetime than an iSCSI
connection, so it doesn't directly relflect the connection state.

Cc: Khazhismel Kumykov <kha...@google.com>
Suggested-by: Junho Ryu <ja...@google.com>
Signed-off-by: Gabriel Krisman Bertazi <kri...@collabora.com>
---
drivers/scsi/libiscsi.c | 7 ++++++-
drivers/scsi/scsi_transport_iscsi.c | 29 ++++++++++++++++++++++++++++-
include/scsi/scsi_transport_iscsi.h | 8 ++++++++
3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 70b99c0e2e67..ca488c57ead4 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3153,13 +3153,18 @@ void iscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)

switch (flag) {
case STOP_CONN_RECOVER:
+ cls_conn->state = ISCSI_CONN_FAILED;
+ break;
case STOP_CONN_TERM:
- iscsi_start_session_recovery(session, conn, flag);
+ cls_conn->state = ISCSI_CONN_DOWN;
break;
default:
iscsi_conn_printk(KERN_ERR, conn,
"invalid stop flag %d\n", flag);
+ return;
}
+
+ iscsi_start_session_recovery(session, conn, flag);
}
EXPORT_SYMBOL_GPL(iscsi_conn_stop);

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 17a45716a0fe..5b0e7df92964 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2276,6 +2276,7 @@ iscsi_create_conn(struct iscsi_cls_session *session, int dd_size, uint32_t cid)
INIT_LIST_HEAD(&conn->conn_list_err);
conn->transport = transport;
conn->cid = cid;
+ conn->state = ISCSI_CONN_DOWN;

/* this is released in the dev's release function */
if (!get_device(&session->dev))
@@ -3709,8 +3710,11 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
break;
case ISCSI_UEVENT_START_CONN:
conn = iscsi_conn_lookup(ev->u.start_conn.sid, ev->u.start_conn.cid);
- if (conn)
+ if (conn) {
ev->r.retcode = transport->start_conn(conn);
+ if (!ev->r.retcode)
+ conn->state = ISCSI_CONN_UP;
+ }
else
err = -EINVAL;
break;
@@ -3907,6 +3911,26 @@ iscsi_conn_attr(tcp_xmit_wsf, ISCSI_PARAM_TCP_XMIT_WSF);
iscsi_conn_attr(tcp_recv_wsf, ISCSI_PARAM_TCP_RECV_WSF);
iscsi_conn_attr(local_ipaddr, ISCSI_PARAM_LOCAL_IPADDR);

+static const struct {
+ int state;
+ char *name;
+} connection_state_name[] = {
+ {ISCSI_CONN_UP, "up"},
+ {ISCSI_CONN_DOWN, "down"},
+ {ISCSI_CONN_FAILED, "failed"}
+};
+
+static ssize_t
+show_conn_state(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct iscsi_cls_conn *conn = iscsi_dev_to_conn(dev->parent);
+
+ return sprintf(buf, "%s\n",
+ connection_state_name[conn->state].name);
+}
+static ISCSI_CLASS_ATTR(conn, state, S_IRUGO, show_conn_state,
+ NULL);

#define iscsi_conn_ep_attr_show(param) \
static ssize_t show_conn_ep_param_##param(struct device *dev, \
@@ -3976,6 +4000,7 @@ static struct attribute *iscsi_conn_attrs[] = {
&dev_attr_conn_tcp_xmit_wsf.attr,
&dev_attr_conn_tcp_recv_wsf.attr,
&dev_attr_conn_local_ipaddr.attr,
+ &dev_attr_conn_state.attr,
NULL,
};

@@ -4047,6 +4072,8 @@ static umode_t iscsi_conn_attr_is_visible(struct kobject *kobj,
param = ISCSI_PARAM_TCP_RECV_WSF;
else if (attr == &dev_attr_conn_local_ipaddr.attr)
param = ISCSI_PARAM_LOCAL_IPADDR;
+ else if (attr == &dev_attr_conn_state.attr)
+ return S_IRUGO;
else {
WARN_ONCE(1, "Invalid conn attr");
return 0;
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
index fa8814245796..b19138a4a774 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -188,6 +188,13 @@ extern void iscsi_ping_comp_event(uint32_t host_no,
uint32_t status, uint32_t pid,
uint32_t data_size, uint8_t *data);

+/* iscsi class connection state */
+enum {
+ ISCSI_CONN_UP,
+ ISCSI_CONN_DOWN,
+ ISCSI_CONN_FAILED,
+};
+
struct iscsi_cls_conn {
struct list_head conn_list; /* item in connlist */
struct list_head conn_list_err; /* item in connlist_err */
@@ -198,6 +205,7 @@ struct iscsi_cls_conn {
struct iscsi_endpoint *ep;

struct device dev; /* sysfs transport/container device */
+ int state;
};

#define iscsi_dev_to_conn(_dev) \
--
2.25.0

[Bart Van Assche]

On 3/4/20 2:57 PM, Gabriel Krisman Bertazi wrote:
> +static const struct {
> + int state;
> + char *name;
> +} connection_state_name[] = {
> + {ISCSI_CONN_UP, "up"},
> + {ISCSI_CONN_DOWN, "down"},
> + {ISCSI_CONN_FAILED, "failed"}
> +};
> +
> +static ssize_t
> +show_conn_state(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct iscsi_cls_conn *conn = iscsi_dev_to_conn(dev->parent);
> +
> + return sprintf(buf, "%s\n",
> + connection_state_name[conn->state].name);
> +}
> +static ISCSI_CLASS_ATTR(conn, state, S_IRUGO, show_conn_state,
> + NULL);

The above code can only work if ISCSI_CONN_UP == 0, ISCSI_CONN_DOWN == 1
and ISCSI_CONN_FAILED == 2. Please don't hardcode such a dependency and
use designated initializers instead.

Thanks,

Bart.

[Gabriel Krisman Bertazi]

If an iSCSI connection happens to fail while the daemon isn't
running (due to a crash or for another reason), the kernel failure
report is not received. When the daemon restarts, there is insufficient
kernel state in sysfs for it to know that this happened. open-iscsi
tries to reopen every connection, but on different initiators, we'd like
to know which connections have failed.

There is session->state, but that has a different lifetime than an iSCSI
connection, so it doesn't directly relflect the connection state.

Cc: Khazhismel Kumykov <kha...@google.com>
Suggested-by: Junho Ryu <ja...@google.com>
Signed-off-by: Gabriel Krisman Bertazi <kri...@collabora.com>
---

drivers/scsi/libiscsi.c | 7 +++++-
drivers/scsi/scsi_transport_iscsi.c | 38 ++++++++++++++++++++++++++++-
include/scsi/scsi_transport_iscsi.h | 8 ++++++
3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 70b99c0e2e67..ca488c57ead4 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3153,13 +3153,18 @@ void iscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)

switch (flag) {
case STOP_CONN_RECOVER:
+ cls_conn->state = ISCSI_CONN_FAILED;
+ break;
case STOP_CONN_TERM:
- iscsi_start_session_recovery(session, conn, flag);
+ cls_conn->state = ISCSI_CONN_DOWN;
break;
default:
iscsi_conn_printk(KERN_ERR, conn,
"invalid stop flag %d\n", flag);
+ return;
}
+
+ iscsi_start_session_recovery(session, conn, flag);
}
EXPORT_SYMBOL_GPL(iscsi_conn_stop);

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c

index 17a45716a0fe..9b6bd03b0963 100644

--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2276,6 +2276,7 @@ iscsi_create_conn(struct iscsi_cls_session *session, int dd_size, uint32_t cid)
INIT_LIST_HEAD(&conn->conn_list_err);
conn->transport = transport;
conn->cid = cid;
+ conn->state = ISCSI_CONN_DOWN;

/* this is released in the dev's release function */
if (!get_device(&session->dev))
@@ -3709,8 +3710,11 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
break;
case ISCSI_UEVENT_START_CONN:
conn = iscsi_conn_lookup(ev->u.start_conn.sid, ev->u.start_conn.cid);
- if (conn)
+ if (conn) {
ev->r.retcode = transport->start_conn(conn);
+ if (!ev->r.retcode)
+ conn->state = ISCSI_CONN_UP;
+ }
else
err = -EINVAL;
break;

@@ -3907,6 +3911,35 @@ iscsi_conn_attr(tcp_xmit_wsf, ISCSI_PARAM_TCP_XMIT_WSF);
iscsi_conn_attr(tcp_recv_wsf, ISCSI_PARAM_TCP_RECV_WSF);
iscsi_conn_attr(local_ipaddr, ISCSI_PARAM_LOCAL_IPADDR);

+static const struct {
+ int value;
+ char *name;
+} connection_state_names[] = {

+ {ISCSI_CONN_UP, "up"},
+ {ISCSI_CONN_DOWN, "down"},
+ {ISCSI_CONN_FAILED, "failed"}
+};
+

+static const char *connection_state_name(int state)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(connection_state_names); i++) {
+ if (connection_state_names[i].value == state)
+ return connection_state_names[i].name;
+ }
+ return NULL;
+}
+
+static ssize_t show_conn_state(struct device *dev,
+ struct device_attribute *attr, char *buf)

+{
+ struct iscsi_cls_conn *conn = iscsi_dev_to_conn(dev->parent);
+

+ return sprintf(buf, "%s\n", connection_state_name(conn->state));

+}
+static ISCSI_CLASS_ATTR(conn, state, S_IRUGO, show_conn_state,
+ NULL);

#define iscsi_conn_ep_attr_show(param) \
static ssize_t show_conn_ep_param_##param(struct device *dev, \

@@ -3976,6 +4009,7 @@ static struct attribute *iscsi_conn_attrs[] = {

&dev_attr_conn_tcp_xmit_wsf.attr,
&dev_attr_conn_tcp_recv_wsf.attr,
&dev_attr_conn_local_ipaddr.attr,
+ &dev_attr_conn_state.attr,
NULL,
};

@@ -4047,6 +4081,8 @@ static umode_t iscsi_conn_attr_is_visible(struct kobject *kobj,

param = ISCSI_PARAM_TCP_RECV_WSF;
else if (attr == &dev_attr_conn_local_ipaddr.attr)
param = ISCSI_PARAM_LOCAL_IPADDR;
+ else if (attr == &dev_attr_conn_state.attr)
+ return S_IRUGO;
else {
WARN_ONCE(1, "Invalid conn attr");
return 0;
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h

index fa8814245796..d710cf48b038 100644

--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -188,6 +188,13 @@ extern void iscsi_ping_comp_event(uint32_t host_no,
uint32_t status, uint32_t pid,
uint32_t data_size, uint8_t *data);

+/* iscsi class connection state */
+enum {

+ ISCSI_CONN_UP = 0,

[Bart Van Assche]

What has been changed in v2 compared to v1? Please always include a
changelog when posting a new version.

Additionally, it seems like the comment I posted on v1 has not been
addressed?

Thanks,

Bart.

[Gabriel Krisman Bertazi]

Hi Bart,

v2 no longer has the dependency for specific values for the state, as
you requested. It follows the pattern in similar places in the iscsi
code. Why would designated initializers be better than my solution?

--
Gabriel Krisman Bertazi

[Bart Van Assche]

Hi Gabriel,

How about removing the loop as follows?

static const char *const connection_state_names[] = {
[ISCSI_CONN_UP] = "up",
[ISCSI_CONN_DOWN] = "down",
[ISCSI_CONN_FAILED] = "failed",
};

...
if ((unsigned)conn->state < ARRAY_SIZE(connection_state_names))

return sprintf(buf, "%s\n",

connection_state_names[conn->state]);
else
...
...

Thanks,

Bart.

[Gabriel Krisman Bertazi]

Bart Van Assche <bvana...@acm.org> writes:

> How about removing the loop as follows?
>

Hey, Thanks for the review.

I understand designated initializers :-P

My point was following the pattern of the other statuses sysfs files in
the iSCSI layer.

But, I really don't mind. What do you think of v3 below?

-- >8 --
Subject: [PATCH v3] iscsi: Report connection state on sysfs

If an iSCSI connection happens to fail while the daemon isn't
running (due to a crash or for another reason), the kernel failure
report is not received. When the daemon restarts, there is insufficient
kernel state in sysfs for it to know that this happened. open-iscsi
tries to reopen every connection, but on different initiators, we'd like
to know which connections have failed.

There is session->state, but that has a different lifetime than an iSCSI
connection, so it doesn't directly relflect the connection state.

Cc: Khazhismel Kumykov <kha...@google.com>
Suggested-by: Junho Ryu <ja...@google.com>
Signed-off-by: Gabriel Krisman Bertazi <kri...@collabora.com>
---

Changes since v2:
- Use designated initializers (Bart)

Changes since v1:
- Remove dependency of state values (Bart)


drivers/scsi/libiscsi.c | 7 ++++++-
drivers/scsi/scsi_transport_iscsi.c | 28 +++++++++++++++++++++++++++-
include/scsi/scsi_transport_iscsi.h | 8 ++++++++
3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 70b99c0e2e67..ca488c57ead4 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3153,13 +3153,18 @@ void iscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)

switch (flag) {
case STOP_CONN_RECOVER:
+ cls_conn->state = ISCSI_CONN_FAILED;
+ break;
case STOP_CONN_TERM:
- iscsi_start_session_recovery(session, conn, flag);
+ cls_conn->state = ISCSI_CONN_DOWN;
break;
default:
iscsi_conn_printk(KERN_ERR, conn,
"invalid stop flag %d\n", flag);
+ return;
}
+
+ iscsi_start_session_recovery(session, conn, flag);
}
EXPORT_SYMBOL_GPL(iscsi_conn_stop);

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c

index 17a45716a0fe..e2a9d48e3d8e 100644

--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2276,6 +2276,7 @@ iscsi_create_conn(struct iscsi_cls_session *session, int dd_size, uint32_t cid)
INIT_LIST_HEAD(&conn->conn_list_err);
conn->transport = transport;
conn->cid = cid;
+ conn->state = ISCSI_CONN_DOWN;

/* this is released in the dev's release function */
if (!get_device(&session->dev))
@@ -3709,8 +3710,11 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
break;
case ISCSI_UEVENT_START_CONN:
conn = iscsi_conn_lookup(ev->u.start_conn.sid, ev->u.start_conn.cid);
- if (conn)
+ if (conn) {
ev->r.retcode = transport->start_conn(conn);
+ if (!ev->r.retcode)
+ conn->state = ISCSI_CONN_UP;
+ }
else
err = -EINVAL;
break;

@@ -3907,6 +3911,25 @@ iscsi_conn_attr(tcp_xmit_wsf, ISCSI_PARAM_TCP_XMIT_WSF);
iscsi_conn_attr(tcp_recv_wsf, ISCSI_PARAM_TCP_RECV_WSF);
iscsi_conn_attr(local_ipaddr, ISCSI_PARAM_LOCAL_IPADDR);

+static const char *const connection_state_names[] = {
+ [ISCSI_CONN_UP] = "up",
+ [ISCSI_CONN_DOWN] = "down",
+ [ISCSI_CONN_FAILED] = "failed"

+};
+
+static ssize_t show_conn_state(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iscsi_cls_conn *conn = iscsi_dev_to_conn(dev->parent);

+ const char *state = "unknown";
+
+ if (conn->state < ARRAY_SIZE(connection_state_names))
+ state = connection_state_names[conn->state];
+
+ return sprintf(buf, "%s\n", state);

+}
+static ISCSI_CLASS_ATTR(conn, state, S_IRUGO, show_conn_state,
+ NULL);

#define iscsi_conn_ep_attr_show(param) \
static ssize_t show_conn_ep_param_##param(struct device *dev, \

@@ -3976,6 +3999,7 @@ static struct attribute *iscsi_conn_attrs[] = {

&dev_attr_conn_tcp_xmit_wsf.attr,
&dev_attr_conn_tcp_recv_wsf.attr,
&dev_attr_conn_local_ipaddr.attr,
+ &dev_attr_conn_state.attr,
NULL,
};

@@ -4047,6 +4071,8 @@ static umode_t iscsi_conn_attr_is_visible(struct kobject *kobj,

param = ISCSI_PARAM_TCP_RECV_WSF;
else if (attr == &dev_attr_conn_local_ipaddr.attr)
param = ISCSI_PARAM_LOCAL_IPADDR;
+ else if (attr == &dev_attr_conn_state.attr)
+ return S_IRUGO;
else {
WARN_ONCE(1, "Invalid conn attr");
return 0;
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h

index fa8814245796..1b8a07b427a3 100644

--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -188,6 +188,13 @@ extern void iscsi_ping_comp_event(uint32_t host_no,
uint32_t status, uint32_t pid,
uint32_t data_size, uint8_t *data);

+/* iscsi class connection state */
+enum {
+ ISCSI_CONN_UP = 0,
+ ISCSI_CONN_DOWN,
+ ISCSI_CONN_FAILED,
+};
+
struct iscsi_cls_conn {
struct list_head conn_list; /* item in connlist */
struct list_head conn_list_err; /* item in connlist_err */
@@ -198,6 +205,7 @@ struct iscsi_cls_conn {
struct iscsi_endpoint *ep;

struct device dev; /* sysfs transport/container device */

+ unsigned int state;
};

#define iscsi_dev_to_conn(_dev) \
--
2.25.0

[Bart Van Assche]

On 3/6/20 11:59 AM, Gabriel Krisman Bertazi wrote:
> +static const char *const connection_state_names[] = {
> + [ISCSI_CONN_UP] = "up",
> + [ISCSI_CONN_DOWN] = "down",
> + [ISCSI_CONN_FAILED] = "failed"
> +};
> +
> +static ssize_t show_conn_state(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct iscsi_cls_conn *conn = iscsi_dev_to_conn(dev->parent);
> + const char *state = "unknown";
> +
> + if (conn->state < ARRAY_SIZE(connection_state_names))
> + state = connection_state_names[conn->state];
> +
> + return sprintf(buf, "%s\n", state);
> +}
> +static ISCSI_CLASS_ATTR(conn, state, S_IRUGO, show_conn_state,
> + NULL);

Thank you for having made this change.

> +/* iscsi class connection state */
> +enum {
> + ISCSI_CONN_UP = 0,
> + ISCSI_CONN_DOWN,
> + ISCSI_CONN_FAILED,
> +};
> +
> struct iscsi_cls_conn {
> struct list_head conn_list; /* item in connlist */
> struct list_head conn_list_err; /* item in connlist_err */
> @@ -198,6 +205,7 @@ struct iscsi_cls_conn {
> struct iscsi_endpoint *ep;
>
> struct device dev; /* sysfs transport/container device */
> + unsigned int state;
> };

Can 'state' have another value than those declared in the enumeration
type? If not, how about changing the type of 'state' from 'unsigned int'
into 'enum ...'? If that change is made, a check will have to be added
in show_conn_state() whether conn->state >= 0 since the C standard does
not guarantee whether enumeration types are signed or unsigned.

Bart.

[Gabriel Krisman Bertazi]

If an iSCSI connection happens to fail while the daemon isn't
running (due to a crash or for another reason), the kernel failure
report is not received. When the daemon restarts, there is insufficient
kernel state in sysfs for it to know that this happened. open-iscsi
tries to reopen every connection, but on different initiators, we'd like
to know which connections have failed.

There is session->state, but that has a different lifetime than an iSCSI
connection, so it doesn't directly relflect the connection state.

Cc: Khazhismel Kumykov <kha...@google.com>
Suggested-by: Junho Ryu <ja...@google.com>
Signed-off-by: Gabriel Krisman Bertazi <kri...@collabora.com>
---

Changes since v3:
- Change state type to enum (Bart)

Changes since v2:
- Use designated initializers (Bart)

Changes since v1:
- Remove dependency of state values (Bart)


drivers/scsi/libiscsi.c | 7 ++++++-

drivers/scsi/scsi_transport_iscsi.c | 29 ++++++++++++++++++++++++++++-
include/scsi/scsi_transport_iscsi.h | 8 ++++++++
3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 70b99c0e2e67..ca488c57ead4 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3153,13 +3153,18 @@ void iscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)

switch (flag) {
case STOP_CONN_RECOVER:
+ cls_conn->state = ISCSI_CONN_FAILED;
+ break;
case STOP_CONN_TERM:
- iscsi_start_session_recovery(session, conn, flag);
+ cls_conn->state = ISCSI_CONN_DOWN;
break;
default:
iscsi_conn_printk(KERN_ERR, conn,
"invalid stop flag %d\n", flag);
+ return;
}
+
+ iscsi_start_session_recovery(session, conn, flag);
}
EXPORT_SYMBOL_GPL(iscsi_conn_stop);

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c

index 17a45716a0fe..0ec1b31c75a9 100644

--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2276,6 +2276,7 @@ iscsi_create_conn(struct iscsi_cls_session *session, int dd_size, uint32_t cid)
INIT_LIST_HEAD(&conn->conn_list_err);
conn->transport = transport;
conn->cid = cid;
+ conn->state = ISCSI_CONN_DOWN;

/* this is released in the dev's release function */
if (!get_device(&session->dev))
@@ -3709,8 +3710,11 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
break;
case ISCSI_UEVENT_START_CONN:
conn = iscsi_conn_lookup(ev->u.start_conn.sid, ev->u.start_conn.cid);
- if (conn)
+ if (conn) {
ev->r.retcode = transport->start_conn(conn);
+ if (!ev->r.retcode)
+ conn->state = ISCSI_CONN_UP;
+ }
else
err = -EINVAL;
break;

@@ -3907,6 +3911,26 @@ iscsi_conn_attr(tcp_xmit_wsf, ISCSI_PARAM_TCP_XMIT_WSF);
iscsi_conn_attr(tcp_recv_wsf, ISCSI_PARAM_TCP_RECV_WSF);
iscsi_conn_attr(local_ipaddr, ISCSI_PARAM_LOCAL_IPADDR);

+static const char *const connection_state_names[] = {
+ [ISCSI_CONN_UP] = "up",
+ [ISCSI_CONN_DOWN] = "down",
+ [ISCSI_CONN_FAILED] = "failed"
+};
+
+static ssize_t show_conn_state(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iscsi_cls_conn *conn = iscsi_dev_to_conn(dev->parent);
+ const char *state = "unknown";
+

+ if (conn->state >= 0 &&
+ conn->state < ARRAY_SIZE(connection_state_names))

+ state = connection_state_names[conn->state];
+
+ return sprintf(buf, "%s\n", state);
+}
+static ISCSI_CLASS_ATTR(conn, state, S_IRUGO, show_conn_state,
+ NULL);

#define iscsi_conn_ep_attr_show(param) \
static ssize_t show_conn_ep_param_##param(struct device *dev, \

@@ -3976,6 +4000,7 @@ static struct attribute *iscsi_conn_attrs[] = {

&dev_attr_conn_tcp_xmit_wsf.attr,
&dev_attr_conn_tcp_recv_wsf.attr,
&dev_attr_conn_local_ipaddr.attr,
+ &dev_attr_conn_state.attr,
NULL,
};

@@ -4047,6 +4072,8 @@ static umode_t iscsi_conn_attr_is_visible(struct kobject *kobj,

param = ISCSI_PARAM_TCP_RECV_WSF;
else if (attr == &dev_attr_conn_local_ipaddr.attr)
param = ISCSI_PARAM_LOCAL_IPADDR;
+ else if (attr == &dev_attr_conn_state.attr)
+ return S_IRUGO;
else {
WARN_ONCE(1, "Invalid conn attr");
return 0;
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h

index fa8814245796..bdcb6d69d154 100644

--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -188,6 +188,13 @@ extern void iscsi_ping_comp_event(uint32_t host_no,
uint32_t status, uint32_t pid,
uint32_t data_size, uint8_t *data);

+/* iscsi class connection state */

+enum iscsi_connection_state {

+ ISCSI_CONN_UP = 0,
+ ISCSI_CONN_DOWN,
+ ISCSI_CONN_FAILED,
+};
+
struct iscsi_cls_conn {
struct list_head conn_list; /* item in connlist */
struct list_head conn_list_err; /* item in connlist_err */
@@ -198,6 +205,7 @@ struct iscsi_cls_conn {
struct iscsi_endpoint *ep;

struct device dev; /* sysfs transport/container device */

+ enum iscsi_connection_state state;
};

#define iscsi_dev_to_conn(_dev) \
--
2.25.0

[Gabriel Krisman Bertazi]

[Lee Duncan]

Reviewed-by: Lee Duncan <ldu...@suse.com>

[Martin K. Petersen]

Gabriel,

> If an iSCSI connection happens to fail while the daemon isn't running
> (due to a crash or for another reason), the kernel failure report is
> not received. When the daemon restarts, there is insufficient kernel
> state in sysfs for it to know that this happened. open-iscsi tries to
> reopen every connection, but on different initiators, we'd like to
> know which connections have failed.

Applied to 5.7/scsi-queue, thanks!

--
Martin K. Petersen Oracle Linux Engineering