On Fri, 2009-05-01 at 12:06 -0700, Joe Eykholt wrote:
> Vasu Dev wrote:
> > This is required for sharing offload xid range across multiple
> > lport on a eth device on demand using unused exch_get interface
> > of the libfc, the first patch in the series describes more
> > details on this.
> >
> > This series is using multiple EM first time by use of another
> > dedicated EM for offload xid which requires adding em_list per
> > lport, em uses count in em_user counter and walk of list of EM
> > in func fc_exch_mgr_reset to reset each EM instance of a lport.
> >
> > Adding multiple EM uses required some more code in fcoe module
> > to identify EM for a incoming fcoe frame to pass them up to libfc
> > with their associated EM instance (offload or default lp-emp).
> >
> > This patch series is based on fcoe-fixes + my three pending patches
> > in internal validation queue, pending patches are:-
> >
> > -- First two patches for improving fcoe pending queue handling,
> > these are same as sent to list before for RFC.
> > -- Third patch to Fix a framing error when using fcoe over VLAN,
> > this is a tiny change so shouldn't prevent from applying
> > this patch series to fcoe-fixes but if you need then I can
> > send that out first as RFC before its validation done.
> >
> > RFC to:
> > 1. have these patches reviewed before getting them queued for
> > internal validation, so that I could send out real patches
> > this week once their internal validation is done.
> > 2. make sure these are for right fcoe tree, this series is basically
> > fixing an issue due to two xid range use within a EM of lport,
> > so should be applicable to fcoe-fixes tree but if you think this
> > should be queue for fcoe-features then let me know and I'll rediff
> > these patches fcoe-features tree. In that case what will be the
> > order with other pending or new features to avoid merging issues.
> >
> > So far I could finish basic testing on a single interface and currently
> > working on testing for :-
> > 1. multiple interfaces over VLAN
> > 2. modified EM reset code paths
> > 3. verifying xid ranges uses for both offload enabled
> > or disabled eth devices.
> >
> > ---
> >
> > Vasu Dev (6):
> > fcoe: adds exch_get function in fcoe to use offload em instance oemp
> > libfc: allocates dedicated EM for offload xid range
> > fcoe: adds fcoe_em_free
> > libfc, fcoe: allows adding existing em instance to a lport
> > libfc, fcoe: adds em_list towards having more than one EM instance
> > for a lport
> > libfc: removes code for reserving offload exch ids (xids) range
> >
> >
> > drivers/scsi/fcoe/fcoe.c | 200
> > ++++++++++++++++++++++++++++--------------
> > drivers/scsi/fcoe/fcoe.h | 1
> > drivers/scsi/libfc/fc_exch.c | 166 ++++++++++++++---------------------
> > include/scsi/libfc.h | 10 +-
> > 4 files changed, 210 insertions(+), 167 deletions(-)
> >
>
> I haven't had as much time to look into this as it deserves.
>
> One thing, though: it seemed problematic that there was a mechanism to
> share an exchange manager between multiple lports via a reference-counting
> scheme.
> How would exchange-manager-reset work in such a scheme? I guess each exchange
> would need an lport pointer which would have to be matched as it goes through
> the list.
>
The fc_exch_mgr_reset matches each exch to be reset against specified
lport sid during reset to locate only exch of caller lport in shared EM,
but we do call fc_exch_mgr_reset with sid = 0 to reset all exches in EM,
so adding suggested lp check will be good to ensure only specific lport
exches are reset from shared EM instance or just don't allow sid = 0. In
any case each exch lp should point to only its own VN_PORT/lport. Good
point.
> I guess this is for an adapter that has a fixed exchange space, but wants to
> share it
> between local ports on different VLANs, in a flexible way. Not bad.
>
And also for NPIV, however this sharing is now in fcoe layer so it is
optional in case adapter doesn't have fixed exchange id space and better
than we had this before in libfc as another range in single EM.
> It might be good to leave the existing interfaces alone as much as possible
> and add
> new ones to: add an em to an lport, take/drop a reference on an em, reset a
> single em.
>
Only interface change in this regard is passing an EM pointer to
fc_exch_mgr_alloc API and other adapter can pass NULL if they don't need
to share a EM across lports. However adding separate API for this is
also doable. I'm fine either way.
Rob is taking over this patchset from here so I'll leave this up to him
to rework for this or not.
> I'd also like to point out a separate problem with sequence ID space.
> Sequence IDs
> should be unique across all exchanges between a port pair, which is something
> we've
> been lax about. Mostly our sequences don't have a long life ... the only
> ones with
> more than one frame are data and discovery sequences. On multiple CPUs,
> though, or
> when there is hardware offload, there could be intermingling of sequences
> during
> transmit. That is, one CPU sending data frames for one sequence and another
> CPU
> (or offload engine) sending data frames on another sequence for the same port
> could
> end up using the same sequence ID.
>
> I propose we correct this by limiting the sequence-ID range by exchange
> manager as well.
> I hope to have patches for this at some point, but if someone else does it
> that'd be great.
>
Implementing sequence-ID by rport could be another option so that
sequence will be always unique to a rport, but that may create more
contention across CPUs for incrementing a sequence-ID. Good point to
think more about this.
Thanks Joe as always for good feedback.
Vasu
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
