Hi leeman

I think I have found the problem.

On the client side, iscsiadm checks if an session exists in sysfs and if 
session_count >= rec->session.nr_sessions.
But without any lock protection.
So when multiple commands run in parallel, multiple same sessions could be 
created because we set rec->session.multiple = 1.

iscsi_login_portal()
|--> rec->session.multiple = 1

On the server side, iscsid runs __session_login_task() serially. And we checks 
if a same session exists in session_is_running().

__session_login_task()
|--> session_is_running()
         |--> iscsi_sysfs_for_each_session()
|--> if (rec->session.multiple)
                 log_debug(2, "Adding a copy of an existing session");          
                // But we didn’t check if session_count >= 
rec->session.nr_sessions here.

I think we should also check if session_count >= rec->session.nr_sessions on 
server side.


Here is a simple patch. If you think it is OK, I’ll send a new one.

Thanks.


diff --git a/usr/initiator.c b/usr/initiator.c
index 9d02f47..d1ceea3 100644
--- a/usr/initiator.c
+++ b/usr/initiator.c
@@ -1870,14 +1870,12 @@ static iscsi_session_t* session_find_by_rec(node_rec_t 
*rec)
  * a session could be running in the kernel but not in iscsid
  * due to a resync or becuase some other app started the session
  */
-static int session_is_running(node_rec_t *rec)
+static int session_is_running(node_rec_t *rec, int *nr_found)
{
-       int nr_found = 0;
-
        if (session_find_by_rec(rec))
                return 1;

-       if (iscsi_sysfs_for_each_session(rec, &nr_found, iscsi_match_session,
+       if (iscsi_sysfs_for_each_session(rec, nr_found, iscsi_match_session,
                                         0))
                return 1;

@@ -1889,12 +1887,17 @@ static int __session_login_task(node_rec_t *rec, 
queue_task_t *qtask)
        iscsi_session_t *session;
        iscsi_conn_t *conn;
        struct iscsi_transport *t;
-       int rc;
-
-       if (session_is_running(rec)) {
-               if (rec->session.multiple)
+       int rc, nr_found;
+
+       if (session_is_running(rec, &nr_found)) {
+               if (rec->session.multiple) {
+                       if (nr_found >= rec->session.nr_sessions) {
+                               log_debug(2, "Cannot add more copy of session,"
+                                               " %d found.\n", nr_found);
+                               return ISCSI_ERR_SESS_EXISTS;
+                       }
                        log_debug(2, "Adding a copy of an existing session");
-               else
+               } else
                        return ISCSI_ERR_SESS_EXISTS;
        }

--
1.8.3.1

From: open-iscsi@googlegroups.com [mailto:open-iscsi@googlegroups.com] On 
Behalf Of Tangchen (UVP)
Sent: Monday, October 09, 2017 9:19 PM
To: open-iscsi@googlegroups.com
Cc: guijianfeng <guijianf...@huawei.com>; Zhangyanfei (YF) 
<yanfei.zh...@huawei.com>
Subject: RE: [iscsiadm] iscsiadm creates multiple same sessions when run with 
--login option in parallel.

Hi leeman

I think it is because iscsi_login_portal() has no protection to avoid parallel 
searching the sysfs for the same session.

Can we use something like sem to serialize the searching from different 
processes ?
Since I don’t see any communication with iscsid in this login operation, I’m 
not sure if it is a good idea to do the synchronization in iscsid.

Thanks.


From: open-iscsi@googlegroups.com<mailto:open-iscsi@googlegroups.com> 
[mailto:open-iscsi@googlegroups.com] On Behalf Of The Lee-Man
Sent: Wednesday, October 04, 2017 12:27 AM
To: open-iscsi <open-iscsi@googlegroups.com<mailto:open-iscsi@googlegroups.com>>
Subject: Re: [iscsiadm] iscsiadm creates multiple same sessions when run with 
--login option in parallel.

Thanks for this report. I'll try to reproduce this myself and see what is going 
on.

It is certainly the case that there is insufficient locking in iscsid with 
respect to multiple threads, IMHO, so I am not surprised.

On Wednesday, September 27, 2017 at 11:43:02 PM UTC-7, Tangchen (UVP) wrote:
Hi guys,

If we run iscsiadm -m node --login command through the same IP address 4 times, 
only one session will be created.
But if we run them in parallel, then 4 same sessions could be created.
( Here, xxx.xxx.xxx.xxx is the IP address to the IPSAN. I'm using the same IP 
in these 4 commands. )

# iscsiadm -m node -p xxx.xxx.xxx.xxx  --login &
# iscsiadm -m node -p xxx.xxx.xxx.xxx  --login &
# iscsiadm -m node -p xxx.xxx.xxx.xxx  --login &
# iscsiadm -m node -p xxx.xxx.xxx.xxx  --login &

Logging in to [iface: default, target: iqn. xxx.xxx.xxx.xxx, portal: 
xxx.xxx.xxx.xxx] (multiple)
Logging in to [iface: default, target: iqn. xxx.xxx.xxx.xxx, portal: 
xxx.xxx.xxx.xxx] (multiple)
Logging in to [iface: default, target: iqn. xxx.xxx.xxx.xxx, portal: 
xxx.xxx.xxx.xxx] (multiple)
Logging in to [iface: default, target: iqn. xxx.xxx.xxx.xxx, portal: 
xxx.xxx.xxx.xxx] (multiple)
Login to [iface: default, target: xxx.xxx.xxx.xxx, portal: xxx.xxx.xxx.xxx] 
successful.
Login to [iface: default, target: xxx.xxx.xxx.xxx, portal: xxx.xxx.xxx.xxx] 
successful.
Login to [iface: default, target: xxx.xxx.xxx.xxx, portal: xxx.xxx.xxx.xxx] 
successful.
Login to [iface: default, target: xxx.xxx.xxx.xxx, portal: xxx.xxx.xxx.xxx] 
successful.

# iscsiadm -m session
tcp: [1] xxx.xxx.xxx.xxx (non-flash)
tcp: [2] xxx.xxx.xxx.xxx (non-flash)
tcp: [3] xxx.xxx.xxx.xxx (non-flash)
tcp: [4] xxx.xxx.xxx.xxx (non-flash)

If we check the net connection in /proc/net/nf_conntrack, they are 4 TCP 
connections with different src ports.
And if we run logout command only once, all the 4 sessions will be destroyed.

Unfortunately, service like multipathd cannot tell the difference between them. 
If we have 4 same sessions,
4 paths will be created connecting to the dm device. But actually they are the 
same paths.

Referring to the code, iscsiadm command does prevent creating same session by 
checking /sys/class/iscsi_session/ dir.
But no multi-thread protection in there.

Any idea how to solve this problem ?

Thanks.
--
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<mailto:open-iscsi+unsubscr...@googlegroups.com>.
To post to this group, send email to 
open-iscsi@googlegroups.com<mailto: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.
--
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<mailto:open-iscsi+unsubscr...@googlegroups.com>.
To post to this group, send email to 
open-iscsi@googlegroups.com<mailto: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.

-- 
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.

Reply via email to