Skip to contentSkip to conversation

Open-FCoE Submission (round 2)

PATCH In review Robert Love 8 participants 17 messages Dec 9, 2008 – Feb 6, 2009

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover Open-FCoE Submission (round 2)

The following series adds the following code to the SCSI subsystem.

1) FC and FCoE protocol definitions in header files
- Files are added in include/scsi/fc/
- New files are: fc_els.h, fc_encaps.h, fc_fc2.h, fc_fcp.h, fc_fs.h, fc_gs.h and fc_ns.h

2) libfc - a FC assist library
- Composed of 5 sub-components: Exchange Manager, Local Port Remote Port, Discovery and SCSI-FCP
- Uses libfc_function_template for cross-layer calls
- Uses struct fc_lport as primary object shared by all sub-modules
- Common code defined in include/scsi/libfc.h

3) fcoe - a FCoE initiator implementation
- Simple encapsulation and unpacking layer
- Interacts with netdev
- Initial support for FCoE offload
- Uses /sys/module/fcoe/parameters/create and destroy for enabling/disabling FCoE connection over a particular Ethernet adapter
- Works on any Ethernet adapter

These patches apply to both the scsi-misc and scsi-rc-fixes trees.

I beleive that we've addressed each of Andi's comments. The largest being a security review where we mostly focused on input validation from the wire as well as removing BUG_ON and WARN_ON statements that could be initiated from an external entity. For a detailed list of recent activity please view our git log on Open-FCoE. Here is a link to our gitweb site:

http://www.open-fcoe.org/openfc/gitweb/?p=openfc/open-fcoe-initiator.git;a=summary

We did not break the patch-set down any further as it would not be bisect-able. Our libfc blocks are organized by file, so using splitdiff or diffsplit anyone should be able to break these patches down into smaller, more review friendly chunks.

Locking comments are mostly in comment blocks at the tops of some files. The largest comment block is in drivers/scsi/libfc/fc_lport.c and would be a good starting point when reviewing the locking.

---

Robert Love (3):
fcoe: Fibre Channel over Ethernet
libfc: A modular Fibre Channel library
FC protocol definition header files

 drivers/scsi/Kconfig                  |   13 
 drivers/scsi/Makefile                 |    2 
 drivers/scsi/fcoe/Makefile            |    8 
 drivers/scsi/fcoe/fc_transport_fcoe.c |  446 +++++++
 drivers/scsi/fcoe/fcoe_sw.c           |  494 ++++++++
 drivers/scsi/fcoe/libfcoe.c           | 1510 +++++++++++++++++++++++
 drivers/scsi/libfc/Makefile           |   12 
 drivers/scsi/libfc/fc_disc.c          |  845 +++++++++++++
 drivers/scsi/libfc/fc_elsct.c         |   71 +
 drivers/scsi/libfc/fc_exch.c          | 1970 +++++++++++++++++++++++++++++++
 drivers/scsi/libfc/fc_fcp.c           | 2130 +++++++++++++++++++++++++++++++++
 drivers/scsi/libfc/fc_frame.c         |   89 +
 drivers/scsi/libfc/fc_lport.c         | 1604 +++++++++++++++++++++++++
 drivers/scsi/libfc/fc_rport.c         | 1291 ++++++++++++++++++++
 include/scsi/fc/fc_els.h              |  816 +++++++++++++
 include/scsi/fc/fc_encaps.h           |  138 ++
 include/scsi/fc/fc_fc2.h              |  124 ++
 include/scsi/fc/fc_fcoe.h             |  114 ++
 include/scsi/fc/fc_fcp.h              |  199 +++
 include/scsi/fc/fc_fs.h               |  340 +++++
 include/scsi/fc/fc_gs.h               |   93 +
 include/scsi/fc/fc_ns.h               |  159 ++
 include/scsi/fc_encode.h              |  309 +++++
 include/scsi/fc_frame.h               |  242 ++++
 include/scsi/fc_transport_fcoe.h      |   54 +
 include/scsi/libfc.h                  |  938 +++++++++++++++
 include/scsi/libfcoe.h                |  176 +++
 27 files changed, 14187 insertions(+), 0 deletions(-)
 create mode 100644 drivers/scsi/fcoe/Makefile
 create mode 100644 drivers/scsi/fcoe/fc_transport_fcoe.c
 create mode 100644 drivers/scsi/fcoe/fcoe_sw.c
 create mode 100644 drivers/scsi/fcoe/libfcoe.c
 create mode 100644 drivers/scsi/libfc/Makefile
 create mode 100644 drivers/scsi/libfc/fc_disc.c
 create mode 100644 drivers/scsi/libfc/fc_elsct.c
 create mode 100644 drivers/scsi/libfc/fc_exch.c
 create mode 100644 drivers/scsi/libfc/fc_fcp.c
 create mode 100644 drivers/scsi/libfc/fc_frame.c
 create mode 100644 drivers/scsi/libfc/fc_lport.c
 create mode 100644 drivers/scsi/libfc/fc_rport.c
 create mode 100644 include/scsi/fc/fc_els.h
 create mode 100644 include/scsi/fc/fc_encaps.h
 create mode 100644 include/scsi/fc/fc_fc2.h
 create mode 100644 include/scsi/fc/fc_fcoe.h
 create mode 100644 include/scsi/fc/fc_fcp.h
 create mode 100644 include/scsi/fc/fc_fs.h
 create mode 100644 include/scsi/fc/fc_gs.h
 create mode 100644 include/scsi/fc/fc_ns.h
 create mode 100644 include/scsi/fc_encode.h
 create mode 100644 include/scsi/fc_frame.h
 create mode 100644 include/scsi/fc_transport_fcoe.h
 create mode 100644 include/scsi/libfc.h
 create mode 100644 include/scsi/libfcoe.h
Signature · 1 line
//Rob
#1 lore raw

PATCH 1/3 FC protocol definition header files

---

 include/scsi/fc/fc_els.h    |  816 +++++++++++++++++++++++++++++++++++++++++++
 include/scsi/fc/fc_encaps.h |  138 +++++++
 include/scsi/fc/fc_fc2.h    |  124 +++++++
 include/scsi/fc/fc_fcoe.h   |  114 ++++++
 include/scsi/fc/fc_fcp.h    |  199 ++++++++++
 include/scsi/fc/fc_fs.h     |  340 ++++++++++++++++++
 include/scsi/fc/fc_gs.h     |   93 +++++
 include/scsi/fc/fc_ns.h     |  159 ++++++++
 8 files changed, 1983 insertions(+), 0 deletions(-)
 create mode 100644 include/scsi/fc/fc_els.h
 create mode 100644 include/scsi/fc/fc_encaps.h
 create mode 100644 include/scsi/fc/fc_fc2.h
 create mode 100644 include/scsi/fc/fc_fcoe.h
 create mode 100644 include/scsi/fc/fc_fcp.h
 create mode 100644 include/scsi/fc/fc_fs.h
 create mode 100644 include/scsi/fc/fc_gs.h
 create mode 100644 include/scsi/fc/fc_ns.h
