1. 08 Jul, 2013 5 commits
    • Nicholas Bellinger's avatar
      iser-target: Add vendor_err debug output · c5a2adbf
      Nicholas Bellinger authored
      Add output for ib_wc.vendor_err in isert_cq_[t,r]x_work(), which
      is useful for debugging future issues.
      Reported-by: default avatarOr Gerlitz <ogerlitz@mellanox.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      c5a2adbf
    • Roland Dreier's avatar
      target: Add (obsolete) checking for PMI/LBA fields in READ CAPACITY(10) · 8dc8632a
      Roland Dreier authored
      The SBC-2 specification of READ CAPACITY(10) has PMI and LOGICAL BLOCK
      ADDRESS fields in the CDB; in SBC-3 these fields are simply listed as
      obsolete.  However, SBC-2 also has the language
      
          If the PMI bit is set to zero and the LOGICAL BLOCK ADDRESS field
          is not set to zero, the device server shall terminate the command
          with CHECK CONDITION status with the sense key set to ILLEGAL
          REQUEST and the additional sense code set to INVALID FIELD IN CDB.
      
      and in fact at least the Windows SCSI compliance test checks this
      behavior.  Since no one following SBC-3 is going to set these fields,
      we might as well include the check from SBC-2 and pass this test.
      Signed-off-by: default avatarRoland Dreier <roland@purestorage.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      8dc8632a
    • Roland Dreier's avatar
      target: Return correct sense data for IO past the end of a device · 09ceadc7
      Roland Dreier authored
      We should use TCM_ADDRESS_OUT_OF_RANGE (-> sense data LOGICAL BLOCK
      ADDRESS OUT OF RANGE) for IOs past the end of a device instead of
      INVALID FIELD IN CDB.
      Signed-off-by: default avatarRoland Dreier <roland@purestorage.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      09ceadc7
    • Roland Dreier's avatar
      target: Add tracepoints for SCSI commands being processed · e5c0d6ad
      Roland Dreier authored
      This patch adds tracepoints to the target code for commands being
      received and being completed, which is quite useful for debugging
      interactions with initiators. For example, one can do something like the
      following to watch commands that are completing unsuccessfully:
      
          # echo 'scsi_status!=0' > /sys/kernel/debug/tracing/events/target/target_cmd_complete/filter
          # echo 1 > /sys/kernel/debug/tracing/events/target/target_cmd_complete/enable
      
              <run command that fails>
      
          # cat /sys/kernel/debug/tracing/trace
               iscsi_trx-0-1902  [003] ...1 990185.810385: target_cmd_complete: iqn.1993-08.org.debian:01:e51ede6aacfd <- LUN 001 status CHECK CONDITION (sense len 18 / 70 00 05 00 00 00 00 0a 00 00 00 00 20 00 00 00 00 00)  0x95 data_length    512  CDB 95 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  (TA:SIMPLE C:00)
      
      (v2: Drop undefined COMPARE_AND_WRITE)
      Signed-off-by: default avatarRoland Dreier <roland@purestorage.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      e5c0d6ad
    • Nicholas Bellinger's avatar
      iser-target: Fix session reset bug with RDMA_CM_EVENT_DISCONNECTED · b2cb9649
      Nicholas Bellinger authored
      This patch addresses a bug where RDMA_CM_EVENT_DISCONNECTED may occur
      before the connection shutdown has been completed by rx/tx threads,
      that causes isert_free_conn() to wait indefinately on ->conn_wait.
      
      This patch allows isert_disconnect_work code to invoke rdma_disconnect
      when isert_disconnect_work() process context is started by client
      session reset before isert_free_conn() code has been reached.
      
      It also adds isert_conn->conn_mutex protection for ->state within
      isert_disconnect_work(), isert_cq_comp_err() and isert_free_conn()
      code, along with isert_check_state() for wait_event usage.
      
      (v2: Add explicit iscsit_cause_connection_reinstatement call
           during isert_disconnect_work() to force conn reset)
      
      Cc: stable@vger.kernel.org  # 3.10+
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      b2cb9649
  2. 07 Jul, 2013 4 commits
    • Nicholas Bellinger's avatar
      iscsi-target: Fix ISCSI_OP_SCSI_TMFUNC handling for iser · 186a9647
      Nicholas Bellinger authored
      This patch adds target_get_sess_cmd reference counting for
      iscsit_handle_task_mgt_cmd(), and adds a target_put_sess_cmd()
      for the failure case.
      
      It also fixes a bug where ISCSI_OP_SCSI_TMFUNC type commands
      where leaking iscsi_cmd->i_conn_node and eventually triggering
      an OOPs during struct isert_conn shutdown.
      
      Cc: stable@vger.kernel.org  # 3.10+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      186a9647
    • Nicholas Bellinger's avatar
      iscsi-target: Fix iscsit_sequence_cmd reject handling for iser · 561bf158
      Nicholas Bellinger authored
      This patch moves ISCSI_OP_REJECT failures into iscsit_sequence_cmd()
      in order to avoid external iscsit_reject_cmd() reject usage for all
      PDU types.
      
      It also updates PDU specific handlers for traditional iscsi-target
      code to not reset the session after posting a ISCSI_OP_REJECT during
      setup.
      
      (v2: Fix CMDSN_LOWER_THAN_EXP for ISCSI_OP_SCSI to call
           target_put_sess_cmd() after iscsit_sequence_cmd() failure)
      
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Mike Christie <michaelc@cs.wisc.edu>
      Cc: stable@vger.kernel.org  # 3.10+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      561bf158
    • Nicholas Bellinger's avatar
      iscsi-target: Fix iscsit_add_reject* usage for iser · ba159914
      Nicholas Bellinger authored
      This patch changes iscsit_add_reject() + iscsit_add_reject_from_cmd()
      usage to not sleep on iscsi_cmd->reject_comp to address a free-after-use
      usage bug in v3.10 with iser-target code.
      
      It saves ->reject_reason for use within iscsit_build_reject() so the
      correct value for both transport cases.  It also drops the legacy
      fail_conn parameter usage throughput iscsi-target code and adds
      two iscsit_add_reject_cmd() and iscsit_reject_cmd helper functions,
      along with various small cleanups.
      
      (v2: Re-enable target_put_sess_cmd() to be called from
           iscsit_add_reject_from_cmd() for rejects invoked after
           target_get_sess_cmd() has been called)
      
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Mike Christie <michaelc@cs.wisc.edu>
      Cc: stable@vger.kernel.org  # 3.10+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      ba159914
    • Nicholas Bellinger's avatar
      iser-target: Fix isert_put_reject payload buffer post · 3df8f68a
      Nicholas Bellinger authored
      This patch adds the missing isert_put_reject() logic to post
      a outgoing payload buffer to hold the 48 bytes of original PDU
      header request payload for the rejected cmd.
      
      It also fixes ISTATE_SEND_REJECT handling in isert_response_completion()
      -> isert_do_control_comp() code, and drops incorrect iscsi_cmd_t->reject_comp
      usage.
      
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Mike Christie <michaelc@cs.wisc.edu>
      Cc: stable@vger.kernel.org  # 3.10+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      3df8f68a
  3. 04 Jul, 2013 8 commits
  4. 26 Jun, 2013 1 commit
    • Nicholas Bellinger's avatar
      iscsi-target: Refactor ISCSI_OP_TEXT_RSP TX handling · 889c8a68
      Nicholas Bellinger authored
      This patch refactoring existing iscsit_send_text_rsp() in order
      to handle iscsi_text_rsp payloads in a transport specific manner.
      
      This includes the addition of iscsit_build_text_rsp() to build
      the response payload and initialize ISCSI_OP_TEXT_RSP.
      
      v2: Make iscsit_build_text_rsp() determine extra padding bytes, and
          drop legacy padding calculation for traditional iSCSI text
          responses within iscsit_send_text_rsp()
      Reported-by: default avatarOr Gerlitz <ogerlitz@mellanox.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Mike Christie <michaelc@cs.wisc.edu>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      889c8a68
  5. 25 Jun, 2013 2 commits
    • Nicholas Bellinger's avatar
      iscsi-target: Refactor ISCSI_OP_TEXT RX handling · 64534aa7
      Nicholas Bellinger authored
      This patch refactors ISCSI_OP_TEXT handling within iscsi-target in
      order to handle iscsi_text payloads in a transport specific manner.
      
      This includes splitting current iscsit_handle_text_cmd() into
      iscsit_setup_text_cmd() and iscsit_process_text_cmd() calls, and
      makes iscsit_handle_text_cmd be only used internally by traditional
      iscsi socket calls.
      
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Mike Christie <michaelc@cs.wisc.edu>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      64534aa7
    • Nicholas Bellinger's avatar
      iscsi/isert-target: Refactor ISCSI_OP_NOOP RX handling · 778de368
      Nicholas Bellinger authored
      This patch refactors ISCSI_OP_NOOP handling within iscsi-target in
      order to handle iscsi_nopout payloads in a transport specific manner.
      
      This includes splitting existing iscsit_handle_nop_out() into
      iscsit_setup_nop_out() and iscsit_process_nop_out() calls, and
      makes iscsit_handle_nop_out() be only used internally by traditional
      iscsi socket calls.
      
      Next update iser-target code to use new callers and add FIXME for
      the handling iscsi_nopout payloads.  Also fix reject response handling
      in iscsit_setup_nop_out() to use proper iscsit_add_reject_from_cmd().
      
      v2: Fix uninitialized iscsit_handle_nop_out() payload_length usage (Fengguang)
      v3: Remove left-over dead code in iscsit_setup_nop_out() (DanC)
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      778de368
  6. 20 Jun, 2013 10 commits
    • Jörn Engel's avatar
      qla_target: remove qlt_check_fcport_exist · 08234e3a
      Jörn Engel authored
      Comment from original 2012 patch:
        In all our testing this function has never returned true.  However, the
        dropping of hardware_lock necessary to call this function seems to cause
        a use-after-free we manage to hit rather frequently.  Given this
        cost-benefit ratio, I'm willing to remove some 100 lines of code.
      
      And since the same problem exists around shutdown_sess and put_sess,
      this patch changes them from taking the hardware_lock to requiring the
      hardware_lock to be taken.  In most cases the caller already had the
      lock and had to drop it for the called method to reacquire it.  At best
      that hurts performance and in rare instances it causes races with fatal
      consequences.
      
      We dropped the original 2012 patch when upgrading our kernel and it took
      us nearly half a year to discover we still need it.
      
      (nab: Fix qla_tgt_sess reference in tcm_qla2xxx_put_sess)
      Signed-off-by: default avatarJoern Engel <joern@logfs.org>
      Cc: Giridhar Malavali <giridhar.malavali@qlogic.com>
      Cc: Chad Dupuis <chad.dupuis@qlogic.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      08234e3a
    • Nicholas Bellinger's avatar
      vhost/scsi: Convert to se_cmd->cmd_kref TARGET_SCF_ACK_KREF usage · 084ed45b
      Nicholas Bellinger authored
      This patch coverts vhost/scsi to se_cmd->cmd_kref TARGET_SCF_ACK_KREF
      usage, instead of assuming that vhost_scsi_free_cmd() is always called
      before TCM processing is completed in the response fast path.
      
      This includes adding vhost_scsi_check_stop_free() -> target_put_sess_cmd()
      to perform the second se_cmd->cmd_kref put, and moving vhost_scsi_free_cmd()
      resource release into tcm_vhost_release_cmd() that is invoked once the last
      se_cmd->cmd_kref put occurs.
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Asias He <asias@redhat.com>
      Cc: Michael S. Tsirkin <mst@redhat.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      084ed45b
    • Nicholas Bellinger's avatar
      vhost/scsi: Drop unnecessary wait_for_tasks=true usage with transport_generic_free_cmd · 6c131d0c
      Nicholas Bellinger authored
      This patch changes vhost_scsi_free_cmd() to call transport_generic_free_cmd()
      with wait_for_tasks=false in order to avoid the extra se_cmd->t_state_lock
      access for the wait_for_tasks=true case.
      
      This is unnecessary because vhost_scsi_free_cmd() is only ever called by
      vhost_scsi_complete_cmd_work() after TCM completion handoff, and by
      vhost_scsi_handle_vq() exception code before TCM submission handoff, so
      there is never a case where se_cmd is still active from TCM's perspective
      when transport_generic_free_cmd() is called.
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Asias He <asias@redhat.com>
      Cc: Michael S. Tsirkin <mst@redhat.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      6c131d0c
    • Nicholas Bellinger's avatar
      target: Drop legacy se_cmd->check_release bit · ca24976a
      Nicholas Bellinger authored
      Now with iscsi-target using modern se_cmd->cmd_kref accounting in
      v3.10 code, it's safe to go ahead and drop the legacy release
      codepath + se_cmd->check_release bit in transport_release_cmd()
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      ca24976a
    • Nicholas Bellinger's avatar
      iscsi-target: Avoid unnecessary t_state_lock during unsolicited data-out check · b9da5826
      Nicholas Bellinger authored
      In modern iscsi-target code, the setup and I/O submission is done within a
      single process context, so there is no need to acquire se_cmd->t_state_lock while
      checking SCF_SUPPORTED_SAM_OPCODE for determining when unsolicited data-out
      should be dumped.
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      b9da5826
    • Nicholas Bellinger's avatar
      target: Drop unnecessary t_state_lock access for SCF_SUPPORTED_SAM_OPCODE assignment · b28e545c
      Nicholas Bellinger authored
      This patch drops the se_cmd->t_state_lock access around SCF_SUPPORTED_SAM_OPCODE
      assignment within target_setup_cmd_from_cdb().
      
      Original v4.0 target code required this as fabrics would be checking for
      this values in different process contexts for setup and I/O submission.
      
      Given that modern v4.1 target code performs setup and I/O submission
      from the same process context, this t_state_lock access is no longer
      required.
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      b28e545c
    • Nicholas Bellinger's avatar
      target: Avoid extra t_state_lock access in __target_execute_cmd · 1a398b97
      Nicholas Bellinger authored
      This patch makes target_execute_cmd() set CMD_T_BUSY|CMD_T_SENT while
      holding se_cmd->t_state_lock, in order to avoid the extra aquire/release
      in __target_execute_cmd().
      
      It also clears these bits in case of a target_handle_task_attr()
      failure.
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      1a398b97
    • Nicholas Bellinger's avatar
      target: Remove legacy t_fe_count + avoid t_state_lock access in transport_put_cmd · c1c35d52
      Nicholas Bellinger authored
      This patch removes legacy se_cmd->t_fe_count usage in order to avoid
      se_cmd->t_state_lock access within transport_put_cmd() during normal
      fast path se_cmd descriptor release.
      
      Also drop the left-over parameter usage within core_tmr_handle_tas_abort()
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      c1c35d52
    • Nicholas Bellinger's avatar
      target: Drop unnecessary CMD_T_DEV_ACTIVE check from transport_lun_remove_cmd · 0b66818a
      Nicholas Bellinger authored
      This patch drops an unnecessary acquire/release of se_cmd->t_state_lock within
      transport_lun_remove_cmd() when checking CMD_T_DEV_ACTIVE for invoking
      target_remove_from_state_list().
      
      For all fast path completion cases, transport_lun_remove_cmd() is always
      called ahead of transport_cmd_check_stop(), and since transport_cmd_check_stop()
      is calling target_remove_from_state_list() when remove_from_lists=true,
      the t_state_lock usage in transport_lun_remove_cmd() can safely be removed.
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      0b66818a
    • Nicholas Bellinger's avatar
      target: Add transport_cmd_check_stop write_pending bit · 862e6389
      Nicholas Bellinger authored
      This patch adds a new transport_cmd_check_stop() parameter for signaling
      when TRANSPORT_WRITE_PENDING needs to be set.
      
      This allows transport_generic_new_cmd() to avoid the extra lock acquire/release
      of ->t_state_lock in the fast path for DMA_TO_DEVICE operations ahead of
      transport_cmd_check_stop() + se_tfo->write_pending().
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@kernel.org>
      Cc: Kent Overstreet <koverstreet@google.com>
      Cc: Or Gerlitz <ogerlitz@mellanox.com>
      Cc: Moussa Ba <moussaba@micron.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      862e6389
  7. 14 Jun, 2013 10 commits