Skip to contentSkip to conversation

Open-iSCSI/Linux-iSCSI-5 High-Performance Initiator

PATCH In review Alex Aizman 10 participants 17 messages Jun 1, 2005 – Aug 6, 2005 SCSI

lore
Share & tools
lore
#0 lore raw

cover Open-iSCSI/Linux-iSCSI-5 High-Performance Initiator

This is open-iscsi/linux-iscsi-5 Initiator. This submission is ready for inclusion into mainline kernel.

It contains:

- iscsi data path (iscsi_tcp)
- iscsi transport class (scsi_transport_iscsi).

This Initiator depends on this transport class, but not vice versa: the transport class is common, generic, and isolated from this concrete iscsi implementation by scsi_transport_iscsi.h interface (see the header file and/or diagrams on the open-iscsi website). Initiator's control plane (supplied separately at http://www.open-iscsi.org/index.html#download) also does not depend on iscsi_tcp data path implementation - it's multi-transport capability is ensured by the the transport class itself.

Changes since 05/05/05 submission
(http://marc.theaimsgroup.com/?l=linux-kernel&m=111526765118450&w=2)
========================
* Stability: fixed a number of races between eh_abort(), conn_destroy(), abort timeout, etc.

* Performance improvements.

* Internally reviewed and cleaned up all the code, to the best of our ability.

* Exported transport and session/connection information via sysfs.

* Removed not-greater-than-page-size restriction; should be able to support applications using sg (tested) and presumably, st as well.

Tarball
====
The entire Initiator source including user space tools is available at
http://www.open-iscsi.org/index.html#download and/or svn (svn.berlios.de/open-iscsi, r363).

The same tarball can be downloaded from the linux-iscsi project page on sourceforge at http://sourceforge.net/project/shownotes.php?release_id=331731

Thanks
======
Many people contributed to testing. In particular, special thanks to:

Pascal Renauld, Harald Kubota, Olivier Galibert, Arne Redlich.

Changelog
========
* misleading printout in top-level Makefile fixed
* new ITT encoding implemented, should protect us from not well behaved targets
* ctask->in_progress state machine depricated. using direct PDU header processing
* using eh_host_reset_handler() to unjam iscsi session in case of interface fluks
* more verificiation done before r2t response processing
* recovery: ctask->SCp.Status deprecated, using ctask->sc (NULL/not NULL) instead
* Mike: scsi_transport_iscsi.h copyright cleanup
* Mike: minor coding style cleanup
* Mike: fixed the error path when session class was not being unregistered
* conn_stop() vs. eh_abort(): ctask->sc serialization via xmitsema
* Mike: iscsi_tcp: remove extra tabs
* fixed race between conn_destroy() and eh_abort()
* properly flushing r2tqueue and stopping ctask from being xmited after TMF Abort
* Mike: data_recv tabs cleanup
* Mike: dockbook style comments
* Mike: rm underscore in labels to match rest of driver
* fixed limitation on number of scattered PDUs across SKBs. Discovered by Harald.
* Mike: fixed some weird braces where locks would never get dropped if run and tabing
* session->state is no longer volatile, since we always access it under lock
* session->generation is not longer volatile; renamed as session->age
* Mike: add kernel printk values
* Mike: rm extra module get/put call
* regression.sh: each test-case now tested with fdisk+mkfs+bonnie
* Mike: do copyright per line
* Mike: merge iscsi_ifev.h and iscsi_if.h
* Mike: rm iscsi_iftrans.h and move include of scsi_transport_iscsi.h
* fixed iscsi_ctask_copy() buffer length bug (observed when using bonnie++ as a part of regression.sh)
* Mike: rm extra braces and add a space between var declarations and code
* enabling data digest causes lock-up (Arne Redlich)
* Mike: whitespaces cleanup
* Mike: coding style cleanup
* Mike: rm extra brace and mv invalid param test
* "un-volatile" tmabort_state (volatile is not needed)
* remved session->conn_lock
* "un-volatile" conn->c_stage
* "un-volatile" conn->stop_stage
* Wang Zhenyu: [io]fmarket typo fix
* regression introduced in r341: fixed now
* send_pdu(): use xmit semaphore, now serialized vs. data_xmit()
* removed: conn->cpu and the (dead) code
* data_xmit_more(): cleaned up and moved to the data_xmit()
* comments in the iscsi_tcp.h in particular: conn->xmitsema, session->lock
* Mike: remove the ISCSI_TRANSPORT_MAX limitation from the kernel using sysfs
* updated to the recent transport class changes backward-compile-2.6.11.patch
* fixed gcc4 compilation warnings
* Mike: don't hold session->lock when deleting timer with del_timer_sync()
* Mike: don't hold xmitsema while waiting for host_busy to decrement
* Mike: deadlock fix: cannot call del_timer sync with session_lock held
* propagating transport->caps up to the daemon, fixes DATA_DIGEST nego bug
* adding max_conn, max_cmd_len and max_lun attributes for iscsi_transport
* force eh_abort to unblock while in conn_destroy() waiting
* fail host_reset() in case of session terminated
* iscsiadm crash fix when transport capability does not match
* connection struct: re-aligned Tx members with Tx, Rx with Rx
* connection suspend: split Tx and Rx suspend, cleanup
* fixed eh_abort/conn_stop() race: clear/set conn->sock under lock
* removed restriction: sg element <= PAGE_SIZE
* removed restriction: non scatter/gather request <= PAGE_SIZE

Patches
=====

1. iscsi-tcp.c.patch - drivers/scsi/iscsi_tcp.c, iscsi data path. This is ~3000 lines 84K file, if it does not make it through, I'll split and resend. In addition, this file is available at http://www.open-iscsi.org/src/iscsi_tcp.c

2. iscsi-tcp.h.patch - drivers/scsi/iscsi_tcp.h, header file.

3. common-iscsi-headers.patch - common header files:
- iscsi_if.h (user/kernel #defines and user/kernel events);
- iscsi_proto.h (RFC3720 #defines and types);
- scsi_transport_iscsi.h (transport API, transport #defines and types).

4. iscsi-netlink.patch - include/linux/netlink.h changes (added new protocol NETLINK_ISCSI).

5. iscsi-Makefile.patch - drivers/scsi/Makefile changes.

6. iscsi-transport.patch - new iscsi transport class (drivers/scsi/scsi_transport_iscsi.c)

7. iscsi-Kconfig.patch - drivers/scsi/Kconfig changes.

Thanks,

Linux-iscsi Team

#1 lore raw
Quoted 2 lines · Alex Aizman

This is open-iscsi/linux-iscsi-5 Initiator. This submission is ready for inclusion into mainline kernel.

Awesome! So is this complete enough so I can, say, play DVDs from one box using an ATAPI DVD drive in another box?

Signature · 4 lines
Patrick "Diablo-D3" McFarland || [email protected]
"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd 
all be running around in darkened rooms, munching magic pills and listening to
repetitive electronic music." -- Kristian Wilson, Nintendo, Inc, 1989
#2 lore raw
Quoted 7 lines · Patrick McFarland
Quoted 1 line · Alex Aizman

This is open-iscsi/linux-iscsi-5 Initiator. This submission is ready for inclusion into mainline kernel.

Awesome! So is this complete enough so I can, say, play DVDs from one box using an ATAPI DVD drive in another box?

Yep, that's what iSCSI is for, in part. You'll need iSCSI target to connect to the SCSI backend, e.g. http://iscsitarget.sourceforge.net/

#3 lore raw
Quoted 10 lines · Alex Aizman
Quoted 6 lines · Patrick McFarland
Quoted 2 lines · Alex Aizman

This is open-iscsi/linux-iscsi-5 Initiator. This submission is ready for inclusion into mainline kernel.

Awesome! So is this complete enough so I can, say, play DVDs from one box using an ATAPI DVD drive in another box?

Yep, that's what iSCSI is for, in part. You'll need iSCSI target to connect to the SCSI backend, e.g. http://iscsitarget.sourceforge.net/

This is really, really good work. Please keep it up :-)

I'm very much looking forward to comparing this to DVD-over-NBD, which works fine, except when you need to change the disc. The NBD server locks the drive when it's running, and you need to restart both client and server after a disc change.

Briefly, what are the security features available? I assume I can do simple things like only allow access to an IP range (can I?), but what about more complex things like encryption?

Signature · 2 lines
Regards,
Christian Iversen
#4 lore raw
Quoted 16 lines · Christian Iversen
Quoted 4 lines · Alex Aizman
Quoted 2 lines · Patrick McFarland

On Wednesday 01 June 2005 04:08 pm, Alex Aizman wrote:

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

This is open-iscsi/linux-iscsi-5 Initiator. This submission is ready for inclusion into mainline kernel.

Awesome! So is this complete enough so I can, say, play DVDs from one box using an ATAPI DVD drive in another box?

Yep, that's what iSCSI is for, in part. You'll need iSCSI target to connect to the SCSI backend, e.g. http://iscsitarget.sourceforge.net/

Quoted 6 lines

This is really, really good work. Please keep it up :-)

I'm very much looking forward to comparing this to DVD-over-NBD, which works fine, except when you need to change the disc. The NBD server locks the drive when it's running, and you need to restart both client and server after a disc change.

Briefly, what are the security features available? I assume I can do simple things like only allow access to an IP range (can I?), but what about more complex things like encryption?

Briefly: in-band authentication at connection level is supported. You will not find it in the submitted code, it's all in user space, where it should be. IPsec encryption, as well as data integrity and confidentiality, is not supported yet. According to RFC, IPsec is a must to implement and optional to use. It's a feature, it can be added.

#5 lore raw
In reply to Patrick McFarland

u need target support as well.

ming

Quoted 7 lines · Patrick McFarland
Quoted 2 lines · Alex Aizman

This is open-iscsi/linux-iscsi-5 Initiator. This submission is ready for inclusion into mainline kernel.

Awesome! So is this complete enough so I can, say, play DVDs from one box using an ATAPI DVD drive in another box?

#6 lore raw
In reply to Alex Aizman
Quoted 2 lines · Alex Aizman

This is open-iscsi/linux-iscsi-5 Initiator. This submission is ready for inclusion into mainline kernel.

OK, I tried to put this into scsi-misc.

FIB has taken your netlink number, so I changed it to 32

__nlm_put() has had an updated prototype, which I can fix (although I'm not sure you're supposed to be using this function...)

I can't fix up the compile errors in iscsi_tcp.c:

CC [M] drivers/scsi/iscsi_tcp.o
drivers/scsi/iscsi_tcp.c: In function `iscsi_hdr_extract':
drivers/scsi/iscsi_tcp.c:160: warning: implicit declaration of function `iscsi_cnx_error'
drivers/scsi/iscsi_tcp.c:161: error: `ISCSI_ERR_PDU_GATHER_FAILED' undeclared (first use in this function) drivers/scsi/iscsi_tcp.c:161: error: (Each undeclared identifier is reported only once
drivers/scsi/iscsi_tcp.c:161: error: for each function it appears in.) drivers/scsi/iscsi_tcp.c: In function `iscsi_tcp_state_change':
drivers/scsi/iscsi_tcp.c:1005: error: `ISCSI_ERR_CNX_FAILED' undeclared (first use in this function)
drivers/scsi/iscsi_tcp.c: In function `iscsi_sendhdr':
drivers/scsi/iscsi_tcp.c:1092: error: `ISCSI_ERR_CNX_FAILED' undeclared (first use in this function)
drivers/scsi/iscsi_tcp.c: In function `iscsi_sendpage':
drivers/scsi/iscsi_tcp.c:1141: error: `ISCSI_ERR_CNX_FAILED' undeclared (first use in this function)
drivers/scsi/iscsi_tcp.c: In function `iscsi_data_xmit_more':
drivers/scsi/iscsi_tcp.c:1707: error: `STOP_CNX_RECOVER' undeclared (first use in this function)
drivers/scsi/iscsi_tcp.c: At top level:
[...]

Do you have an updated driver that will work in the current tree?

Thanks,

James

#7 lore raw
Quoted 10 lines · James Bottomley
Quoted 2 lines · Alex Aizman

This is open-iscsi/linux-iscsi-5 Initiator. This submission is ready for inclusion into mainline kernel.

OK, I tried to put this into scsi-misc.

FIB has taken your netlink number, so I changed it to 32

OK. Hopefully that'll remain.

Quoted 5 lines

__nlm_put() has had an updated prototype, which I can fix (although I'm not sure you're supposed to be using this function...)

I can't fix up the compile errors in iscsi_tcp.c:

Yes, the 06/01 submission was for 2.6.11. It will not compile.

Here's the latest development tarball 0.3rc7:

http://www.open-iscsi.org/bits/open-iscsi-0.3rc7-383.tar.gz
(This, as well as all the previous code drops can be downloaded from http://www.open-iscsi.org/index.html#download.)

Will work on producing a new patchset for 2.6.13. The only pending item now is code style cleanup.

Changelog, etc. Monday 08/01 is the latest.

Quoted 36 lines

CC [M] drivers/scsi/iscsi_tcp.o
drivers/scsi/iscsi_tcp.c: In function `iscsi_hdr_extract':
drivers/scsi/iscsi_tcp.c:160: warning: implicit declaration of function `iscsi_cnx_error'
drivers/scsi/iscsi_tcp.c:161: error: `ISCSI_ERR_PDU_GATHER_FAILED' undeclared (first use in this function) drivers/scsi/iscsi_tcp.c:161: error: (Each undeclared identifier is reported only once
drivers/scsi/iscsi_tcp.c:161: error: for each function it appears in.) drivers/scsi/iscsi_tcp.c: In function `iscsi_tcp_state_change':
drivers/scsi/iscsi_tcp.c:1005: error: `ISCSI_ERR_CNX_FAILED' undeclared (first use in this function)
drivers/scsi/iscsi_tcp.c: In function `iscsi_sendhdr':
drivers/scsi/iscsi_tcp.c:1092: error: `ISCSI_ERR_CNX_FAILED' undeclared (first use in this function)
drivers/scsi/iscsi_tcp.c: In function `iscsi_sendpage':
drivers/scsi/iscsi_tcp.c:1141: error: `ISCSI_ERR_CNX_FAILED' undeclared (first use in this function)
drivers/scsi/iscsi_tcp.c: In function `iscsi_data_xmit_more':
drivers/scsi/iscsi_tcp.c:1707: error: `STOP_CNX_RECOVER' undeclared (first use in this function)
drivers/scsi/iscsi_tcp.c: At top level:
[...]

Do you have an updated driver that will work in the current tree?

Thanks,

James

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw
Quoted 1 line · Alex Aizman

OK. Hopefully that'll remain.

Please ask davem for a netlink number allocation.

#9 lore raw

From: Christoph Hellwig <[email protected]>
Subject: Re: [ANNOUNCE 0/7] Open-iSCSI/Linux-iSCSI-5 High-Performance Initiator
Date: Sat, 30 Jul 2005 20:38:03 +0200

Quoted 4 lines · Alex Aizman
Quoted 1 line

OK. Hopefully that'll remain.

Please ask davem for a netlink number allocation.

There are none to allocate, see my other email as to how we have to start handling the issue of many new netlink users and the fact that we are out of numbers to give to people.

#10 lore raw
In reply to James Bottomley

From: James Bottomley <[email protected]>
Date: Sat, 30 Jul 2005 12:32:42 -0500

Quoted 1 line

FIB has taken your netlink number, so I changed it to 32

MAX_LINKS is 32, so there is no way this reassignment would work.

You have to pick something in the range 0 --> 32, and as is no surprise, there are no numbers available :-)