include/scsi/fc/fc_els.h new file +816 -0
new file mode 100644
index 0000000..195ca01
@@ -0,0 +1,816 @@
+/*
+ * Copyright(c) 2007 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Maintained at www.Open-FCoE.org
+ */
+
+#ifndef _FC_ELS_H_
+#define	_FC_ELS_H_
+
+/*
+ * Fibre Channel Switch - Enhanced Link Services definitions.
+ * From T11 FC-LS Rev 1.2 June 7, 2005.
+ */
+
+/*
+ * ELS Command codes - byte 0 of the frame payload
+ */
+enum fc_els_cmd {
+	ELS_LS_RJT =	0x01,	/* ESL reject */
+	ELS_LS_ACC =	0x02,	/* ESL Accept */
+	ELS_PLOGI =	0x03,	/* N_Port login */
+	ELS_FLOGI =	0x04,	/* F_Port login */
+	ELS_LOGO =	0x05,	/* Logout */
+	ELS_ABTX =	0x06,	/* Abort exchange - obsolete */
+	ELS_RCS =	0x07,	/* read connection status */
+	ELS_RES =	0x08,	/* read exchange status block */
+	ELS_RSS =	0x09,	/* read sequence status block */
+	ELS_RSI =	0x0a,	/* read sequence initiative */
+	ELS_ESTS =	0x0b,	/* establish streaming */
+	ELS_ESTC =	0x0c,	/* estimate credit */
+	ELS_ADVC =	0x0d,	/* advise credit */
+	ELS_RTV =	0x0e,	/* read timeout value */
+	ELS_RLS =	0x0f,	/* read link error status block */
+	ELS_ECHO =	0x10,	/* echo */
+	ELS_TEST =	0x11,	/* test */
+	ELS_RRQ =	0x12,	/* reinstate recovery qualifier */
+	ELS_REC =	0x13,	/* read exchange concise */
+	ELS_SRR =	0x14,	/* sequence retransmission request */
+	ELS_PRLI =	0x20,	/* process login */
+	ELS_PRLO =	0x21,	/* process logout */
+	ELS_SCN =	0x22,	/* state change notification */
+	ELS_TPLS =	0x23,	/* test process login state */
+	ELS_TPRLO =	0x24,	/* third party process logout */
+	ELS_LCLM =	0x25,	/* login control list mgmt (obs) */
+	ELS_GAID =	0x30,	/* get alias_ID */
+	ELS_FACT =	0x31,	/* fabric activate alias_id */
+	ELS_FDACDT =	0x32,	/* fabric deactivate alias_id */
+	ELS_NACT =	0x33,	/* N-port activate alias_id */
+	ELS_NDACT =	0x34,	/* N-port deactivate alias_id */
+	ELS_QOSR =	0x40,	/* quality of service request */
+	ELS_RVCS =	0x41,	/* read virtual circuit status */
+	ELS_PDISC =	0x50,	/* discover N_port service params */
+	ELS_FDISC =	0x51,	/* discover F_port service params */
+	ELS_ADISC =	0x52,	/* discover address */
+	ELS_RNC =	0x53,	/* report node cap (obs) */
+	ELS_FARP_REQ =	0x54,	/* FC ARP request */
+	ELS_FARP_REPL =	0x55,	/* FC ARP reply */
+	ELS_RPS =	0x56,	/* read port status block */
+	ELS_RPL =	0x57,	/* read port list */
+	ELS_RPBC =	0x58,	/* read port buffer condition */
+	ELS_FAN =	0x60,	/* fabric address notification */
+	ELS_RSCN =	0x61,	/* registered state change notification */
+	ELS_SCR =	0x62,	/* state change registration */
+	ELS_RNFT =	0x63,	/* report node FC-4 types */
+	ELS_CSR =	0x68,	/* clock synch. request */
+	ELS_CSU =	0x69,	/* clock synch. update */
+	ELS_LINIT =	0x70,	/* loop initialize */
+	ELS_LSTS =	0x72,	/* loop status */
+	ELS_RNID =	0x78,	/* request node ID data */
+	ELS_RLIR =	0x79,	/* registered link incident report */
+	ELS_LIRR =	0x7a,	/* link incident record registration */
+	ELS_SRL =	0x7b,	/* scan remote loop */
+	ELS_SBRP =	0x7c,	/* set bit-error reporting params */
+	ELS_RPSC =	0x7d,	/* report speed capabilities */
+	ELS_QSA =	0x7e,	/* query security attributes */
+	ELS_EVFP =	0x7f,	/* exchange virt. fabrics params */
+	ELS_LKA =	0x80,	/* link keep-alive */
+	ELS_AUTH_ELS =	0x90,	/* authentication ELS */
+};
+
+/*
+ * Initializer useful for decoding table.
+ * Please keep this in sync with the above definitions.
+ */
+#define	FC_ELS_CMDS_INIT {			\
+	[ELS_LS_RJT] =	"LS_RJT",		\
+	[ELS_LS_ACC] =	"LS_ACC",		\
+	[ELS_PLOGI] =	"PLOGI",		\
+	[ELS_FLOGI] =	"FLOGI",		\
+	[ELS_LOGO] =	"LOGO",			\
+	[ELS_ABTX] =	"ABTX",			\
+	[ELS_RCS] =	"RCS",			\
+	[ELS_RES] =	"RES",			\
+	[ELS_RSS] =	"RSS",			\
+	[ELS_RSI] =	"RSI",			\
+	[ELS_ESTS] =	"ESTS",			\
+	[ELS_ESTC] =	"ESTC",			\
+	[ELS_ADVC] =	"ADVC",			\
+	[ELS_RTV] =	"RTV",			\
+	[ELS_RLS] =	"RLS",			\
+	[ELS_ECHO] =	"ECHO",			\
+	[ELS_TEST] =	"TEST",			\
+	[ELS_RRQ] =	"RRQ",			\
+	[ELS_REC] =	"REC",			\
+	[ELS_SRR] =	"SRR",			\
+	[ELS_PRLI] =	"PRLI",			\
+	[ELS_PRLO] =	"PRLO",			\
+	[ELS_SCN] =	"SCN",			\
+	[ELS_TPLS] =	"TPLS",			\
+	[ELS_TPRLO] =	"TPRLO",		\
+	[ELS_LCLM] =	"LCLM",			\
+	[ELS_GAID] =	"GAID",			\
+	[ELS_FACT] =	"FACT",			\
+	[ELS_FDACDT] =	"FDACDT",		\
+	[ELS_NACT] =	"NACT",			\
+	[ELS_NDACT] =	"NDACT",		\
+	[ELS_QOSR] =	"QOSR",			\
+	[ELS_RVCS] =	"RVCS",			\
+	[ELS_PDISC] =	"PDISC",		\
+	[ELS_FDISC] =	"FDISC",		\
+	[ELS_ADISC] =	"ADISC",		\
+	[ELS_RNC] =	"RNC",			\
+	[ELS_FARP_REQ] = "FARP_REQ",		\
+	[ELS_FARP_REPL] =  "FARP_REPL",		\
+	[ELS_RPS] =	"RPS",			\
+	[ELS_RPL] =	"RPL",			\
+	[ELS_RPBC] =	"RPBC",			\
+	[ELS_FAN] =	"FAN",			\
+	[ELS_RSCN] =	"RSCN",			\
+	[ELS_SCR] =	"SCR",			\
+	[ELS_RNFT] =	"RNFT",			\
+	[ELS_CSR] =	"CSR",			\
+	[ELS_CSU] =	"CSU",			\
+	[ELS_LINIT] =	"LINIT",		\
+	[ELS_LSTS] =	"LSTS",			\
+	[ELS_RNID] =	"RNID",			\
+	[ELS_RLIR] =	"RLIR",			\
+	[ELS_LIRR] =	"LIRR",			\
+	[ELS_SRL] =	"SRL",			\
+	[ELS_SBRP] =	"SBRP",			\
+	[ELS_RPSC] =	"RPSC",			\
+	[ELS_QSA] =	"QSA",			\
+	[ELS_EVFP] =	"EVFP",			\
+	[ELS_LKA] =	"LKA",			\
+	[ELS_AUTH_ELS] = "AUTH_ELS",		\
+}
+
+/*
+ * LS_ACC payload.
+ */
+struct fc_els_ls_acc {
+	__u8          la_cmd;		/* command code ELS_LS_ACC */
+	__u8          la_resv[3];	/* reserved */
+};
+
+/*
+ * ELS reject payload.
+ */
+struct fc_els_ls_rjt {
+	__u8	er_cmd;		/* command code ELS_LS_RJT */
+	__u8	er_resv[4];	/* reserved must be zero */
+	__u8	er_reason;	/* reason (enum fc_els_rjt_reason below) */
+	__u8	er_explan;	/* explanation (enum fc_els_rjt_explan below) */
+	__u8	er_vendor;	/* vendor specific code */
+};
+
+/*
+ * ELS reject reason codes (er_reason).
+ */
+enum fc_els_rjt_reason {
+	ELS_RJT_NONE =		0,	/* no reject - not to be sent */
+	ELS_RJT_INVAL =		0x01,	/* invalid ELS command code */
+	ELS_RJT_LOGIC =		0x03,	/* logical error */
+	ELS_RJT_BUSY =		0x05,	/* logical busy */
+	ELS_RJT_PROT =		0x07,	/* protocol error */
+	ELS_RJT_UNAB =		0x09,	/* unable to perform command request */
+	ELS_RJT_UNSUP =		0x0b,	/* command not supported */
+	ELS_RJT_INPROG =	0x0e,	/* command already in progress */
+	ELS_RJT_VENDOR =	0xff,	/* vendor specific error */
+};
+
+
+/*
+ * reason code explanation (er_explan).
+ */
+enum fc_els_rjt_explan {
+	ELS_EXPL_NONE =		0x00,	/* No additional explanation */
+	ELS_EXPL_SPP_OPT_ERR =	0x01,	/* service parameter error - options */
+	ELS_EXPL_SPP_ICTL_ERR =	0x03,	/* service parm error - initiator ctl */
+	ELS_EXPL_AH =		0x11,	/* invalid association header */
+	ELS_EXPL_AH_REQ =	0x13,	/* association_header required */
+	ELS_EXPL_SID =		0x15,	/* invalid originator S_ID */
+	ELS_EXPL_OXID_RXID =	0x17,	/* invalid OX_ID-RX_ID combination */
+	ELS_EXPL_INPROG =	0x19,	/* Request already in progress */
+	ELS_EXPL_PLOGI_REQD =	0x1e,	/* N_Port login required */
+	ELS_EXPL_INSUF_RES =	0x29,	/* insufficient resources */
+	ELS_EXPL_UNAB_DATA =	0x2a,	/* unable to supply requested data */
+	ELS_EXPL_UNSUPR =	0x2c,	/* Request not supported */
+	ELS_EXPL_INV_LEN =	0x2d,	/* Invalid payload length */
+	/* TBD - above definitions incomplete */
+};
+
+/*
+ * Common service parameters (N ports).
+ */
+struct fc_els_csp {
+	__u8		sp_hi_ver;	/* highest version supported (obs.) */
+	__u8		sp_lo_ver;	/* highest version supported (obs.) */
+	__be16		sp_bb_cred;	/* buffer-to-buffer credits */
+	__be16		sp_features;	/* common feature flags */
+	__be16		sp_bb_data;	/* b-b state number and data field sz */
+	union {
+		struct {
+			__be16	_sp_tot_seq; /* total concurrent sequences */
+			__be16	_sp_rel_off; /* rel. offset by info cat */
+		} sp_plogi;
+		struct {
+			__be32	_sp_r_a_tov; /* resource alloc. timeout msec */
+		} sp_flogi_acc;
+	} sp_u;
+	__be32		sp_e_d_tov;	/* error detect timeout value */
+};
+#define	sp_tot_seq	sp_u.sp_plogi._sp_tot_seq
+#define	sp_rel_off	sp_u.sp_plogi._sp_rel_off
+#define	sp_r_a_tov	sp_u.sp_flogi_acc._sp_r_a_tov
+
+#define	FC_SP_BB_DATA_MASK 0xfff /* mask for data field size in sp_bb_data */
+
+/*
+ * Minimum and maximum values for max data field size in service parameters.
+ */
+#define	FC_SP_MIN_MAX_PAYLOAD	FC_MIN_MAX_PAYLOAD
+#define	FC_SP_MAX_MAX_PAYLOAD	FC_MAX_PAYLOAD
+
+/*
+ * sp_features
+ */
+#define	FC_SP_FT_CIRO	0x8000	/* continuously increasing rel. off. */
+#define	FC_SP_FT_CLAD	0x8000	/* clean address (in FLOGI LS_ACC) */
+#define	FC_SP_FT_RAND	0x4000	/* random relative offset */
+#define	FC_SP_FT_VAL	0x2000	/* valid vendor version level */
+#define	FC_SP_FT_FPORT	0x1000	/* F port (1) vs. N port (0) */
+#define	FC_SP_FT_ABB	0x0800	/* alternate BB_credit management */
+#define	FC_SP_FT_EDTR	0x0400	/* E_D_TOV Resolution is nanoseconds */
+#define	FC_SP_FT_MCAST	0x0200	/* multicast */
+#define	FC_SP_FT_BCAST	0x0100	/* broadcast */
+#define	FC_SP_FT_HUNT	0x0080	/* hunt group */
+#define	FC_SP_FT_SIMP	0x0040	/* dedicated simplex */
+#define	FC_SP_FT_SEC	0x0020	/* reserved for security */
+#define	FC_SP_FT_CSYN	0x0010	/* clock synch. supported */
+#define	FC_SP_FT_RTTOV	0x0008	/* R_T_TOV value 100 uS, else 100 mS */
+#define	FC_SP_FT_HALF	0x0004	/* dynamic half duplex */
+#define	FC_SP_FT_SEQC	0x0002	/* SEQ_CNT */
+#define	FC_SP_FT_PAYL	0x0001	/* FLOGI payload length 256, else 116 */
+
+/*
+ * Class-specific service parameters.
+ */
+struct fc_els_cssp {
+	__be16		cp_class;	/* class flags */
+	__be16		cp_init;	/* initiator flags */
+	__be16		cp_recip;	/* recipient flags */
+	__be16		cp_rdfs;	/* receive data field size */
+	__be16		cp_con_seq;	/* concurrent sequences */
+	__be16		cp_ee_cred;	/* N-port end-to-end credit */
+	__u8		cp_resv1;	/* reserved */
+	__u8		cp_open_seq;	/* open sequences per exchange */
+	__u8		_cp_resv2[2];	/* reserved */
+};
+
+/*
+ * cp_class flags.
+ */
+#define	FC_CPC_VALID	0x8000		/* class valid */
+#define	FC_CPC_IMIX	0x4000		/* intermix mode */
+#define	FC_CPC_SEQ	0x0800		/* sequential delivery */
+#define	FC_CPC_CAMP	0x0200		/* camp-on */
+#define	FC_CPC_PRI	0x0080		/* priority */
+
+/*
+ * cp_init flags.
+ * (TBD: not all flags defined here).
+ */
+#define	FC_CPI_CSYN	0x0010		/* clock synch. capable */
+
+/*
+ * cp_recip flags.
+ */
+#define	FC_CPR_CSYN	0x0008		/* clock synch. capable */
+
+/*
+ * NFC_ELS_FLOGI: Fabric login request.
+ * NFC_ELS_PLOGI: Port login request (same format).
+ */
+struct fc_els_flogi {
+	__u8		fl_cmd;		/* command */
+	__u8		_fl_resvd[3];	/* must be zero */
+	struct fc_els_csp fl_csp;	/* common service parameters */
+	__be64		fl_wwpn;	/* port name */
+	__be64		fl_wwnn;	/* node name */
+	struct fc_els_cssp fl_cssp[4];	/* class 1-4 service parameters */
+	__u8		fl_vend[16];	/* vendor version level */
+} __attribute__((__packed__));
+
+/*
+ * Process login service parameter page.
+ */
+struct fc_els_spp {
+	__u8		spp_type;	/* type code or common service params */
+	__u8		spp_type_ext;	/* type code extension */
+	__u8		spp_flags;
+	__u8		_spp_resvd;
+	__be32		spp_orig_pa;	/* originator process associator */
+	__be32		spp_resp_pa;	/* responder process associator */
+	__be32		spp_params;	/* service parameters */
+};
+
+/*
+ * spp_flags.
+ */
+#define	FC_SPP_OPA_VAL	    0x80	/* originator proc. assoc. valid */
+#define	FC_SPP_RPA_VAL	    0x40	/* responder proc. assoc. valid */
+#define	FC_SPP_EST_IMG_PAIR 0x20	/* establish image pair */
+#define	FC_SPP_RESP_MASK    0x0f	/* mask for response code (below) */
+
+/*
+ * SPP response code in spp_flags - lower 4 bits.
+ */
+enum fc_els_spp_resp {
+	FC_SPP_RESP_ACK	=	1,	/* request executed */
+	FC_SPP_RESP_RES =	2,	/* unable due to lack of resources */
+	FC_SPP_RESP_INIT =	3,	/* initialization not complete */
+	FC_SPP_RESP_NO_PA = 	4,	/* unknown process associator */
+	FC_SPP_RESP_CONF = 	5,	/* configuration precludes image pair */
+	FC_SPP_RESP_COND = 	6,	/* request completed conditionally */
+	FC_SPP_RESP_MULT = 	7,	/* unable to handle multiple SPPs */
+	FC_SPP_RESP_INVL = 	8,	/* SPP is invalid */
+};
+
+/*
+ * ELS_RRQ - Reinstate Recovery Qualifier
+ */
+struct fc_els_rrq {
+	__u8		rrq_cmd;	/* command (0x12) */
+	__u8		rrq_zero[3];	/* specified as zero - part of cmd */
+	__u8		rrq_resvd;	/* reserved */
+	__u8		rrq_s_id[3];	/* originator FID */
+	__be16		rrq_ox_id;	/* originator exchange ID */
+	__be16		rrq_rx_id;	/* responders exchange ID */
+};
+
+/*
+ * ELS_REC - Read exchange concise.
+ */
+struct fc_els_rec {
+	__u8		rec_cmd;	/* command (0x13) */
+	__u8		rec_zero[3];	/* specified as zero - part of cmd */
+	__u8		rec_resvd;	/* reserved */
+	__u8		rec_s_id[3];	/* originator FID */
+	__be16		rec_ox_id;	/* originator exchange ID */
+	__be16		rec_rx_id;	/* responders exchange ID */
+};
+
+/*
+ * ELS_REC LS_ACC payload.
+ */
+struct fc_els_rec_acc {
+	__u8		reca_cmd;	/* accept (0x02) */
+	__u8		reca_zero[3];	/* specified as zero - part of cmd */
+	__be16		reca_ox_id;	/* originator exchange ID */
+	__be16		reca_rx_id;	/* responders exchange ID */
+	__u8		reca_resvd1;	/* reserved */
+	__u8		reca_ofid[3];	/* originator FID */
+	__u8		reca_resvd2;	/* reserved */
+	__u8		reca_rfid[3];	/* responder FID */
+	__be32		reca_fc4value;	/* FC4 value */
+	__be32		reca_e_stat;	/* ESB (exchange status block) status */
+};
+
+/*
+ * ELS_PRLI - Process login request and response.
+ */
+struct fc_els_prli {
+	__u8		prli_cmd;	/* command */
+	__u8		prli_spp_len;	/* length of each serv. parm. page */
+	__be16		prli_len;	/* length of entire payload */
+	/* service parameter pages follow */
+};
+
+/*
+ * ELS_ADISC payload
+ */
+struct fc_els_adisc {
+	__u8		adisc_cmd;
+	__u8		adisc_resv[3];
+	__u8            adisc_resv1;
+	__u8            adisc_hard_addr[3];
+	__be64          adisc_wwpn;
+	__be64          adisc_wwnn;
+	__u8            adisc_resv2;
+	__u8            adisc_port_id[3];
+} __attribute__((__packed__));
+
+/*
+ * ELS_LOGO - process or fabric logout.
+ */
+struct fc_els_logo {
+	__u8		fl_cmd;		/* command code */
+	__u8		fl_zero[3];	/* specified as zero - part of cmd */
+	__u8		fl_resvd;	/* reserved */
+	__u8		fl_n_port_id[3];/* N port ID */
+	__be64		fl_n_port_wwn;	/* port name */
+};
+
+/*
+ * ELS_RTV - read timeout value.
+ */
+struct fc_els_rtv {
+	__u8		rtv_cmd;	/* command code 0x0e */
+	__u8		rtv_zero[3];	/* specified as zero - part of cmd */
+};
+
+/*
+ * LS_ACC for ELS_RTV - read timeout value.
+ */
+struct fc_els_rtv_acc {
+	__u8		rtv_cmd;	/* command code 0x02 */
+	__u8		rtv_zero[3];	/* specified as zero - part of cmd */
+	__be32		rtv_r_a_tov;	/* resource allocation timeout value */
+	__be32		rtv_e_d_tov;	/* error detection timeout value */
+	__be32		rtv_toq;	/* timeout qualifier (see below) */
+};
+
+/*
+ * rtv_toq bits.
+ */
+#define	FC_ELS_RTV_EDRES (1 << 26)	/* E_D_TOV resolution is nS else mS */
+#define	FC_ELS_RTV_RTTOV (1 << 19)	/* R_T_TOV is 100 uS else 100 mS */
+
+/*
+ * ELS_SCR - state change registration payload.
+ */
+struct fc_els_scr {
+	__u8		scr_cmd;	/* command code */
+	__u8		scr_resv[6];	/* reserved */
+	__u8		scr_reg_func;	/* registration function (see below) */
+};
+
+enum fc_els_scr_func {
+	ELS_SCRF_FAB =	1,	/* fabric-detected registration */
+	ELS_SCRF_NPORT = 2,	/* Nx_Port-detected registration */
+	ELS_SCRF_FULL =	3,	/* full registration */
+	ELS_SCRF_CLEAR = 255,	/* remove any current registrations */
+};
+
+/*
+ * ELS_RSCN - registered state change notification payload.
+ */
+struct fc_els_rscn {
+	__u8		rscn_cmd;	/* RSCN opcode (0x61) */
+	__u8		rscn_page_len;	/* page length (4) */
+	__be16		rscn_plen;	/* payload length including this word */
+
+	/* followed by 4-byte generic affected Port_ID pages */
+};
+
+struct fc_els_rscn_page {
+	__u8		rscn_page_flags; /* event and address format */
+	__u8		rscn_fid[3];	/* fabric ID */
+};
+
+#define	ELS_RSCN_EV_QUAL_BIT	2	/* shift count for event qualifier */
+#define	ELS_RSCN_EV_QUAL_MASK	0xf	/* mask for event qualifier */
+#define	ELS_RSCN_ADDR_FMT_BIT	0	/* shift count for address format */
+#define	ELS_RSCN_ADDR_FMT_MASK	0x3	/* mask for address format */
+
+enum fc_els_rscn_ev_qual {
+	ELS_EV_QUAL_NONE = 0,		/* unspecified */
+	ELS_EV_QUAL_NS_OBJ = 1,		/* changed name server object */
+	ELS_EV_QUAL_PORT_ATTR = 2,	/* changed port attribute */
+	ELS_EV_QUAL_SERV_OBJ = 3,	/* changed service object */
+	ELS_EV_QUAL_SW_CONFIG = 4,	/* changed switch configuration */
+	ELS_EV_QUAL_REM_OBJ = 5,	/* removed object */
+};
+
+enum fc_els_rscn_addr_fmt {
+	ELS_ADDR_FMT_PORT = 0,	/* rscn_fid is a port address */
+	ELS_ADDR_FMT_AREA = 1,	/* rscn_fid is a area address */
+	ELS_ADDR_FMT_DOM = 2,	/* rscn_fid is a domain address */
+	ELS_ADDR_FMT_FAB = 3,	/* anything on fabric may have changed */
+};
+
+/*
+ * ELS_RNID - request Node ID.
+ */
+struct fc_els_rnid {
+	__u8		rnid_cmd;	/* RNID opcode (0x78) */
+	__u8		rnid_resv[3];	/* reserved */
+	__u8		rnid_fmt;	/* data format */
+	__u8		rnid_resv2[3];	/* reserved */
+};
+
+/*
+ * Node Identification Data formats (rnid_fmt)
+ */
+enum fc_els_rnid_fmt {
+	ELS_RNIDF_NONE = 0,		/* no specific identification data */
+	ELS_RNIDF_GEN = 0xdf,		/* general topology discovery format */
+};
+
+/*
+ * ELS_RNID response.
+ */
+struct fc_els_rnid_resp {
+	__u8		rnid_cmd;	/* response code (LS_ACC) */
+	__u8		rnid_resv[3];	/* reserved */
+	__u8		rnid_fmt;	/* data format */
+	__u8		rnid_cid_len;	/* common ID data length */
+	__u8		rnid_resv2;	/* reserved */
+	__u8		rnid_sid_len;	/* specific ID data length */
+};
+
+struct fc_els_rnid_cid {
+	__be64		rnid_wwpn;	/* N port name */
+	__be64		rnid_wwnn;	/* node name */
+};
+
+struct fc_els_rnid_gen {
+	__u8		rnid_vend_id[16]; /* vendor-unique ID */
+	__be32		rnid_atype;	/* associated type (see below) */
+	__be32		rnid_phys_port;	/* physical port number */
+	__be32		rnid_att_nodes;	/* number of attached nodes */
+	__u8		rnid_node_mgmt;	/* node management (see below) */
+	__u8		rnid_ip_ver;	/* IP version (see below) */
+	__be16		rnid_prot_port;	/* UDP / TCP port number */
+	__be32		rnid_ip_addr[4]; /* IP address */
+	__u8		rnid_resvd[2];	/* reserved */
+	__be16		rnid_vend_spec;	/* vendor-specific field */
+};
+
+enum fc_els_rnid_atype {
+	ELS_RNIDA_UNK =		0x01,	/* unknown */
+	ELS_RNIDA_OTHER =	0x02,	/* none of the following */
+	ELS_RNIDA_HUB =		0x03,
+	ELS_RNIDA_SWITCH =	0x04,
+	ELS_RNIDA_GATEWAY =	0x05,
+	ELS_RNIDA_CONV =	0x06,   /* Obsolete, do not use this value */
+	ELS_RNIDA_HBA =	        0x07,   /* Obsolete, do not use this value */
+	ELS_RNIDA_PROXY =       0x08,   /* Obsolete, do not use this value */
+	ELS_RNIDA_STORAGE =	0x09,
+	ELS_RNIDA_HOST =	0x0a,
+	ELS_RNIDA_SUBSYS =	0x0b,	/* storage subsystem (e.g., RAID) */
+	ELS_RNIDA_ACCESS =	0x0e,	/* access device (e.g. media changer) */
+	ELS_RNIDA_NAS =		0x11,	/* NAS server */
+	ELS_RNIDA_BRIDGE =	0x12,	/* bridge */
+	ELS_RNIDA_VIRT =	0x13,	/* virtualization device */
+	ELS_RNIDA_MF =		0xff,	/* multifunction device (bits below) */
+	ELS_RNIDA_MF_HUB =	1UL << 31, 	/* hub */
+	ELS_RNIDA_MF_SW =	1UL << 30, 	/* switch */
+	ELS_RNIDA_MF_GW =	1UL << 29,	/* gateway */
+	ELS_RNIDA_MF_ST =	1UL << 28,	/* storage */
+	ELS_RNIDA_MF_HOST =	1UL << 27,	/* host */
+	ELS_RNIDA_MF_SUB =	1UL << 26,	/* storage subsystem */
+	ELS_RNIDA_MF_ACC =	1UL << 25,	/* storage access dev */
+	ELS_RNIDA_MF_WDM =	1UL << 24,	/* wavelength division mux */
+	ELS_RNIDA_MF_NAS =	1UL << 23,	/* NAS server */
+	ELS_RNIDA_MF_BR =	1UL << 22,	/* bridge */
+	ELS_RNIDA_MF_VIRT =	1UL << 21,	/* virtualization device */
+};
+
+enum fc_els_rnid_mgmt {
+	ELS_RNIDM_SNMP =	0,
+	ELS_RNIDM_TELNET =	1,
+	ELS_RNIDM_HTTP =	2,
+	ELS_RNIDM_HTTPS =	3,
+	ELS_RNIDM_XML =		4,	/* HTTP + XML */
+};
+
+enum fc_els_rnid_ipver {
+	ELS_RNIDIP_NONE =	0,	/* no IP support or node mgmt. */
+	ELS_RNIDIP_V4 =		1,	/* IPv4 */
+	ELS_RNIDIP_V6 =		2,	/* IPv6 */
+};
+
+/*
+ * ELS RPL - Read Port List.
+ */
+struct fc_els_rpl {
+	__u8		rpl_cmd;	/* command */
+	__u8		rpl_resv[5];	/* reserved - must be zero */
+	__be16		rpl_max_size;	/* maximum response size or zero */
+	__u8		rpl_resv1;	/* reserved - must be zero */
+	__u8		rpl_index[3];	/* starting index */
+};
+
+/*
+ * Port number block in RPL response.
+ */
+struct fc_els_pnb {
+	__be32		pnb_phys_pn;	/* physical port number */
+	__u8		pnb_resv;	/* reserved */
+	__u8		pnb_port_id[3];	/* port ID */
+	__be64		pnb_wwpn;	/* port name */
+};
+
+/*
+ * RPL LS_ACC response.
+ */
+struct fc_els_rpl_resp {
+	__u8		rpl_cmd;	/* ELS_LS_ACC */
+	__u8		rpl_resv1;	/* reserved - must be zero */
+	__be16		rpl_plen;	/* payload length */
+	__u8		rpl_resv2;	/* reserved - must be zero */
+	__u8		rpl_llen[3];	/* list length */
+	__u8		rpl_resv3;	/* reserved - must be zero */
+	__u8		rpl_index[3];	/* starting index */
+	struct fc_els_pnb rpl_pnb[1];	/* variable number of PNBs */
+};
+
+/*
+ * Link Error Status Block.
+ */
+struct fc_els_lesb {
+	__be32		lesb_link_fail;	/* link failure count */
+	__be32		lesb_sync_loss;	/* loss of synchronization count */
+	__be32		lesb_sig_loss;	/* loss of signal count */
+	__be32		lesb_prim_err;	/* primitive sequence error count */
+	__be32		lesb_inv_word;	/* invalid transmission word count */
+	__be32		lesb_inv_crc;	/* invalid CRC count */
+};
+
+/*
+ * ELS RPS - Read Port Status Block request.
+ */
+struct fc_els_rps {
+	__u8		rps_cmd;	/* command */
+	__u8		rps_resv[2];	/* reserved - must be zero */
+	__u8		rps_flag;	/* flag - see below */
+	__be64		rps_port_spec;	/* port selection */
+};
+
+enum fc_els_rps_flag {
+	FC_ELS_RPS_DID =	0x00,	/* port identified by D_ID of req. */
+	FC_ELS_RPS_PPN =	0x01,	/* port_spec is physical port number */
+	FC_ELS_RPS_WWPN =	0x02,	/* port_spec is port WWN */
+};
+
+/*
+ * ELS RPS LS_ACC response.
+ */
+struct fc_els_rps_resp {
+	__u8		rps_cmd;	/* command - LS_ACC */
+	__u8		rps_resv[2];	/* reserved - must be zero */
+	__u8		rps_flag;	/* flag - see below */
+	__u8		rps_resv2[2];	/* reserved */
+	__be16		rps_status;	/* port status - see below */
+	struct fc_els_lesb rps_lesb;	/* link error status block */
+};
+
+enum fc_els_rps_resp_flag {
+	FC_ELS_RPS_LPEV =	0x01,	/* L_port extension valid */
+};
+
+enum fc_els_rps_resp_status {
+	FC_ELS_RPS_PTP =	1 << 5,	/* point-to-point connection */
+	FC_ELS_RPS_LOOP =	1 << 4,	/* loop mode */
+	FC_ELS_RPS_FAB =	1 << 3,	/* fabric present */
+	FC_ELS_RPS_NO_SIG =	1 << 2,	/* loss of signal */
+	FC_ELS_RPS_NO_SYNC =	1 << 1,	/* loss of synchronization */
+	FC_ELS_RPS_RESET =	1 << 0,	/* in link reset protocol */
+};
+
+/*
+ * ELS LIRR - Link Incident Record Registration request.
+ */
+struct fc_els_lirr {
+	__u8		lirr_cmd;	/* command */
+	__u8		lirr_resv[3];	/* reserved - must be zero */
+	__u8		lirr_func;	/* registration function */
+	__u8		lirr_fmt;	/* FC-4 type of RLIR requested */
+	__u8		lirr_resv2[2];	/* reserved - must be zero */
+};
+
+enum fc_els_lirr_func {
+	ELS_LIRR_SET_COND = 	0x01,	/* set - conditionally receive */
+	ELS_LIRR_SET_UNCOND = 	0x02,	/* set - unconditionally receive */
+	ELS_LIRR_CLEAR = 	0xff	/* clear registration */
+};
+
+/*
+ * ELS SRL - Scan Remote Loop request.
+ */
+struct fc_els_srl {
+	__u8		srl_cmd;	/* command */
+	__u8		srl_resv[3];	/* reserved - must be zero */
+	__u8		srl_flag;	/* flag - see below */
+	__u8		srl_flag_param[3];	/* flag parameter */
+};
+
+enum fc_els_srl_flag {
+	FC_ELS_SRL_ALL =	0x00,	/* scan all FL ports */
+	FC_ELS_SRL_ONE =	0x01,	/* scan specified loop */
+	FC_ELS_SRL_EN_PER =	0x02,	/* enable periodic scanning (param) */
+	FC_ELS_SRL_DIS_PER =	0x03,	/* disable periodic scanning */
+};
+
+/*
+ * ELS RLS - Read Link Error Status Block request.
+ */
+struct fc_els_rls {
+	__u8		rls_cmd;	/* command */
+	__u8		rls_resv[4];	/* reserved - must be zero */
+	__u8		rls_port_id[3];	/* port ID */
+};
+
+/*
+ * ELS RLS LS_ACC Response.
+ */
+struct fc_els_rls_resp {
+	__u8		rls_cmd;	/* ELS_LS_ACC */
+	__u8		rls_resv[3];	/* reserved - must be zero */
+	struct fc_els_lesb rls_lesb;	/* link error status block */
+};
+
+/*
+ * ELS RLIR - Registered Link Incident Report.
+ * This is followed by the CLIR and the CLID, described below.
+ */
+struct fc_els_rlir {
+	__u8		rlir_cmd;	/* command */
+	__u8		rlir_resv[3];	/* reserved - must be zero */
+	__u8		rlir_fmt;	/* format (FC4-type if type specific) */
+	__u8		rlir_clr_len;	/* common link incident record length */
+	__u8		rlir_cld_len;	/* common link incident desc. length */
+	__u8		rlir_slr_len;	/* spec. link incident record length */
+};
+
+/*
+ * CLIR - Common Link Incident Record Data. - Sent via RLIR.
+ */
+struct fc_els_clir {
+	__be64		clir_wwpn;	/* incident port name */
+	__be64		clir_wwnn;	/* incident port node name */
+	__u8		clir_port_type;	/* incident port type */
+	__u8		clir_port_id[3];	/* incident port ID */
+
+	__be64		clir_conn_wwpn;	/* connected port name */
+	__be64		clir_conn_wwnn;	/* connected node name */
+	__be64		clir_fab_name;	/* fabric name */
+	__be32		clir_phys_port;	/* physical port number */
+	__be32		clir_trans_id;	/* transaction ID */
+	__u8		clir_resv[3];	/* reserved */
+	__u8		clir_ts_fmt;	/* time stamp format */
+	__be64		clir_timestamp;	/* time stamp */
+};
+
+/*
+ * CLIR clir_ts_fmt - time stamp format values.
+ */
+enum fc_els_clir_ts_fmt {
+	ELS_CLIR_TS_UNKNOWN = 	0,	/* time stamp field unknown */
+	ELS_CLIR_TS_SEC_FRAC = 	1,	/* time in seconds and fractions */
+	ELS_CLIR_TS_CSU =	2,	/* time in clock synch update format */
+};
+
+/*
+ * Common Link Incident Descriptor - sent via RLIR.
+ */
+struct fc_els_clid {
+	__u8		clid_iq;	/* incident qualifier flags */
+	__u8		clid_ic;	/* incident code */
+	__be16		clid_epai;	/* domain/area of ISL */
+};
+
+/*
+ * CLID incident qualifier flags.
+ */
+enum fc_els_clid_iq {
+	ELS_CLID_SWITCH =	0x20,	/* incident port is a switch node */
+	ELS_CLID_E_PORT =	0x10,	/* incident is an ISL (E) port */
+	ELS_CLID_SEV_MASK =	0x0c,	/* severity 2-bit field mask */
+	ELS_CLID_SEV_INFO =	0x00,	/* report is informational */
+	ELS_CLID_SEV_INOP =	0x08,	/* link not operational */
+	ELS_CLID_SEV_DEG =	0x04,	/* link degraded but operational */
+	ELS_CLID_LASER =	0x02,	/* subassembly is a laser */
+	ELS_CLID_FRU =		0x01,	/* format can identify a FRU */
+};
+
+/*
+ * CLID incident code.
+ */
+enum fc_els_clid_ic {
+	ELS_CLID_IC_IMPL =	1,	/* implicit incident */
+	ELS_CLID_IC_BER =	2,	/* bit-error-rate threshold exceeded */
+	ELS_CLID_IC_LOS =	3,	/* loss of synch or signal */
+	ELS_CLID_IC_NOS =	4,	/* non-operational primitive sequence */
+	ELS_CLID_IC_PST =	5,	/* primitive sequence timeout */
+	ELS_CLID_IC_INVAL =	6,	/* invalid primitive sequence */
+	ELS_CLID_IC_LOOP_TO =	7,	/* loop initialization time out */
+	ELS_CLID_IC_LIP =	8,	/* receiving LIP */
+};
+
+#endif /* _FC_ELS_H_ */
include/scsi/fc/fc_encaps.h new file +138 -0

