On Mon, 2009-06-01 at 11:27 -0700, Robert Love wrote:
> On Mon, 2009-06-01 at 10:14 -0700, Vasu Dev wrote:
> > Currently part of total xid range of default exchange manger (EM)
> > lp->emp is reserved as offload xids(DDP) range, this is not
> > effective for these reason:
> > 
> >  1. This won't allow full DDP xid range sharing on demand across
> >     multiple VN_PORT on a eth device for either NPIV or VLAN,
> >     the VN_PORT is the lport instance in libfc implementation.
> > 
> >  2. Requires additional checks in fast path for two xid range
> >     management within a single EM instance
> > 
> > This patch removes code reserving offload xids range and instead
> > by end of this patch series, a dedicated EM instance will be
> > allocated for this range to allow effective offload xid space
> > sharing across multiple lports on a eth device.
> > 
> > Completely removes func fc_em_alloc_xid needed for selecting a
> > xid from two ranges, instead adds simple logic to allocate xid
> > with fewer args to fc_exch_alloc, removes no longer needed fp
> > and xid args to fc_exch_alloc.
> > 
> > RFC notes:-
> >  1. Get review comments for modified code to address review comments
> >     from http://www.open-fcoe.org/pipermail/devel/2009-May/002329.html
> >  2. I've tested this series for single interface since I could n't
> >     get setup ready for multiple VLAN on eth interface on fabric
> >     side to test shared em code. Now that NPIV is ready so I will
> >     test with NPIV but NPIV code is also new code, so finding any
> >     issues by review will be more helpful to test shared EM code with
> >     NPIV, so I'd appreciate your feedback on any issues on this.
> >  3. The patch 3 could be combined with patch 2, but to help review
> >     and keep original code authorship clean, I kept patch 3 separate.
> >     However if Rob agrees then I'll merge this patch 3 with patch 2.
> 
> Thank you for considering me. For this case please merge the patches if
> it makes sense, you don't need to retain any of my authorship on this
> series.
>  

Okay np, this will be quick just a command a away being just next patch
in series.

> >  4. The discussed em_list with match function will require more design
> >     thoughts for shared em instance across lports, so I implemented
> >     shared em using previously designed libfc interface of exch_get,
> >     therefore the em_list with match func for shared em cannot be done
> >     now or any soon. I think going into that direction will possible
> >     delay other features depending on shared EM patches since as
> >     I'm told NPIV is waiting for shared EM instance feature in line.
> > 
> > This series is based on following commit on fcoe-features tree:
> > 
> >  commit 9da17a6c18a4db817d7c7605f2a7a98dfe41e572
> >  Author: Vasu Dev <[email protected]>
> >  Date:   Thu May 28 15:34:56 2009 -0700
> >  fcoe: removes fcoe_watchdog
> > 
> > Signed-off-by: Vasu Dev <[email protected]>
> > ---
> > 
> >  drivers/scsi/libfc/fc_exch.c |   92 
> > +++++++-----------------------------------
> >  include/scsi/libfc.h         |    5 --
> >  2 files changed, 17 insertions(+), 80 deletions(-)
> > 
> > diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> > index 2bc22be..bbabd2c 100644
> > --- a/drivers/scsi/libfc/fc_exch.c
> > +++ b/drivers/scsi/libfc/fc_exch.c
> > @@ -60,9 +60,7 @@ struct fc_exch_mgr {
> >     u16             last_xid;       /* last allocated exchange ID */
> >     u16             min_xid;        /* min exchange ID */
> >     u16             max_xid;        /* max exchange ID */
> > -   u16             max_read;       /* max exchange ID for read */
> > -   u16             last_read;      /* last xid allocated for read */
> > -   u32     total_exches;           /* total allocated exchanges */
> > +   u32             total_exches;   /* total allocated exchanges */
> >     struct list_head        ex_list;        /* allocated exchanges list */
> >     struct fc_lport *lp;            /* fc device instance */
> >     mempool_t       *ep_pool;       /* reserve ep's */
> > @@ -461,64 +459,15 @@ static struct fc_seq *fc_seq_alloc(struct fc_exch 
> > *ep, u8 seq_id)
> >  }
> >  
> >  /*
> > - * fc_em_alloc_xid - returns an xid based on request type
> > - * @lp : ptr to associated lport
> > - * @fp : ptr to the assocated frame
> > - *
> > - * check the associated fc_fsp_pkt to get scsi command type and
> > - * command direction to decide from which range this exch id
> > - * will be allocated from.
> > - *
> > - * Returns : 0 or an valid xid
> > - */
> > -static u16 fc_em_alloc_xid(struct fc_exch_mgr *mp, const struct fc_frame 
> > *fp)
> > -{
> > -   u16 xid, min, max;
> > -   u16 *plast;
> > -   struct fc_exch *ep = NULL;
> > -
> > -   if (mp->max_read) {
> > -           if (fc_fcp_is_read(fr_fsp(fp))) {
> 
> Doesn't the removal of this code break offloads? Maybe I don't
> understand. After this patch is applied how does the EM use the offload
> XID range?
> 

No it doesn't break, since this patch doesn't remove call to
fc_fcp_ddp_setup() and DDP will be still set for a allocated exchange id
for read  IO within offload xid range by fc_fcp_ddp_setup() =>
fc_fcp_ddp_setup, the fc_fcp_ddp_setup already checks for offload xid
range and read type IO.

        Vasu

_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel

Reply via email to