Re: [PATCH] iscsi tools: Add support for some host statistics
On 03/25/2016 04:08 AM, Adheer Chandravanshi wrote: > > > From: "open-iscsi@googlegroups.com <mailto:open-iscsi@googlegroups.com>" > mailto:open-iscsi@googlegroups.com>> on > behalf of Adheer Chandravanshi <mailto:adheer.chandravan...@qlogic.com>> > Reply-To: "open-iscsi@googlegroups.com > <mailto:open-iscsi@googlegroups.com>" <mailto:open-iscsi@googlegroups.com>> > Date: Thursday, 10 March 2016 9:54 am > To: Mike Christie mailto:micha...@cs.wisc.edu>>, > "open-iscsi@googlegroups.com <mailto:open-iscsi@googlegroups.com>" > mailto:open-iscsi@googlegroups.com>> > Cc: Shyam Sundar <mailto:shyam.sun...@qlogic.com>>, Saurav Kashyap > mailto:saurav.kash...@qlogic.com>> > Subject: Re: [PATCH] iscsi tools: Add support for some host statistics > > > > From: Mike Christie mailto:micha...@cs.wisc.edu>> > Date: Friday, 4 March 2016 6:11 am > To: "open-iscsi@googlegroups.com > <mailto:open-iscsi@googlegroups.com>" <mailto:open-iscsi@googlegroups.com>> > Cc: Shyam Sundar <mailto:shyam.sun...@qlogic.com>>, Saurav Kashyap > mailto:saurav.kash...@qlogic.com>>, > Adheer Chandravanshi <mailto:adheer.chandravan...@qlogic.com>> > Subject: Re: [PATCH] iscsi tools: Add support for some host statistics > > On 03/02/2016 02:04 AM, adheer.chandravan...@qlogic.com > <mailto:adheer.chandravan...@qlogic.com> wrote: > > From: Adheer Chandravanshi <mailto:adheer.chandravan...@qlogic.com>> > Add support to maintain and show some host statistics in iscsid. > This provides following host specific stats: > iscsi_login_accept_rsps > iscsi_login_other_fail_rsps > iscsi_login_negotiate_fails > iscsi_login_authenticate_fails > iscsi_login_auth_fail_rsps > iscsi_login_redirect_rsps > iscsi_logout_normals > iscsi_logout_others > iscsi_connection_timeout_errors > iscsi_session_failures > > > What happened? I actually liked your guys's originally attempt > where it > was kernel based better. > > > We think that putting the host statistics in userspace looks cleaner > and is also inline with current design of partial offload solutions > (bnx2i and likes) and software initiator. > The stats to be supported currently can be easily managed with this > patch as they are updated with the current PDU parsing flow in > userspace. > > > How are you going to support qla4xxx session with this approach? > Were > you going to do this and also a kernel interface for that type > of driver? > > > For qla4xxx, it already has support for host stats through current > offload host stats interface. > And in case its needed, we can support extra stats using > iscsi_host_stats_custom field of struct iscsi_offload_host_stats. > > > For this userspace based patch, what was the last issue with it? > Were > you guys having some issue about when to add the host or delete > it or > something and did you solve the issue? > > > Yes, it was related to host deletion and dropping the reference. > It has been solved by listening to udev events for iscsi_host removal. > > As correctly pointed out by Chris, following points should have been > added to description. > 1) adds a refcounted iscsi_host structure > 2) adds a uevent listener to iscsid > > > > Hello, > > Any other feedback for this patch? > Hey, I think the patch is ugly. I know this is partially my fault because I worked on it :) It is just a lot of code to add login stats. I can live with it though. If however, we end up needing stats like these for qla4xxx type drivers I will block any patches that try to do it using custom stats. We will need to do it generically in kernel. In userspace we have flexibility. Could you break up the patch like Chris requested. I have not done a line by line check of the patch. -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at https://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH] iscsi tools: Add support for some host statistics
From: "open-iscsi@googlegroups.com<mailto:open-iscsi@googlegroups.com>" mailto:open-iscsi@googlegroups.com>> on behalf of Adheer Chandravanshi mailto:adheer.chandravan...@qlogic.com>> Reply-To: "open-iscsi@googlegroups.com<mailto:open-iscsi@googlegroups.com>" mailto:open-iscsi@googlegroups.com>> Date: Thursday, 10 March 2016 9:54 am To: Mike Christie mailto:micha...@cs.wisc.edu>>, "open-iscsi@googlegroups.com<mailto:open-iscsi@googlegroups.com>" mailto:open-iscsi@googlegroups.com>> Cc: Shyam Sundar mailto:shyam.sun...@qlogic.com>>, Saurav Kashyap mailto:saurav.kash...@qlogic.com>> Subject: Re: [PATCH] iscsi tools: Add support for some host statistics From: Mike Christie mailto:micha...@cs.wisc.edu>> Date: Friday, 4 March 2016 6:11 am To: "open-iscsi@googlegroups.com<mailto:open-iscsi@googlegroups.com>" mailto:open-iscsi@googlegroups.com>> Cc: Shyam Sundar mailto:shyam.sun...@qlogic.com>>, Saurav Kashyap mailto:saurav.kash...@qlogic.com>>, Adheer Chandravanshi mailto:adheer.chandravan...@qlogic.com>> Subject: Re: [PATCH] iscsi tools: Add support for some host statistics On 03/02/2016 02:04 AM, adheer.chandravan...@qlogic.com<mailto:adheer.chandravan...@qlogic.com> wrote: From: Adheer Chandravanshi mailto:adheer.chandravan...@qlogic.com>> Add support to maintain and show some host statistics in iscsid. This provides following host specific stats: iscsi_login_accept_rsps iscsi_login_other_fail_rsps iscsi_login_negotiate_fails iscsi_login_authenticate_fails iscsi_login_auth_fail_rsps iscsi_login_redirect_rsps iscsi_logout_normals iscsi_logout_others iscsi_connection_timeout_errors iscsi_session_failures What happened? I actually liked your guys's originally attempt where it was kernel based better. We think that putting the host statistics in userspace looks cleaner and is also inline with current design of partial offload solutions (bnx2i and likes) and software initiator. The stats to be supported currently can be easily managed with this patch as they are updated with the current PDU parsing flow in userspace. How are you going to support qla4xxx session with this approach? Were you going to do this and also a kernel interface for that type of driver? For qla4xxx, it already has support for host stats through current offload host stats interface. And in case its needed, we can support extra stats using iscsi_host_stats_custom field of struct iscsi_offload_host_stats. For this userspace based patch, what was the last issue with it? Were you guys having some issue about when to add the host or delete it or something and did you solve the issue? Yes, it was related to host deletion and dropping the reference. It has been solved by listening to udev events for iscsi_host removal. As correctly pointed out by Chris, following points should have been added to description. 1) adds a refcounted iscsi_host structure 2) adds a uevent listener to iscsid Hello, Any other feedback for this patch? Thanks, Adheer -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at https://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH] iscsi tools: Add support for some host statistics
From: Mike Christie mailto:micha...@cs.wisc.edu>> Date: Friday, 4 March 2016 6:11 am To: "open-iscsi@googlegroups.com<mailto:open-iscsi@googlegroups.com>" mailto:open-iscsi@googlegroups.com>> Cc: Shyam Sundar mailto:shyam.sun...@qlogic.com>>, Saurav Kashyap mailto:saurav.kash...@qlogic.com>>, Adheer Chandravanshi mailto:adheer.chandravan...@qlogic.com>> Subject: Re: [PATCH] iscsi tools: Add support for some host statistics On 03/02/2016 02:04 AM, adheer.chandravan...@qlogic.com<mailto:adheer.chandravan...@qlogic.com> wrote: From: Adheer Chandravanshi mailto:adheer.chandravan...@qlogic.com>> Add support to maintain and show some host statistics in iscsid. This provides following host specific stats: iscsi_login_accept_rsps iscsi_login_other_fail_rsps iscsi_login_negotiate_fails iscsi_login_authenticate_fails iscsi_login_auth_fail_rsps iscsi_login_redirect_rsps iscsi_logout_normals iscsi_logout_others iscsi_connection_timeout_errors iscsi_session_failures What happened? I actually liked your guys's originally attempt where it was kernel based better. We think that putting the host statistics in userspace looks cleaner and is also inline with current design of partial offload solutions (bnx2i and likes) and software initiator. The stats to be supported currently can be easily managed with this patch as they are updated with the current PDU parsing flow in userspace. How are you going to support qla4xxx session with this approach? Were you going to do this and also a kernel interface for that type of driver? For qla4xxx, it already has support for host stats through current offload host stats interface. And in case its needed, we can support extra stats using iscsi_host_stats_custom field of struct iscsi_offload_host_stats. For this userspace based patch, what was the last issue with it? Were you guys having some issue about when to add the host or delete it or something and did you solve the issue? Yes, it was related to host deletion and dropping the reference. It has been solved by listening to udev events for iscsi_host removal. As correctly pointed out by Chris, following points should have been added to description. 1) adds a refcounted iscsi_host structure 2) adds a uevent listener to iscsid Thanks, Adheer -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at https://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH] iscsi tools: Add support for some host statistics
On 03/02/2016 02:04 AM, adheer.chandravan...@qlogic.com wrote: > From: Adheer Chandravanshi > > Add support to maintain and show some host statistics in iscsid. > This provides following host specific stats: > iscsi_login_accept_rsps > iscsi_login_other_fail_rsps > iscsi_login_negotiate_fails > iscsi_login_authenticate_fails > iscsi_login_auth_fail_rsps > iscsi_login_redirect_rsps > iscsi_logout_normals > iscsi_logout_others > iscsi_connection_timeout_errors > iscsi_session_failures What happened? I actually liked your guys's originally attempt where it was kernel based better. How are you going to support qla4xxx session with this approach? Were you going to do this and also a kernel interface for that type of driver? For this userspace based patch, what was the last issue with it? Were you guys having some issue about when to add the host or delete it or something and did you solve the issue? -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at https://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH] iscsi tools: Add support for some host statistics
On Wed, Mar 02, 2016 at 03:04:46AM -0500, adheer.chandravan...@qlogic.com wrote: > From: Adheer Chandravanshi > > Add support to maintain and show some host statistics in iscsid. > This provides following host specific stats: > iscsi_login_accept_rsps > iscsi_login_other_fail_rsps > iscsi_login_negotiate_fails > iscsi_login_authenticate_fails > iscsi_login_auth_fail_rsps > iscsi_login_redirect_rsps > iscsi_logout_normals > iscsi_logout_others > iscsi_connection_timeout_errors > iscsi_session_failures There's a lot going on in this patch. Some of which could be split out into a earlier patch before adding the new statistics, but should at least be described in my opinion. 1) adds a refcounted iscsi_host structure 2) adds a uevent listener to iscsid - Chris -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at https://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
[PATCH] iscsi tools: Add support for some host statistics
From: Adheer Chandravanshi
Add support to maintain and show some host statistics in iscsid.
This provides following host specific stats:
iscsi_login_accept_rsps
iscsi_login_other_fail_rsps
iscsi_login_negotiate_fails
iscsi_login_authenticate_fails
iscsi_login_auth_fail_rsps
iscsi_login_redirect_rsps
iscsi_logout_normals
iscsi_logout_others
iscsi_connection_timeout_errors
iscsi_session_failures
Signed-off-by: Adheer Chandravanshi
---
include/iscsi_if.h | 1 +
usr/discovery.c| 32 +-
usr/event_poll.c | 12 +++-
usr/event_poll.h | 3 +-
usr/host.c | 45 ++
usr/host.h | 14 +
usr/initiator.c| 100 +-
usr/initiator.h| 16 -
usr/initiator_common.c | 162 -
usr/iscsiadm.c | 25 +---
usr/iscsid.c | 12 +++-
usr/iscsistart.c | 10 ++-
usr/mgmt_ipc.c | 19 ++
usr/mgmt_ipc.h | 9 +++
usr/netlink.c | 3 +-
usr/transport.c| 1 +
usr/transport.h| 1 +
17 files changed, 416 insertions(+), 49 deletions(-)
diff --git a/include/iscsi_if.h b/include/iscsi_if.h
index 9d15811..0576dfb 100644
--- a/include/iscsi_if.h
+++ b/include/iscsi_if.h
@@ -536,6 +536,7 @@ enum iscsi_err {
ISCSI_ERR_XMIT_FAILED = ISCSI_ERR_BASE + 19,
ISCSI_ERR_TCP_CONN_CLOSE= ISCSI_ERR_BASE + 20,
ISCSI_ERR_SCSI_EH_SESSION_RST = ISCSI_ERR_BASE + 21,
+ ISCSI_ERR_NOP_TIMEDOUT = ISCSI_ERR_BASE + 22,
};
/*
diff --git a/usr/discovery.c b/usr/discovery.c
index 593d226..4d23381 100644
--- a/usr/discovery.c
+++ b/usr/discovery.c
@@ -1060,6 +1060,11 @@ done:
conn->socket_fd = -1;
}
session->id = -1;
+
+ if (session->host) {
+ iscsi_host_put(session->host);
+ session->host = NULL;
+ }
}
static int iscsi_create_leading_conn(struct iscsi_session *session)
@@ -1104,7 +1109,15 @@ static int iscsi_create_leading_conn(struct
iscsi_session *session)
* if offload is used.
*/
session->conn[0].bind_ep = 1;
- session->hostno = host_no;
+ /*
+* some offload functions need the host so create now for
+* them
+*/
+ session->host = iscsi_host_create(host_no);
+ if (!session->host) {
+ rc = ISCSI_ERR_NOMEM;
+ goto close_ipc;
+ }
}
rc = iscsi_host_set_net_params(iface, session);
@@ -1113,7 +1126,7 @@ static int iscsi_create_leading_conn(struct iscsi_session
*session)
rc);
if (rc != ISCSI_ERR_AGAIN)
rc = ISCSI_ERR_INTERNAL;
- goto close_ipc;
+ goto free_host;
}
/* create interconnect endpoint */
@@ -1121,7 +1134,7 @@ static int iscsi_create_leading_conn(struct iscsi_session
*session)
rc = t->template->ep_connect(conn, 1);
if (rc < 0) {
rc = ISCSI_ERR_TRANS;
- goto close_ipc;
+ goto free_host;
}
do {
@@ -1150,6 +1163,15 @@ static int iscsi_create_leading_conn(struct
iscsi_session *session)
rc = ISCSI_ERR_INTERNAL;
goto disconnect;
}
+
+ if (!session->host) {
+ session->host = iscsi_host_create(host_no);
+ if (!session->host) {
+ rc = ISCSI_ERR_NOMEM;
+ goto disconnect;
+ }
+ }
+
log_debug(2, "%s discovery created session %u", __FUNCTION__,
session->id);
session->isid[3] = (session->id >> 16) & 0xff;
@@ -1196,6 +1218,10 @@ disconnect:
session->id = -1;
}
+free_host:
+ iscsi_host_put(session->host);
+ session->host = NULL;
+
close_ipc:
if (conn->socket_fd >= 0) {
ipc->ctldev_close();
diff --git a/usr/event_poll.c b/usr/event_poll.c
index 209ee02..52391db 100644
--- a/usr/event_poll.c
+++ b/usr/event_poll.c
@@ -38,6 +38,7 @@
#include "actor.h"
#include "initiator.h"
#include "iscsi_err.h"
+#include "host.h"
static unsigned int reap_count;
@@ -121,7 +122,8 @@ static int shutdown_wait_pids(void)
#define POLL_CTRL 0
#define POLL_IPC 1
#define POLL_ALARM 2
-#define POLL_MAX 3
+#define POLL_UDEV 3
+#define POLL_MAX 4
static int event_loop_stop;
static queue_task_t *shutdown_qtask;
@@ -132,7 +134,8 @@ void event_loop_exit(queue_task_t *qtask)
event_loop_stop = 1;
}
-void event_loop(struct iscsi_ipc *ipc, int control_fd, int mgmt_ipc_fd)
+void event_loop(struct iscsi_ipc *ipc, int control_fd, int mgmt_ipc_fd,
+
