On 10/28/10 11:48 AM, Kiran Patil wrote:
> From: Kiran Patil <[email protected]>
>
> Problem: When initaitor sends write command to target, target tries to assign
> new sequence. It
> allocates new exchangeID (RX_ID) always from non-offloaded pool
> (Non-offload EMA)
>
> Fix: Enhanced fcoe_oem_match routine to look at F_CTL flags and if it is
> exchange responder and
> command type is WRITEDATA, then function returns TRUE instead of
> FALSE. This function
> is used to determine which pool to use (offload pool of exchange is
> used only if this
> function returns TRUE).
>
> Technical Notes: N/A
>
> Signed-off-by: Kiran Patil <[email protected]>
> ---
>
> drivers/scsi/fcoe/fcoe.c | 21 +++++++++++++++++++--
> 1 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 844d618..a3e465f 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -747,8 +747,25 @@ static int fcoe_shost_config(struct fc_lport *lport,
> struct device *dev)
> */
> bool fcoe_oem_match(struct fc_frame *fp)
BTW, this should be static. It isn't exported. Pre-existing problem, though.
> {
> - return fc_fcp_is_read(fr_fsp(fp)) &&
> - (fr_fsp(fp)->data_len > fcoe_ddp_min);
> + bool bret = false;
> + struct fc_frame_header *fh = fc_frame_header_get(fp);
There should be a blank line here after declarations before code starts.
> + if (fc_fcp_is_read(fr_fsp(fp)) &&
> + (fr_fsp(fp)->data_len > fcoe_ddp_min))
> + bret = true;
> + else {
> + /*
> + * Means operating as exchange originator (target received
> + * write request from initiator
> + */
The comment doesn't seem to be indented correctly ... maybe my mailer messed it
up.
It's also wrong because at this point we don't know we're the target.
Also, we're operating as the recipient, not the exchange originator.
I know what you mean, but it isn't clear. Better to leave the comment out.
> + struct fcp_cmnd *fcp;
This declaration should be at the top.
> + if (!(ntoh24(fh->fh_f_ctl) & FC_FC_EX_CTX)) {
This can now be an "else if"
> + fcp = fc_frame_payload_get(fp, sizeof(*fcp));
> + if ((ntohs(fh->fh_rx_id) == FC_XID_UNKNOWN) &&
> + (fcp && (fcp->fc_flags & FCP_CFL_WRDATA)))
You can eliminate the extra parens, per our earlier discussion.
They're OK, though. It's good that you checked for fcp non-NULL!
Anyway, it looks like it works.
> + bret = true;
> + }
> + }
> + return bret;
> }
>
> /**
>
> _______________________________________________
> devel mailing list
> [email protected]
> http://www.open-fcoe.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
