Yes, please send a proper patch or pull request.
On Wednesday, October 11, 2017 at 6:44:17 AM UTC-7, Tangchen (UVP) wrote:
>
> 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
> <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>
> *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.
> 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.
>
> --
> 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.
>
--
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.