138 lines not on this page · raw message

include/scsi/fc/fc_fc2.h new file +124 -0

124 lines not on this page · raw message

include/scsi/fc/fc_fcoe.h new file +114 -0

114 lines not on this page · raw message

include/scsi/fc/fc_fcp.h new file +199 -0

199 lines not on this page · raw message

include/scsi/fc/fc_fs.h new file +340 -0

340 lines not on this page · raw message

include/scsi/fc/fc_gs.h new file +93 -0

93 lines not on this page · raw message

include/scsi/fc/fc_ns.h new file +159 -0

159 lines not on this page · raw message

#2 lore raw

PATCH 2/3 libfc: A modular Fibre Channel library

In reply to Robert Love

libFC is composed of 4 blocks supported by an exchange manager and a framing library. The upper 4 layers are fc_lport, fc_disc, fc_rport and fc_fcp. A LLD that uses libfc could choose to either use libfc's block, or using the transport template defined in libfc.h, override one or more blocks with its own implementation.

The EM (Exchange Manager) manages exhcanges/sequences for all commands- ELS, CT and FCP.

The framing library frames ELS and CT commands.

The fc_lport block manages the library's representation of the host's FC enabled ports.

The fc_disc block manages discovery of targets as well as handling changes that occur in the FC fabric (via. RSCN events).

