From 604e257984185ecd666efa4f8db473471b743f2e Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Mon, 7 Jul 2014 09:37:22 +0000 Subject: [PATCH] Teach ctl_add_initiator() to dynamically allocate IIDs from pool. If port passed negative IID value, the function will try to allocate IID from the pool of unused, based on passed wwpn or name arguments. It does all its best to make IID unique and persistent across reconnects. This makes persistent reservation properly work for iSCSI. Previously, in case of reconnects, reservation could be unexpectedly lost, or even migrate between intiators. --- sys/cam/ctl/ctl.c | 195 ++++++++++++++++++------------- sys/cam/ctl/ctl_frontend.c | 20 +++- sys/cam/ctl/ctl_frontend.h | 35 +++--- sys/cam/ctl/ctl_frontend_iscsi.c | 52 +++------ sys/cam/ctl/ctl_frontend_iscsi.h | 2 - sys/cam/ctl/ctl_private.h | 8 -- sys/cam/ctl/scsi_ctl.c | 10 +- 7 files changed, 172 insertions(+), 150 deletions(-) diff --git a/sys/cam/ctl/ctl.c b/sys/cam/ctl/ctl.c index 6d62eb8727b..fa620faf4f7 100644 --- a/sys/cam/ctl/ctl.c +++ b/sys/cam/ctl/ctl.c @@ -1088,11 +1088,6 @@ ctl_init(void) if (bootverbose) printf("ctl: CAM Target Layer loaded\n"); - /* - * Initialize the initiator and portname mappings - */ - memset(softc->wwpn_iid, 0, sizeof(softc->wwpn_iid)); - /* * Initialize the ioctl front end. */ @@ -1365,32 +1360,24 @@ ctl_ioctl_offline(void *arg) /* * Remove an initiator by port number and initiator ID. - * Returns 0 for success, 1 for failure. + * Returns 0 for success, -1 for failure. */ int -ctl_remove_initiator(int32_t targ_port, uint32_t iid) +ctl_remove_initiator(struct ctl_port *port, int iid) { - struct ctl_softc *softc; - - softc = control_softc; + struct ctl_softc *softc = control_softc; mtx_assert(&softc->ctl_lock, MA_NOTOWNED); - if ((targ_port < 0) - || (targ_port > CTL_MAX_PORTS)) { - printf("%s: invalid port number %d\n", __func__, targ_port); - return (1); - } if (iid > CTL_MAX_INIT_PER_PORT) { printf("%s: initiator ID %u > maximun %u!\n", __func__, iid, CTL_MAX_INIT_PER_PORT); - return (1); + return (-1); } mtx_lock(&softc->ctl_lock); - - softc->wwpn_iid[targ_port][iid].in_use = 0; - + port->wwpn_iid[iid].in_use--; + port->wwpn_iid[iid].last_use = time_uptime; mtx_unlock(&softc->ctl_lock); return (0); @@ -1398,41 +1385,91 @@ ctl_remove_initiator(int32_t targ_port, uint32_t iid) /* * Add an initiator to the initiator map. - * Returns 0 for success, 1 for failure. + * Returns iid for success, < 0 for failure. */ int -ctl_add_initiator(uint64_t wwpn, int32_t targ_port, uint32_t iid) +ctl_add_initiator(struct ctl_port *port, int iid, uint64_t wwpn, char *name) { - struct ctl_softc *softc; - int retval; - - softc = control_softc; + struct ctl_softc *softc = control_softc; + time_t best_time; + int i, best; mtx_assert(&softc->ctl_lock, MA_NOTOWNED); - retval = 0; - - if ((targ_port < 0) - || (targ_port > CTL_MAX_PORTS)) { - printf("%s: invalid port number %d\n", __func__, targ_port); - return (1); - } - if (iid > CTL_MAX_INIT_PER_PORT) { - printf("%s: WWPN %#jx initiator ID %u > maximun %u!\n", + if (iid >= CTL_MAX_INIT_PER_PORT) { + printf("%s: WWPN %#jx initiator ID %u > maximum %u!\n", __func__, wwpn, iid, CTL_MAX_INIT_PER_PORT); - return (1); + free(name, M_CTL); + return (-1); } mtx_lock(&softc->ctl_lock); - if (softc->wwpn_iid[targ_port][iid].in_use != 0) { + if (iid < 0 && (wwpn != 0 || name != NULL)) { + for (i = 0; i < CTL_MAX_INIT_PER_PORT; i++) { + if (wwpn != 0 && wwpn == port->wwpn_iid[i].wwpn) { + iid = i; + break; + } + if (name != NULL && port->wwpn_iid[i].name != NULL && + strcmp(name, port->wwpn_iid[i].name) == 0) { + iid = i; + break; + } + } + } + + if (iid < 0) { + for (i = 0; i < CTL_MAX_INIT_PER_PORT; i++) { + if (port->wwpn_iid[i].in_use == 0 && + port->wwpn_iid[i].wwpn == 0 && + port->wwpn_iid[i].name == NULL) { + iid = i; + break; + } + } + } + + if (iid < 0) { + best = -1; + best_time = INT32_MAX; + for (i = 0; i < CTL_MAX_INIT_PER_PORT; i++) { + if (port->wwpn_iid[i].in_use == 0) { + if (port->wwpn_iid[i].last_use < best_time) { + best = i; + best_time = port->wwpn_iid[i].last_use; + } + } + } + iid = best; + } + + if (iid < 0) { + mtx_unlock(&softc->ctl_lock); + free(name, M_CTL); + return (-2); + } + + if (port->wwpn_iid[iid].in_use > 0 && (wwpn != 0 || name != NULL)) { /* - * We don't treat this as an error. + * This is not an error yet. */ - if (softc->wwpn_iid[targ_port][iid].wwpn == wwpn) { - printf("%s: port %d iid %u WWPN %#jx arrived again?\n", - __func__, targ_port, iid, (uintmax_t)wwpn); - goto bailout; + if (wwpn != 0 && wwpn == port->wwpn_iid[iid].wwpn) { +#if 0 + printf("%s: port %d iid %u WWPN %#jx arrived" + " again\n", __func__, port->targ_port, + iid, (uintmax_t)wwpn); +#endif + goto take; + } + if (name != NULL && port->wwpn_iid[iid].name != NULL && + strcmp(name, port->wwpn_iid[iid].name) == 0) { +#if 0 + printf("%s: port %d iid %u name '%s' arrived" + " again\n", __func__, port->targ_port, + iid, name); +#endif + goto take; } /* @@ -1440,26 +1477,25 @@ ctl_add_initiator(uint64_t wwpn, int32_t targ_port, uint32_t iid) * driver is telling us we have a new WWPN for this * initiator ID, so we pretty much need to use it. */ - printf("%s: port %d iid %u WWPN %#jx arrived, WWPN %#jx is " - "still at that address\n", __func__, targ_port, iid, - (uintmax_t)wwpn, - (uintmax_t)softc->wwpn_iid[targ_port][iid].wwpn); + printf("%s: port %d iid %u WWPN %#jx '%s' arrived," + " but WWPN %#jx '%s' is still at that address\n", + __func__, port->targ_port, iid, wwpn, name, + (uintmax_t)port->wwpn_iid[iid].wwpn, + port->wwpn_iid[iid].name); /* * XXX KDM clear have_ca and ua_pending on each LUN for * this initiator. */ } - softc->wwpn_iid[targ_port][iid].in_use = 1; - softc->wwpn_iid[targ_port][iid].iid = iid; - softc->wwpn_iid[targ_port][iid].wwpn = wwpn; - softc->wwpn_iid[targ_port][iid].port = targ_port; - -bailout: - +take: + free(port->wwpn_iid[iid].name, M_CTL); + port->wwpn_iid[iid].name = name; + port->wwpn_iid[iid].wwpn = wwpn; + port->wwpn_iid[iid].in_use++; mtx_unlock(&softc->ctl_lock); - return (retval); + return (iid); } static int @@ -2872,23 +2908,11 @@ ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag, break; } case CTL_DUMP_STRUCTS: { - int i, j, k; + int i, j, k, idx; struct ctl_port *port; struct ctl_frontend *fe; - printf("CTL IID to WWPN map start:\n"); - for (i = 0; i < CTL_MAX_PORTS; i++) { - for (j = 0; j < CTL_MAX_INIT_PER_PORT; j++) { - if (softc->wwpn_iid[i][j].in_use == 0) - continue; - - printf("port %d iid %u WWPN %#jx\n", - softc->wwpn_iid[i][j].port, - softc->wwpn_iid[i][j].iid, - (uintmax_t)softc->wwpn_iid[i][j].wwpn); - } - } - printf("CTL IID to WWPN map end\n"); + mtx_lock(&softc->ctl_lock); printf("CTL Persistent Reservation information start:\n"); for (i = 0; i < CTL_MAX_LUNS; i++) { struct ctl_lun *lun; @@ -2901,33 +2925,46 @@ ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag, for (j = 0; j < (CTL_MAX_PORTS * 2); j++) { for (k = 0; k < CTL_MAX_INIT_PER_PORT; k++){ - if (lun->per_res[j+k].registered == 0) + idx = j * CTL_MAX_INIT_PER_PORT + k; + if (lun->per_res[idx].registered == 0) continue; - printf("LUN %d port %d iid %d key " + printf(" LUN %d port %d iid %d key " "%#jx\n", i, j, k, (uintmax_t)scsi_8btou64( - lun->per_res[j+k].res_key.key)); + lun->per_res[idx].res_key.key)); } } } printf("CTL Persistent Reservation information end\n"); printf("CTL Ports:\n"); + STAILQ_FOREACH(port, &softc->port_list, links) { + printf(" Port %d '%s' Frontend '%s' Type %u pp %d vp %d WWNN " + "%#jx WWPN %#jx\n", port->targ_port, port->port_name, + port->frontend->name, port->port_type, + port->physical_port, port->virtual_port, + (uintmax_t)port->wwnn, (uintmax_t)port->wwpn); + for (j = 0; j < CTL_MAX_INIT_PER_PORT; j++) { + if (port->wwpn_iid[j].in_use == 0 && + port->wwpn_iid[j].wwpn == 0 && + port->wwpn_iid[j].name == NULL) + continue; + + printf(" iid %u use %d WWPN %#jx '%s'\n", + j, port->wwpn_iid[j].in_use, + (uintmax_t)port->wwpn_iid[j].wwpn, + port->wwpn_iid[j].name); + } + } + printf("CTL Port information end\n"); + mtx_unlock(&softc->ctl_lock); /* * XXX KDM calling this without a lock. We'd likely want * to drop the lock before calling the frontend's dump * routine anyway. */ - STAILQ_FOREACH(port, &softc->port_list, links) { - printf("Port %s Frontend %s Type %u pport %d vport %d WWNN " - "%#jx WWPN %#jx\n", port->port_name, - port->frontend->name, port->port_type, - port->physical_port, port->virtual_port, - (uintmax_t)port->wwnn, (uintmax_t)port->wwpn); - } - printf("CTL Port information end\n"); printf("CTL Frontends:\n"); STAILQ_FOREACH(fe, &softc->fe_list, links) { - printf("Frontend %s\n", fe->name); + printf(" Frontend '%s'\n", fe->name); if (fe->fe_dump != NULL) fe->fe_dump(); } diff --git a/sys/cam/ctl/ctl_frontend.c b/sys/cam/ctl/ctl_frontend.c index 7af7a71b555..ecd5cbafeda 100644 --- a/sys/cam/ctl/ctl_frontend.c +++ b/sys/cam/ctl/ctl_frontend.c @@ -157,6 +157,17 @@ ctl_port_register(struct ctl_port *port, int master_shelf) control_softc->num_ports++; mtx_unlock(&control_softc->ctl_lock); + /* + * Initialize the initiator and portname mappings + */ + port->max_initiators = CTL_MAX_INIT_PER_PORT; + port->wwpn_iid = malloc(sizeof(*port->wwpn_iid) * port->max_initiators, + M_CTL, M_NOWAIT | M_ZERO); + if (port->wwpn_iid == NULL) { + retval = ENOMEM; + goto error; + } + /* * We add 20 to whatever the caller requests, so he doesn't get * burned by queueing things back to the pending sense queue. In @@ -168,6 +179,8 @@ ctl_port_register(struct ctl_port *port, int master_shelf) retval = ctl_pool_create(control_softc, CTL_POOL_FETD, port->num_requested_ctl_io + 20, &pool); if (retval != 0) { + free(port->wwpn_iid, M_CTL); +error: port->targ_port = -1; mtx_lock(&control_softc->ctl_lock); ctl_clear_mask(&control_softc->ctl_port_mask, port_num); @@ -181,7 +194,6 @@ ctl_port_register(struct ctl_port *port, int master_shelf) mtx_lock(&control_softc->ctl_lock); port->targ_port = port_num + (master_shelf != 0 ? 0 : CTL_MAX_PORTS); - port->max_initiators = CTL_MAX_INIT_PER_PORT; STAILQ_INSERT_TAIL(&port->frontend->port_list, port, fe_links); STAILQ_INSERT_TAIL(&control_softc->port_list, port, links); control_softc->ctl_ports[port_num] = port; @@ -194,8 +206,7 @@ int ctl_port_deregister(struct ctl_port *port) { struct ctl_io_pool *pool; - int port_num; - int retval; + int port_num, retval, i; retval = 0; @@ -223,6 +234,9 @@ ctl_port_deregister(struct ctl_port *port) port->port_devid = NULL; free(port->target_devid, M_CTL); port->target_devid = NULL; + for (i = 0; i < port->max_initiators; i++) + free(port->wwpn_iid[i].name, M_CTL); + free(port->wwpn_iid, M_CTL); bailout: return (retval); diff --git a/sys/cam/ctl/ctl_frontend.h b/sys/cam/ctl/ctl_frontend.h index 2582ecca619..f252678cadf 100644 --- a/sys/cam/ctl/ctl_frontend.h +++ b/sys/cam/ctl/ctl_frontend.h @@ -80,6 +80,13 @@ typedef int (*fe_ioctl_t)(struct cdev *dev, u_long cmd, caddr_t addr, int flag, MODULE_DEPEND(name, ctl, 1, 1, 1); \ MODULE_DEPEND(name, cam, 1, 1, 1) +struct ctl_wwpn_iid { + int in_use; + time_t last_use; + uint64_t wwpn; + char *name; +}; + /* * The ctl_frontend structure is the registration mechanism between a FETD * (Front End Target Driver) and the CTL layer. Here is a description of @@ -228,6 +235,7 @@ struct ctl_port { int32_t targ_port; /* passed back to FETD */ void *ctl_pool_ref; /* passed back to FETD */ uint32_t max_initiators; /* passed back to FETD */ + struct ctl_wwpn_iid *wwpn_iid; /* used by CTL */ uint64_t wwnn; /* set by CTL before online */ uint64_t wwpn; /* set by CTL before online */ ctl_port_status status; /* used by CTL */ @@ -269,13 +277,13 @@ struct ctl_frontend * ctl_frontend_find(char *frontend_name); * This may block until resources are allocated. Called at FETD module load * time. Returns 0 for success, non-zero for failure. */ -int ctl_port_register(struct ctl_port *fe, int master_SC); +int ctl_port_register(struct ctl_port *port, int master_SC); /* * Called at FETD module unload time. * Returns 0 for success, non-zero for failure. */ -int ctl_port_deregister(struct ctl_port *fe); +int ctl_port_deregister(struct ctl_port *port); /* * Called to set the WWNN and WWPN for a particular frontend. @@ -312,21 +320,18 @@ int ctl_queue(union ctl_io *io); int ctl_queue_sense(union ctl_io *io); /* - * This routine adds an initiator to CTL's port database. The WWPN should - * be the FC WWPN, if available. The targ_port field should be the same as - * the targ_port passed back from CTL in the ctl_frontend structure above. + * This routine adds an initiator to CTL's port database. + * The iid field should be the same as the iid passed in the nexus of each + * ctl_io from this initiator. + * The WWPN should be the FC WWPN, if available. + */ +int ctl_add_initiator(struct ctl_port *port, int iid, uint64_t wwpn, char *name); + +/* + * This routine will remove an initiator from CTL's port database. * The iid field should be the same as the iid passed in the nexus of each * ctl_io from this initiator. */ -int ctl_add_initiator(uint64_t wwpn, int32_t targ_port, uint32_t iid); - -/* - * This routine will remove an initiator from CTL's port database. The - * targ_port field should be the same as the targ_port passed back in the - * ctl_frontend structure above. The iid field should be the same as the - * iid passed in the nexus of each ctl_io from this initiator. - */ -int -ctl_remove_initiator(int32_t targ_port, uint32_t iid); +int ctl_remove_initiator(struct ctl_port *port, int iid); #endif /* _CTL_FRONTEND_H_ */ diff --git a/sys/cam/ctl/ctl_frontend_iscsi.c b/sys/cam/ctl/ctl_frontend_iscsi.c index caf3af11b4f..11030f46f71 100644 --- a/sys/cam/ctl/ctl_frontend_iscsi.c +++ b/sys/cam/ctl/ctl_frontend_iscsi.c @@ -1146,41 +1146,25 @@ cfiscsi_session_terminate(struct cfiscsi_session *cs) static int cfiscsi_session_register_initiator(struct cfiscsi_session *cs) { - int error, i; - struct cfiscsi_softc *softc; + struct cfiscsi_target *ct; + char *name; + int i; KASSERT(cs->cs_ctl_initid == -1, ("already registered")); - softc = &cfiscsi_softc; - - mtx_lock(&softc->lock); - for (i = 0; i < softc->max_initiators; i++) { - if (softc->ctl_initids[i] == 0) - break; - } - if (i == softc->max_initiators) { - CFISCSI_SESSION_WARN(cs, "too many concurrent sessions (%d)", - softc->max_initiators); - mtx_unlock(&softc->lock); - return (1); - } - softc->ctl_initids[i] = 1; - mtx_unlock(&softc->lock); - -#if 0 - CFISCSI_SESSION_DEBUG(cs, "adding initiator id %d, max %d", - i, softc->max_initiators); -#endif - cs->cs_ctl_initid = i; - error = ctl_add_initiator(0x0, cs->cs_target->ct_port.targ_port, cs->cs_ctl_initid); - if (error != 0) { - CFISCSI_SESSION_WARN(cs, "ctl_add_initiator failed with error %d", error); - mtx_lock(&softc->lock); - softc->ctl_initids[cs->cs_ctl_initid] = 0; - mtx_unlock(&softc->lock); + ct = cs->cs_target; + name = strdup(cs->cs_initiator_id, M_CTL); + i = ctl_add_initiator(&ct->ct_port, -1, 0, name); + if (i < 0) { + CFISCSI_SESSION_WARN(cs, "ctl_add_initiator failed with error %d", + i); cs->cs_ctl_initid = -1; return (1); } + cs->cs_ctl_initid = i; +#if 0 + CFISCSI_SESSION_DEBUG(cs, "added initiator id %d", i); +#endif return (0); } @@ -1189,21 +1173,15 @@ static void cfiscsi_session_unregister_initiator(struct cfiscsi_session *cs) { int error; - struct cfiscsi_softc *softc; if (cs->cs_ctl_initid == -1) return; - softc = &cfiscsi_softc; - - error = ctl_remove_initiator(cs->cs_target->ct_port.targ_port, cs->cs_ctl_initid); + error = ctl_remove_initiator(&cs->cs_target->ct_port, cs->cs_ctl_initid); if (error != 0) { CFISCSI_SESSION_WARN(cs, "ctl_remove_initiator failed with error %d", error); } - mtx_lock(&softc->lock); - softc->ctl_initids[cs->cs_ctl_initid] = 0; - mtx_unlock(&softc->lock); cs->cs_ctl_initid = -1; } @@ -1300,8 +1278,6 @@ cfiscsi_init(void) TAILQ_INIT(&softc->sessions); TAILQ_INIT(&softc->targets); - softc->max_initiators = CTL_MAX_INIT_PER_PORT; - cfiscsi_data_wait_zone = uma_zcreate("cfiscsi_data_wait", sizeof(struct cfiscsi_data_wait), NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, 0); diff --git a/sys/cam/ctl/ctl_frontend_iscsi.h b/sys/cam/ctl/ctl_frontend_iscsi.h index 76d8254d7b0..0ac0e98ce46 100644 --- a/sys/cam/ctl/ctl_frontend_iscsi.h +++ b/sys/cam/ctl/ctl_frontend_iscsi.h @@ -112,8 +112,6 @@ struct cfiscsi_softc { unsigned int last_session_id; TAILQ_HEAD(, cfiscsi_target) targets; TAILQ_HEAD(, cfiscsi_session) sessions; - char ctl_initids[CTL_MAX_INIT_PER_PORT]; - int max_initiators; #ifdef ICL_KERNEL_PROXY struct icl_listen *listener; struct cv accept_cv; diff --git a/sys/cam/ctl/ctl_private.h b/sys/cam/ctl/ctl_private.h index e783d02a0c7..9f43b1e0af3 100644 --- a/sys/cam/ctl/ctl_private.h +++ b/sys/cam/ctl/ctl_private.h @@ -414,13 +414,6 @@ typedef enum { CTL_FLAG_MASTER_SHELF = 0x04 } ctl_gen_flags; -struct ctl_wwpn_iid { - int in_use; - uint64_t wwpn; - uint32_t iid; - int32_t port; -}; - #define CTL_MAX_THREADS 16 struct ctl_thread { @@ -453,7 +446,6 @@ struct ctl_softc { int targ_online; uint32_t ctl_lun_mask[CTL_MAX_LUNS >> 5]; struct ctl_lun *ctl_luns[CTL_MAX_LUNS]; - struct ctl_wwpn_iid wwpn_iid[CTL_MAX_PORTS][CTL_MAX_INIT_PER_PORT]; uint32_t ctl_port_mask; uint64_t aps_locked_lun; STAILQ_HEAD(, ctl_lun) lun_list; diff --git a/sys/cam/ctl/scsi_ctl.c b/sys/cam/ctl/scsi_ctl.c index 8d38b024c09..85a34bbc070 100644 --- a/sys/cam/ctl/scsi_ctl.c +++ b/sys/cam/ctl/scsi_ctl.c @@ -484,14 +484,14 @@ ctlfeasync(void *callback_arg, uint32_t code, struct cam_path *path, void *arg) break; } if (dev_chg->arrived != 0) { - retval = ctl_add_initiator(dev_chg->wwpn, - softc->port.targ_port, dev_chg->target); + retval = ctl_add_initiator(&softc->port, + dev_chg->target, dev_chg->wwpn, NULL); } else { - retval = ctl_remove_initiator( - softc->port.targ_port, dev_chg->target); + retval = ctl_remove_initiator(&softc->port, + dev_chg->target); } - if (retval != 0) { + if (retval < 0) { printf("%s: could not %s port %d iid %u " "WWPN %#jx!\n", __func__, (dev_chg->arrived != 0) ? "add" :