Robert Love wrote:
> From: Vasu Dev <[email protected]>
>
> Previously when a netdev supported offloaded Rx it was
> indicating the range of XIDs for offload to the singular
> EM that was associated with the lport on the port.
>
> Previous patches have made it so an EM can be shared across
> multiple lports. This patch adds an offload EM to the fcoe_softc
> and allows the fcoe module to override the exch_get() function
> poiner in libfc's template so that requests for exchanges are
> handled by fcoe. fcoe can then choose from either the default
> EM associated with the lport the exchange is on or the offload
> EM associated with the lport's netdev.
>
> To accomplish this the patch does the following:
>
> - Adds oem (offload EM) pointer to the fcoe_softc
>
> - Reorganize EM freeing to consider the offload EM
>
> - Reorganize fcoe_em_alloc() (previously fcoe_em_config()) to
> assign the same EMs to any VN_Ports (lports) associated with
> the same netdev->phys_dev.
>
> - Implements fcoe_exch_recv() to handle incoming frames and
> determine with EM should handle the inbound exchange.
>
> - Overrides the exch_get() routine of the libfc template
> so that fcoe can use offload EM when offload criteria is met
> (i.e. exchange will be for a read request).
>
> Signed-off-by: Vasu Dev <[email protected]>
> Signed-off-by: Robert Love <[email protected]>
> ---
>
> drivers/scsi/fcoe/fcoe.c | 134
> +++++++++++++++++++++++++++++++++++++++---
> drivers/scsi/fcoe/fcoe.h | 1
> drivers/scsi/libfc/fc_exch.c | 92 +++++------------------------
> include/scsi/libfc.h | 5 --
> 4 files changed, 141 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index e4ba631..f589a47 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -407,20 +407,78 @@ static int fcoe_shost_config(struct fc_lport *lp,
> struct Scsi_Host *shost,
> }
>
> /**
> - * fcoe_em_config() - allocates em for this lport
> + * fcoe_em_alloc() - allocates em for this lport
> * @lp: the port that em is to allocated for
> *
> * Returns : 0 on success
> */
> -static inline int fcoe_em_config(struct fc_lport *lp)
> +static inline int fcoe_em_alloc(struct fc_lport *lp)
> {
> + struct fcoe_softc *fc = lport_priv(lp);
> + struct fcoe_softc *oldfc = NULL;
> + int rc = 0;
> +
> BUG_ON(lp->emp);
>
> + /*
> + * Check if need to allocate an em instance for
> + * offload exchange ids to be shared across all VN_PORTs/lport.
> + */
> + if (!lp->lro_enabled || !lp->lro_xid || (lp->lro_xid >= FCOE_MAX_XID)) {
> + lp->lro_xid = 0;
> + goto skip_oem;
> + }
> +
> + /*
> + * Reuse exitsing offload em instance in case
typo: existing
> + * it is already allocated on phys_dev
> + */
> + write_lock(&fcoe_hostlist_lock);
> + list_for_each_entry(oldfc, &fcoe_hostlist, list) {
> + if (oldfc->phys_dev == fc->phys_dev) {
> + fc->oem = oldfc->oem;
Take a reference here?
What if the other other instance gets deleted, including the EM
before we start using it? Maybe do the exch_mgr_alloc inside the lock?
> + break;
> + }
> + }
> + write_unlock(&fcoe_hostlist_lock);
> +
> + fc->oem = fc_exch_mgr_alloc(lp, fc->oem, FC_CLASS_3,
> + FCOE_MIN_XID, lp->lro_xid);
> +
> + if (!fc->oem) {
> + printk(KERN_ERR "fcoe_em_alloc: failed to "
> + "allocate em for offload%s\n", fc->real_dev->name);
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> +skip_oem:
> lp->emp = fc_exch_mgr_alloc(lp, NULL, FC_CLASS_3,
> - FCOE_MIN_XID, FCOE_MAX_XID);
> - if (!lp->emp)
> - return -ENOMEM;
> + FCOE_MIN_XID + lp->lro_xid,
> + FCOE_MAX_XID);
> +
> + if (!lp->emp) {
> + printk(KERN_ERR "Failed to allocate an Exchange "
> + "Manager for %s\n", fc->real_dev->name);
> + rc = -ENOMEM;
> + }
> +out:
> + return rc;
> +}
>
> +/**
> + * fcoe_em_free() - frees allocated em for a lport
> + * @lp: the port that em is to allocated for
> + *
> + * Returns : 0 on success
> + */
> +static inline int fcoe_em_free(struct fc_lport *lp)
> +{
> + struct fcoe_softc *fc = lport_priv(lp);
> +
> + if (fc->oem)
> + fc_exch_mgr_free(fc->oem);
> + fc_exch_mgr_free(lp->emp);
> return 0;
> }
>
> @@ -468,7 +526,7 @@ static int fcoe_if_destroy(struct net_device *netdev)
>
> /* There are no more rports or I/O, free the EM */
> if (lp->emp)
> - fc_exch_mgr_free(lp->emp);
> + fcoe_em_free(lp);
>
> /* Free the per-CPU revieve threads */
> fcoe_percpu_clean(lp);
> @@ -486,6 +544,59 @@ static int fcoe_if_destroy(struct net_device *netdev)
> return 0;
> }
>
> +/**
> + * fcoe_exch_recv() - receive a incoming frame
> + * @lp: the associated local port
> + * @fp: received fc_frame
> + *
> + * Locates exchange manager instance for received frame
> + * and pass it up to libfc using fc_exch_recv api.
> + */
> +static void fcoe_exch_recv(struct fc_lport *lp, struct fc_frame *fp)
> +{
> + struct fcoe_softc *fc = lport_priv(lp);
> + struct fc_frame_header *fh = fc_frame_header_get(fp);
> + u16 oxid = ntohs(fh->fh_ox_id);
> + u32 ctx;
> +
> + if (lp->lro_enabled && (oxid <= lp->lro_xid)) {
Should this be < instead of <= ?
If I'm correct, the first XID belonging to the non-offload EM is lro_xid.
BTW, parens are not needed around the comparison.
Since lro_xid is 0 when lro is disabled, we don't need to check lro_enabled.
Just:
if (oxid < lp->lro_xid) {
> + ctx = ntoh24(fh->fh_f_ctl) & (FC_FC_EX_CTX | FC_FC_SEQ_CTX);
> + if (ctx == FC_FC_EX_CTX) {
Maybe this should allow SEQ_CTX to be 1 or 0, not just 0.
That would permit class 2 action someday. Unless class 2 is supported,
it isn't necessary to check SEQ_CTX here. OK if you want to leave it as is.
> + fc_exch_recv(lp, fc->oem, fp);
> + return;
> + }
> + }
> + fc_exch_recv(lp, lp->emp, fp);
> +}
> +
> +/**
> + * fcoe_exch_get() - allocates an exchange
> + * @lp: the associated local port
> + * @fp: the fc_frame to be transmitted
> + *
> + * Allocates exchange from fc->oem for offload eligible frames
> + * otherwise allocates from exhcange from the default EM instance.
typo: exchange
> + */
> +static struct fc_exch *fcoe_exch_get(struct fc_lport *lp, struct fc_frame
> *fp)
> +{
> + struct fcoe_softc *fc = lport_priv(lp);
> + struct fc_exch *ep = NULL;
Don't initialize ep here. It won't be needed if we
rearrange to do ddp setup in the read case only.
> +
> + if (fc_fcp_is_read(fr_fsp(fp)))
{
> + ep = fc_exch_alloc(fc->oem);
> +
> + /*
> + * if valid ep allocated from fc->ofemp
> + * then set up ddp here, else allocate
> + * ep from other em.
> + */
> + if (ep)
This should be part of the above if-statement.
Test this only in the case of a read.
> + fc_fcp_ddp_setup(fr_fsp(fp), ep->xid);
> + else
> + ep = fc_exch_alloc(lp->emp);
> + return ep;
> +}
> +
> /*
> * fcoe_ddp_setup - calls LLD's ddp_setup through net_device
> * @lp: the corresponding fc_lport
> @@ -590,6 +701,9 @@ static int fcoe_if_create(struct net_device *netdev)
> goto out_netdev_cleanup;
> }
>
> + /* set exch_get if lro enabled */
> + if (lp->lro_enabled)
> + fcoe_libfc_fcn_templ.exch_get = fcoe_exch_get;
> /* Initialize the library */
> rc = fcoe_libfc_config(lp, &fcoe_libfc_fcn_templ);
> if (rc) {
> @@ -598,7 +712,7 @@ static int fcoe_if_create(struct net_device *netdev)
> }
>
> /* lport exch manager allocation */
> - rc = fcoe_em_config(lp);
> + rc = fcoe_em_alloc(lp);
> if (rc) {
> FCOE_NETDEV_DBG(netdev, "Could not configure the EM for the "
> "interface\n");
> @@ -620,7 +734,7 @@ static int fcoe_if_create(struct net_device *netdev)
> return rc;
>
> out_lp_destroy:
> - fc_exch_mgr_free(lp->emp); /* Free the EM */
> + fcoe_em_free(lp);
> out_netdev_cleanup:
> fcoe_netdev_cleanup(fc);
> out_host_put:
> @@ -1279,7 +1393,7 @@ int fcoe_percpu_receive_thread(void *arg)
> fh = fc_frame_header_get(fp);
> if (fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA &&
> fh->fh_type == FC_TYPE_FCP) {
> - fc_exch_recv(lp, lp->emp, fp);
> + fcoe_exch_recv(lp, fp);
> continue;
> }
> if (fr_flags(fp) & FCPHF_CRC_UNCHECKED) {
> @@ -1300,7 +1414,7 @@ int fcoe_percpu_receive_thread(void *arg)
> fc_frame_free(fp);
> continue;
> }
> - fc_exch_recv(lp, lp->emp, fp);
> + fcoe_exch_recv(lp, fp);
> }
> return 0;
> }
> diff --git a/drivers/scsi/fcoe/fcoe.h b/drivers/scsi/fcoe/fcoe.h
> index 917aae8..4db2330 100644
> --- a/drivers/scsi/fcoe/fcoe.h
> +++ b/drivers/scsi/fcoe/fcoe.h
> @@ -57,6 +57,7 @@ struct fcoe_softc {
> struct list_head list;
> struct net_device *real_dev;
> struct net_device *phys_dev; /* device with ethtool_ops */
> + struct fc_exch_mgr *oem; /* offload exchane manager */
typo: exchange
> struct packet_type fcoe_packet_type;
> struct packet_type fip_packet_type;
> struct sk_buff_head fcoe_pending_queue;
> diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> index d429e8b..0664366 100644
> --- a/drivers/scsi/libfc/fc_exch.c
> +++ b/drivers/scsi/libfc/fc_exch.c
> @@ -72,9 +72,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 list_head em_list; /* fc_exch_mgr list */
> struct fc_lport *lp; /* fc device instance */
> @@ -474,64 +472,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))) {
> - min = mp->min_xid;
> - max = mp->max_read;
> - plast = &mp->last_read;
> - } else {
> - min = mp->max_read + 1;
> - max = mp->max_xid;
> - plast = &mp->last_xid;
> - }
> - } else {
> - min = mp->min_xid;
> - max = mp->max_xid;
> - plast = &mp->last_xid;
> - }
> - xid = *plast;
> - do {
> - xid = (xid == max) ? min : xid + 1;
> - ep = mp->exches[xid - mp->min_xid];
> - } while ((ep != NULL) && (xid != *plast));
> -
> - if (unlikely(ep))
> - xid = 0;
> - else
> - *plast = xid;
> -
> - return xid;
> -}
> -
> -/*
> - * fc_exch_alloc - allocate an exchange.
> + * fc_exch_alloc - allocate an exchange and its exchange id.
> * @mp : ptr to the exchange manager
> - * @xid: input xid
> *
> - * if xid is supplied zero then assign next free exchange ID
> - * from exchange manager, otherwise use supplied xid.
> * Returns with exch lock held.
> */
> -struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp,
> - struct fc_frame *fp, u16 xid)
> +struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp)
> {
> struct fc_exch *ep;
> + u16 min, max, xid;
>
> /* allocate memory for exchange */
> ep = mempool_alloc(mp->ep_pool, GFP_ATOMIC);
> @@ -541,16 +490,18 @@ struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp,
> }
> memset(ep, 0, sizeof(*ep));
>
> + min = mp->min_xid;
> + max = mp->max_xid;
> +
> spin_lock_bh(&mp->em_lock);
> - /* alloc xid if input xid 0 */
> - if (!xid) {
> - /* alloc a new xid */
> - xid = fc_em_alloc_xid(mp, fp);
> - if (!xid) {
> - printk(KERN_ERR "fc_em_alloc_xid() failed\n");
> + xid = mp->last_xid;
> + /* alloc a new xid */
> + while (mp->exches[xid - min]) {
> + xid = (xid == max) ? min : xid + 1;
> + if (xid == mp->last_xid)
> goto err;
> - }
> }
> + mp->last_xid = xid;
>
> fc_exch_hold(ep); /* hold for exch in mp */
> spin_lock_init(&ep->ex_lock);
> @@ -1761,7 +1712,6 @@ struct fc_exch_mgr *fc_exch_mgr_alloc(struct fc_lport
> *lp,
> /*
> * Memory need for EM
> */
> -#define xid_ok(i, m1, m2) (((i) >= (m1)) && ((i) <= (m2)))
> len = (max_xid - min_xid + 1) * (sizeof(struct fc_exch *));
> len += sizeof(struct fc_exch_mgr);
>
> @@ -1775,17 +1725,7 @@ struct fc_exch_mgr *fc_exch_mgr_alloc(struct fc_lport
> *lp,
> /* adjust em exch xid range for offload */
> mp->min_xid = min_xid;
> mp->max_xid = max_xid;
> - mp->last_xid = min_xid - 1;
> - mp->max_read = 0;
> - mp->last_read = 0;
> - if (lp->lro_enabled && xid_ok(lp->lro_xid, min_xid, max_xid)) {
> - mp->max_read = lp->lro_xid;
> - mp->last_read = min_xid - 1;
> - mp->last_xid = mp->max_read;
> - } else {
> - /* disable lro if no xid control over read */
> - lp->lro_enabled = 0;
> - }
> + mp->last_xid = min_xid;
>
> INIT_LIST_HEAD(&mp->ex_list);
> spin_lock_init(&mp->em_lock);
> @@ -1830,7 +1770,7 @@ struct fc_exch *fc_exch_get(struct fc_lport *lp, struct
> fc_frame *fp)
> if (!lp || !lp->emp)
> return NULL;
>
> - return fc_exch_alloc(lp->emp, fp, 0);
> + return fc_exch_alloc(lp->emp);
> }
> EXPORT_SYMBOL(fc_exch_get);
>
> @@ -1867,8 +1807,6 @@ struct fc_seq *fc_exch_seq_send(struct fc_lport *lp,
> fc_exch_setup_hdr(ep, fp, ep->f_ctl);
> sp->cnt++;
>
> - fc_fcp_ddp_setup(fr_fsp(fp), ep->xid);
> -
> if (unlikely(lp->tt.frame_send(lp, fp)))
> goto err;
>
> diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h
> index 68f7a61..99d687c 100644
> --- a/include/scsi/libfc.h
> +++ b/include/scsi/libfc.h
> @@ -992,11 +992,8 @@ struct fc_exch *fc_exch_get(struct fc_lport *lp, struct
> fc_frame *fp);
>
> /*
> * Allocate a new exchange and sequence pair.
> - * if ex_id is zero then next free exchange id
> - * from specified exchange manger mp will be assigned.
> */
> -struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp,
> - struct fc_frame *fp, u16 ex_id);
> +struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp);
> /*
> * Start a new sequence on the same exchange as the supplied sequence.
> */
>
> _______________________________________________
> 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