The fc_rport block manages the library's representation of other entities in the FC fabric. Currently the library uses this block for targets, its peer when in point-to-point mode and the directory server, but can be extended for other entities if needed.

The fc_fcp block interacts with the scsi-ml and handles all I/O.

---

 drivers/scsi/Kconfig          |    6 
 drivers/scsi/Makefile         |    1 
 drivers/scsi/libfc/Makefile   |   12 
 drivers/scsi/libfc/fc_disc.c  |  845 ++++++++++++++++
 drivers/scsi/libfc/fc_elsct.c |   71 +
 drivers/scsi/libfc/fc_exch.c  | 1970 ++++++++++++++++++++++++++++++++++++++
 drivers/scsi/libfc/fc_fcp.c   | 2130 +++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/libfc/fc_frame.c |   89 ++
 drivers/scsi/libfc/fc_lport.c | 1604 +++++++++++++++++++++++++++++++
 drivers/scsi/libfc/fc_rport.c | 1291 +++++++++++++++++++++++++
 include/scsi/fc_encode.h      |  309 ++++++
 include/scsi/fc_frame.h       |  242 +++++
 include/scsi/libfc.h          |  938 ++++++++++++++++++
 13 files changed, 9508 insertions(+), 0 deletions(-)
 create mode 100644 drivers/scsi/libfc/Makefile
 create mode 100644 drivers/scsi/libfc/fc_disc.c
 create mode 100644 drivers/scsi/libfc/fc_elsct.c
 create mode 100644 drivers/scsi/libfc/fc_exch.c
 create mode 100644 drivers/scsi/libfc/fc_fcp.c
 create mode 100644 drivers/scsi/libfc/fc_frame.c
 create mode 100644 drivers/scsi/libfc/fc_lport.c
 create mode 100644 drivers/scsi/libfc/fc_rport.c
 create mode 100644 include/scsi/fc_encode.h
 create mode 100644 include/scsi/fc_frame.h
 create mode 100644 include/scsi/libfc.h