Since ethertap has been deleted, 16-->31 could be made allocatable once more, but I simply do not want to do that and have the flood gates open up for folks allocating random netlink numbers.

Instead, we need to take one of those netlink numbers, and turn it into a multiplexable layer that can support an arbitrary number of sub-netlink types. Said protocol would need some shim header that just says the "sub-netlink" protocol number, something as simple as just a "u32", this gets pulled off the front of the netlink packet and then it's passed on down to the real protocol.

#11 lore raw
Quoted 7 lines · David S. Miller

From: James Bottomley <[email protected]>
Date: Sat, 30 Jul 2005 12:32:42 -0500

Quoted 1 line

FIB has taken your netlink number, so I changed it to 32

MAX_LINKS is 32, so there is no way this reassignment would work.

Actually, I saw this and increased MAX_LINKS as well. I was going to query all of this on the net-dev mailing list if we'd managed to get the code compileable.

Quoted 7 lines

You have to pick something in the range 0 --> 32, and as is no surprise, there are no numbers available :-)

Since ethertap has been deleted, 16-->31 could be made allocatable once more, but I simply do not want to do that and have the flood gates open up for folks allocating random netlink numbers.

Quoted 7 lines

Instead, we need to take one of those netlink numbers, and turn it into a multiplexable layer that can support an arbitrary number of sub-netlink types. Said protocol would need some shim header that just says the "sub-netlink" protocol number, something as simple as just a "u32", this gets pulled off the front of the netlink packet and then it's passed on down to the real protocol.

