[Open-FCoE] [PATCH 2/2] libfc: updated sequence counter when sending down burst len data

Joe Eykholt jre at nuovasystems.com
Thu Oct 2 18:29:47 UTC 2008


Hi Vasu,

Vasu Dev wrote:
> Also passed down FC max frame size(mfs) to LLD in fc_frame header field
> fr_mfs to indicate FC frame size in data burst.

> 
> Signed-off-by: Vasu Dev <vasu.dev at intel.com>
> ---
> 
>  drivers/scsi/libfc/fc_exch.c  |   10 ++++++++++
>  drivers/scsi/libfc/fc_fcp.c   |    1 +
>  include/scsi/libfc/fc_frame.h |    2 ++
>  3 files changed, 13 insertions(+), 0 deletions(-)
> 
> 
> diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> index ed74d95..470b142 100644
> --- a/drivers/scsi/libfc/fc_exch.c
> +++ b/drivers/scsi/libfc/fc_exch.c
> @@ -873,6 +873,7 @@ int fc_seq_send(struct fc_lport *lp, struct fc_seq *sp,
>  	enum fc_class class;
>  	u16 fill = 0;
>  	int error;
> +	int len;
>  
>  	ep = fc_seq_exch(sp);
>  	WARN_ON((ep->esb_stat & ESB_ST_SEQ_INIT) != ESB_ST_SEQ_INIT);
> @@ -916,6 +917,15 @@ int fc_seq_send(struct fc_lport *lp, struct fc_seq *sp,
>  	fh->fh_seq_cnt = htons(sp->cnt++);
>  
>  	/*
> +	 * update sequence counter if this frame is carrying

The comment should say "in case" instead of "if".
Is this for recovery purposes?  I mean, it would be strange
to send multiple sequences and have the LLD break them up further,
right?  Do we need a separate field to say what the max frame
size is that we can pass to the LLD?

> +	 * multiple FC frames when sequence offload is enabled
> +	 * by LLD.
> +	 */
> +	len = fr_len(fp) - fr_mfs(fp) - sizeof(struct fc_frame_header);
> +	if (fr_mfs(fp) && len > 0)
> +		sp->cnt += len / fr_mfs(fp);

This doesn't seem right.  I assume fr_mfs() includes the FC header,
as the name implies.  How about:

	if (fr_mfs(fp)) {
		u_int max_payload;
	
		max_payload = fr_mfs(fp) - sizeof(struct fc_frame_header);
		payload_len = fr_len(fp) - sizeof(struct fc_frame_header);
		sp->cnt = (payload_len + max_payload - 1) / max_payload - 1;
	}

The first -1 is to round up (I couldn't find a common ROUND_UP macro).
The second -1 is because presumably sp->cnt is incremented by 1 somewhere else.

> +
> +	/*
>  	 * Send the frame.
>  	 */
>  	error = lp->tt.frame_send(lp, fp);
> diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
> index 2a59c61..58cb385 100644
> --- a/drivers/scsi/libfc/fc_fcp.c
> +++ b/drivers/scsi/libfc/fc_fcp.c
> @@ -569,6 +569,7 @@ static int fc_fcp_send_data(struct fc_fcp_pkt *fsp, struct fc_seq *sp,
>  			}
>  			fc_frame_setup(fp, FC_RCTL_DD_SOL_DATA, FC_TYPE_FCP);
>  			fc_frame_set_offset(fp, frame_offset);
> +			fr_mfs(fp) = fsp->max_payload;


To keep things straight, let's use payload when we don't include the header, and
frame when we do.  If mfs stands for max frame size, that should include the header.
Assigning mfs to payload raised a red flag to me.  For the above calculation,
payload is much more convenient, so maybe fr_mfs() should be max_payload
instead of mfs?  If you agree, please adjust the calculation.

	Regards,
	Joe


>  		}
>  		sg_bytes = min(tlen, sg->length - offset);
>  		if (using_sg) {
> diff --git a/include/scsi/libfc/fc_frame.h b/include/scsi/libfc/fc_frame.h
> index c7a52bb..11e9401 100644
> --- a/include/scsi/libfc/fc_frame.h
> +++ b/include/scsi/libfc/fc_frame.h
> @@ -51,6 +51,7 @@
>  #define fr_sof(fp)	(fr_cb(fp)->fr_sof)
>  #define fr_eof(fp)	(fr_cb(fp)->fr_eof)
>  #define fr_flags(fp)	(fr_cb(fp)->fr_flags)
> +#define fr_mfs(fp)	(fr_cb(fp)->fr_mfs)
>  
>  struct fc_frame {
>  	struct sk_buff skb;
> @@ -63,6 +64,7 @@ struct fcoe_rcv_info {
>  	enum fc_sof	fr_sof;		/* start of frame delimiter */
>  	enum fc_eof	fr_eof;		/* end of frame delimiter */
>  	u8		fr_flags;	/* flags - see below */
> +	u16		fr_mfs;		/* max FC frame size */
>  };
>  
>  /*
> 
> _______________________________________________
> devel mailing list
> devel at open-fcoe.org
> http://www.open-fcoe.org/mailman/listinfo/devel




More information about the devel mailing list