drivers/scsi/Kconfig +6 -0
index 1badcec..24d762a 100644
@@ -603,6 +603,12 @@ config SCSI_FLASHPOINT
 	  substantial, so users of MultiMaster Host Adapters may not
 	  wish to include it.
 
+config LIBFC
+	tristate "LibFC module"
+	depends on SCSI && SCSI_FC_ATTRS
+	---help---
+	  Fibre Channel library module
+
 config SCSI_DMX3191D
 	tristate "DMX3191D SCSI support"
 	depends on PCI && SCSI
drivers/scsi/Makefile +1 -0
index b89aedf..87355f5 100644
@@ -36,6 +36,7 @@ obj-$(CONFIG_SCSI_SAS_LIBSAS)	+= libsas/
 obj-$(CONFIG_SCSI_SRP_ATTRS)	+= scsi_transport_srp.o
 obj-$(CONFIG_SCSI_DH)		+= device_handler/
 
+obj-$(CONFIG_LIBFC)		+= libfc/
 obj-$(CONFIG_ISCSI_TCP) 	+= libiscsi.o	libiscsi_tcp.o iscsi_tcp.o
 obj-$(CONFIG_INFINIBAND_ISER) 	+= libiscsi.o
 obj-$(CONFIG_SCSI_A4000T)	+= 53c700.o	a4000t.o
drivers/scsi/libfc/Makefile new file +12 -0
new file mode 100644
index 0000000..55f982d
@@ -0,0 +1,12 @@
+# $Id: Makefile
+
+obj-$(CONFIG_LIBFC) += libfc.o
+
+libfc-objs := \
+	fc_disc.o \
+	fc_exch.o \
+	fc_elsct.o \
+	fc_frame.o \
+	fc_lport.o \
+	fc_rport.o \
+	fc_fcp.o
drivers/scsi/libfc/fc_disc.c new file +845 -0

845 lines not on this page · raw message

drivers/scsi/libfc/fc_elsct.c new file +71 -0

71 lines not on this page · raw message

drivers/scsi/libfc/fc_exch.c new file +1970 -0

1,970 lines not on this page · raw message

drivers/scsi/libfc/fc_fcp.c new file +2130 -0

83 lines not on this page · raw message

drivers/scsi/libfc/fc_frame.c new file +89 -0

89 lines not on this page · raw message

drivers/scsi/libfc/fc_lport.c new file +1604 -0

1,604 lines not on this page · raw message

drivers/scsi/libfc/fc_rport.c new file +1291 -0

1,291 lines not on this page · raw message

include/scsi/fc_encode.h new file +309 -0

309 lines not on this page · raw message

include/scsi/fc_frame.h new file +242 -0

242 lines not on this page · raw message

include/scsi/libfc.h new file +938 -0

938 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Part of the diff is not shown here. View the raw message.

#3 lore raw

Re: [PATCH 2/3] libfc: A modular Fibre Channel library

Quoted 6 lines · Robert Love

libFC is composed of 4 blocks supported by an exchange manager and a framing library. The upper 4 layers are fc_lport, fc_disc, fc_rport and fc_fcp. A LLD that uses libfc could choose to either use libfc's block, or using the transport template defined in libfc.h, override one or more blocks with its own implementation.

Just some more stuff I noticed while looking briefly at libfcoe.c Not a complete review again.

230 #ifdef CONFIG_SMP
231 /*

 232          * The exchange ID are ANDed with num of online CPUs,
 233          * so that will have the least lock contention in
 234          * handling the exchange. if there is no thread
 235          * for a given idx then use first online cpu.
 236          */

237 cpu_idx = oxid & (num_online_cpus() >> 1);
238 if (fcoe_percpu[cpu_idx] == NULL)
239 cpu_idx = first_cpu(cpu_online_map);
240 #endif

What is this supposed to do? It looks quite dubious. Shouldn't this use the current CPU number?

include/scsi/libfc.h: struct fcoe_dev_stats *dev_stats[NR_CPUS];

NR_CPUS arrays are strongly discouraged because they lead to waste on kernels compiled with large NR_CPUS, ideally use per CPU data or the per cpu allocator or at least a dynamic allocation with num_possible_cpus() In general every use of NR_CPUS is dubious. e.g. you have some loops over that, these should be all over possible cpus. fcoe_percpu[] should be also per cpu data, not an array.

It would probably simplify the code a lot if you just went to global per cpu counters instead of per cpu device counters. I don't know if that is feasible or not.

static int __init fcoe_init(void)

There are CPU hotplug races between the notifier and the for_each_online_cpu() loop (probably not a serious problem, such races are common all over the code base, just wanted to mention it)

static int fcoe_device_notification(struct notifier_block *notifier,
ulong event, void *ptr)

the link_status field seems to be only protected by the rtnl semaphore here which is normally hold in this notifier chain. But if any other users in the fcoe stack change that too there might be races. Also normally it's better to check "event" first before doing anything expensive like walking lists. There are definitely more events you're not handling, but right now you would slow them down.

fcoe_ethdrv_get/put

It's not quite clear to me why you go to the trouble to lock the underlying NIC module. Why would it be a problem if it goes away? Normally one would rather keep a reference to the device, but even that might be not needed.

static int fcoe_get_paged_crc_eof(struct sk_buff *skb, int tlen)

    get_page(page);
        skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags, page,
                           fps->crc_eof_offset, tlen);
        skb->len += tlen;
        skb->data_len += tlen;
        skb->truesize += tlen;
        fps->crc_eof_offset += sizeof(struct fcoe_crc_eof);

        if (fps->crc_eof_offset >= PAGE_SIZE) {
                fps->crc_eof_page = NULL;
                fps->crc_eof_offset = 0;
                put_page(page);
        }

Is the put_page() here really correct? The original reference in the skb will be still around, won't it?

static void fcoe_insert_wait_queue(struct fc_lport *lp,
struct sk_buff *skb)

{
        struct fcoe_softc *fc;

        fc = fcoe_softc(lp);
        spin_lock_bh(&fc->fcoe_pending_queue.lock);
        __skb_queue_tail(&fc->fcoe_pending_queue, skb);
        spin_unlock_bh(&fc->fcoe_pending_queue.lock);
}

That's just skb_queue_tail() (except that the lock is hardirq safe there, but that should be fine)

        /* adjust skb netowrk/transport offsets to match mac/fcoe/fc */

typo

Quoted 12 lines

The EM (Exchange Manager) manages exhcanges/sequences for all commands- ELS, CT and FCP.

The framing library frames ELS and CT commands.

The fc_lport block manages the library's representation of the host's FC enabled ports.

The fc_disc block manages discovery of targets as well as handling changes that occur in the FC fabric (via. RSCN events).

Quoted 8 lines

The fc_rport block manages the library's representation of other entities in the FC fabric. Currently the library uses this block for targets, its peer when in point-to-point mode and the directory server, but can be extended for other entities if needed.

The fc_fcp block interacts with the scsi-ml and handles all I/O.

A cheatsheet for all the acronyms would be nice.

-Andi

#4 lore raw

Re: [PATCH 2/3] libfc: A modular Fibre Channel library

Quoted 20 lines · Andi Kleen
Quoted 7 lines · Robert Love

libFC is composed of 4 blocks supported by an exchange manager and a framing library. The upper 4 layers are fc_lport, fc_disc, fc_rport and fc_fcp. A LLD that uses libfc could choose to either use libfc's block, or using the transport template defined in libfc.h, override one or more blocks with its own implementation.

Just some more stuff I noticed while looking briefly at libfcoe.c Not a complete review again.

230 #ifdef CONFIG_SMP
231 /*

 232          * The exchange ID are ANDed with num of online CPUs,
 233          * so that will have the least lock contention in
 234          * handling the exchange. if there is no thread
 235          * for a given idx then use first online cpu.
 236          */
Quoted 7 lines

237 cpu_idx = oxid & (num_online_cpus() >> 1);
238 if (fcoe_percpu[cpu_idx] == NULL)
239 cpu_idx = first_cpu(cpu_online_map);
240 #endif

What is this supposed to do? It looks quite dubious.

