On Wed, 2011-07-27 at 14:28 -0700, Kiran Patil wrote:
> From: Kiran Patil <kiran.patil@xxxxxxxxx>
>
> Problem: Explicit bug ON is not necessary, as well sequence change abort
> is not needed since exchange is not originated by target in this case.
>
> Fix: Remove explicit bug ON. Remove code which aborts sequence and exchange.
> Let other end (in this case FCoE initiator - which is also the originator
> of this exchange) timeout and retry the IO. Also changed log level from
> INFO to WARNING.
>
> Code was moved around to minimize printk and simplify log messsage. Removed
> one of the printk.
>
> Additional Notes: Due to this handling failure scrnario, target would drop
> the frame and hence initiator will not see RSP. As a result of that, IO
> will timeout (as expected) and will be retried. It has effect on IO/s and MB/s.
>
> Signed-off-by: Kiran Patil <kiran.patil@xxxxxxxxx>
Hi Kiran,
This change looks OK to me. It has been applied into lio-core-2.6.git
with some light patch breakage wrt to recent v3.1 conversion of
drivers/target/tcm_fc to printk -> pr_debug + pr_err macro usage..
Also queueing up this patch into target-pending.git/for-next with the
other outstanding drivers/target/ bugfixes for the next mainline pull.
Thanks!
--nab
> ---
>
> drivers/target/tcm_fc/tfc_io.c | 36 +++++++++++++++---------------------
> 1 files changed, 15 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
> index 8c4a240..c80ba9f 100644
> --- a/drivers/target/tcm_fc/tfc_io.c
> +++ b/drivers/target/tcm_fc/tfc_io.c
> @@ -243,21 +243,27 @@ void ft_recv_write_data(struct ft_cmd *cmd, struct fc_frame *fp)
> if (!(ntoh24(fh->fh_f_ctl) & FC_FC_REL_OFF))
> goto drop;
>
> + f_ctl = ntoh24(fh->fh_f_ctl);
> + ep = fc_seq_exch(seq);
> + lport = ep->lp;
> + if (cmd->was_ddp_setup) {
> + BUG_ON(!ep);
> + BUG_ON(!lport);
> + }
> /*
> - * Doesn't expect even single byte of payload. Payload
> + * Doesn't expect payload if DDP is setup. Payload
> * is expected to be copied directly to user buffers
> - * due to DDP (Large Rx offload) feature, hence
> - * BUG_ON if BUF is non-NULL
> + * due to DDP (Large Rx offload),
> */
> buf = fc_frame_payload_get(fp, 1);
> if (cmd->was_ddp_setup && buf) {
> - printk(KERN_INFO "%s: When DDP was setup, not expected to"
> - "receive frame with payload, Payload shall be"
> - "copied directly to buffer instead of coming "
> - "via. legacy receive queues\n", __func__);
> - BUG_ON(buf);
> + printk(KERN_ERR "%s: xid 0x%x, f_ctl 0x%x, cmd->sg %p, "
> + "cmd->sg_cnt 0x%x. DDP was setup"
> + " hence not expected to receive frame with "
> + "payload, Frame will be dropped if TSI bit"
> + "in f_ctl is not set\n",
> + __func__, ep->xid, f_ctl, cmd->sg, cmd->sg_cnt);
> }
> -
> /*
> * If ft_cmd indicated 'ddp_setup', in that case only the last frame
> * should come with 'TSI bit being set'. If 'TSI bit is not set and if
> @@ -265,14 +271,7 @@ void ft_recv_write_data(struct ft_cmd *cmd, struct fc_frame *fp)
> * release the DDP context (ddp_put) and in error case, as well
> * initiate error recovery mechanism.
> */
> - ep = fc_seq_exch(seq);
> - if (cmd->was_ddp_setup) {
> - BUG_ON(!ep);
> - lport = ep->lp;
> - BUG_ON(!lport);
> - }
> if (cmd->was_ddp_setup && ep->xid != FC_XID_UNKNOWN) {
> - f_ctl = ntoh24(fh->fh_f_ctl);
> /*
> * If TSI bit set in f_ctl, means last write data frame is
> * received successfully where payload is posted directly
> @@ -289,13 +288,8 @@ void ft_recv_write_data(struct ft_cmd *cmd, struct fc_frame *fp)
> * this point, but just in case if required in future
> * for debugging or any other purpose
> */
> - printk(KERN_ERR "%s: Received frame with TSI bit not"
> - " being SET, dropping the frame, "
> - "cmd->sg <%p>, cmd->sg_cnt <0x%x>\n",
> - __func__, cmd->sg, cmd->sg_cnt);
> cmd->write_data_len = lport->tt.ddp_done(lport,
> ep->xid);
> - lport->tt.seq_exch_abort(cmd->seq, 0);
> goto drop;
> }
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
