On 11/12/10 3:39 PM, Nicholas A. Bellinger wrote: > On Fri, 2010-11-12 at 14:46 -0800, Joe Eykholt wrote: >> >> On 11/11/10 5:10 PM, Nicholas A. Bellinger wrote: >>> On Thu, 2010-11-11 at 17:07 -0800, Joe Eykholt wrote: >>>> >>>> On 11/11/10 5:00 PM, Nicholas A. Bellinger wrote: >>>>> On Wed, 2010-11-10 at 17:44 -0800, Kiran Patil wrote: >>>>>> From: Kiran Patil <[email protected]> >>>>>> >>>>>> Problem: Existing implementation of TCM/LIO target isn't taking >>>>>> advantage of underlying HW (NIC) >>>>>> Large Send Offload (LSO) feature. Unable to use non-linear SKBs >>>>>> for CONTROL_NONSG CDBs >>>>>> such as INQUIRY, etc... >>>>>> >>>>>> Fix: Enhance function (ft_queue_data_in) which sends data back to >>>>>> initiator to make use of >>>>>> LSO feature of underlying HW and fix the issues related to >>>>>> CONTROL_NONSG, where code >>>>>> was unable to map TCM provided buffer (t_task_buf) as >>>>>> non-linear SKB. >>>>>> >>>>>> Technical Details: Enhancement benefits in all threee cases , use_sg, >>>>>> mem, and task_buf. This changes >>>>>> are applicable only for all CDBs (DATA/CONTROL SG/NONSG). >>>>>> - Addressed comments from Joe w.r.t checking and making use_sg >>>>>> = 0 only >>>>>> for DATA_SG whereas it is applicale for >>>>>> CONTROL_SG/CONTROL_NONSG. >>>>>> - Made the prink rate limited (in case where "unable to send >>>>>> frame") >>>>>> - Fixed 2 issues/bugs related to CONTROL_NONSG and non-linear >>>>>> SKB. >>>>>> - Added a BUG_ON(!page) >>>>>> >>>>>> Dependencies: This depends on TCM and Joe's FC4 patches for TCM >>>>>> >>>>>> Signed-off-by: Kiran Patil <[email protected]> >>>>>> Signed-off-by: Yi Zou <[email protected]> >>>>> >>>>> Btw, I think this patch looks good guys.. >>>>> >>>>> Joe, can I get your ACK and get this merged into lio-core-2.6.git for >>>>> testing..? >>>> >>>> I reviewed it but didn't test it. Based on that, I'll give my ack. >>>> >>>> Acked-by: Joe Eykholt <[email protected]> >>>> >>> >>> Thanks Joe! Commited as: >>> >>> [tcm_fc_ddp_offload b035e31] TCM/OpenFCoE : Added Large Send Offload >>> Author: Kiran Patil <[email protected]> >>> 1 files changed, 37 insertions(+), 7 deletions(-) >>> >>> and pushed into lio-core-2.6.git/tcm_fc_ddp_offload for testing along >>> with the latest lio-4.0 changes from hch. >>> >>> Thanks Kiran and Yi! >>> >>> --nab >>> > > <SNIP> > >>>>>> @@ -167,7 +191,8 @@ int ft_queue_data_in(struct se_cmd *se_cmd) >>>>>> frame_len -= tlen; >>>>>> remaining -= tlen; >>>>>> >>>>>> - if (frame_len) >>>>>> + if (frame_len && >>>>>> + (skb_shinfo(fp_skb(fp))->nr_frags < >>>>>> FC_FRAME_SG_LEN)) >>>>>> continue; >> >> If we fall through at this point, we should set frame_len to 0, so that >> we'll get a new frame next time around the loop. Previously frame_len would >> be 0 at this point, but if we limited out (very correctly) on the SG_LEN >> check, then we should set frame_len to 0 here. I'm testing a similar fix >> for fcst and ran into this. >> > > Thanks for reporting Joe. Below is the patch being pushed to > lio-core-2.6.git/tcm_fc_ddp_offload now.. > > Also, this needs to be fixed in non tcm_fc_ddp_offload tcm_fc code as > well, yes..?
This is shared for both, I believe. >>From 437651c709056ce3a6c327bb0a74dad518039335 Mon Sep 17 00:00:00 2001 > From: Nicholas Bellinger <[email protected]> > Date: Fri, 12 Nov 2010 23:40:58 +0000 > Subject: [PATCH] tcm_fc: Clear frame_len before skipping to next frame > > If we fall through at this point, we should set frame_len to 0, so that > we'll get a new frame next time around the loop. Previously frame_len would > be 0 at this point, but if we limited out (very correctly) on the SG_LEN > check, then we should set frame_len to 0 here. > > Reported-by: Joe Eykholt <[email protected]> > Signed-off-by: Nicholas A. Bellinger <[email protected]> > --- > drivers/target/tcm_fc/tfc_io.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c > index 4c3c0ef..b3c7a28 100644 > --- a/drivers/target/tcm_fc/tfc_io.c > +++ b/drivers/target/tcm_fc/tfc_io.c > @@ -192,8 +192,10 @@ int ft_queue_data_in(struct se_cmd *se_cmd) > remaining -= tlen; > > if (frame_len && > - (skb_shinfo(fp_skb(fp))->nr_frags < FC_FRAME_SG_LEN)) > + (skb_shinfo(fp_skb(fp))->nr_frags < FC_FRAME_SG_LEN)) { > + frame_len = 0; > continue; > + } Not quite. I still don't have the full fix. I was thinking it should be: > if (frame_len && > (skb_shinfo(fp_skb(fp))->nr_frags < FC_FRAME_SG_LEN)) > continue; > + frame_len = 0; So it'll get a new frame next time around. But, that messes up the fh_offset value, which was computed on the frame_len we had when we started. So I moved that around to fix it as well. I'll see if I can send you a fix later. > if (!remaining) > f_ctl |= FC_FC_END_SEQ; > fc_fill_fc_hdr(fp, FC_RCTL_DD_SOL_DATA, ep->did, ep->sid, _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