It had load balancing issue but now it is fixed, related latest submitted code with its updated comment is:-

	/*
	 * The incoming frame exchange id(oxid) is ANDed with num of online
	 * cpu bits to get cpu_idx and then this cpu_idx is used for selecting
	 * a per cpu kernel thread from fcoe_percpu. In case the cpu is
	 * offline or no kernel thread for derived cpu_idx then cpu_idx is
	 * initialize to first online cpu index.
	 */
	cpu_idx = oxid & (num_online_cpus() - 1);
	if (!fcoe_percpu[cpu_idx] || !cpu_online(cpu_idx))
		cpu_idx = first_cpu(cpu_online_map);

The incoming FC frame is distributed across all fcoe_percpu_receive_thread for better load balancing, these per CPU threads are created by fcoe module during fcoe_init. This should help in performance but not sure how much since performance tuning is yet to be done.

Quoted 3 lines

Shouldn't this use the current CPU number?

The current cpu could be used here with some other ingress FC frames distribution/hashing across all cpus instead this hash logic "oxid & (num_online_cpus() - 1)", possibly ingress FC frame distribution at soft irq level by oxid and then passing up to current cpu's kernel thread fcoe_percpu_receive_thread.

#5 lore raw

Re: [PATCH 2/3] libfc: A modular Fibre Channel library

Quoted 4 lines · Vasu Dev

It had load balancing issue but now it is fixed, related latest submitted code with its updated comment is:-

Quoted 9 lines

/*
* The incoming frame exchange id(oxid) is ANDed with num of online
* cpu bits to get cpu_idx and then this cpu_idx is used for selecting
* a per cpu kernel thread from fcoe_percpu. In case the cpu is
* offline or no kernel thread for derived cpu_idx then cpu_idx is
* initialize to first online cpu index.
*/
cpu_idx = oxid & (num_online_cpus() - 1);

First note that num_online_cpus() is not guaranteed to be a power of two,
- 1 is not guaranteed to give a suitable mask. So you might actually lose random bits. Also your load balancing scheme is unusual to say at least. e.g. when you're just talking to a single frame exchange you would always transfer data between CPUs instead of keeping it all on the CPU that processes the interrupt. Normally the rule of thumb is to use local data as much as possible. Or when you distribute like this at least stay in the same socket. But probably the scheme is too clever anyways.

-Andi

Signature · 1 line
[email protected]
#6 lore raw

Re: [PATCH 2/3] libfc: A modular Fibre Channel library

Quoted 15 lines · Andi Kleen
Quoted 12 lines · Vasu Dev

It had load balancing issue but now it is fixed, related latest submitted code with its updated comment is:-

/*
* The incoming frame exchange id(oxid) is ANDed with num of online
* cpu bits to get cpu_idx and then this cpu_idx is used for selecting
* a per cpu kernel thread from fcoe_percpu. In case the cpu is
* offline or no kernel thread for derived cpu_idx then cpu_idx is
* initialize to first online cpu index.
*/
cpu_idx = oxid & (num_online_cpus() - 1);

Quoted 3 lines

First note that num_online_cpus() is not guaranteed to be a power of two,
- 1 is not guaranteed to give a suitable mask. So you might actually lose random bits.

Correct, this will work best for only power of 2 online cpus and that would be the most common typical use case. I agree it won't load balance better in non power of 2 cpus case.

Quoted 7 lines

Also your load balancing scheme is unusual to say at least.
e.g. when you're just talking to a single frame exchange you would always transfer data between CPUs instead of keeping it all on the CPU that processes the interrupt.

Normally the rule of thumb is to use local
data as much as possible. Or when you distribute like this at least stay in the same socket.

We cannot control what cpu to get interrupted for a FC frame in a typical generic NIC, so we may end up receiving mostly all FC frames on a single same cpu though system might have several other cpus available. In this scenario if frame are passed up to same cpu as suggested above then that won't do any load balancing, therefore some sort of load balancing is required based on some FC frame attributes here.

As I said in my last response that "performance tuning is yet to be done" but you bring up some good related points now of cross socket frame migration and balancing on non power of 2 cpus system. These should be considered during pending performance tuning but for now I can add additional check to select cpu within same socket but not sure how to do that, any kernel call for this ? This might cause more locking contentions on libfc structs so really we have to experiment these thing during performance tuning. Thanks Andi for these hints on performance consideration.

Vasu

#7 lore raw

Re: [PATCH 2/3] libfc: A modular Fibre Channel library

Quoted 49 lines · Vasu Dev
Quoted 16 lines · Andi Kleen
Quoted 13 lines · Vasu Dev

It had load balancing issue but now it is fixed, related latest submitted code with its updated comment is:-

/*
* The incoming frame exchange id(oxid) is ANDed with num of online
* cpu bits to get cpu_idx and then this cpu_idx is used for
selecting
* a per cpu kernel thread from fcoe_percpu. In case the cpu is
* offline or no kernel thread for derived cpu_idx then cpu_idx is
* initialize to first online cpu index.
*/
cpu_idx = oxid & (num_online_cpus() - 1);

Quoted 3 lines

First note that num_online_cpus() is not guaranteed to be a power of two,
- 1 is not guaranteed to give a suitable mask. So you might actually lose random bits.

Correct, this will work best for only power of 2 online cpus and that would be the most common typical use case. I agree it won't load balance better in non power of 2 cpus case.

Quoted 6 lines

Also your load balancing scheme is unusual to say at least. e.g. when
you're just talking to a single frame exchange you would always transfer data between CPUs instead of keeping it all on the CPU that
processes the interrupt. Normally the rule of thumb is to use local
data as much as possible. Or when you distribute like this at least stay in the same socket.

We cannot control what cpu to get interrupted for a FC frame in a typical generic NIC, so we may end up receiving mostly all FC frames on a single same cpu though system might have several other cpus available. In this scenario if frame are passed up to same cpu as suggested above then that won't do any load balancing, therefore some sort of load balancing is required based on some FC frame attributes here.

As I said in my last response that "performance tuning is yet to be done" but you bring up some good related points now of cross socket frame migration and balancing on non power of 2 cpus system. These should be considered during pending performance tuning but for now I can add additional check to select cpu within same socket but not sure how to do that, any kernel call for this ? This might cause more locking contentions on libfc structs so really we have to experiment these thing during performance tuning. Thanks Andi for these hints on performance consideration.

Quoted 1 line

Vasu

Somehow the exchange IDs should be allocated so that responses can be directed back to the same CPU that issued the requests. At one time the mask was a computed mask such that it was always a power of 2.

If we made a separate exchange ID space per CPU and the LLD allocated the XIDs, then it could also be sure it was doing the right thing with the responses.

If we used a separate exchange manager for each CPU, there would be less
lock contention in the EM code. As we diffy up the exchange ID space
between the multiple EMs, we could use the high-order bits to be the CPU number. We would probably want to limit the number of EMs to something like 8 or 16 even on systems with more CPUs.

Like you said, Vasu, this is for performance tuning time, whenever that is. However, I think that might be a few releases away or a gradual evolution since there might be some significant changes involved.

Joe

#8 lore raw

Re: [PATCH 2/3] libfc: A modular Fibre Channel library

In reply to Andi Kleen
Quoted 2 lines · Andi Kleen

Is the put_page() here really correct? The original reference in the skb will be still around, won't it?

But you didn't quote the part with the alloc_page call. Here's the whole function.

Quoted 27 lines

static int fcoe_get_paged_crc_eof(struct sk_buff *skb, int tlen)

{
	struct fcoe_percpu_s *fps;
	struct page *page;
	int cpu_idx;

	cpu_idx = get_cpu();
	fps = fcoe_percpu[cpu_idx];
	page = fps->crc_eof_page;
	if (!page) {
		page = alloc_page(GFP_ATOMIC);
		if (!page) {
			put_cpu();
			return -ENOMEM;
		}
		fps->crc_eof_page = page;
		WARN_ON(fps->crc_eof_offset != 0);
	}

	get_page(page);
	skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags, page,
			   fps->crc_eof_offset, tlen);
	skb->len += tlen;
	skb->data_len += tlen;
	skb->truesize += tlen;
	fps->crc_eof_offset += sizeof(struct fcoe_crc_eof);
Quoted 8 lines
	if (fps->crc_eof_offset >= PAGE_SIZE) {
		fps->crc_eof_page = NULL;
		fps->crc_eof_offset = 0;
		put_page(page);
	}
	put_cpu();
	return 0;
}

FCoE needs to attach an 8 byte trailer to all FC frames created by libfc before transmitting. If the skb is non-linear, say a SCSI write data frame, the only way to do that is to attach a fragment page to the skb.

In order to avoid repeated calls to alloc_page when only 8 bytes are needed, the fcoe module keeps a reference to the page using it repeatedly until it reaches the end. Then then call to put_page happens, and a new page is allocated on the next transmit. At this point the page may still have references from any skbs that are still outstanding (get_page before skb_fill_page_desc). With 4k pages 512 fragmented frames can be sent before calling alloc_page again.

- Chris

#9 lore raw

Re: [PATCH 2/3] libfc: A modular Fibre Channel library

In reply to Andi Kleen
Quoted 2 lines · Andi Kleen

Is the put_page() here really correct? The original reference in the skb will be still around, won't it?

But you didn't quote the part with the alloc_page call. Here's the whole function.

Quoted 27 lines

static int fcoe_get_paged_crc_eof(struct sk_buff *skb, int tlen)

{
	struct fcoe_percpu_s *fps;
	struct page *page;
	int cpu_idx;

	cpu_idx = get_cpu();
	fps = fcoe_percpu[cpu_idx];
	page = fps->crc_eof_page;
	if (!page) {
		page = alloc_page(GFP_ATOMIC);
		if (!page) {
			put_cpu();
			return -ENOMEM;
		}
		fps->crc_eof_page = page;
		WARN_ON(fps->crc_eof_offset != 0);
	}

	get_page(page);
	skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags, page,
			   fps->crc_eof_offset, tlen);
	skb->len += tlen;
	skb->data_len += tlen;
	skb->truesize += tlen;
	fps->crc_eof_offset += sizeof(struct fcoe_crc_eof);
Quoted 8 lines
	if (fps->crc_eof_offset >= PAGE_SIZE) {
		fps->crc_eof_page = NULL;
		fps->crc_eof_offset = 0;
		put_page(page);
	}
	put_cpu();
	return 0;
}

FCoE needs to attach an 8 byte trailer to all FC frames created by libfc before transmitting. If the skb is non-linear, say a SCSI write data frame, the only way to do that is to attach a fragment page to the skb.

In order to avoid repeated calls to alloc_page when only 8 bytes are needed, the fcoe module keeps a reference to the page using it repeatedly until it reaches the end. Then then call to put_page happens, and a new page is allocated on the next transmit. At this point the page may still have references from any skbs that are still outstanding (get_page before skb_fill_page_desc). With 4k pages 512 fragmented frames can be sent before calling alloc_page again.

- Chris

#10 lore raw

RE: [PATCH 2/3] libfc: A modular Fibre Channel library

In reply to Andi Kleen
Quoted 16 lines

include/scsi/libfc.h: struct fcoe_dev_stats *dev_stats[NR_CPUS];

NR_CPUS arrays are strongly discouraged because they lead to waste on kernels compiled with large NR_CPUS, ideally use per CPU data or the per cpu allocator or at least a dynamic allocation with num_possible_cpus()
In general every use of NR_CPUS is dubious. e.g. you have some loops over that,
these should be all over possible cpus. fcoe_percpu[] should be also per cpu data, not an array.