I'll let the iSCSI people try this ...

Alternatively, if they don't fancy it, I think the kobject_uevent mechanism (which already has a netlink number) looks like it might be amenable for use for most of the things they want to do.

James

#12 lore raw
Quoted 28 lines · James Bottomley
Quoted 7 lines · David S. Miller

From: James Bottomley <[email protected]>
Date: Sat, 30 Jul 2005 12:32:42 -0500

Quoted 1 line

FIB has taken your netlink number, so I changed it to 32

MAX_LINKS is 32, so there is no way this reassignment would work.

Actually, I saw this and increased MAX_LINKS as well. I was going to query all of this on the net-dev mailing list if we'd managed to get the code compileable.

Quoted 7 lines

You have to pick something in the range 0 --> 32, and as is no surprise, there are no numbers available :-)

Since ethertap has been deleted, 16-->31 could be made allocatable once more, but I simply do not want to do that and have the flood gates open up for folks allocating random netlink numbers.

Quoted 7 lines

Instead, we need to take one of those netlink numbers, and turn it into a multiplexable layer that can support an arbitrary number of sub-netlink types. Said protocol would need some shim header that just says the "sub-netlink" protocol number, something as simple as just a "u32", this gets pulled off the front of the netlink packet and then it's passed on down to the real protocol.

