Ronnie, Currently there is NOP-IN generation in iser LLD. Is there any chance we produce a common code for that? Or if we keep them separate at least organize the flow similarly so that one day it could be merged? On Wed, Jun 19, 2013 at 5:49 AM, ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote: > Please ignore this patch for now. > I need to check and work some more on it. > > I will send a new patch in a few days or so. > > > On Mon, Jun 17, 2013 at 8:53 PM, Ronnie Sahlberg > <ronniesahlberg@xxxxxxxxx> wrote: >> Add support for setting NOP-OUT probes to detect dead initiators. >> We can set the interval(how frequently to send the probes) >> and the count(after how many failures will we tear down the connection) >> globally from the tgtd command line. >> >> Optionally, using tgtadm, it is possible to configure the interval/count >> for individual targets. >> >> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx> >> --- >> doc/tgtadm.8.xml | 42 +++++++++++++ >> doc/tgtd.8.xml | 30 +++++++++ >> usr/iscsi/iscsi_tcp.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++- >> usr/iscsi/iscsid.c | 49 +++++++++++++--- >> usr/iscsi/iscsid.h | 10 +++ >> usr/iscsi/target.c | 14 ++++ >> usr/iscsi/transport.h | 1 + >> usr/mgmt.c | 3 +- >> usr/target.c | 4 + >> usr/tgtadm.c | 7 +- >> 10 files changed, 307 insertions(+), 14 deletions(-) >> >> diff --git a/doc/tgtadm.8.xml b/doc/tgtadm.8.xml >> index c17f823..680e177 100644 >> --- a/doc/tgtadm.8.xml >> +++ b/doc/tgtadm.8.xml >> @@ -663,6 +663,48 @@ Account information: >> </refsect1> >> >> >> + <refsect1><title>NOP-OUT Probes</title> >> + <para> >> + TGTD can send NOP-OUT probes to connected initiators to determine when >> + an initiator is dead and then automatically clear and tear down the >> + TCP connection. This can either be set as a global default from the >> + tgtd command-line or it can be set for individual targets using the >> + tgtadm command. >> + </para> >> + <refsect2><title>Check the current NOP-OUT setting</title> >> + <para> >> + The tgtadm command is used to view the current setting for if/when >> + to send NOP-OUT probes to connected initiators. >> + </para> >> + <para> >> + If the target is configured to send NOP-OUT probes this will show up >> + as two parameter lines in the target printout. If the target is not >> + configured to send NOP-OUT these lines will not be printed at all. >> + </para> >> + <screen format="linespecific"> >> +tgtadm --lld iscsi --op show --mode target >> + >> +Target 1: iqn.ronnie.test >> + System information: >> + Driver: iscsi >> + State: ready >> + Nop interval: 5 >> + Nop count: 5 >> + I_T nexus information: >> + </screen> >> + </refsect2> >> + <refsect2><title>Setting NOP-OUT for a target</title> >> + <para> >> + The tgtadm command is used to change the NOP-OUT settings. >> + </para> >> + <screen format="linespecific"> >> +tgtadm --op update --mode target --tid 1 -n nop_count -v 5 >> +tgtadm --op update --mode target --tid 1 -n nop_interval -v 5 >> + </screen> >> + </refsect2> >> + </refsect1> >> + >> + >> <refsect1><title>iSCSI PORTALS</title> >> <para> >> iSCSI portals can be viewed, added and removed at runtime. >> diff --git a/doc/tgtd.8.xml b/doc/tgtd.8.xml >> index ce734ce..a0020c6 100644 >> --- a/doc/tgtd.8.xml >> +++ b/doc/tgtd.8.xml >> @@ -129,6 +129,36 @@ >> </screen> >> </para> >> </refsect2> >> + <refsect2><title>nop_interval=<integer></title> >> + <para> >> + This sets the default interval for sending NOP-OUT to probe for >> + connected initiators. >> + This parameter only controls the default value for targets. >> + Individual targets can be controlled using tgtadm. >> + </para> >> + <para> >> + The default value is 0 which means that the feature is disabled >> + TGTD will not send any NOP-OUT probes. >> + </para> >> + </refsect2> >> + <refsect2><title>nop_count=<integer></title> >> + <para> >> + This sets the default value for after how many failed probes TGTD >> + will consider the initiator dead and tear down the session. >> + This parameter only controls the default value for targets. >> + Individual targets can be controlled using tgtadm. >> + </para> >> + <para> >> + The default value is 0. >> + </para> >> + <para> >> + Example: send NOP-OUT every 5 seconds and abort the session after >> + 6 failures. >> + <screen format="linespecific"> >> + tgtd --iscsi portal=192.0.2.1:3260,nop_interval=5,nop_count=6 >> + </screen> >> + </para> >> + </refsect2> >> </refsect1> >> >> >> diff --git a/usr/iscsi/iscsi_tcp.c b/usr/iscsi/iscsi_tcp.c >> index 0c43039..b80e108 100644 >> --- a/usr/iscsi/iscsi_tcp.c >> +++ b/usr/iscsi/iscsi_tcp.c >> @@ -35,15 +35,27 @@ >> #include "iscsid.h" >> #include "tgtd.h" >> #include "util.h" >> +#include "work.h" >> >> static void iscsi_tcp_event_handler(int fd, int events, void *data); >> +static struct iscsi_task *iscsi_tcp_alloc_task(struct iscsi_connection *conn, >> + size_t ext_len); >> +static void iscsi_tcp_free_task(struct iscsi_task *task); >> +static void iscsi_tcp_release(struct iscsi_connection *conn); >> >> static int listen_fds[8]; >> static struct iscsi_transport iscsi_tcp; >> +static long nop_ttt; >> >> struct iscsi_tcp_connection { >> - int fd; >> + struct list_head tcp_conn_siblings; >> + int nop_inflight_count; >> + int nop_interval; >> + int nop_tick; >> + int nop_count; >> >> + int fd; >> + long ttt; >> struct iscsi_connection iscsi_conn; >> }; >> >> @@ -52,6 +64,74 @@ static inline struct iscsi_tcp_connection *TCP_CONN(struct iscsi_connection *con >> return container_of(conn, struct iscsi_tcp_connection, iscsi_conn); >> } >> >> +/* all iscsi connections */ >> +static struct list_head iscsi_tcp_conn_list; >> + >> +static struct tgt_work nop_work; >> + >> +static int iscsi_send_ping_nop_in(struct iscsi_tcp_connection *tcp_conn) >> +{ >> + struct iscsi_connection *conn = &tcp_conn->iscsi_conn; >> + struct iscsi_task *task = NULL; >> + >> + task = iscsi_tcp_alloc_task(&tcp_conn->iscsi_conn, 0); >> + task->conn = conn; >> + >> + task->tag = ISCSI_RESERVED_TAG; >> + task->req.opcode = ISCSI_OP_NOOP_IN; >> + task->req.itt = cpu_to_be32(ISCSI_RESERVED_TAG); >> + task->req.ttt = cpu_to_be32(tcp_conn->ttt); >> + >> + list_add_tail(&task->c_list, &task->conn->tx_clist); >> + task->conn->tp->ep_event_modify(task->conn, EPOLLIN | EPOLLOUT); >> + >> + return 0; >> +} >> + >> +static void iscsi_tcp_nop_work_handler(void *data) >> +{ >> + struct iscsi_tcp_connection *tcp_conn; >> + >> + list_for_each_entry(tcp_conn, &iscsi_tcp_conn_list, tcp_conn_siblings) { >> + tcp_conn->nop_inflight_count++; >> + if (tcp_conn->nop_interval == 0) >> + continue; >> + >> + tcp_conn->nop_tick--; >> + if (tcp_conn->nop_tick > 0) >> + continue; >> + >> + tcp_conn->nop_tick = tcp_conn->nop_interval; >> + >> + if (tcp_conn->nop_inflight_count > tcp_conn->nop_count) { >> + eprintf("tcp connection timed out after %d failed " \ >> + "NOP-OUT\n", tcp_conn->nop_count); >> + iscsi_tcp_release(&tcp_conn->iscsi_conn); >> + /* cant/shouldnt delete tcp_conn from within the loop */ >> + break; >> + } >> + nop_ttt++; >> + if (nop_ttt == ISCSI_RESERVED_TAG) >> + nop_ttt = 1; >> + >> + tcp_conn->ttt = nop_ttt; >> + iscsi_send_ping_nop_in(tcp_conn); >> + } >> + >> + add_work(&nop_work, 1); >> +} >> + >> +static void iscsi_tcp_nop_reply(long ttt) >> +{ >> + struct iscsi_tcp_connection *tcp_conn; >> + >> + list_for_each_entry(tcp_conn, &iscsi_tcp_conn_list, tcp_conn_siblings) { >> + if (tcp_conn->ttt != ttt) >> + continue; >> + tcp_conn->nop_inflight_count = 0; >> + } >> +} >> + >> static int set_keepalive(int fd) >> { >> int ret, opt; >> @@ -144,6 +224,8 @@ static void accept_connection(int afd, int events, void *data) >> goto out; >> } >> >> + list_add(&tcp_conn->tcp_conn_siblings, &iscsi_tcp_conn_list); >> + >> return; >> out: >> close(fd); >> @@ -279,6 +361,42 @@ int iscsi_add_portal(char *addr, int port, int tpgt) >> return 0; >> }; >> >> +void iscsi_set_nop_interval(int interval) >> +{ >> + default_nop_interval = interval; >> +} >> + >> +void iscsi_set_nop_count(int count) >> +{ >> + default_nop_count = count; >> +} >> + >> +int iscsi_update_target_nop_count(int tid, int count) >> +{ >> + struct iscsi_target *target; >> + >> + list_for_each_entry(target, &iscsi_targets_list, tlist) { >> + if (target->tid != tid) >> + continue; >> + target->nop_count = count; >> + return 0; >> + } >> + return -1; >> +} >> + >> +int iscsi_update_target_nop_interval(int tid, int interval) >> +{ >> + struct iscsi_target *target; >> + >> + list_for_each_entry(target, &iscsi_targets_list, tlist) { >> + if (target->tid != tid) >> + continue; >> + target->nop_interval = interval; >> + return 0; >> + } >> + return -1; >> +} >> + >> int iscsi_delete_portal(char *addr, int port) >> { >> struct iscsi_portal *portal; >> @@ -313,6 +431,12 @@ static int iscsi_tcp_init(void) >> iscsi_add_portal(NULL, 3260, 1); >> } >> >> + nop_work.func = iscsi_tcp_nop_work_handler; >> + nop_work.data = &nop_work; >> + add_work(&nop_work, 1); >> + >> + INIT_LIST_HEAD(&iscsi_tcp_conn_list); >> + >> return 0; >> } >> >> @@ -326,8 +450,41 @@ static void iscsi_tcp_exit(void) >> } >> } >> >> +void iscsi_print_nop_settings(struct concat_buf *b, int tid) >> +{ >> + struct iscsi_target *target; >> + >> + list_for_each_entry(target, &iscsi_targets_list, tlist) { >> + if (target->tid != tid) >> + continue; >> + if (target->nop_interval == 0) >> + continue; >> + >> + concat_printf(b, >> + _TAB2 "Nop interval: %d\n" >> + _TAB2 "Nop count: %d\n", >> + target->nop_interval, >> + target->nop_count); >> + break; >> + } >> +} >> + >> static int iscsi_tcp_conn_login_complete(struct iscsi_connection *conn) >> { >> + struct iscsi_tcp_connection *tcp_conn = >> + (struct iscsi_tcp_connection *)conn->tp; >> + struct iscsi_target *target; >> + >> + list_for_each_entry(target, &iscsi_targets_list, tlist) { >> + if (target->tid != conn->tid) >> + continue; >> + >> + tcp_conn->nop_count = target->nop_count; >> + tcp_conn->nop_interval = target->nop_interval; >> + tcp_conn->nop_tick = target->nop_interval; >> + break; >> + } >> + >> return 0; >> } >> >> @@ -370,6 +527,7 @@ static void iscsi_tcp_release(struct iscsi_connection *conn) >> >> conn_exit(conn); >> close(tcp_conn->fd); >> + list_del(&tcp_conn->tcp_conn_siblings); >> free(tcp_conn); >> } >> >> @@ -480,6 +638,7 @@ static struct iscsi_transport iscsi_tcp = { >> .free_data_buf = iscsi_tcp_free_data_buf, >> .ep_getsockname = iscsi_tcp_getsockname, >> .ep_getpeername = iscsi_tcp_getpeername, >> + .ep_nop_reply = iscsi_tcp_nop_reply, >> }; >> >> __attribute__((constructor)) static void iscsi_transport_init(void) >> diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c >> index 260989f..3757938 100644 >> --- a/usr/iscsi/iscsid.c >> +++ b/usr/iscsi/iscsid.c >> @@ -43,6 +43,9 @@ >> >> #define MAX_QUEUE_CMD 128 >> >> +int default_nop_interval; >> +int default_nop_count; >> + >> LIST_HEAD(iscsi_portals_list); >> >> char *portal_arguments; >> @@ -1674,13 +1677,9 @@ static int iscsi_noop_out_rx_start(struct iscsi_connection *conn) >> >> dprintf("%x %x %u\n", req->ttt, req->itt, ntoh24(req->dlength)); >> if (req->ttt != cpu_to_be32(ISCSI_RESERVED_TAG)) { >> - /* >> - * We don't request a NOP-Out by sending a NOP-In. >> - * See 10.18.2 in the draft 20. >> - */ >> - eprintf("initiator bug\n"); >> - err = -ISCSI_REASON_PROTOCOL_ERROR; >> - goto out; >> + if ((req->opcode & ISCSI_OPCODE_MASK) == ISCSI_OP_NOOP_OUT) { >> + goto good; >> + } >> } >> >> if (req->itt == cpu_to_be32(ISCSI_RESERVED_TAG)) { >> @@ -1691,6 +1690,7 @@ static int iscsi_noop_out_rx_start(struct iscsi_connection *conn) >> } >> } >> >> +good: >> conn->exp_stat_sn = be32_to_cpu(req->exp_statsn); >> >> len = ntoh24(req->dlength); >> @@ -1719,8 +1719,8 @@ static int iscsi_task_rx_done(struct iscsi_connection *conn) >> >> op = hdr->opcode & ISCSI_OPCODE_MASK; >> switch (op) { >> - case ISCSI_OP_SCSI_CMD: >> case ISCSI_OP_NOOP_OUT: >> + case ISCSI_OP_SCSI_CMD: >> case ISCSI_OP_SCSI_TMFUNC: >> case ISCSI_OP_LOGOUT: >> err = iscsi_task_queue(task); >> @@ -1836,6 +1836,28 @@ static int iscsi_logout_tx_start(struct iscsi_task *task) >> return 0; >> } >> >> +static int iscsi_noop_in_tx_start(struct iscsi_task *task) >> +{ >> + struct iscsi_connection *conn = task->conn; >> + struct iscsi_data_rsp *rsp = (struct iscsi_data_rsp *) &conn->rsp.bhs; >> + >> + memset(rsp, 0, sizeof(*rsp)); >> + rsp->opcode = ISCSI_OP_NOOP_IN; >> + rsp->flags = ISCSI_FLAG_CMD_FINAL; >> + rsp->itt = task->req.itt; >> + rsp->ttt = task->req.ttt; >> + rsp->statsn = cpu_to_be32(conn->stat_sn); >> + rsp->exp_cmdsn = cpu_to_be32(conn->session->exp_cmd_sn); >> + rsp->max_cmdsn = cpu_to_be32(conn->session->exp_cmd_sn + MAX_QUEUE_CMD); >> + >> + /* TODO: honor max_burst */ >> + conn->rsp.datasize = task->len; >> + hton24(rsp->dlength, task->len); >> + conn->rsp.data = task->data; >> + >> + return 0; >> +} >> + >> static int iscsi_noop_out_tx_start(struct iscsi_task *task, int *is_rsp) >> { >> struct iscsi_connection *conn = task->conn; >> @@ -1843,6 +1865,10 @@ static int iscsi_noop_out_tx_start(struct iscsi_task *task, int *is_rsp) >> >> if (task->req.itt == cpu_to_be32(ISCSI_RESERVED_TAG)) { >> *is_rsp = 0; >> + >> + if (conn->tp->ep_nop_reply) >> + conn->tp->ep_nop_reply(be32_to_cpu(task->req.ttt)); >> + >> iscsi_free_task(task); >> } else { >> *is_rsp = 1; >> @@ -1955,6 +1981,9 @@ static int iscsi_task_tx_start(struct iscsi_connection *conn) >> case ISCSI_OP_SCSI_CMD: >> err = iscsi_scsi_cmd_tx_start(task); >> break; >> + case ISCSI_OP_NOOP_IN: >> + err = iscsi_noop_in_tx_start(task); >> + break; >> case ISCSI_OP_NOOP_OUT: >> err = iscsi_noop_out_tx_start(task, &is_rsp); >> if (!is_rsp) >> @@ -2450,6 +2479,10 @@ int iscsi_param_parse_portals(char *p, int do_add, >> return -1; >> } >> } >> + } else if (!strncmp(p, "nop_interval", 12)) { >> + iscsi_set_nop_interval(atoi(p+13)); >> + } else if (!strncmp(p, "nop_count", 9)) { >> + iscsi_set_nop_count(atoi(p+10)); >> } >> >> p += strcspn(p, ","); >> diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h >> index e7e08ae..ae878a5 100644 >> --- a/usr/iscsi/iscsid.h >> +++ b/usr/iscsi/iscsid.h >> @@ -239,6 +239,9 @@ struct iscsi_connection { >> >> #define INCOMING_BUFSIZE 8192 >> >> +extern int default_nop_interval; >> +extern int default_nop_count; >> + >> struct iscsi_target { >> struct list_head tlist; >> >> @@ -262,6 +265,8 @@ struct iscsi_target { >> struct list_head isns_list; >> >> int rdma; >> + int nop_interval; >> + int nop_count; >> }; >> >> enum task_flags { >> @@ -316,6 +321,8 @@ extern int iscsi_scsi_cmd_execute(struct iscsi_task *task); >> extern int iscsi_transportid(int tid, uint64_t itn_id, char *buf, int size); >> extern int iscsi_add_portal(char *addr, int port, int tpgt); >> extern int iscsi_delete_portal(char *addr, int port); >> +extern void iscsi_set_nop_interval(int interval); >> +extern void iscsi_set_nop_count(int count); >> extern int iscsi_param_parse_portals(char *p, int do_add, int do_delete); >> extern void iscsi_update_conn_stats_rx(struct iscsi_connection *conn, int size, int opcode); >> extern void iscsi_update_conn_stats_tx(struct iscsi_connection *conn, int size, int opcode); >> @@ -342,6 +349,9 @@ extern int iscsi_target_create(struct target *); >> extern void iscsi_target_destroy(int tid, int force); >> extern tgtadm_err iscsi_target_show(int mode, int tid, uint64_t sid, uint32_t cid, >> uint64_t lun, struct concat_buf *b); >> +extern void iscsi_print_nop_settings(struct concat_buf *b, int tid); >> +extern int iscsi_update_target_nop_count(int tid, int count); >> +extern int iscsi_update_target_nop_interval(int tid, int interval); >> extern tgtadm_err iscsi_stat(int mode, int tid, uint64_t sid, uint32_t cid, >> uint64_t lun, struct concat_buf *b); >> extern tgtadm_err iscsi_target_update(int mode, int op, int tid, uint64_t sid, uint64_t lun, >> diff --git a/usr/iscsi/target.c b/usr/iscsi/target.c >> index 916c84a..20fb1da 100644 >> --- a/usr/iscsi/target.c >> +++ b/usr/iscsi/target.c >> @@ -470,6 +470,8 @@ int iscsi_target_create(struct target *t) >> INIT_LIST_HEAD(&target->sessions_list); >> INIT_LIST_HEAD(&target->isns_list); >> target->tid = tid; >> + target->nop_interval = default_nop_interval; >> + target->nop_count = default_nop_count; >> list_add_tail(&target->tlist, &iscsi_targets_list); >> >> isns_target_register(tgt_targetname(tid)); >> @@ -545,6 +547,18 @@ tgtadm_err iscsi_target_update(int mode, int op, int tid, uint64_t sid, uint64_t >> break; >> } >> adm_err = TGTADM_SUCCESS; >> + } else if (!strncmp(name, "nop_count", 9)) { >> + err = iscsi_update_target_nop_count(tid, >> + atoi(&name[10])); >> + adm_err = !err ? TGTADM_SUCCESS : >> + TGTADM_INVALID_REQUEST; >> + break; >> + } else if (!strncmp(name, "nop_interval", 12)) { >> + err = iscsi_update_target_nop_interval(tid, >> + atoi(&name[13])); >> + adm_err = !err ? TGTADM_SUCCESS : >> + TGTADM_INVALID_REQUEST; >> + break; >> } >> >> idx = param_index_by_name(name, session_keys); >> diff --git a/usr/iscsi/transport.h b/usr/iscsi/transport.h >> index af4af21..f4300c1 100644 >> --- a/usr/iscsi/transport.h >> +++ b/usr/iscsi/transport.h >> @@ -39,6 +39,7 @@ struct iscsi_transport { >> struct sockaddr *sa, socklen_t *len); >> int (*ep_getpeername)(struct iscsi_connection *conn, >> struct sockaddr *sa, socklen_t *len); >> + void (*ep_nop_reply) (long ttt); >> }; >> >> extern int iscsi_transport_register(struct iscsi_transport *); >> diff --git a/usr/mgmt.c b/usr/mgmt.c >> index c0bf7be..d108e33 100644 >> --- a/usr/mgmt.c >> +++ b/usr/mgmt.c >> @@ -171,10 +171,11 @@ static tgtadm_err target_mgmt(int lld_no, struct mgmt_task *mtask) >> >> if (!strcmp(mtask->req_buf, "state")) { >> adm_err = tgt_set_target_state(req->tid, p); >> - } else if (tgt_drivers[lld_no]->update) >> + } else if (tgt_drivers[lld_no]->update) { >> adm_err = tgt_drivers[lld_no]->update(req->mode, req->op, req->tid, >> req->sid, req->lun, >> req->cid, mtask->req_buf); >> + } >> break; >> } >> case OP_SHOW: >> diff --git a/usr/target.c b/usr/target.c >> index 9a8f2fd..23b5766 100644 >> --- a/usr/target.c >> +++ b/usr/target.c >> @@ -36,6 +36,7 @@ >> #include "driver.h" >> #include "target.h" >> #include "scsi.h" >> +#include "iscsi/iscsid.h" >> #include "tgtadm.h" >> #include "parser.h" >> #include "spc.h" >> @@ -2000,6 +2001,9 @@ tgtadm_err tgt_target_show_all(struct concat_buf *b) >> tgt_drivers[target->lid]->name, >> target_state_name(target->target_state)); >> >> + if (!strcmp(tgt_drivers[target->lid]->name, "iscsi")) >> + iscsi_print_nop_settings(b, target->tid); >> + >> concat_printf(b, _TAB1 "I_T nexus information:\n"); >> >> list_for_each_entry(nexus, &target->it_nexus_list, >> diff --git a/usr/tgtadm.c b/usr/tgtadm.c >> index 90803f2..9090069 100644 >> --- a/usr/tgtadm.c >> +++ b/usr/tgtadm.c >> @@ -707,14 +707,13 @@ int main(int argc, char **argv) >> case OP_UPDATE: >> rc = verify_mode_params(argc, argv, "LmotnvC"); >> if (rc) { >> - eprintf("target mode: option '-%c' is not " >> + eprintf("target mode: option '-%c' is not " \ >> "allowed/supported\n", rc); >> exit(EINVAL); >> } >> if ((!name || !value)) { >> - eprintf("update operation requires 'name'" >> - " and 'value' options\n"); >> - exit(EINVAL); >> + eprintf("update operation requires 'name'" \ >> + " and 'value' options\n"); >> } >> break; >> default: >> -- >> 1.7.3.1 >> > -- > To unsubscribe from this list: send the line "unsubscribe stgt" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe stgt" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html