It would probably simplify the code a lot if you just went to global per cpu counters instead of per cpu device counters. I don't know if that is feasible or not.

Quoted 8 lines

static int __init fcoe_init(void)

There are CPU hotplug races between the notifier and the for_each_online_cpu()
loop (probably not a serious problem, such races are common all over the
code base, just wanted to mention it)

Hi, Andi,
Thanks for the comments, I have submitted a patch to open-fcoe.org to convert the foce_percpu[] to be kernel percpu var as you suggested. We also have a dev_stats[NR_CPUS] as a member of the struct fc_lport. It stores an array of pointers to struct fcoe_dev_stats, which keeps track of all the statistics of fcoe traffic. The fc_lport is dynamically allocated upon creating fcoe instance from a given network interface name. The dev_stats are allocated initially in a for_each_online_cpu)() loop, and after that, corresponding dev_stats is allocated/freed upon CPU_ONLINE/CPU_DEAD event callback fcoe_cpu_callback(), do you think this is also a problem in using NR_CPUS?

Thanks.

yi

#11 lore raw

Re: [PATCH 2/3] libfc: A modular Fibre Channel library

Quoted 4 lines

name. The dev_stats are allocated initially in a for_each_online_cpu)() loop, and after that, corresponding dev_stats is allocated/freed upon CPU_ONLINE/CPU_DEAD event callback fcoe_cpu_callback(), do you think this is also a problem in using NR_CPUS?

Yes, it's also a problem. Use num_possible_cpus()

-Andi

#12 lore raw

PATCH 3/3 fcoe: Fibre Channel over Ethernet

In reply to Robert Love

Encapsulation protocol for running Fibre Channel over Ethernet interfaces. Creates virtual Fibre Channel host adapters using libfc.

This layer is the LLD to the scsi-ml. It allocates the Scsi_Host, utilizes libfc for Fibre Channel protocol processing and interacts with netdev to send/receive Ethernet packets.

---

 drivers/scsi/Kconfig                  |    7 
 drivers/scsi/Makefile                 |    1 
 drivers/scsi/fcoe/Makefile            |    8 
 drivers/scsi/fcoe/fc_transport_fcoe.c |  446 ++++++++++
 drivers/scsi/fcoe/fcoe_sw.c           |  494 +++++++++++
 drivers/scsi/fcoe/libfcoe.c           | 1510 +++++++++++++++++++++++++++++++++
 include/scsi/fc_transport_fcoe.h      |   54 +
 include/scsi/libfcoe.h                |  176 ++++
 8 files changed, 2696 insertions(+), 0 deletions(-)
 create mode 100644 drivers/scsi/fcoe/Makefile
 create mode 100644 drivers/scsi/fcoe/fc_transport_fcoe.c
 create mode 100644 drivers/scsi/fcoe/fcoe_sw.c
 create mode 100644 drivers/scsi/fcoe/libfcoe.c
 create mode 100644 include/scsi/fc_transport_fcoe.h
 create mode 100644 include/scsi/libfcoe.h
drivers/scsi/Kconfig +7 -0
index 24d762a..673463e 100644
@@ -609,6 +609,13 @@ config LIBFC
 	---help---
 	  Fibre Channel library module
 
+config FCOE
+	tristate "FCoE module"
+	depends on SCSI
+	select LIBFC
+	---help---
+	  Fibre Channel over Ethernet module
+
 config SCSI_DMX3191D
 	tristate "DMX3191D SCSI support"
 	depends on PCI && SCSI
drivers/scsi/Makefile +1 -0
index 87355f5..07d0f58 100644
@@ -37,6 +37,7 @@ obj-$(CONFIG_SCSI_SRP_ATTRS)	+= scsi_transport_srp.o
 obj-$(CONFIG_SCSI_DH)		+= device_handler/
 
 obj-$(CONFIG_LIBFC)		+= libfc/
+obj-$(CONFIG_FCOE)		+= fcoe/
 obj-$(CONFIG_ISCSI_TCP) 	+= libiscsi.o	libiscsi_tcp.o iscsi_tcp.o
 obj-$(CONFIG_INFINIBAND_ISER) 	+= libiscsi.o
 obj-$(CONFIG_SCSI_A4000T)	+= 53c700.o	a4000t.o
drivers/scsi/fcoe/Makefile new file +8 -0
new file mode 100644
index 0000000..b78da06
@@ -0,0 +1,8 @@
+# $Id: Makefile
+
+obj-$(CONFIG_FCOE) += fcoe.o
+
+fcoe-y := \
+	libfcoe.o \
+	fcoe_sw.o \
+	fc_transport_fcoe.o
drivers/scsi/fcoe/fc_transport_fcoe.c new file +446 -0

446 lines not on this page · raw message

drivers/scsi/fcoe/fcoe_sw.c new file +494 -0

494 lines not on this page · raw message

drivers/scsi/fcoe/libfcoe.c new file +1510 -0

1,510 lines not on this page · raw message

include/scsi/fc_transport_fcoe.h new file +54 -0

54 lines not on this page · raw message

include/scsi/libfcoe.h new file +176 -0

176 lines not on this page · raw message

#13 lore raw

Re: [PATCH 3/3] fcoe: Fibre Channel over Ethernet

Quoted 7 lines · On Tue, 09 Dec 2008 15:10:24 -0800 Robert Love <[email protected]> wrote:

Encapsulation protocol for running Fibre Channel over Ethernet interfaces. Creates virtual Fibre Channel host adapters using libfc.

This layer is the LLD to the scsi-ml. It allocates the Scsi_Host, utilizes libfc for Fibre Channel protocol processing and interacts with netdev to send/receive Ethernet packets.

I stumbled across this while looking for new and weird kthread API usages..

Quoted 12 lines

...

+/**
+ * fcoe_transport_lookup - check if the transport is already registered
+ * @t: the transport to be looked up
+ *
+ * This compares the parent device (pci) vendor and device id
+ *
+ * Returns: NULL if not found
+ *
+ * TODO - return default sw transport if no other transport is found
+ **/

The kerneldoc comments consistently close with

**/

which is consistently unconventional. Not wrong, just odd.

Quoted 10 lines

...

drivers/scsi/fcoe/fc_transport_fcoe.c new file +7 -0
+int fcoe_transport_register(struct fcoe_transport *t)
+{
+	struct fcoe_transport *tt;
+
+	/* TODO - add fcoe_transport specific initialization here */
+	mutex_lock(&fcoe_transports_lock);
+	list_for_each_entry(tt, &fcoe_transports, list) {

drivers/scsi/fcoe/fc_transport_fcoe.c line 263

Quoted 10 lines
drivers/scsi/fcoe/fc_transport_fcoe.c new file +10 -0
+		if (tt == t) {
+			mutex_unlock(&fcoe_transports_lock);
+			return -EEXIST;
+		}
+	}
+	list_add_tail(&t->list, &fcoe_transports);
+	mutex_unlock(&fcoe_transports_lock);
+
+	mutex_init(&t->devlock);
+	INIT_LIST_HEAD(&t->devlist);

Seems wrong to make this available for lookups pror to completing its initialisation.

Quoted 14 lines
drivers/scsi/fcoe/fc_transport_fcoe.c new file +11 -0
+	printk(KERN_DEBUG "fcoe_transport_register:%s\n", t->name);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fcoe_transport_register);
+
 
...
 