Quoted 5 lines

I'll let the iSCSI people try this ...

Alternatively, if they don't fancy it, I think the kobject_uevent mechanism (which already has a netlink number) looks like it might be amenable for use for most of the things they want to do.

In fact, during design phase we've considered to use kobject_uevent() as well but (if i recall correctly), it didn't fit for the simple reason that if we want to have that much code in user-space, than we need to have more control on netlink socket and need to pass binary data back and forth.

It would be nice to set MAX_LINKS to 64 and close this issue for now, since I'm pretty sure some other apps might find out kobject_uevent() not suitable for their needs too.

Dima

#13 lore raw

From: Dmitry Yusupov <[email protected]>
Date: Sat, 30 Jul 2005 14:25:28 -0700

Quoted 1 line

It would be nice to set MAX_LINKS to 64 and close this issue for now,

James and Dmitry, increasing MAX_LINKS does't work, did you actually try to open a netlink socket with a protocol number larger than 32?

It will not work. Don't you think I'd implement such a simple fix to expand the protocol number space if it was that easy? :-)

You can't increase MAX_LINKS past 32 because this is also the value of NPROTO. It is impossible to create a new socket with a protocol value larger than NPROTO, and this value is exported into userspace as well.

#14 lore raw
In reply to Dmitry Yusupov
Quoted 40 lines · Dmitry Yusupov
Quoted 31 lines · James Bottomley
Quoted 8 lines · David S. Miller

