Re: [PATCH v3] iscsi: Perform connection failure entirely in kernel space

2020-01-01 Thread Lee Duncan
On 12/26/19 12:47 PM, Gabriel Krisman Bertazi wrote:
> From: Bharath Ravi 
> 
> Connection failure processing depends on a daemon being present to (at
> least) stop the connection and start recovery.  This is a problem on a
> multipath scenario, where if the daemon failed for whatever reason, the
> SCSI path is never marked as down, multipath won't perform the
> failover and IO to the device will be forever waiting for that
> connection to come back.
> 
> This patch performs the connection failure entirely inside the kernel.
> This way, the failover can happen and pending IO can continue even if
> the daemon is dead. Once the daemon comes alive again, it can execute
> recovery procedures if applicable.
> 
> Changes since v2:
>   - Don't hold rx_mutex for too long at once
> 
> Changes since v1:
>   - Remove module parameter.
>   - Always do kernel-side stop work.
>   - Block recovery timeout handler if system is dying.
>   - send a CONN_TERM stop if the system is dying.
> 
> Cc: Mike Christie 
> Cc: Lee Duncan 
> Cc: Bart Van Assche 
> Co-developed-by: Dave Clausen 
> Signed-off-by: Dave Clausen 
> Co-developed-by: Nick Black 
> Signed-off-by: Nick Black 
> Co-developed-by: Vaibhav Nagarnaik 
> Signed-off-by: Vaibhav Nagarnaik 
> Co-developed-by: Anatol Pomazau 
> Signed-off-by: Anatol Pomazau 
> Co-developed-by: Tahsin Erdogan 
> Signed-off-by: Tahsin Erdogan 
> Co-developed-by: Frank Mayhar 
> Signed-off-by: Frank Mayhar 
> Co-developed-by: Junho Ryu 
> Signed-off-by: Junho Ryu 
> Co-developed-by: Khazhismel Kumykov 
> Signed-off-by: Khazhismel Kumykov 
> Signed-off-by: Bharath Ravi 
> Co-developed-by: Gabriel Krisman Bertazi 
> Signed-off-by: Gabriel Krisman Bertazi 
> ---
>  drivers/scsi/scsi_transport_iscsi.c | 63 +
>  include/scsi/scsi_transport_iscsi.h |  1 +
>  2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c 
> b/drivers/scsi/scsi_transport_iscsi.c
> index 271afea654e2..c6db6ded60a1 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -86,6 +86,12 @@ struct iscsi_internal {
>   struct transport_container session_cont;
>  };
>  
> +/* Worker to perform connection failure on unresponsive connections
> + * completely in kernel space.
> + */
> +static void stop_conn_work_fn(struct work_struct *work);
> +static DECLARE_WORK(stop_conn_work, stop_conn_work_fn);
> +
>  static atomic_t iscsi_session_nr; /* sysfs session id for next new session */
>  static struct workqueue_struct *iscsi_eh_timer_workq;
>  
> @@ -1611,6 +1617,7 @@ static DEFINE_MUTEX(rx_queue_mutex);
>  static LIST_HEAD(sesslist);
>  static DEFINE_SPINLOCK(sesslock);
>  static LIST_HEAD(connlist);
> +static LIST_HEAD(connlist_err);
>  static DEFINE_SPINLOCK(connlock);
>  
>  static uint32_t iscsi_conn_get_sid(struct iscsi_cls_conn *conn)
> @@ -2247,6 +2254,7 @@ iscsi_create_conn(struct iscsi_cls_session *session, 
> int dd_size, uint32_t cid)
>  
>   mutex_init(>ep_mutex);
>   INIT_LIST_HEAD(>conn_list);
> + INIT_LIST_HEAD(>conn_list_err);
>   conn->transport = transport;
>   conn->cid = cid;
>  
> @@ -2293,6 +2301,7 @@ int iscsi_destroy_conn(struct iscsi_cls_conn *conn)
>  
>   spin_lock_irqsave(, flags);
>   list_del(>conn_list);
> + list_del(>conn_list_err);
>   spin_unlock_irqrestore(, flags);
>  
>   transport_unregister_device(>dev);
> @@ -2407,6 +2416,51 @@ int iscsi_offload_mesg(struct Scsi_Host *shost,
>  }
>  EXPORT_SYMBOL_GPL(iscsi_offload_mesg);
>  
> +static void stop_conn_work_fn(struct work_struct *work)
> +{
> + struct iscsi_cls_conn *conn, *tmp;
> + unsigned long flags;
> + LIST_HEAD(recovery_list);
> +
> + spin_lock_irqsave(, flags);
> + if (list_empty(_err)) {
> + spin_unlock_irqrestore(, flags);
> + return;
> + }
> + list_splice_init(_err, _list);
> + spin_unlock_irqrestore(, flags);
> +
> + list_for_each_entry_safe(conn, tmp, _list, conn_list_err) {
> + uint32_t sid = iscsi_conn_get_sid(conn);
> + struct iscsi_cls_session *session;
> +
> + mutex_lock(_queue_mutex);
> +
> + session = iscsi_session_lookup(sid);
> + if (session) {
> + if (system_state != SYSTEM_RUNNING) {
> + session->recovery_tmo = 0;
> + conn->transport->stop_conn(conn,
> +STOP_CONN_TERM);
> + } else {
> + conn->transport->stop_conn(conn,
> +STOP_CONN_RECOVER);
> + }
> + }
> +
> + list_del_init(>conn_list_err);
> +
> + mutex_unlock(_queue_mutex);
> +
> + /* we don't want to hold rx_queue_mutex for too long,
> +  * for instance if many conns failed at the same time,
> +

Re: Open-iSCSI in research paper

2020-01-01 Thread The Lee-Man
 On Tuesday, December 31, 2019 at 7:49:49 AM UTC-8, Bobby wrote:
>
> Hi all,
>
> I have come across this research paper (attached) called "*Design and 
> implementation of IP-based iSCSI Offoad Engine on an FPGA*"  and the 
> authors have mentioned they have used open source software based 
> *Open-iSCSI* for their research. At the moment there are 2 questions 
> based on this paper.
>
> *Question 1:*
> On page 3 and under section 2.4 ( *Performance Analysis of Open-iSCSI*), 
> they have started the paragraph with following lines:
>
> "*We analyzed iSCSI traffic with Wireshark, the open source network 
> packet analyzer. We measured traffic between a software initiator and a 
> target by using a set of microbenchmarks. The microbenchmarks transmitted 
> arbitrary number of data in both directions* "
>
> The question is...what are these microbenchmarks. There is no reference to 
> these microbenchmarks in this paper. Any idea, what are these 
> microbenchmarks? 
>

I have no idea. They didn't consult me when doing this paper. :) 

>
> *Question 2:*
> Similarly, on the same page 3 and under section 2.3 (Related Work), they 
> have written "*The most common software implementations in the research 
> community are open source Open-iSCSI and UNH-iSCSI projects*".
>
> After my research on UNH-iSCSI, I have found some work where some 
> researchers have proposed a hardware accelerator for data transfer iSCSI 
> functions. They analyzed UNH-iSCSI source code and presented a general 
> methodology that transforms the software C code into the hardware HDL 
> (FPGA) implementation. Hence their hardware accelerator is designed with 
> direct C-to-HDL translation of specific sub-modules of UNH-iSCSI software.
>
> The question: Is there any similar work like this for Open-iSCSI where 
> specific sub-modules of Open-iSCSI are translated to a hardware language 
> like Verilog/VHDL on hardware (FPGA)? If not, can you please give a hint 
> what would possibly a starting point in case of Open-iSCSI? Because the 
> attached paper does not mention the specific functions of Open-iSCSI code 
> that could be translated to HDL. 
>

No, none that I know of.

There are really two major chunks of open-iscsi: user-land and kernel 
driver(s). The user-land is only used for error handling, setting up 
connections, tearing them down, and other administrative tasks (like 
directing discovery). The kernel code is where all the IO goes on.

There are several adapters available for Linux that move the iSCSI stack 
into hardware. See the qedi driver, for example. These effectively act as 
the "transport" for open-iscsi, when available. I'd be interested in 
comparing throughput using these available adapters to the FPGA in the 
paper -- if I had infinite time. :)

>
> 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 view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/e6557758-3d4e-4f71-8374-c308e2f09835%40googlegroups.com.