Skip to contentSkip to conversation

Open-iSCSI High-Performance Initiator for Linux

PATCH In review Alex Aizman 2 participants 2 messages Mar 7, 2005 – Mar 9, 2005

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

PATCH 1/6 Open-iSCSI High-Performance Initiator for Linux

SCSI LLDD consists of 3 files:
- iscsi_if.c (iSCSI open interface over netlink);
- iscsi_tcp.[ch] (iSCSI transport over TCP/IP).

drivers/scsi/iscsi_if.c +257 -0
diff -Nru --exclude Kconfig --exclude Makefile linux-2.6.11.orig/drivers/scsi/iscsi_if.c linux-2.6.11.dima/drivers/scsi/iscsi_if.c
@@ -0,0 +1,257 @@
+/*
+ * iSCSI Initiator Kernel/User Interface
+ *
+ * Copyright (C) 2004 Dmitry Yusupov, Alex Aizman
+ * maintained by [email protected]
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that 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.
+ *
+ * See the file COPYING included with this distribution for more details.
+ */
+
+#include <linux/module.h>
+#include <net/tcp.h>
+#include <scsi/iscsi_if.h>
+#include <scsi/iscsi_ifev.h>
+
+MODULE_AUTHOR("Dmitry Yusupov <[email protected]>, "
+	      "Alex Aizman <[email protected]>");
+MODULE_DESCRIPTION("iSCSI Open Interface");
+MODULE_LICENSE("GPL");
+
+static struct iscsi_transport *transport_table[ISCSI_TRANSPORT_MAX];
+static struct sock *nls;
+static int daemon_pid;
+
+int
+iscsi_control_recv_pdu(iscsi_cnx_h cp_cnx, struct iscsi_hdr *hdr,
+				char *data, uint32_t data_size)
+{
+	struct nlmsghdr	*nlh;
+	struct sk_buff	*skb;
+	struct iscsi_uevent *ev;
+	char *pdu;
+	int len = NLMSG_SPACE(sizeof(*ev) + sizeof(struct iscsi_hdr) +
+			      data_size);
+
+	skb = alloc_skb(len, gfp_any());
+	if (!skb) {
+		return -ENOMEM;
+	}
+
+	nlh = __nlmsg_put(skb, daemon_pid, 0, 0, (len - sizeof(*nlh)));
+	ev = NLMSG_DATA(nlh);
+	memset(ev, 0, sizeof(*ev));
+	ev->transport_id = 0;
+	ev->type = ISCSI_KEVENT_RECV_PDU;
+	ev->r.recv_req.cnx_handle = cp_cnx;
+	pdu = (char*)ev + sizeof(*ev);
+	memcpy(pdu, hdr, sizeof(struct iscsi_hdr));
+	memcpy(pdu + sizeof(struct iscsi_hdr), data, data_size);
+	skb_get(skb);
+	netlink_unicast(nls, skb, daemon_pid, MSG_DONTWAIT);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(iscsi_control_recv_pdu);
+
+void
+iscsi_control_cnx_error(iscsi_cnx_h cp_cnx, iscsi_err_e error)
+{
+	struct nlmsghdr	*nlh;
+	struct sk_buff	*skb;
+	struct iscsi_uevent *ev;
+	int len = NLMSG_SPACE(sizeof(*ev));
+
+	skb = alloc_skb(len, gfp_any());
+	if (!skb) {
+		return;
+	}
+
+	nlh = __nlmsg_put(skb, daemon_pid, 0, 0, (len - sizeof(*nlh)));
+	ev = NLMSG_DATA(nlh);
+	ev->transport_id = 0;
+	ev->type = ISCSI_KEVENT_CNX_ERROR;
+	ev->r.cnxerror.error = error;
+	ev->r.cnxerror.cnx_handle = cp_cnx;
+	skb_get(skb);
+	netlink_unicast(nls, skb, daemon_pid, MSG_DONTWAIT);
+}
+EXPORT_SYMBOL_GPL(iscsi_control_cnx_error);
+
+static struct iscsi_transport*
+iscsi_if_transport_lookup(int id)
+{
+	/* FIXME: implement transport's container */
+	if (id != 0)
+		return NULL;
+	return transport_table[id];
+}
+
+static int
+iscsi_if_send_reply(int pid, int seq, int type, int done, int multi,
+		      void *payload, int size)
+{
+	struct sk_buff	*skb;
+	struct nlmsghdr	*nlh;
+	int len = NLMSG_SPACE(size);
+	int flags = multi ? NLM_F_MULTI : 0;
+	int t = done ? NLMSG_DONE  : type;
+
+	skb = alloc_skb(len, GFP_KERNEL);
+	if (!skb) {
+		return -ENOMEM;
+	}
+
+	nlh = __nlmsg_put(skb, pid, seq, t, (len - sizeof(*nlh)));
+	nlh->nlmsg_flags = flags;
+	memcpy(NLMSG_DATA(nlh), payload, size);
+	netlink_unicast(nls, skb, pid, MSG_DONTWAIT);
+	return 0;
+}
+
+static int
+iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
+{
+	int err = 0;
+	struct iscsi_transport *transport;
+	struct iscsi_uevent *ev = NLMSG_DATA(nlh);
+
+	if ((transport = iscsi_if_transport_lookup(ev->transport_id)) == NULL)
+		return -EEXIST;
+
+	daemon_pid = NETLINK_CREDS(skb)->pid;
+
+	switch (nlh->nlmsg_type) {
+	case ISCSI_UEVENT_CREATE_SESSION:
+		ev->r.c_session_ret.handle = transport->create_session(
+		       ev->u.c_session.session_handle,
+		       ev->u.c_session.initial_cmdsn, &ev->r.c_session_ret.sid);
+		break;
+	case ISCSI_UEVENT_DESTROY_SESSION:
+		transport->destroy_session(
+			ev->u.d_session.session_handle);
+		break;
+	case ISCSI_UEVENT_CREATE_CNX:
+		ev->r.handle = transport->create_cnx(
+			ev->u.c_cnx.session_handle,
+			ev->u.c_cnx.cnx_handle,
+			ev->u.c_cnx.cid);
+		break;
+	case ISCSI_UEVENT_DESTROY_CNX:
+		transport->destroy_cnx(ev->u.d_cnx.cnx_handle);
+		break;
+	case ISCSI_UEVENT_BIND_CNX:
+		ev->r.retcode = transport->bind_cnx(
+			ev->u.b_cnx.session_handle,
+			ev->u.b_cnx.cnx_handle,
+			ev->u.b_cnx.transport_fd,
+			ev->u.b_cnx.is_leading);
+		break;
+	case ISCSI_UEVENT_SET_PARAM:
+		ev->r.retcode = transport->set_param(
+			ev->u.set_param.cnx_handle,
+			ev->u.set_param.param, ev->u.set_param.value);
+		break;
+	case ISCSI_UEVENT_START_CNX:
+		ev->r.retcode = transport->start_cnx(
+			ev->u.start_cnx.cnx_handle);
+		break;
+	case ISCSI_UEVENT_STOP_CNX:
+		transport->stop_cnx(
+			ev->u.stop_cnx.cnx_handle);
+		break;
+	case ISCSI_UEVENT_SEND_PDU:
+		ev->r.retcode = transport->send_pdu(
+		       ev->u.send_pdu.cnx_handle,
+		       (struct iscsi_hdr*)((char*)ev + sizeof(*ev)),
+		       (char*)ev + sizeof(*ev) + ev->u.send_pdu.hdr_size,
+			ev->u.send_pdu.data_size);
+		break;
+	default:
+		err = -EINVAL;
+		break;
+	}
+
+	return err;
+}
+
+/* Get message from skb (based on rtnetlink_rcv_skb).  Each message is
+ * processed by iscsi_if_recv_msg.  Malformed skbs with wrong length are
+ * discarded silently.  */
+static void
+iscsi_if_rx(struct sock *sk, int len)
+{
+	struct sk_buff *skb;
+	while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
+		while (skb->len >= NLMSG_SPACE(0)) {
+			int err;
+			uint32_t rlen;
+			struct nlmsghdr	*nlh;
+
+			nlh = (struct nlmsghdr *)skb->data;
+			if (nlh->nlmsg_len < sizeof(*nlh) ||
+			    skb->len < nlh->nlmsg_len) {
+				break;
+			}
+			rlen = NLMSG_ALIGN(nlh->nlmsg_len);
+			if (rlen > skb->len)
+				rlen = skb->len;
+			err = iscsi_if_recv_msg(skb, nlh);
+			if (err) {
+				netlink_ack(skb, nlh, -err);
+			} else {
+				u32 seq  = nlh->nlmsg_seq;
+				u32 pid  = NETLINK_CREDS(skb)->pid;
+				struct iscsi_uevent *ev = NLMSG_DATA(nlh);
+				err = iscsi_if_send_reply(pid, seq,
+					nlh->nlmsg_type, 0, 0, ev, sizeof(*ev));
+				if (err)
+					netlink_ack(skb, nlh, -err);
+			}
+			skb_pull(skb, rlen);
+		}
+		kfree_skb(skb);
+	}
+}
+
+int iscsi_register_transport(struct iscsi_transport *ops, int id)
+{
+	transport_table[id] = ops;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(iscsi_register_transport);
+
+void iscsi_unregister_transport(int id)
+{
+	transport_table[id] = NULL;
+}
+EXPORT_SYMBOL_GPL(iscsi_unregister_transport);
+
+int __init iscsi_if_init(void)
+{
+	printk(KERN_INFO "Open-iSCSI Interface, version "
+			ISCSI_VERSION_STR " variant (" ISCSI_DATE_STR ")\n");
+
+	nls = netlink_kernel_create(NETLINK_ISCSI, iscsi_if_rx);
+	if (nls == NULL)
+		return -ENOBUFS;
+
+	return 0;
+}
+
+static void __exit
+iscsi_if_exit(void)
+{
+	sock_release(nls->sk_socket);
+}
+
+module_init(iscsi_if_init);
+module_exit(iscsi_if_exit);
drivers/scsi/iscsi_tcp.c +2669 -0

2,669 lines not on this page · raw message

drivers/scsi/iscsi_tcp.h +283 -0

74 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.

#1 lore raw
Quoted 6 lines · Alex Aizman

SCSI LLDD consists of 3 files:
- iscsi_if.c (iSCSI open interface over netlink);
- iscsi_tcp.[ch] (iSCSI transport over TCP/IP).

Some minor comments..

drivers/scsi/iscsi_if.c line 36

Quoted 3 lines
drivers/scsi/iscsi_if.c +3 -0
+int
+iscsi_control_recv_pdu(iscsi_cnx_h cp_cnx, struct iscsi_hdr *hdr,
+				char *data, uint32_t data_size)

Return value on the same line as the function name, please.

drivers/scsi/iscsi_if.c line 39

Quoted 3 lines
drivers/scsi/iscsi_if.c +3 -0
+{
+	struct nlmsghdr	*nlh;
+	struct sk_buff	*skb;

Tabs except at the beginning of the line are a nuisance.

drivers/scsi/iscsi_if.c line 48

Quoted 3 lines
drivers/scsi/iscsi_if.c +3 -0
+	if (!skb) {
+		return -ENOMEM;
+	}

Drop the braces around single statements.

Quoted 1 line

+EXPORT_SYMBOL_GPL(iscsi_control_recv_pdu);

Is this more than one module?

Quoted 1 line

+iscsi_control_cnx_error(iscsi_cnx_h cp_cnx, iscsi_err_e error)

Your type-naming scheme is a little unusual for the kernel. err_e appears redundant. And _h for handle could simply be _t for (opaque) type.

drivers/scsi/iscsi_if.c line 193

Quoted 5 lines
drivers/scsi/iscsi_if.c +5 -0
+static void
+iscsi_if_rx(struct sock *sk, int len)
+{
+	struct sk_buff *skb;
+	while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {

Assignments in tests are somewhat discouraged and !f is preferred to f != NULL.

drivers/scsi/iscsi_tcp.c line 69

Quoted 6 lines
drivers/scsi/iscsi_tcp.c +6 -0
+#ifndef DEBUG_ASSERT
+#ifdef BUG_ON
+#undef BUG_ON
+#endif
+#define BUG_ON(expr)
+#endif

Don't do that, please. An assertion that's not enabled is worse than no assertion at all.

drivers/scsi/iscsi_tcp.c line 108

Quoted 9 lines
drivers/scsi/iscsi_tcp.c +9 -0
+static inline void
+iscsi_buf_init_hdr(struct iscsi_conn *conn, struct iscsi_buf *ibuf,
+		   char *vbuf, u8 *crc)
+{
+	iscsi_buf_init_virt(ibuf, vbuf, sizeof(struct iscsi_hdr));
+	if (conn->hdrdgst_en) {
+		crypto_digest_init(conn->tx_tfm);
+		crypto_digest_update(conn->tx_tfm, &ibuf->sg, 1);
+		crypto_digest_final(conn->tx_tfm, crc);

I believe you'll find that crypto_digest_digest does that all for you.

drivers/scsi/iscsi_tcp.c line 114

Quoted 2 lines
drivers/scsi/iscsi_tcp.c +2 -0
+#define iscsi_conn_get(rdd) (struct iscsi_conn*)(rdd)->arg.data
+#define iscsi_conn_set(rdd, conn) (rdd)->arg.data = conn

Inlines, please. The second one is slightly broken without parens.

drivers/scsi/iscsi_tcp.c line 145

Quoted 1 line
drivers/scsi/iscsi_tcp.c +1 -0
+		 * PDU header scattered accross SKB's,

"across"

drivers/scsi/iscsi_tcp.c line 556

Quoted 2 lines
drivers/scsi/iscsi_tcp.c +2 -0
+		switch(conn->in.opcode) {
+		case ISCSI_OP_SCSI_CMD_RSP:

This switch looks like it could happily be in another function. The indentation is making it cramped.

drivers/scsi/iscsi_tcp.c line 678

Quoted 10 lines
drivers/scsi/iscsi_tcp.c +10 -0
+/*
+ * iscsi_ctask_copy - copy skb bits to the destanation cmd task
+ *
+ * The function calls skb_copy_bits() and updates per-connection and
+ * per-cmd byte counters.
+ */
+static inline int
+iscsi_ctask_copy(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask,
+		void *buf, int buf_size)
+{

Almost kerneldoc style, but not quite.

drivers/scsi/iscsi_tcp.c line 776

Quoted 1 line
drivers/scsi/iscsi_tcp.c +1 -0
+	    BUG_ON(ctask != (void*)sc->SCp.ptr);

Strange cast here.

drivers/scsi/iscsi_tcp.c line 782

Quoted 2 lines
drivers/scsi/iscsi_tcp.c +2 -0
+	    if (sc->use_sg) {
+		int i;

Mixed tabs and spaces.

drivers/scsi/iscsi_tcp.c line 876

Quoted 2 lines
drivers/scsi/iscsi_tcp.c +2 -0
+	default:
+		BUG_ON(1);

BUG()?

drivers/scsi/iscsi_tcp.c line 975

Quoted 4 lines
drivers/scsi/iscsi_tcp.c +4 -0
+static void
+iscsi_tcp_data_ready(struct sock *sk, int flag)
+{
+	struct iscsi_conn *conn = (struct iscsi_conn*)sk->sk_user_data;

Redundant cast here.

drivers/scsi/iscsi_tcp.c line 1018

Quoted 7 lines
drivers/scsi/iscsi_tcp.c +7 -0
+static void
+iscsi_write_space(struct sock *sk)
+{
+	struct iscsi_conn *conn = (struct iscsi_conn*)sk->sk_user_data;
+	conn->old_write_space(sk);
+	debug_tcp("iscsi_write_space: cid %d\n", conn->id);
+	conn->suspend = 0; wmb();

Sneaky. Separate line, please, and maybe a comment as to why the barrier is needed.

drivers/scsi/iscsi_tcp.c line 1075

Quoted 2 lines
drivers/scsi/iscsi_tcp.c +2 -0
+	debug_tcp("sendhdr %lx %d bytes at offset %d sent %d res %d\n",
+		(long)page_address(buf->sg.page), size, offset, buf->sent, res);

%p for page_address?

Quoted 5 lines
drivers/scsi/iscsi_tcp.c +5 -0
+static inline int
+iscsi_sendpage(struct iscsi_conn *conn, struct iscsi_buf *buf,
+	       int *count, int *sent)
+{
+	ssize_t (*sendpage)(struct socket *, struct page *, int, size_t, int);

[...]

drivers/scsi/iscsi_tcp.c line 1118

Quoted 3 lines
drivers/scsi/iscsi_tcp.c +3 -0
+	sendpage = sk->ops->sendpage ? : sock_no_sendpage;
+
+	res = sendpage(sk, buf->sg.page, offset, size, flags);

Can't we just do an if (a) a() else b() here? The ? <empty> : syntax is nonstandard.

drivers/scsi/iscsi_tcp.c line 1161

Quoted 13 lines
drivers/scsi/iscsi_tcp.c +13 -0
+iscsi_solicit_data_cont(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask,
+			struct iscsi_r2t_info *r2t, int left)
+{
+	struct iscsi_data *hdr;
+	struct iscsi_data_task *dtask;
+	struct scsi_cmnd *sc = ctask->sc;
+	int new_offset;
+
+	dtask = mempool_alloc(ctask->datapool, GFP_ATOMIC);
+	hdr = &dtask->hdr;
+	hdr->flags = 0;
+	hdr->rsvd2[0] = hdr->rsvd2[1] = hdr->rsvd3 =
+		hdr->rsvd4 = hdr->rsvd5 = hdr->rsvd6 = 0;

That looks odd, memset?

drivers/scsi/iscsi_tcp.c line 1208

Quoted 10 lines
drivers/scsi/iscsi_tcp.c +10 -0
+static void
+iscsi_unsolicit_data_init(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
+{
+	struct iscsi_data *hdr;
+	struct iscsi_data_task *dtask;
+
+	dtask = mempool_alloc(ctask->datapool, GFP_ATOMIC);
+	hdr = &dtask->hdr;
+	hdr->rsvd2[0] = hdr->rsvd2[1] = hdr->rsvd3 =
+		hdr->rsvd4 = hdr->rsvd5 = hdr->rsvd6 = 0;

And that looks familiar..

...

Boy howdy, this patch goes on for ever. Giving up at 9%.

Signature · 1 line
Mathematics is the supreme nostalgia of our time.