+int fcoe_load_transport_driver(struct net_device *netdev)
+{
+	struct pci_dev *pci;
+	struct device *dev = netdev->dev.parent;
+

drivers/scsi/fcoe/fc_transport_fcoe.c line 325

Quoted 10 lines
drivers/scsi/fcoe/fc_transport_fcoe.c new file +10 -0
+	if (fcoe_transport_lookup(netdev)) {
+		/* load default transport */
+		printk(KERN_DEBUG "fcoe: already loaded transport for %s\n",
+		       netdev->name);
+		return -EEXIST;
+	}
+
+	pci = to_pci_dev(dev);
+	if (dev->bus != &pci_bus_type) {
+		printk(KERN_DEBUG "fcoe: support noly PCI device\n");

typo

Quoted 24 lines
drivers/scsi/fcoe/fcoe_sw.c new file +21 -0
+		return -ENODEV;
+	}
+	printk(KERN_DEBUG "fcoe: loading driver fcoe-pci-0x%04x-0x%04x\n",
+	       pci->vendor, pci->device);
+
+	return request_module("fcoe-pci-0x%04x-0x%04x",
+			      pci->vendor, pci->device);
+
 
...
 
+static int fcoe_sw_lport_config(struct fc_lport *lp)
+{
+	int i = 0;
+
+	lp->link_status = 0;
+	lp->max_retry_count = 3;
+	lp->e_d_tov = 2 * 1000;	/* FC-FS default */
+	lp->r_a_tov = 2 * 2 * 1000;
+	lp->service_params = (FCP_SPPF_INIT_FCN | FCP_SPPF_RD_XRDY_DIS |
+			      FCP_SPPF_RETRY | FCP_SPPF_CONF_COMPL);
+
+	/*
+	 * allocate per cpu stats block

drivers/scsi/fcoe/fcoe_sw.c line 137

Quoted 10 lines
drivers/scsi/fcoe/fcoe_sw.c new file +10 -0
+	 */
+	for_each_online_cpu(i)
+		lp->dev_stats[i] = kzalloc(sizeof(struct fcoe_dev_stats),
+					   GFP_KERNEL);
+
+	/* lport fc_lport related configuration */
+	fc_lport_config(lp);
+
+	return 0;
+}

Has the CPU hotplugging code been tested? This seems to not do things which fcoe_create_percpu_data() does.

Quoted 21 lines
drivers/scsi/fcoe/fcoe_sw.c new file +18 -0
+/*
+ * fcoe_sw_netdev_config - sets up fcoe_softc for lport and network
+ * related properties
+ * @lp : ptr to the fc_lport
+ * @netdev : ptr to the associated netdevice struct
+ *
+ * Must be called after fcoe_sw_lport_config() as it will use lport mutex
+ *
+ * Returns : 0 for success
+ *
+ */
 
...
 
+static int fcoe_cpu_callback(struct notifier_block *nfb, unsigned long action,
+			     void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+
+	switch (action) {
+	case CPU_ONLINE:

drivers/scsi/fcoe/fcoe_sw.c line 225

Quoted 10 lines
drivers/scsi/fcoe/fcoe_sw.c new file +10 -0
+		fcoe_create_percpu_data(cpu);
+		break;
+	case CPU_DEAD:
+		fcoe_destroy_percpu_data(cpu);
+		break;
+	default:
+		break;
+	}
+	return NOTIFY_OK;
+}

This function forgets to create and destroy the per-cpu kernel threads.

Quoted 8 lines

...

+/*
+ * fcoe_percpu_clean - frees skb of the corresponding lport from the per
+ * cpu queue.
+ * @lp: the fc_lport
+ */

This purports to bea kerneldoc comment but mieese the /** token.

The first line of the kernedoc comment should not have a linbreak - kerneldoc doesn't understand that.

Quoted 8 lines

...

drivers/scsi/fcoe/libfcoe.c new file +5 -0
+static struct fcoe_softc *fcoe_hostlist_lookup_softc(
+	const struct net_device *dev)
+{
+	struct fcoe_softc *fc;
+

drivers/scsi/fcoe/libfcoe.c line 1309

Quoted 10 lines
drivers/scsi/fcoe/libfcoe.c new file +10 -0
+	read_lock(&fcoe_hostlist_lock);
+	list_for_each_entry(fc, &fcoe_hostlist, list) {
+		if (fc->real_dev == dev) {
+			read_unlock(&fcoe_hostlist_lock);
+			return fc;
+		}
+	}
+	read_unlock(&fcoe_hostlist_lock);
+	return NULL;
+}

It's strange to take a lock, look something up, drop the lock and then return the looked-up object without having taken any steps to prevent some other thread of control from destroying that object while the caller is playing with it. (ie: take a reference on it).

Quoted 5 lines

...

drivers/scsi/fcoe/libfcoe.c new file +2 -0
+static int __init fcoe_init(void)
+{

drivers/scsi/fcoe/libfcoe.c line 1411

Quoted 10 lines
drivers/scsi/fcoe/libfcoe.c new file +10 -0
+	int cpu;
+	struct fcoe_percpu_s *p;
+
+
+	INIT_LIST_HEAD(&fcoe_hostlist);
+	rwlock_init(&fcoe_hostlist_lock);
+
+#ifdef CONFIG_HOTPLUG_CPU
+	register_cpu_notifier(&fcoe_cpu_notifier);
+#endif /* CONFIG_HOTPLUG_CPU */

This could/should use hotcpu_notifier(), lose the ifdefs.

Quoted 18 lines
drivers/scsi/fcoe/libfcoe.c new file +18 -0
+
+	/*
+	 * initialize per CPU interrupt thread
+	 */
+	for_each_online_cpu(cpu) {
+		p = kzalloc(sizeof(struct fcoe_percpu_s), GFP_KERNEL);
+		if (p) {
+			p->thread = kthread_create(fcoe_percpu_receive_thread,
+						   (void *)p,
+						   "fcoethread/%d", cpu);
+
+			/*
+			 * if there is no error then bind the thread to the cpu
+			 * initialize the semaphore and skb queue head
+			 */
+			if (likely(!IS_ERR(p->thread))) {
+				p->cpu = cpu;
+				fcoe_percpu[cpu] = p;

drivers/scsi/fcoe/libfcoe.c line 1439

Quoted 10 lines
drivers/scsi/fcoe/libfcoe.c new file +10 -0
+				skb_queue_head_init(&p->fcoe_rx_list);
+				kthread_bind(p->thread, cpu);
+				wake_up_process(p->thread);
+			} else {
+				fcoe_percpu[cpu] = NULL;
+				kfree(p);
+
+			}
+		}
+	}

afaict nothing is done here to avoid races against CPU hotplug.

What happens if the kzalloc() failed?

drivers/scsi/fcoe/libfcoe.c line 1448

Quoted 8 lines
drivers/scsi/fcoe/libfcoe.c new file +8 -0
+	/*
+	 * setup link change notification
+	 */
+	fcoe_dev_setup();
+
+	init_timer(&fcoe_timer);
+	fcoe_timer.data = 0;
+	fcoe_timer.function = fcoe_watchdog;

Could use setup_timer() here.

drivers/scsi/fcoe/libfcoe.c line 1450

Quoted 2 lines
drivers/scsi/fcoe/libfcoe.c new file +2 -0
+	fcoe_timer.expires = (jiffies + (10 * HZ));
+	add_timer(&fcoe_timer);

Could actually use mod_timer() here.

Quoted 17 lines
drivers/scsi/fcoe/libfcoe.c new file +17 -0
+	/* initiatlize the fcoe transport */
+	fcoe_transport_init();
+
+	fcoe_sw_init();
+
+	return 0;
+}
+module_init(fcoe_init);
+
+/**
+ * fcoe_exit - fcoe module unloading cleanup
+ *
+ * Returns 0 on success, negative on failure
+ **/
+static void __exit fcoe_exit(void)
+{
+	u32 idx;

drivers/scsi/fcoe/libfcoe.c line 1478

Quoted 10 lines
drivers/scsi/fcoe/libfcoe.c new file +10 -0
+	struct fcoe_softc *fc, *tmp;
+	struct fcoe_percpu_s *p;
+	struct sk_buff *skb;
+
+	/*
+	 * Stop all call back interfaces
+	 */
+#ifdef CONFIG_HOTPLUG_CPU
+	unregister_cpu_notifier(&fcoe_cpu_notifier);
+#endif /* CONFIG_HOTPLUG_CPU */

Use unregister_hotcpu_notifier(), lose ifdef (I think).

drivers/scsi/fcoe/libfcoe.c line 1486

Quoted 8 lines
drivers/scsi/fcoe/libfcoe.c new file +8 -0
+	fcoe_dev_cleanup();
+
+	/*
+	 * stop timer
+	 */
+	del_timer_sync(&fcoe_timer);
+
+	/* releases the assocaited fcoe transport for each lport */

typo

Quoted 6 lines
drivers/scsi/fcoe/libfcoe.c new file +6 -0
+	list_for_each_entry_safe(fc, tmp, &fcoe_hostlist, list)
+		fcoe_transport_release(fc->real_dev);
+
+	for (idx = 0; idx < NR_CPUS; idx++) {
+		if (fcoe_percpu[idx]) {
+			kthread_stop(fcoe_percpu[idx]->thread);

drivers/scsi/fcoe/libfcoe.c line 1502

Quoted 10 lines
drivers/scsi/fcoe/libfcoe.c new file +10 -0
+			p = fcoe_percpu[idx];
+			spin_lock_bh(&p->fcoe_rx_list.lock);
+			while ((skb = __skb_dequeue(&p->fcoe_rx_list)) != NULL)
+				kfree_skb(skb);
+			spin_unlock_bh(&p->fcoe_rx_list.lock);
+			if (fcoe_percpu[idx]->crc_eof_page)
+				put_page(fcoe_percpu[idx]->crc_eof_page);
+			kfree(fcoe_percpu[idx]);
+		}
+	}

The iterating across NR_CPUS is rather, umm, old-fashioned.

It'd be better to lock against cpu hotplugging then use for_each_online_cpu().

drivers/scsi/fcoe/libfcoe.c

Quoted 11 lines
drivers/scsi/fcoe/libfcoe.c new file +8 -0
+	/* remove sw trasnport */
+	fcoe_sw_exit();
+
+	/* detach the transport */
+	fcoe_transport_exit();
+}
 
...
 
+static inline struct fcoe_softc *fcoe_softc(
+	const struct fc_lport *lp)

weird code layout, used everywhere.

include/scsi/libfcoe.h line 64

Quoted 2 lines
include/scsi/libfcoe.h new file +2 -0
+{
+	return (struct fcoe_softc *)lport_priv(lp);

unneeded/undesirable cast of void*. There are probably zillions of instances of this - there always are.

Quoted 5 lines
drivers/scsi/fcoe/fc_transport_fcoe.c new file +2 -0
+}
+
 
...
#14 lore raw

Re: [PATCH 3/3] fcoe: Fibre Channel over Ethernet

Quoted 5 lines · Andrew Morton
Quoted 3 lines · On Tue, 09 Dec 2008 15:10:24 -0800 Robert Love <[email protected]> wrote:

Encapsulation protocol for running Fibre Channel over Ethernet interfaces. Creates virtual Fibre Channel host adapters using libfc.

Quoted 7 lines
Quoted 4 lines

This layer is the LLD to the scsi-ml. It allocates the Scsi_Host, utilizes libfc for Fibre Channel protocol processing and interacts with netdev to send/receive Ethernet packets.

I stumbled across this while looking for new and weird kthread API usages..

I just sent a patchset of Open-FCoE updates to linux-scsi. Patches in that set should take care of the kernel-doc issues, the initialization ordering problem for fcoe transports, the typos, the timer comments, and the unnecessary casting problem.

Quoted 13 lines
Quoted 12 lines

...

+/**
+ * fcoe_transport_lookup - check if the transport is already registered
+ * @t: the transport to be looked up
+ *
+ * This compares the parent device (pci) vendor and device id
+ *
+ * Returns: NULL if not found
+ *
+ * TODO - return default sw transport if no other transport is found
+ **/

Quoted 6 lines

The kerneldoc comments consistently close with

**/

which is consistently unconventional. Not wrong, just odd.

I did change the kernel-doc function comment blocks to end with '*/', since you called it out. I looked at the other comments under drivers/scsi/ and most did end in '*/'. However, the Documentation/kernel-doc-nano-HOWTO.txt has an example for function headers that shows '**/' as the suggested style. Should the kernel-doc HOWTO be updated?

Example kernel-doc function comment:

/**
* foobar() - short function description of foobar

 * @arg1:       Describe the first argument to foobar.
 * @arg2:       Describe the second argument to foobar.
 *              One can provide multiple line descriptions
 *              for arguments.

*
* A longer description, with more discussion of the function foobar()
* that might be useful to those using or modifying it. Begins with
* empty comment line, and may include additional embedded empty
* comment lines.
*
* The longer description can have multiple paragraphs.
**/

#15 lore raw

Re: [PATCH 3/3] fcoe: Fibre Channel over Ethernet

Quoted 7 lines · On Fri, 06 Feb 2009 11:05:17 -0800 Robert Love <[email protected]> wrote:
Quoted 6 lines

The kerneldoc comments consistently close with

**/

which is consistently unconventional. Not wrong, just odd.

Quoted 6 lines

I did change the kernel-doc function comment blocks to end with '*/', since you called it out. I looked at the other comments under drivers/scsi/ and most did end in '*/'. However, the Documentation/kernel-doc-nano-HOWTO.txt has an example for function headers that shows '**/' as the suggested style. Should the kernel-doc HOWTO be updated?

I think so - we might as well be consistent about these things.

Quoted 8 lines

Example kernel-doc function comment:

/**
* foobar() - short function description of foobar

 * @arg1:       Describe the first argument to foobar.
 * @arg2:       Describe the second argument to foobar.
 *              One can provide multiple line descriptions
 *              for arguments.
Quoted 8 lines

*
* A longer description, with more discussion of the function foobar()
* that might be useful to those using or modifying it. Begins with
* empty comment line, and may include additional embedded empty
* comment lines.
*
* The longer description can have multiple paragraphs.
**/

That's RandyStuff.

#16 lore raw

PATCH 1/1 kernel-doc: preferred ending marker and examples

From: Randy Dunlap <[email protected]>

Fix kernel-doc-nano-HOWTO.txt to use */ as the ending marker in kernel-doc examples and state that */ is the preferred ending marker.

---
 Documentation/kernel-doc-nano-HOWTO.txt |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/kernel-doc-nano-HOWTO.txt +4 -3
@@ -43,7 +43,8 @@ Only comments so marked will be consider
 and any comment so marked must be in kernel-doc format.  Do not use
 "/**" to be begin a comment block unless the comment block contains
 kernel-doc formatted comments.  The closing comment marker for
-kernel-doc comments can be either "*/" or "**/".
+kernel-doc comments can be either "*/" or "**/", but "*/" is
+preferred in the Linux kernel tree.
 
 Kernel-doc comments should be placed just before the function
 or data structure being described.
@@ -63,7 +64,7 @@ Example kernel-doc function comment:
  * comment lines.
  *
  * The longer description can have multiple paragraphs.
- **/
+ */
 
 The first line, with the short description, must be on a single line.
 
@@ -85,7 +86,7 @@ Example kernel-doc data structure commen
  *		perhaps with more lines and words.
  *
  * Longer description of this structure.
- **/
+ */
 
 The kernel-doc function comments describe each parameter to the
 function, in order, with the @name lines.