From: James Bottomley <[email protected]>
Date: Sat, 30 Jul 2005 12:32:42 -0500

Quoted 1 line

FIB has taken your netlink number, so I changed it to 32

MAX_LINKS is 32, so there is no way this reassignment would work.

Actually, I saw this and increased MAX_LINKS as well. I was going to query all of this on the net-dev mailing list if we'd managed to get the code compileable.

Quoted 7 lines

You have to pick something in the range 0 --> 32, and as is no surprise, there are no numbers available :-)

Since ethertap has been deleted, 16-->31 could be made allocatable once more, but I simply do not want to do that and have the flood gates open up for folks allocating random netlink numbers.

Quoted 7 lines

Instead, we need to take one of those netlink numbers, and turn it into a multiplexable layer that can support an arbitrary number of sub-netlink types. Said protocol would need some shim header that just says the "sub-netlink" protocol number, something as simple as just a "u32", this gets pulled off the front of the netlink packet and then it's passed on down to the real protocol.

Quoted 5 lines

I'll let the iSCSI people try this ...

Alternatively, if they don't fancy it, I think the kobject_uevent mechanism (which already has a netlink number) looks like it might be amenable for use for most of the things they want to do.

Quoted 6 lines

In fact, during design phase we've considered to use kobject_uevent() as well but (if i recall correctly), it didn't fit for the simple reason that if we want to have that much code in user-space, than we need to have more control on netlink socket and need to pass binary data back and forth.

