bidi, varlen, bsg success report
Subsystems (4)
I spent the last couple of days rebasing my working set of kernel patches from 2.6.22-rc5 to 2.6.24-rc6. I want to use the BSG interface from userspace to submit variable-length bidirectional SCSI commands to targets via iSCSI TCP.
Many thanks to all the work with scsi data accessors and iscsi that makes this much easier, and to Boaz for continuing to maintain patches. Here's what I needed to do to make it work.
Start with linus-2.6.24-rc6.
Merge scsi-misc.
Apply the following outstanding patches from Boaz:
bidi
scsi: scsi_data_buffer
http://marc.info/?l=linux-scsi&m=119754650614813&w=2
scsi: bidi support
http://marc.info/?l=linux-scsi&m=119754667915055&w=2varlen
[PATCH 1/3] Let scsi_cmnd->cmnd use request->cmd buffer
http://marc.info/?l=linux-scsi&m=119798523823750&w=2
[PATCH 2/2] block layer varlen-cdb
http://marc.info/?l=linux-scsi&m=119798548624218&w=2
[PATCH 3/3] scsi: varlen extended and vendor-specific cdbs
http://marc.info/?l=linux-scsi&m=119798563924476&w=2Then a few more, two modified from earlier Boaz patches, and three new ones by me, mainly to make bsg work. They follow.
1. iscsi: extended cdb support
Adapted from Boaz
http://marc.info/?l=linux-scsi&m=119394074127857&w=2
2. iscsi: bidi support
Adapted from two old Boaz patches that can be found in this git:
http://www.bhalevy.com/git/gitweb.cgi?p=open-osd/.git;a=commitdiff;h=b5a97649eadb9434acbc4170a8aebd962af6d960
http://www.bhalevy.com/git/gitweb.cgi?p=open-osd/.git;a=commitdiff;h=cf3d4684a22bde6787f2eccee66d7b0ee9c9182c
3. bsg: accept varlen commands
4. bsg: bidi fix
5. iscsi tcp: bidi capable
Suggestions: Boaz is probably the best qualified to shepherd his outstanding five patches plus the two others I mention above (#1, #2). The first bsg patch (#3) won't apply without the varlen patches. The last two (#4, #5) are harmless and can go in now, but won't matter until the bidi patches are in. Comments on these welcome.
Hope this is helpful. I'm definitely looking forward to seeing all this merged into mainline eventually.
-- Pete
PATCH 1/5 iscsi: extended cdb support
From: Boaz Harrosh <[email protected]>
Once varlen cdbs are supported by the block and scsi-ml layers we can apply this patch to support extended CDBs in iscsi.
--- drivers/scsi/iscsi_tcp.c | 2 +- drivers/scsi/libiscsi.c | 56 ++++++++++++++++++++++++++++++++++++++++---- include/scsi/iscsi_proto.h | 6 +++- 3 files changed, 56 insertions(+), 8 deletions(-)
drivers/scsi/iscsi_tcp.c +1 -1
@@ -1973,7 +1973,7 @@ static struct iscsi_transport iscsi_tcp_transport = { .host_template = &iscsi_sht, .conndata_size = sizeof(struct iscsi_conn), .max_conn = 1, - .max_cmd_len = 16, + .max_cmd_len = SCSI_MAX_VARLEN_CDB_SIZE,/* session management */ .create_session = iscsi_tcp_session_create, .destroy_session = iscsi_tcp_session_destroy,
drivers/scsi/libiscsi.c +51 -5
74 lines not on this page · raw message
include/scsi/iscsi_proto.h +4 -2
@@ -112,6 +112,7 @@ struct iscsi_ahs_hdr {#define ISCSI_AHSTYPE_CDB 1#define ISCSI_AHSTYPE_RLENGTH 2 +#define ISCSI_CDB_SIZE 16/* iSCSI PDU Header */ struct iscsi_cmd {
@@ -125,7 +126,7 @@ struct iscsi_cmd {__be32 data_length;__be32 cmdsn;__be32 exp_statsn; -uint8_t cdb[16 ];/* SCSI Command Block */ +uint8_t cdb[ISCSI_CDB_SIZE ];/* SCSI Command Block */ /* Additional Data (Command Dependent) */ };
@@ -154,7 +155,8 @@ struct iscsi_ecdb_ahdr {__be16 ahslength;/* CDB length - 15, including reserved byte */ uint8_t ahstype;uint8_t reserved; -uint8_t ecdb[260 -16 ];/* 4-byte aligned extended CDB spillover */ +/* 4-byte aligned extended CDB spillover */ +uint8_t ecdb[260 -ISCSI_CDB_SIZE ]; };/* SCSI Response Header */
Signature · 1 line
1.5.3.6
PATCH 1/5 iscsi: extended cdb support
In reply to Pete Wyckoff[Resend, date was in ancient history.]
From: Boaz Harrosh <[email protected]>
Once varlen cdbs are supported by the block and scsi-ml layers we can apply this patch to support extended CDBs in iscsi.
--- drivers/scsi/iscsi_tcp.c | 2 +- drivers/scsi/libiscsi.c | 56 ++++++++++++++++++++++++++++++++++++++++---- include/scsi/iscsi_proto.h | 6 +++- 3 files changed, 56 insertions(+), 8 deletions(-)
drivers/scsi/iscsi_tcp.c +1 -1
@@ -1973,7 +1973,7 @@ static struct iscsi_transport iscsi_tcp_transport = { .host_template = &iscsi_sht, .conndata_size = sizeof(struct iscsi_conn), .max_conn = 1, - .max_cmd_len = 16, + .max_cmd_len = SCSI_MAX_VARLEN_CDB_SIZE,/* session management */ .create_session = iscsi_tcp_session_create, .destroy_session = iscsi_tcp_session_destroy,
drivers/scsi/libiscsi.c +51 -5
74 lines not on this page · raw message
include/scsi/iscsi_proto.h +4 -2
@@ -112,6 +112,7 @@ struct iscsi_ahs_hdr {#define ISCSI_AHSTYPE_CDB 1#define ISCSI_AHSTYPE_RLENGTH 2 +#define ISCSI_CDB_SIZE 16/* iSCSI PDU Header */ struct iscsi_cmd {
@@ -125,7 +126,7 @@ struct iscsi_cmd {__be32 data_length;__be32 cmdsn;__be32 exp_statsn; -uint8_t cdb[16 ];/* SCSI Command Block */ +uint8_t cdb[ISCSI_CDB_SIZE ];/* SCSI Command Block */ /* Additional Data (Command Dependent) */ };
@@ -154,7 +155,8 @@ struct iscsi_ecdb_ahdr {__be16 ahslength;/* CDB length - 15, including reserved byte */ uint8_t ahstype;uint8_t reserved; -uint8_t ecdb[260 -16 ];/* 4-byte aligned extended CDB spillover */ +/* 4-byte aligned extended CDB spillover */ +uint8_t ecdb[260 -ISCSI_CDB_SIZE ]; };/* SCSI Response Header */
Signature · 1 line
1.5.3.6
PATCH 2/5 iscsi bidi support
In reply to Pete WyckoffFrom: Boaz Harrosh <[email protected]>
Adapted from separate libiscsi and TCP patches by Boaz to update for iscsi changes and iscsi-tcp rewrite. Dropped some needless conversion to scsi_out() in some places. Original log follows.
Once bidi support is accepted at block and scsi-ml layers
below bidi support to iscsi can be applied.
The assumed bidi API from scsi-ml is based on scsi_sgtables
but it is not at all far from any other implementation as far
as iscsi code is concerned:
- scsi_bidi_cmd() - return true if the command is bidi. Note
that this is a deviation from previous RFCs
in that we no longer have sc_dma_direction
== DMA_BIDIRECTIONAL. The command direction is DMA_TO_DEVICE but with this flag on.
- scsi_{in,out}() - Which will return the right sg_table for IN or OUT
operation, in a generic way. The small difference here due to sgtable implementation is that in the past we had scsi_in()->sglist == NULL for DMA_NONE and now we have scsi_in() == NULL.
At the generic libiscsi level
- prepare the additional bidi_read rlength header.
- access the right scsi_in() and/or scsi_out() side of things. also for resid.
--- drivers/scsi/iscsi_tcp.c | 14 ++++--- drivers/scsi/libiscsi.c | 81 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 17 deletions(-)
drivers/scsi/iscsi_tcp.c +8 -6
@@ -542,10 +542,11 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask) tcp_ctask->exp_datasn++; tcp_ctask->data_offset =be32_to_cpu (rhdr->offset); -if (tcp_ctask->data_offset + tcp_conn->in.datalen >scsi_bufflen (sc)) { +if (tcp_ctask->data_offset + tcp_conn->in.datalen > +scsi_in (sc)->length) {debug_tcp ("%s: data_offset(%d) + data_len(%d) > total_length_in(%d)\n" , __FUNCTION__, tcp_ctask->data_offset, - tcp_conn->in.datalen,scsi_bufflen (sc)); + tcp_conn->in.datalen,scsi_in (sc)->length);return ISCSI_ERR_DATA_OFFSET ; }
@@ -558,8 +559,8 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)if (res_count >0 && (rhdr->flags &ISCSI_FLAG_CMD_OVERFLOW || - res_count <=scsi_bufflen (sc))) -scsi_set_resid (sc, res_count); + res_count <=scsi_in (sc)->length)) +scsi_in (sc)->resid = res_count;else sc->result = (DID_BAD_TARGET <<16 ) | rhdr->cmd_status;
@@ -783,8 +784,9 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr) tcp_ctask->data_offset, tcp_conn->in.datalen); return iscsi_segment_seek_sg(&tcp_conn->in.segment, - scsi_sglist(ctask->sc), - scsi_sg_count(ctask->sc), + scsi_in(ctask->sc)->sglist, + scsi_in(ctask->sc)-> + sg_count, tcp_ctask->data_offset, tcp_conn->in.datalen, iscsi_tcp_process_data_in,
drivers/scsi/libiscsi.c +70 -11
149 lines not on this page · raw message
Signature · 1 line
1.5.3.6
PATCH 3/5 bsg: accept varlen commands
In reply to Pete WyckoffAccept variable length SCSI commands through BSG.
--- block/bsg.c | 34 +++++++++++++++++++++++++--------- 1 files changed, 25 insertions(+), 9 deletions(-)
block/bsg.c +25 -9
@@ -175,14 +175,29 @@ unlock:static int blk_fill_sgv4_hdr_rq (struct request_queue *q,struct request *rq,struct sg_io_v4 *hdr,int has_write_perm) { -memset (rq->cmd,0 ,BLK_MAX_CDB );/* ATAPI hates garbage after CDB */ - -if (copy_from_user (rq->cmd, (void *)(unsigned long )hdr->request, - hdr->request_len)) -return -EFAULT ; +void __user *buf = (void __user *)(unsigned long ) hdr->request; +int len = hdr->request_len; +unsigned char *cmd; + +if (len >BLK_MAX_CDB ) { + rq->cmd_len =0 ; + rq->varlen_cdb_len = len; + rq->varlen_cdb =kmalloc (len,GFP_KERNEL ); +if (rq->varlen_cdb ==NULL ) +return -ENOMEM ; +if (copy_from_user (rq->varlen_cdb, buf, len)) +return -EFAULT ; + cmd = &rq->varlen_cdb[0 ]; + }else { + rq->cmd_len = len; +memset (rq->cmd,0 ,BLK_MAX_CDB );/* for ATAPI */ +if (copy_from_user (rq->cmd, buf, len)) +return -EFAULT ; + cmd = &rq->cmd[0 ]; + }if (hdr->subprotocol ==BSG_SUB_PROTOCOL_SCSI_CMD ) { -if (blk_verify_command (rq->cmd, has_write_perm)) +if (blk_verify_command (cmd, has_write_perm))return -EPERM ; }else if (!capable (CAP_SYS_RAWIO ))return -EPERM ;
@@ -190,7 +205,6 @@ static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,/* * fill in request structure */ - rq->cmd_len = hdr->request_len; rq->cmd_type =REQ_TYPE_BLOCK_PC ; rq->timeout = (hdr->timeout *HZ ) /1000 ;
@@ -212,8 +226,6 @@ bsg_validate_sgv4_hdr(struct request_queue *q, struct sg_io_v4 *hdr, int *rw)if (hdr->guard !='Q' )return -EINVAL ; -if (hdr->request_len >BLK_MAX_CDB ) -return -EINVAL ;if (hdr->dout_xfer_len > (q->max_sectors <<9 ) || hdr->din_xfer_len > (q->max_sectors <<9 ))return -EIO ;
@@ -302,6 +314,8 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr) }return rq; out: +if (rq->varlen_cdb_len) +kfree (rq->varlen_cdb);blk_put_request (rq);if (next_rq) {blk_rq_unmap_user (next_rq->bio);
@@ -446,6 +460,8 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq, struct sg_io_v4 *hdr, hdr->dout_resid = rq->data_len;blk_rq_unmap_user (bio); +if (rq->varlen_cdb_len) +kfree (rq->varlen_cdb);blk_put_request (rq);return ret;
Signature · 1 line
1.5.3.6
PATCH 3/5 bsg: accept varlen commands
In reply to Pete Wyckoff[Resend, date was in ancient history.]
Accept variable length SCSI commands through BSG.
--- block/bsg.c | 34 +++++++++++++++++++++++++--------- 1 files changed, 25 insertions(+), 9 deletions(-)
block/bsg.c +25 -9
@@ -175,14 +175,29 @@ unlock:static int blk_fill_sgv4_hdr_rq (struct request_queue *q,struct request *rq,struct sg_io_v4 *hdr,int has_write_perm) { -memset (rq->cmd,0 ,BLK_MAX_CDB );/* ATAPI hates garbage after CDB */ - -if (copy_from_user (rq->cmd, (void *)(unsigned long )hdr->request, - hdr->request_len)) -return -EFAULT ; +void __user *buf = (void __user *)(unsigned long ) hdr->request; +int len = hdr->request_len; +unsigned char *cmd; + +if (len >BLK_MAX_CDB ) { + rq->cmd_len =0 ; + rq->varlen_cdb_len = len; + rq->varlen_cdb =kmalloc (len,GFP_KERNEL ); +if (rq->varlen_cdb ==NULL ) +return -ENOMEM ; +if (copy_from_user (rq->varlen_cdb, buf, len)) +return -EFAULT ; + cmd = &rq->varlen_cdb[0 ]; + }else { + rq->cmd_len = len; +memset (rq->cmd,0 ,BLK_MAX_CDB );/* for ATAPI */ +if (copy_from_user (rq->cmd, buf, len)) +return -EFAULT ; + cmd = &rq->cmd[0 ]; + }if (hdr->subprotocol ==BSG_SUB_PROTOCOL_SCSI_CMD ) { -if (blk_verify_command (rq->cmd, has_write_perm)) +if (blk_verify_command (cmd, has_write_perm))return -EPERM ; }else if (!capable (CAP_SYS_RAWIO ))return -EPERM ;
@@ -190,7 +205,6 @@ static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,/* * fill in request structure */ - rq->cmd_len = hdr->request_len; rq->cmd_type =REQ_TYPE_BLOCK_PC ; rq->timeout = (hdr->timeout *HZ ) /1000 ;
@@ -212,8 +226,6 @@ bsg_validate_sgv4_hdr(struct request_queue *q, struct sg_io_v4 *hdr, int *rw)if (hdr->guard !='Q' )return -EINVAL ; -if (hdr->request_len >BLK_MAX_CDB ) -return -EINVAL ;if (hdr->dout_xfer_len > (q->max_sectors <<9 ) || hdr->din_xfer_len > (q->max_sectors <<9 ))return -EIO ;
@@ -302,6 +314,8 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr) }return rq; out: +if (rq->varlen_cdb_len) +kfree (rq->varlen_cdb);blk_put_request (rq);if (next_rq) {blk_rq_unmap_user (next_rq->bio);
@@ -446,6 +460,8 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq, struct sg_io_v4 *hdr, hdr->dout_resid = rq->data_len;blk_rq_unmap_user (bio); +if (rq->varlen_cdb_len) +kfree (rq->varlen_cdb);blk_put_request (rq);return ret;
Signature · 1 line
1.5.3.6
PATCH 4/5 bsg: bidi fix
In reply to Pete WyckoffFixes for bsg to handle bidirectional commands. The next_rq part of a bidi command must be marked REQ_TYPE_BLOCK_PC for scsi_init_sgtable.
--- block/bsg.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
block/bsg.c +1 -0
@@ -291,6 +291,7 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr) goto out; } rq->next_rq = next_rq; + next_rq->cmd_type =REQ_TYPE_BLOCK_PC ; dxferp = (void *)(unsigned long )hdr->din_xferp; ret =blk_rq_map_user (q, next_rq, dxferp, hdr->din_xfer_len);
Signature · 1 line
1.5.3.6
PATCH 5/5 iscsi tcp bidi capable
In reply to Pete WyckoffMark iscsi_tcp as being capable of bidirectional transfers. Perhaps instead we could remove the check in bsg, as that is the only entry to SCSI that bothers to check the flag.
--- drivers/scsi/iscsi_tcp.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
drivers/scsi/iscsi_tcp.c +7 -0
@@ -1915,6 +1915,12 @@ static void iscsi_tcp_session_destroy(struct iscsi_cls_session *cls_session)iscsi_session_teardown (cls_session); } +static int iscsi_tcp_slave_alloc (struct scsi_device *sdev) +{ +set_bit (QUEUE_FLAG_BIDI , &sdev->request_queue->queue_flags); +return 0 ; +} +static int iscsi_tcp_slave_configure (struct scsi_device *sdev) {blk_queue_bounce_limit (sdev->request_queue,BLK_BOUNCE_ANY );
@@ -1936,6 +1942,7 @@ static struct scsi_host_template iscsi_sht = { .eh_host_reset_handler = iscsi_eh_host_reset, .use_clustering = DISABLE_CLUSTERING, .use_sg_chaining = ENABLE_SG_CHAINING, + .slave_alloc = iscsi_tcp_slave_alloc, .slave_configure = iscsi_tcp_slave_configure, .proc_name = "iscsi_tcp", .this_id = -1,
Signature · 1 line
1.5.3.6
Quoted 55 lines · Pete Wyckoff
I spent the last couple of days rebasing my working set of kernel patches from 2.6.22-rc5 to 2.6.24-rc6. I want to use the BSG interface from userspace to submit variable-length bidirectional SCSI commands to targets via iSCSI TCP.
Many thanks to all the work with scsi data accessors and iscsi that makes this much easier, and to Boaz for continuing to maintain patches. Here's what I needed to do to make it work.
Start with linus-2.6.24-rc6.
Merge scsi-misc.
Apply the following outstanding patches from Boaz:bidi
scsi: scsi_data_buffer http://marc.info/?l=linux-scsi&m=119754650614813&w=2 scsi: bidi support http://marc.info/?l=linux-scsi&m=119754667915055&w=2varlen
[PATCH 1/3] Let scsi_cmnd->cmnd use request->cmd buffer http://marc.info/?l=linux-scsi&m=119798523823750&w=2 [PATCH 2/2] block layer varlen-cdb http://marc.info/?l=linux-scsi&m=119798548624218&w=2 [PATCH 3/3] scsi: varlen extended and vendor-specific cdbs http://marc.info/?l=linux-scsi&m=119798563924476&w=2Then a few more, two modified from earlier Boaz patches, and three new ones by me, mainly to make bsg work. They follow.
1. iscsi: extended cdb support
Adapted from Boaz
http://marc.info/?l=linux-scsi&m=119394074127857&w=22. iscsi: bidi support
Adapted from two old Boaz patches that can be found in this git:
http://www.bhalevy.com/git/gitweb.cgi?p=open-osd/.git;a=commitdiff;h=b5a97649eadb9434acbc4170a8aebd962af6d960
http://www.bhalevy.com/git/gitweb.cgi?p=open-osd/.git;a=commitdiff;h=cf3d4684a22bde6787f2eccee66d7b0ee9c9182c3. bsg: accept varlen commands
4. bsg: bidi fix
5. iscsi tcp: bidi capable
Suggestions: Boaz is probably the best qualified to shepherd his outstanding five patches plus the two others I mention above (#1, #2). The first bsg patch (#3) won't apply without the varlen patches. The last two (#4, #5) are harmless and can go in now, but won't matter until the bidi patches are in. Comments on these welcome.
Quoted 5 lines
Hope this is helpful. I'm definitely looking forward to seeing all this merged into mainline eventually.
-- Pete-
Dear peter.
This is grate to know. I know from passed experience that code might work well for your own set of tests, but otherwise hide bugs.
Please forgive me about the iSCSI patches, I have an updated set here in my trees, but I have not posted them, because I thought that no one cares, but me.
I did not yet get any principle OK about the varlen API. that is - scsi_cmnd->cmnd changes to a pointer that points to either a static buffer at request or a dynamic buffer at ULD - Because if this principle is accepted, than please Note that the patch "iscsi: extended cdb support" can be applied now, before the actual cleanups and implementation are applied.
I attributed the complete silence treatment from both block and scsi maintainers, to the holiday session. I hope they come with renewed strength next year ;)
I will compare our iscsi patches and put a freshen set at:git://git.bhalevy.com/open-osd osd_dev
or on the web
http://git.bhalevy.com/git/gitweb.cgi?p=open-osd/.git;a=summary
(give me until the end of today)
I have inspected your other patches, and they all look good. I will include them in above git.
Happy holidays
Boaz