Re: [PATCH V3 2/2] qedf: Add QLogic FastLinQ offload FCoE driver framework.
On Mon, 6 Feb 2017, 3:56pm -, Hannes Reinecke wrote:
> On 02/03/2017 08:17 PM, Dupuis, Chad wrote:
> > From: "Dupuis, Chad"
> >
> > The QLogic FastLinQ Driver for FCoE (qedf) is the FCoE specific module for
> > 41000
> > Series Converged Network Adapters by QLogic. This patch consists of
> > following
> > changes:
> >
> > - MAINTAINERS Makefile and Kconfig changes for qedf
> > - PCI driver registration
> > - libfc/fcoe host level initialization
> > - SCSI host template initialization and callbacks
> > - Debugfs and log level infrastructure
> > - Link handling
> > - Firmware interface structures
> > - QED core module initialization
> > - Light L2 interface callbacks
> > - I/O request initialization
> > - Firmware I/O completion handling
> > - Firmware ELS request/response handling
> > - FIP request/response handled by the driver itself
> >
> > Signed-off-by: Nilesh Javali
> > Signed-off-by: Manish Rangankar
> > Signed-off-by: Saurav Kashyap
> > Signed-off-by: Arun Easi
> > Signed-off-by: Chad Dupuis
> > ---
> [ .. ]
> > +static void qedf_process_l2_frame_compl(struct qedf_rport *fcport,
> > + unsigned char *buf,
> > + u32 frame_len, u16 l2_oxid)
> > +{
> > + struct fc_lport *lport = fcport->qedf->lport;
> > + struct fc_frame_header *fh;
> > + struct fc_frame *fp;
> > + u32 payload_len;
> > + u32 crc;
> > +
> > + payload_len = frame_len - sizeof(struct fc_frame_header);
> > +
> > + fp = fc_frame_alloc(lport, payload_len);
> > + if (!fp) {
> > + QEDF_ERR(&(fcport->qedf->dbg_ctx),
> > + "fc_frame_alloc failure.\n");
> > + return;
> > + }
> > +
> > + /* Copy FC Frame header and payload into the frame */
> > + fh = (struct fc_frame_header *)fc_frame_header_get(fp);
> > + memcpy(fh, buf, frame_len);
> > +
> > + /* Set the OXID we return to what libfc used */
> > + if (l2_oxid != FC_XID_UNKNOWN)
> > + fh->fh_ox_id = htons(l2_oxid);
> > +
> > + /* Setup header fields */
> > + fh->fh_r_ctl = FC_RCTL_ELS_REP;
> > + fh->fh_type = FC_TYPE_ELS;
> > + /* Last sequence, end sequence */
> > + fh->fh_f_ctl[0] = 0x98;
> > + hton24(fh->fh_d_id, lport->port_id);
> > + hton24(fh->fh_s_id, fcport->rdata->ids.port_id);
> > + fh->fh_rx_id = 0x;
> > +
> > + /* Set frame attributes */
> > + crc = fcoe_fc_crc(fp);
> > + fc_frame_init(fp);
> > + fr_dev(fp) = lport;
> > + fr_sof(fp) = FC_SOF_I3;
> > + fr_eof(fp) = FC_EOF_T;
> > + fr_crc(fp) = cpu_to_le32(~crc);
> > +
> > + /* Send completed request to libfc */
> > + fc_exch_recv(lport, fp);
> > +}
> > +
> > +/*
> > + * In instances where an ELS command times out we may need to restart the
> > + * rport by logging out and then logging back in.
> > + */
> > +void qedf_restart_rport(struct qedf_rport *fcport)
> > +{
> > + struct fc_lport *lport;
> > + struct fc_rport_priv *rdata;
> > + u32 port_id;
> > +
> > + if (!fcport)
> > + return;
> > +
> > + rdata = fcport->rdata;
> > + if (rdata) {
> > + lport = fcport->qedf->lport;
> > + port_id = rdata->ids.port_id;
> > + QEDF_ERR(&(fcport->qedf->dbg_ctx),
> > + "LOGO port_id=%x.\n", port_id);
> > + fc_rport_logoff(rdata);
> > + /* Recreate the rport and log back in */
> > + rdata = fc_rport_create(lport, port_id);
> > + if (rdata)
> > + fc_rport_login(rdata);
> > + }
> > +}
> > +
> > +static void qedf_l2_els_compl(struct qedf_els_cb_arg *cb_arg)
> > +{
> > + struct qedf_ioreq *els_req;
> > + struct qedf_rport *fcport;
> > + struct qedf_mp_req *mp_req;
> > + struct fc_frame_header *fc_hdr;
> > + unsigned char *buf;
> > + void *resp_buf;
> > + u32 resp_len, hdr_len;
> > + u16 l2_oxid;
> > + int frame_len;
> > +
> > + l2_oxid = cb_arg->l2_oxid;
> > + els_req = cb_arg->io_req;
> > +
> > + if (!els_req) {
> > + QEDF_ERR(NULL, "els_req is NULL.\n");
> > + goto free_arg;
> > + }
> > +
> > + /*
> > +* If we are flushing the co
Re: [PATCH V2 2/2] qedf: Add QLogic FastLinQ offload FCoE driver framework.
On Mon, 30 Jan 2017, 10:34am -, Hannes Reinecke wrote:
> On 01/25/2017 09:33 PM, Dupuis, Chad wrote:
> > +static int qedf_request_msix_irq(struct qedf_ctx *qedf)
> > +{
> > + int i, rc, cpu;
> > +
> > + cpu = cpumask_first(cpu_online_mask);
> > + for (i = 0; i < qedf->num_queues; i++) {
> > + rc = request_irq(qedf->int_info.msix[i].vector,
> > + qedf_msix_handler, 0, "qedf", &qedf->fp_array[i]);
> > +
> > + if (rc) {
> > + QEDF_WARN(&(qedf->dbg_ctx), "request_irq failed.\n");
> > + qedf_sync_free_irqs(qedf);
> > + return rc;
> > + }
> > +
> > + qedf->int_info.used_cnt++;
> > + rc = irq_set_affinity_hint(qedf->int_info.msix[i].vector,
> > + get_cpu_mask(cpu));
> > + cpu = cpumask_next(cpu, cpu_online_mask);
> > + }
> > +
> > + return 0;
> > +}
> > +
> Please use pci_alloc_irq_vectors() here; that avoid you having to do SMP
> affinity setting yourself.
This wil be difficult to coordinate with three other drivers (qedi, qede
and qedr) using the same vector allocation code in the qed module.
>
> Cheers,
>
> Hannes
>
Re: [PATCH V2 2/2] qedf: Add QLogic FastLinQ offload FCoE driver framework.
On Mon, 30 Jan 2017, 10:34am -, Hannes Reinecke wrote:
> On 01/25/2017 09:33 PM, Dupuis, Chad wrote:
> > From: "Dupuis, Chad"
> >
> > The QLogic FastLinQ Driver for FCoE (qedf) is the FCoE specific module for
> > 41000
> > Series Converged Network Adapters by QLogic. This patch consists of
> > following
> > changes:
> >
> > - MAINTAINERS Makefile and Kconfig changes for qedf
> > - PCI driver registration
> > - libfc/fcoe host level initialization
> > - SCSI host template initialization and callbacks
> > - Debugfs and log level infrastructure
> > - Link handling
> > - Firmware interface structures
> > - QED core module initialization
> > - Light L2 interface callbacks
> > - I/O request initialization
> > - Firmware I/O completion handling
> > - Firmware ELS request/response handling
> > - FIP request/response handled by the driver itself
> >
> > Signed-off-by: Nilesh Javali
> > Signed-off-by: Manish Rangankar
> > Signed-off-by: Saurav Kashyap
> > Signed-off-by: Arun Easi
> > Signed-off-by: Chad Dupuis
> > ---
> [ .. ]
> > +#define QEDF_IO_WORK_MIN 64
> > + mempool_t *io_mempool;
> > + struct workqueue_struct *dpc_wq;
> > +
> > + u32 slow_sge_ios;
> > + u32 fast_sge_ios;
> > + u32 single_sge_ios;
> > +
> > + uint8_t *grcdump;
> > + uint32_t grcdump_size;
> > +
> > + struct qedf_io_log io_trace_buf[QEDF_IO_TRACE_SIZE];
> > + spinlock_t io_trace_lock;
> > + uint16_t io_trace_idx;
> > +
> > + bool stop_io_on_error;
> > +
> > + u32 flogi_cnt;
> > + u32 flogi_failed;
> > +
> > + /* Used for fc statistics */
> > + u64 input_requests;
> > + u64 output_requests;
> > + u64 control_requests;
> > + u64 packet_aborts;
> > + u64 alloc_failures;
> > +};
> > +
> > +/*
> > + * 4 regs size $$KEEP_ENDIANNESS$$
> > + */
> > +
> What is this supposed to mean?
> Does it refer to the next structure?
I think it's a superfluous comment. I'll remove it.
>
> > +struct io_bdt {
> > + struct qedf_ioreq *io_req;
> > + struct fcoe_sge *bd_tbl;
> > + dma_addr_t bd_tbl_dma;
> > + u16 bd_valid;
> > +};
> > +
> [ .. ]
>
> > +
> > +static inline void qedf_stop_all_io(struct qedf_ctx *qedf)
> > +{
> > + set_bit(QEDF_UNLOADING, &qedf->flags);
> > +}
> > +
> This looks like a misnomer.
> Why is 'UNLOADING' equivalent to stopping all I/O?
> I could imagine quite some situations where I would want to stop I/O
> without unloading the driver.
>
> Please use better naming here; either name the bit 'QEDF_STOP_IO' or
> something or call the function 'qedf_is_unloading'.
>
This is more a debugfs mechanism to be able to stop i/o from a script or
other outside entity. I was piggy-backing off of functionality that stops
I/O submission while the driver is unloading but I'll add a define
specific to debug stopping of I/O.
>
> [ .. ]
> > +/*
> > + * In instances where an ELS command times out we may need to restart the
> > + * rport by logging out and then logging back in.
> > + */
> > +void qedf_restart_rport(struct qedf_rport *fcport)
> > +{
> > + struct fc_lport *lport;
> > + struct fc_rport_priv *rdata;
> > + u32 port_id;
> > +
> > + if (!fcport)
> > + return;
> > +
> > + rdata = fcport->rdata;
> > + if (rdata) {
> > + lport = fcport->qedf->lport;
> > + port_id = rdata->ids.port_id;
> > + QEDF_ERR(&(fcport->qedf->dbg_ctx),
> > + "LOGO port_id=%x.\n", port_id);
> > + mutex_lock(&lport->disc.disc_mutex);
> > + fc_rport_logoff(rdata);
> > + /* Recreate the rport and log back in */
> > + rdata = fc_rport_create(lport, port_id);
> > + if (rdata)
> > + fc_rport_login(rdata);
> > + mutex_unlock(&lport->disc.disc_mutex);
> > + }
> > +}
> > +
> Please don't hold the discovery mutex when calling fc_rport_logoff().
> Calling rport_logoff might call into exch_mgr_reset() which in turn
> might need to take the discover mutex.
Looking at other code it looked like this mutex was held when logging off
the port and then logging on. I'll remove it however.
>
> [ .. ]
> > + if (opcode == ELS_LS_RJT) {
> > + rjt = fc_frame_pa
Re: [PATCH 1/2] qed: Add support for hardware offloaded FCoE.
I forgot to add netdev-next to the subject line. Is a repost needed here? On Mon, 16 Jan 2017, 7:53pm -, Dupuis, Chad wrote: > From: Arun Easi > > This adds the backbone required for the various HW initalizations > which are necessary for the FCoE driver (qedf) for QLogic FastLinQ > 4 line of adapters - FW notification, resource initializations, etc. > > Signed-off-by: Arun Easi > Signed-off-by: Yuval Mintz > --- > drivers/net/ethernet/qlogic/Kconfig | 3 + > drivers/net/ethernet/qlogic/qed/Makefile | 1 + > drivers/net/ethernet/qlogic/qed/qed.h | 11 + > drivers/net/ethernet/qlogic/qed/qed_cxt.c | 98 ++- > drivers/net/ethernet/qlogic/qed/qed_cxt.h | 3 + > drivers/net/ethernet/qlogic/qed/qed_dcbx.c| 11 + > drivers/net/ethernet/qlogic/qed/qed_dcbx.h| 1 + > drivers/net/ethernet/qlogic/qed/qed_dev.c | 205 - > drivers/net/ethernet/qlogic/qed/qed_dev_api.h | 42 + > drivers/net/ethernet/qlogic/qed/qed_fcoe.c| 990 > ++ > drivers/net/ethernet/qlogic/qed/qed_fcoe.h| 52 ++ > drivers/net/ethernet/qlogic/qed/qed_hsi.h | 781 - > drivers/net/ethernet/qlogic/qed/qed_hw.c | 3 + > drivers/net/ethernet/qlogic/qed/qed_ll2.c | 25 + > drivers/net/ethernet/qlogic/qed/qed_ll2.h | 2 +- > drivers/net/ethernet/qlogic/qed/qed_main.c| 7 + > drivers/net/ethernet/qlogic/qed/qed_mcp.c | 3 + > drivers/net/ethernet/qlogic/qed/qed_mcp.h | 1 + > drivers/net/ethernet/qlogic/qed/qed_reg_addr.h| 8 + > drivers/net/ethernet/qlogic/qed/qed_sp.h | 4 + > drivers/net/ethernet/qlogic/qed/qed_sp_commands.c | 3 + > include/linux/qed/common_hsi.h| 10 +- > include/linux/qed/fcoe_common.h | 715 > include/linux/qed/qed_fcoe_if.h | 145 > include/linux/qed/qed_if.h| 39 + > 25 files changed, 3152 insertions(+), 11 deletions(-) > create mode 100644 drivers/net/ethernet/qlogic/qed/qed_fcoe.c > create mode 100644 drivers/net/ethernet/qlogic/qed/qed_fcoe.h > create mode 100644 include/linux/qed/fcoe_common.h > create mode 100644 include/linux/qed/qed_fcoe_if.h >
Re: [Open-FCoE] [PATCH RFC 5/5] qedf: Add FIP request handling
On Wed, 28 Dec 2016, 9:11am -, Hannes Reinecke wrote:
> On 12/23/2016 08:17 PM, Dupuis, Chad wrote:
> > From: "Dupuis, Chad"
> >
> > This patch adds handling for FIP requests and responses that are handled by
> > the driver itself and not by libfcoe.
> >
> > Signed-off-by: Nilesh Javali
> > Signed-off-by: Manish Rangankar
> > Signed-off-by: Saurav Kashyap
> > Signed-off-by: Chad Dupuis
> > ---
> > drivers/scsi/qedf/qedf_fip.c | 267
> > +++
> > 1 file changed, 267 insertions(+)
> > create mode 100644 drivers/scsi/qedf/qedf_fip.c
> >
> > diff --git a/drivers/scsi/qedf/qedf_fip.c b/drivers/scsi/qedf/qedf_fip.c
> > new file mode 100644
> > index 000..4f185c6
> > --- /dev/null
> > +++ b/drivers/scsi/qedf/qedf_fip.c
> > @@ -0,0 +1,267 @@
> > +/*
> > + * QLogic FCoE Offload Driver
> > + * Copyright (c) 2016 Cavium Inc.
> > + *
> > + * This software is available under the terms of the GNU General Public
> > License
> > + * (GPL) Version 2, available from the file COPYING in the main directory
> > of
> > + * this source tree.
> > + */
> > +#include
> > +#include
> > +#include "qedf.h"
> > +
> > +extern const struct qed_fcoe_ops *qed_ops;
> > +/*
> > + * FIP VLAN functions that will eventually move to libfcoe.
> > + */
> > +
> > +void qedf_fcoe_send_vlan_req(struct qedf_ctx *qedf)
> > +{
> > + struct sk_buff *skb;
> > + char *eth_fr;
> > + int fr_len;
> > + struct fip_vlan *vlan;
> > +#define MY_FIP_ALL_FCF_MACS((__u8[6]) { 1, 0x10, 0x18, 1, 0, 2 })
> > + static u8 my_fcoe_all_fcfs[ETH_ALEN] = MY_FIP_ALL_FCF_MACS;
>
> Do you support VN2VN, too?
Not currently, no.
>
> Cheers,
>
> Hannes
>
Re: [Open-FCoE] [PATCH RFC 3/5] qedf: Add offloaded I/O request functions.
On Wed, 28 Dec 2016, 9:08am -, Hannes Reinecke wrote:
> On 12/23/2016 08:17 PM, Dupuis, Chad wrote:
> > From: "Dupuis, Chad"
> >
> > This patch adds various I/O requests types that are handled in firmware:
> >
> > - Normal I/O requests
> > - ABTS requests
> > - Cleanup requests
> > - Task management requests
> >
> > It also contains:
> >
> > - I/O request initialization
> > - Firmware completion handling
> >
> > Signed-off-by: Nilesh Javali
> > Signed-off-by: Manish Rangankar
> > Signed-off-by: Saurav Kashyap
> > Signed-off-by: Chad Dupuis
> > ---
> > drivers/scsi/qedf/qedf_hsi.h | 427
> > drivers/scsi/qedf/qedf_io.c | 2303
> > ++
> > 2 files changed, 2730 insertions(+)
> > create mode 100644 drivers/scsi/qedf/qedf_hsi.h
> > create mode 100644 drivers/scsi/qedf/qedf_io.c
> >
> [ .. ]
>
> > +static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd
> > *sc_cmd,
> > + uint8_t tm_flags)
> > +{
> > + struct qedf_ioreq *io_req;
> > + struct qedf_mp_req *tm_req;
> > + struct fcoe_task_context *task;
> > + struct fc_frame_header *fc_hdr;
> > + struct fcp_cmnd *fcp_cmnd;
> > + struct qedf_ctx *qedf = fcport->qedf;
> > + int rc = 0;
> > + uint16_t xid;
> > + uint32_t sid, did;
> > + int tmo = 0;
> > + unsigned long flags;
> > +
> > + if (!sc_cmd) {
> > + QEDF_ERR(&(qedf->dbg_ctx), "invalid arg\n");
> > + return FAILED;
> > + }
> > +
> > + if (!(test_bit(QEDF_RPORT_SESSION_READY, &fcport->flags))) {
> > + QEDF_ERR(&(qedf->dbg_ctx), "fcport not offloaded\n");
> > + rc = FAILED;
> > + return FAILED;
> > + }
> > +
> > + scsi_block_requests(qedf->lport->host);
> > +
> Typically, EH commands will be executed after the scsi host is stopped
> and no commands are outstanding.
> So there's no point in issuing 'scsi_block_requests()' here.
>
Will remove.
> Cheers,
>
> Hannes
>
Re: [Open-FCoE] [PATCH RFC 2/5] qedf: Add QLogic FastLinQ offload FCoE driver framework.
On Wed, 28 Dec 2016, 9:00am -, Hannes Reinecke wrote:
> On 12/23/2016 08:17 PM, Dupuis, Chad wrote:
> > From: "Dupuis, Chad"
> >
> > The QLogic FastLinQ Driver for FCoE (qedf) is the FCoE specific module
> > for 41000 Series Converged Network Adapters by QLogic.
> >
> > This patch consists of following changes:
> > - MAINTAINERS Makefile and Kconfig changes for qedf
> > - PCI driver registration
> > - libfc/fcoe host level initialization
> > - SCSI host template initialization and callbacks
> > - Debugfs and log level infrastructure
> > - Link handling
> > - Firmware interface structures
> > - QED core module initialization
> > - Light L2 interface callbacks
> >
> > Signed-off-by: Nilesh Javali
> > Signed-off-by: Manish Rangankar
> > Signed-off-by: Saurav Kashyap
> > Signed-off-by: Chad Dupuis
> > ---
> > MAINTAINERS |6 +
> > drivers/scsi/Kconfig |1 +
> > drivers/scsi/qedf/Kconfig| 11 +
> > drivers/scsi/qedf/Makefile |5 +
> > drivers/scsi/qedf/qedf.h | 555 ++
> > drivers/scsi/qedf/qedf_attr.c| 165 ++
> > drivers/scsi/qedf/qedf_dbg.c | 192 +++
> > drivers/scsi/qedf/qedf_dbg.h | 153 ++
> > drivers/scsi/qedf/qedf_debugfs.c | 472 +
> > drivers/scsi/qedf/qedf_main.c| 3519
> > ++
> > drivers/scsi/qedf/qedf_version.h | 15 +
> > 11 files changed, 5094 insertions(+)
> > create mode 100644 drivers/scsi/qedf/Kconfig
> > create mode 100644 drivers/scsi/qedf/Makefile
> > create mode 100644 drivers/scsi/qedf/qedf.h
> > create mode 100644 drivers/scsi/qedf/qedf_attr.c
> > create mode 100644 drivers/scsi/qedf/qedf_dbg.c
> > create mode 100644 drivers/scsi/qedf/qedf_dbg.h
> > create mode 100644 drivers/scsi/qedf/qedf_debugfs.c
> > create mode 100644 drivers/scsi/qedf/qedf_main.c
> > create mode 100644 drivers/scsi/qedf/qedf_version.h
> >
> [ .. ]
> > +/* Returns true if we have a valid vlan, false otherwise */
> > +static bool qedf_initiate_fipvlan_req(struct qedf_ctx *qedf)
> > +{
> > + int rc;
> > +
> > + if (atomic_read(&qedf->link_state) != QEDF_LINK_UP) {
> > + QEDF_ERR(&(qedf->dbg_ctx), "Link not up.\n");
> > + return false;
> > + }
> > +
> > + while (qedf->fipvlan_retries--) {
> > + if (qedf->vlan_id > 0)
> > + return true;
> Some weird FCoE bridges (most notably HP VirtualConnect) return a VLAN
> ID of '0'. Shouldn't you rather test for '>= 0' here?
Will look into this but isn't a VLAN ID of 0 not valid?
>
> [ .. ]
> > +
> > +static void qedf_flogi_resp(struct fc_seq *seq, struct fc_frame *fp,
> > + void *arg)
> > +{
> > + struct fc_exch *exch = fc_seq_exch(seq);
> > + struct fc_lport *lport = exch->lp;
> > + struct qedf_ctx *qedf = lport_priv(lport);
> > +
> > + if (!qedf) {
> > + QEDF_ERR(NULL, "qedf is NULL.\n");
> > + return;
> > + }
> > +
> > + /*
> > +* If ERR_PTR is set then don't try to stat anything as it will cause
> > +* a crash when we access fp.
> > +*/
> > + if (fp == ERR_PTR(-FC_EX_TIMEOUT) ||
> > + fp == ERR_PTR(-FC_EX_CLOSED)) {
> > + QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_ELS,
> > + "fp has ERR_PTR() set.\n");
> > + goto skip_stat;
> > + }
>
> Please use
>
> if (IS_ERR(fp)) {
>
> here instead of checking for individual error codes; if 'fp' has a
> different error value you'll continue with an invalid fp from here on.
>
Will fix up.
> [ .. ]
>
> > +/**
> > + * qedf_xmit - qedf FCoE frame transmit function
> > + *
> > + */
> > +static int qedf_xmit(struct fc_lport *lport, struct fc_frame *fp)
> > +{
> > + struct fc_lport *base_lport;
> > + struct qedf_ctx *qedf;
> > + struct ethhdr *eh;
> > + struct fcoe_crc_eof *cp;
> > + struct sk_buff *skb;
> > + struct fc_frame_header *fh;
> > + struct fcoe_hdr *hp;
> > + u8 sof, eof;
> > + u32 crc;
> > + unsigned inthlen, tlen, elen;
> > + int wlen;
> > + struct fc_stats *stats;
> > + struct fc_lport *tmp_lport;
>
Re: [PATCH] bnx2fc: reduce stack usage in __bnx2fc_enable
On Wed, 7 Oct 2015, Maurizio Lombardi wrote: Hi, On 10/07/2015 03:11 PM, Arnd Bergmann wrote: - memset(&npiv_tbl, 0, sizeof(npiv_tbl)); - if (hba->cnic->get_fc_npiv_tbl(hba->cnic, &npiv_tbl)) + npiv_tbl = kzalloc(sizeof(struct cnic_fc_npiv_tbl), GFP_KERNEL); + if (!npiv_tbl) goto done; If kzalloc() fails perhaps the function should return -ENOMEM, not zero. The enablement of the fcoe interface can still proceed if this particular allocation fails so returning 0 would be appropriate here. This patch looks good to me. Regards, Maurizio Lombardi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
