On 01/11/2011 12:25 AM, Or Gerlitz wrote:
Mike Christie<[email protected]> wrote:
+ ep->conn = cls_conn;
+ cls_conn->ep = ep;
if not, it doesn't look like ep can be null here...
Will fix. Tested offload and did not test software iscsi.
Mike, are you pushing this to 2.6.38? if yes, could you send it to
review @ open-iscsi before?
Not sure when I am going to send it. Too late for this feature window,
and I am not sure it counts as a critical bug fix.
I attached the current version of the patch. It just adds some checks
for the bug you spotted above.
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/open-iscsi?hl=en.
commit 9a2c1b712b82301ea93928bfcb2ceba814114d67
Author: Mike Christie <[email protected]>
Date: Tue Jan 11 16:31:22 2011 -0500
iscsi: fix iscsi_endpoint leak
When iscsid restarts it does not know the connection's
endpoint, so it is getting leaked. This fixes the problem
by having the iscsi class force a disconnect before a
new connection is bound.
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 7b2fc98..921c2cb 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -331,8 +331,7 @@ iscsi_iser_conn_destroy(struct iscsi_cls_conn *cls_conn)
static int
iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
- struct iscsi_cls_conn *cls_conn, uint64_t transport_eph,
- int is_leading)
+ struct iscsi_cls_conn *cls_conn, uint64_t transport_eph)
{
struct iscsi_conn *conn = cls_conn->dd_data;
struct iscsi_iser_conn *iser_conn;
@@ -340,10 +339,6 @@ iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
struct iscsi_endpoint *ep;
int error;
- error = iscsi_conn_bind(cls_session, cls_conn, is_leading);
- if (error)
- return error;
-
/* the transport ep handle comes from user space so it must be
* verified against the global ib connections list */
ep = iscsi_lookup_endpoint(transport_eph);
@@ -352,6 +347,11 @@ iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
(unsigned long long)transport_eph);
return -EINVAL;
}
+
+ error = iscsi_conn_bind(cls_session, cls_conn, ep);
+ if (error)
+ return error;
+
ib_conn = ep->dd_data;
/* binds the iSER connection retrieved from the previously
diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index eaaa881..97533e7 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -174,7 +174,7 @@ static int beiscsi_bindconn_cid(struct beiscsi_hba *phba,
*/
int beiscsi_conn_bind(struct iscsi_cls_session *cls_session,
struct iscsi_cls_conn *cls_conn,
- u64 transport_fd, int is_leading)
+ u64 transport_fd)
{
struct iscsi_conn *conn = cls_conn->dd_data;
struct beiscsi_conn *beiscsi_conn = conn->dd_data;
@@ -191,7 +191,7 @@ int beiscsi_conn_bind(struct iscsi_cls_session *cls_session,
beiscsi_ep = ep->dd_data;
- if (iscsi_conn_bind(cls_session, cls_conn, is_leading))
+ if (iscsi_conn_bind(cls_session, cls_conn, ep))
return -EINVAL;
if (beiscsi_ep->phba != phba) {
diff --git a/drivers/scsi/be2iscsi/be_iscsi.h b/drivers/scsi/be2iscsi/be_iscsi.h
index 8950a70..4047ec1 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.h
+++ b/drivers/scsi/be2iscsi/be_iscsi.h
@@ -46,7 +46,7 @@ struct iscsi_cls_conn *beiscsi_conn_create(struct
iscsi_cls_session
int beiscsi_conn_bind(struct iscsi_cls_session *cls_session,
struct iscsi_cls_conn *cls_conn,
- uint64_t transport_fd, int is_leading);
+ uint64_t transport_fd);
int beiscsi_conn_get_param(struct iscsi_cls_conn *cls_conn,
enum iscsi_param param, char *buf);
diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index fb50efb..ec41503 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -1370,7 +1370,7 @@ free_conn:
*/
static int bnx2i_conn_bind(struct iscsi_cls_session *cls_session,
struct iscsi_cls_conn *cls_conn,
- uint64_t transport_fd, int is_leading)
+ uint64_t transport_fd)
{
struct iscsi_conn *conn = cls_conn->dd_data;
struct bnx2i_conn *bnx2i_conn = conn->dd_data;
@@ -1390,7 +1390,7 @@ static int bnx2i_conn_bind(struct iscsi_cls_session
*cls_session,
/* Peer disconnect via' FIN or RST */
return -EINVAL;
- if (iscsi_conn_bind(cls_session, cls_conn, is_leading))
+ if (iscsi_conn_bind(cls_session, cls_conn, ep))
return -EINVAL;
if (bnx2i_ep->hba != hba) {
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index be56617..84125e9 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -2264,7 +2264,7 @@ EXPORT_SYMBOL_GPL(cxgbi_create_conn);
int cxgbi_bind_conn(struct iscsi_cls_session *cls_session,
struct iscsi_cls_conn *cls_conn,
- u64 transport_eph, int is_leading)
+ u64 transport_eph)
{
struct iscsi_conn *conn = cls_conn->dd_data;
struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
@@ -2285,7 +2285,7 @@ int cxgbi_bind_conn(struct iscsi_cls_session *cls_session,
if (err < 0)
return err;
- err = iscsi_conn_bind(cls_session, cls_conn, is_leading);
+ err = iscsi_conn_bind(cls_session, cls_conn, ep);
if (err)
return -EINVAL;
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index c57d59d..1bd50e3 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -718,7 +718,7 @@ int cxgbi_set_conn_param(struct iscsi_cls_conn *,
int cxgbi_get_conn_param(struct iscsi_cls_conn *, enum iscsi_param, char *);
struct iscsi_cls_conn *cxgbi_create_conn(struct iscsi_cls_session *, u32);
int cxgbi_bind_conn(struct iscsi_cls_session *,
- struct iscsi_cls_conn *, u64, int);
+ struct iscsi_cls_conn *, u64);
void cxgbi_destroy_session(struct iscsi_cls_session *);
struct iscsi_cls_session *cxgbi_create_session(struct iscsi_endpoint *,
u16, u16, u32);
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index fec47de..9961c22 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -651,8 +651,7 @@ free_addr:
static int
iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
- struct iscsi_cls_conn *cls_conn, uint64_t transport_eph,
- int is_leading)
+ struct iscsi_cls_conn *cls_conn, uint64_t transport_eph)
{
struct Scsi_Host *shost = iscsi_session_to_shost(cls_session);
struct iscsi_host *ihost = shost_priv(shost);
@@ -685,7 +684,7 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session
*cls_session,
if (err)
goto free_socket;
- err = iscsi_conn_bind(cls_session, cls_conn, is_leading);
+ err = iscsi_conn_bind(cls_session, cls_conn, NULL);
if (err)
goto free_socket;
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index b0481a8..d382b84 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3136,16 +3136,20 @@ void iscsi_conn_stop(struct iscsi_cls_conn *cls_conn,
int flag)
EXPORT_SYMBOL_GPL(iscsi_conn_stop);
int iscsi_conn_bind(struct iscsi_cls_session *cls_session,
- struct iscsi_cls_conn *cls_conn, int is_leading)
+ struct iscsi_cls_conn *cls_conn,
+ struct iscsi_endpoint *ep)
{
struct iscsi_session *session = cls_session->dd_data;
struct iscsi_conn *conn = cls_conn->dd_data;
spin_lock_bh(&session->lock);
- if (is_leading)
- session->leadconn = conn;
+ session->leadconn = conn;
spin_unlock_bh(&session->lock);
+ if (ep) {
+ ep->conn = cls_conn;
+ cls_conn->ep = ep;
+ }
/*
* Unblock xmitworker(), Login Phase will pass through.
*/
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index 332387a..98b5bc9 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1430,6 +1430,26 @@ release_host:
return err;
}
+static int iscsi_if_ep_disconnect(struct iscsi_transport *transport,
+ u64 ep_handle)
+{
+ struct iscsi_cls_conn *conn;
+ struct iscsi_endpoint *ep;
+
+ if (!transport->ep_disconnect)
+ return -EINVAL;
+
+ ep = iscsi_lookup_endpoint(ep_handle);
+ if (!ep)
+ return -EINVAL;
+ conn = ep->conn;
+
+ transport->ep_disconnect(ep);
+ if (conn)
+ conn->ep = NULL;
+ return 0;
+}
+
static int
iscsi_if_transport_ep(struct iscsi_transport *transport,
struct iscsi_uevent *ev, int msg_type)
@@ -1454,14 +1474,8 @@ iscsi_if_transport_ep(struct iscsi_transport *transport,
ev->u.ep_poll.timeout_ms);
break;
case ISCSI_UEVENT_TRANSPORT_EP_DISCONNECT:
- if (!transport->ep_disconnect)
- return -EINVAL;
-
- ep = iscsi_lookup_endpoint(ev->u.ep_disconnect.ep_handle);
- if (!ep)
- return -EINVAL;
-
- transport->ep_disconnect(ep);
+ rc = iscsi_if_ep_disconnect(transport,
+ ev->u.ep_disconnect.ep_handle);
break;
}
return rc;
@@ -1609,10 +1623,12 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr
*nlh, uint32_t *group)
session = iscsi_session_lookup(ev->u.b_conn.sid);
conn = iscsi_conn_lookup(ev->u.b_conn.sid, ev->u.b_conn.cid);
+ if (conn && conn->ep)
+ iscsi_if_ep_disconnect(transport, conn->ep->id);
+
if (session && conn)
ev->r.retcode = transport->bind_conn(session, conn,
- ev->u.b_conn.transport_eph,
- ev->u.b_conn.is_leading);
+ ev->u.b_conn.transport_eph);
else
err = -EINVAL;
break;
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index f1ff600..10b537e 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -388,7 +388,7 @@ extern void iscsi_conn_teardown(struct iscsi_cls_conn *);
extern int iscsi_conn_start(struct iscsi_cls_conn *);
extern void iscsi_conn_stop(struct iscsi_cls_conn *, int);
extern int iscsi_conn_bind(struct iscsi_cls_session *, struct iscsi_cls_conn *,
- int);
+ struct iscsi_endpoint *);
extern void iscsi_conn_failure(struct iscsi_conn *conn, enum iscsi_err err);
extern void iscsi_session_failure(struct iscsi_session *session,
enum iscsi_err err);
diff --git a/include/scsi/scsi_transport_iscsi.h
b/include/scsi/scsi_transport_iscsi.h
index 7fff94b..45c1329 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -95,7 +95,7 @@ struct iscsi_transport {
uint32_t cid);
int (*bind_conn) (struct iscsi_cls_session *session,
struct iscsi_cls_conn *cls_conn,
- uint64_t transport_eph, int is_leading);
+ uint64_t transport_eph);
int (*start_conn) (struct iscsi_cls_conn *conn);
void (*stop_conn) (struct iscsi_cls_conn *conn, int flag);
void (*destroy_conn) (struct iscsi_cls_conn *conn);
@@ -160,6 +160,7 @@ struct iscsi_cls_conn {
void *dd_data; /* LLD private data */
struct iscsi_transport *transport;
uint32_t cid; /* connection id */
+ struct iscsi_endpoint *ep;
int active; /* must be accessed with the connlock */
struct device dev; /* sysfs transport/container device */
@@ -222,6 +223,7 @@ struct iscsi_endpoint {
void *dd_data; /* LLD private data */
struct device dev;
uint64_t id;
+ struct iscsi_cls_conn *conn;
};
/*