I have been trying to modify open-iscsi to use the exisitng kobject_uevent code similar to how we tried to do this with the old sfnet driver. Basically there are two problems. As Dimtry described above, open-iscsi pushes a lot of iSCSI command processing to userspace. And to accomplish this it must send at least this struct (or a one similar) to userspace to be processed.

struct iscsi_hdr {
        uint8_t         opcode;
        uint8_t         flags;          /* Final bit */
        uint8_t         rsvd2[2];
        uint8_t         hlength;        /* AHSs total length */
        uint8_t         dlength[3];     /* Data length */
        uint8_t         lun[8];
        __be32          itt;            /* Initiator Task Tag */
        __be32          ttt;            /* Target Task Tag */
        __be32          statsn;
        __be32          exp_statsn;
        uint8_t         other[16];
};

There could also be data as part of the command too thought (like how a SCSI read or write command has the cdb part and the payload).

For the iscsi_hdr we could encode the values of the fields into strings and send it back and forth between userspace and the kernel using sysfs to get the comamnd into the kernel and modifed kobject_uevent functions (kobject_uevent would need to be modified to be able to take this extra header info instead of just doing add, remove, etc that are predefined uevents). This starts to get ulgy though and when you consider that we still have some payload that also needs to go to userspace it gets uglier.

Also for the sysfs value per file rule, breaking up the iSCSI command header into a file per field to pass commands into the kernel then using a binary sysfs file for the data part of the command, and then reassembling all this in the driver is not so nice. A major advantage open-iscsi has over sfnet was that it was simple to send all the session info for session creation and setup and iSCSI command info through a netlink message.

#15 lore raw
In reply to James Bottomley

From: James Bottomley <[email protected]>
Date: Sat, 30 Jul 2005 15:23:20 -0500

Quoted 1 line

Actually, I saw this and increased MAX_LINKS as well.

That does absolutely nothing, you cannot create sockets with protocol numbers larger than NPROTOS which like MAX_LINKS has the value 32. And NPROTOS is something we cannot change.

#16 lore raw
Quoted 8 lines · David S. Miller

From: James Bottomley <[email protected]>
Date: Sat, 30 Jul 2005 15:23:20 -0500

Quoted 1 line

Actually, I saw this and increased MAX_LINKS as well.

That does absolutely nothing, you cannot create sockets with protocol numbers larger than NPROTOS which like MAX_LINKS has the value 32. And NPROTOS is something we cannot change.

I couldn't find any relationship between MAX_LINKS and NPROTO. NPROTO is the maximum number of protocol families, the only limitation for MAX_LINKS seems to be the size of sk_protocol which is an unsigned char. So I think we can safely increase it.