nvmf/fc: Data path should not use nport and rport.

nport and rport data structures are supposed to be used only
in control path but not datapath. FC data path scaling is based
on connection and connection object contains all the required
information for command processing.

Signed-off-by: Naresh Gottumukkala <raju.gottumukkala@broadcom.com>
Change-Id: I70f6896955d97abc86bc7c5e6b9f160d622e60d7
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5501
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Anil Veerabhadrappa <anil.veerabhadrappa@broadcom.com>

lib/nvmf/fc.c

@@ -1285,6 +1285,12 @@ nvmf_fc_hwqp_handle_request(struct spdk_nvmf_fc_hwqp *hwqp, struct spdk_nvmf_fc_
 	struct spdk_nvmf_fc_cmnd_iu *cmd_iu = NULL;
 	struct spdk_nvmf_fc_conn *fc_conn = NULL;
 	enum spdk_nvme_data_transfer xfer;
+	uint32_t s_id, d_id;
+
+	s_id = (uint32_t)frame->s_id;
+	d_id = (uint32_t)frame->d_id;
+	s_id = from_be32(&s_id) >> 8;
+	d_id = from_be32(&d_id) >> 8;
 
 	cmd_iu = buffer->virt;
 	cmnd_len = cmd_iu->cmnd_iu_len;
@@ -1316,6 +1322,19 @@ nvmf_fc_hwqp_handle_request(struct spdk_nvmf_fc_hwqp *hwqp, struct spdk_nvmf_fc_
 		return -ENODEV;
 	}
 
+	/* Validate s_id and d_id */
+	if (s_id != fc_conn->s_id) {
+		hwqp->counters.rport_invalid++;
+		SPDK_ERRLOG("Frame s_id invalid for connection %ld\n", rqst_conn_id);
+		return -ENODEV;
+	}
+
+	if (d_id != fc_conn->d_id) {
+		hwqp->counters.nport_invalid++;
+		SPDK_ERRLOG("Frame d_id invalid for connection %ld\n", rqst_conn_id);
+		return -ENODEV;
+	}
+
 	/* If association/connection is being deleted - return */
 	if (fc_conn->fc_assoc->assoc_state !=  SPDK_NVMF_FC_OBJECT_CREATED) {
 		SPDK_ERRLOG("Association state not valid\n");
@@ -1336,7 +1355,6 @@ nvmf_fc_hwqp_handle_request(struct spdk_nvmf_fc_hwqp *hwqp, struct spdk_nvmf_fc_
 	/* allocate a request buffer */
 	fc_req = nvmf_fc_hwqp_alloc_fc_request(hwqp);
 	if (fc_req == NULL) {
-		/* Should not happen. Since fc_reqs == RQ buffers */
 		return -ENOMEM;
 	}
 
@@ -1353,10 +1371,8 @@ nvmf_fc_hwqp_handle_request(struct spdk_nvmf_fc_hwqp *hwqp, struct spdk_nvmf_fc_
 	fc_req->hwqp = hwqp;
 	fc_req->fc_conn = fc_conn;
 	fc_req->req.xfer = xfer;
-	fc_req->s_id = (uint32_t)frame->s_id;
+	fc_req->s_id = s_id;
-	fc_req->d_id = (uint32_t)frame->d_id;
+	fc_req->d_id = d_id;
-	fc_req->s_id = from_be32(&fc_req->s_id) >> 8;
-	fc_req->d_id = from_be32(&fc_req->d_id) >> 8;
 
 	nvmf_fc_record_req_trace_point(fc_req, SPDK_NVMF_FC_REQ_INIT);
 
@@ -1440,16 +1456,19 @@ nvmf_fc_hwqp_process_frame(struct spdk_nvmf_fc_hwqp *hwqp,
 	s_id = from_be32(&s_id) >> 8;
 	d_id = from_be32(&d_id) >> 8;
 
-	/* Note: In tracelog below, we directly do endian conversion on rx_id and.
-	 * ox_id Since these are fields, we can't pass address to from_be16().
-	 * Since ox_id and rx_id are only needed for tracelog, assigning to local
-	 * vars. and doing conversion is a waste of time in non-debug builds. */
 	SPDK_DEBUGLOG(nvmf_fc,
 		      "Process NVME frame s_id:0x%x d_id:0x%x oxid:0x%x rxid:0x%x.\n",
 		      s_id, d_id,
 		      ((frame->ox_id << 8) & 0xff00) | ((frame->ox_id >> 8) & 0xff),
 		      ((frame->rx_id << 8) & 0xff00) | ((frame->rx_id >> 8) & 0xff));
 
+	if ((frame->r_ctl == FCNVME_R_CTL_LS_REQUEST) &&
+	    (frame->type == FCNVME_TYPE_NVMF_DATA)) {
+		struct spdk_nvmf_fc_rq_buf_ls_request *req_buf = buffer->virt;
+		struct spdk_nvmf_fc_ls_rqst *ls_rqst;
+
+		SPDK_DEBUGLOG(nvmf_fc, "Process LS NVME frame\n");
+
 		rc = nvmf_fc_hwqp_find_nport_and_rport(hwqp, d_id, &nport, s_id, &rport);
 		if (rc) {
 			if (nport == NULL) {
@@ -1472,13 +1491,6 @@ nvmf_fc_hwqp_process_frame(struct spdk_nvmf_fc_hwqp *hwqp,
 			return -EACCES;
 		}
 
-	if ((frame->r_ctl == FCNVME_R_CTL_LS_REQUEST) &&
-	    (frame->type == FCNVME_TYPE_NVMF_DATA)) {
-		struct spdk_nvmf_fc_rq_buf_ls_request *req_buf = buffer->virt;
-		struct spdk_nvmf_fc_ls_rqst *ls_rqst;
-
-		SPDK_DEBUGLOG(nvmf_fc, "Process LS NVME frame\n");
-
 		/* Use the RQ buffer for holding LS request. */
 		ls_rqst = (struct spdk_nvmf_fc_ls_rqst *)&req_buf->ls_rqst;
 

lib/nvmf/fc_ls.c

@@ -349,6 +349,8 @@ nvmf_fc_ls_new_connection(struct spdk_nvmf_fc_association *assoc, uint16_t qid,
 	TAILQ_INIT(&fc_conn->qpair.outstanding);
 	fc_conn->esrp_ratio = esrp_ratio;
 	fc_conn->fc_assoc = assoc;
+	fc_conn->s_id = assoc->s_id;
+	fc_conn->d_id = assoc->tgtport->d_id;
 	fc_conn->rpi = rpi;
 	fc_conn->max_queue_depth = sq_size + 1;
 

lib/nvmf/nvmf_fc.h

@@ -207,6 +207,8 @@ struct spdk_nvmf_fc_conn {
 	struct spdk_nvmf_qpair qpair;
 	struct spdk_nvme_transport_id trid;
 
+	uint32_t s_id;
+	uint32_t d_id;
 	uint64_t conn_id;
 	struct spdk_nvmf_fc_hwqp *hwqp;
 	uint16_t esrp_ratio;