RE: [PATCH 1/2] RFC: Proposal for BSG Interface
Sorry for late response. Please see our comments inline.
We are trying to figure out how we can leverage this interface
for our offload solution.
Thanks,
Vikas.
+/*
+ * iSCSI Host Messages
+ */
+
+struct ip_address_subnet_format {
+ u16 sizeofstructure;
Why do we need this field ?
+ u8 iptype;
+ u8 ipv6_prefix_length;
+ u8 ipaddress[16];
+ u8 subnetmask[16];
+ u32 rsvd0;
+} __packed;
+
+struct iscsi_ip_addr_record {
+ u32 action;
What's does this field implies ? Looks like be2iscsi specific ?
+ u32 interface_hndl;
Again what this interface handle for ?
+ struct ip_address_subnet_format ip_address;
+ u32 status;
Why do we need *status* ? And its status of what ?
+} __packed;
+
+struct iscsi_ip_addr_record_params {
+ u32 record_entry_count;
+ struct iscsi_ip_addr_record ip_record[1];
+} __packed;
+
+struct ip_address_format {
+ u16 sizeofstructure;
+ u8 reserved;
+ u8 ip_type;
+ u8 ip_address[16];
+ u32 rsvd0;
+} __packed;
+
So this structure is needed to distinguish between IPV4 and IPV6. Correct ?
Once we have a better understanding we can really assess whether it will
work for our interface or not, or it will require further changes to make it
generic
so that it will work for us.
+struct iscsi_bsg_common_format {
+ u8 opcode; /* dword 0 */
+ u8 subsystem; /* dword 0 */
+ u8 port_number; /* dword 0 */
+ u8 domain; /* dword 0 */
What does *port_number* and *domain* indicate ?
+ u32 timeout;/* dword 1 */
Is it bsg command time out?
+ u32 request_length; /* dword 2 */
+ u32 rsvd0; /* dword 3 */
Why *rsvd0* here? Why not at end of struct.
+ u32action;
What is expected data here ? So at the app's level what *action*
is supposed to be set to ?
+ struct ip_address_format gateway;
+ u32 interface_hndl;
What is expected data here? Why we need *interface_hndl* here.
We also have a *port_number* as struct member.
+ u32 vlan_priority;
+ u32 flags;
Again *flags* is supposed to be set to what ?
+ u8 iscsiname[224];
+ u8 iscsialias[32];
+ u32 ip_type;
+ u32 retry_count;
+ struct iscsi_ip_addr_record_params ip_params;
Can we add *u32 vendor_data[0]* for vendor specific data?
+} __packed;
+
+/* ISCSI_BSG_HST_NET_CONFIG : */
+
+/* bsg network parameter ids */
+enum iscsi_bsg_netparam {
+ BSG_NETPARAM_UNKNOWN= 0x,
+ BSG_NETPARAM_MAC= BSG_NETPARAM_UNKNOWN + 1,
+ BSG_NETPARAM_MTU= BSG_NETPARAM_UNKNOWN + 2,
+ BSG_NETPARAM_VLAN = BSG_NETPARAM_UNKNOWN + 3,
+ BSG_NETPARAM_BOOTPROTO = BSG_NETPARAM_UNKNOWN + 4,
+ BSG_NETPARAM_IPV4_ADDR = BSG_NETPARAM_UNKNOWN + 5,
+ BSG_NETPARAM_IPV4_MASK = BSG_NETPARAM_UNKNOWN + 6,
+ BSG_NETPARAM_IPV4_GW= BSG_NETPARAM_UNKNOWN + 7,
+ BSG_NETPARAM_IPV6_ADDR = BSG_NETPARAM_UNKNOWN + 8,
+ BSG_NETPARAM_IPV6_MASK = BSG_NETPARAM_UNKNOWN + 9,
+ BSG_NETPARAM_IPV6_GW= BSG_NETPARAM_UNKNOWN + 10,
+ BSG_NETPARAM_IPV6_AUTO_PARAM= BSG_NETPARAM_UNKNOWN + 11,
+};
+
+/* network parameter TLV (type, length, value) structure */
+struct iscsi_bsg_netparam_tlv {
+ uint32_tnetparam;
+ uint32_tlength;
+ uint8_t value[1];
+};
So *value[1]* you mean *value[0]*, just to be consistent ? Also the idea behind
this TLV is for App's to set whatever network param needs to be set or get in
one
or multiple shot, can be configured and passed back forth ? Is that how
you guys envisioned it ?
+
+/* Request:
+ * This message sets or gets Network configuration parameters on/from the
+ * iscsi host network interface.
+ *
+ * On set operations, the request payload buffer contains a set of
+ * netparam tlv's with the values to be set.
+ *
+ * On get operations, the request payload is unused.
+ */
+struct iscsi_bsg_host_net_config {
+ /*
+ * Specifies the operation type: 0x0 = GET; 0x1 = SET
+ */
+ uint8_t set_op;
Looking at the implementation, looks like *set_op* is set to ISCSI_SET_IP_ADDR
for ex, ? Basically not consistent as outlined in the comment.
+
+ /*
+ * If SET operation, contains the total number of netparam tlv's
+ * in the request payload.
+ */
+ uint8_t netparam_count;
+};
--
You received this message because you are subscribed to the Google Groups
open-iscsi group.
To post to this group, send email to open-is...@googlegroups.com.
To unsubscribe from this group, send email to
open-iscsi+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/open-iscsi?hl=en.
Re: [PATCH 1/2] RFC: Proposal for BSG Interface
On 03/17/2010 08:06 PM, Jayamohan Kallickal wrote: This patch contains the bsg interface based on the bsg interface in FC. What? where? who? how? why? w*\? ... You are proposing a new Kernel ABI/API here right. I think it is not acceptable without a detailed documentation, including every little bit. Nothing missing. If lots of it is already written for FC then grate most of your job is done. But for us mortals please put some text on an introductory message. I use iscsi everyday for 4 years, your mail should answer the question: what am I missing? Developed and submitted earlier by James Smart. Signed-off-by: James Smart james.sm...@emulex.com Signed-off-by: Jayamohan Kallickal jayamoh...@serverengines.com Boaz -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
[PATCH 1/2] RFC: Proposal for BSG Interface
This patch contains the bsg interface based on
the bsg interface in FC.
Developed and submitted earlier by James Smart.
Signed-off-by: James Smart james.sm...@emulex.com
Signed-off-by: Jayamohan Kallickal jayamoh...@serverengines.com
---
drivers/scsi/scsi_transport_iscsi.c | 459 ++-
include/scsi/Kbuild |1 +
include/scsi/scsi_bsg_iscsi.h | 253 +++
include/scsi/scsi_transport_iscsi.h | 47
4 files changed, 759 insertions(+), 1 deletions(-)
create mode 100644 include/scsi/scsi_bsg_iscsi.h
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index ea3892e..2b7218c 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -29,6 +29,8 @@
#include scsi/scsi_transport.h
#include scsi/scsi_transport_iscsi.h
#include scsi/iscsi_if.h
+#include scsi/scsi_bsg_iscsi.h
+#include scsi/scsi_cmnd.h
#define ISCSI_SESSION_ATTRS 22
#define ISCSI_CONN_ATTRS 13
@@ -268,6 +270,447 @@ struct iscsi_endpoint *iscsi_lookup_endpoint(u64 handle)
}
EXPORT_SYMBOL_GPL(iscsi_lookup_endpoint);
+
+/*
+ * BSG support
+ */
+
+/**
+ * iscsi_destroy_bsgjob - routine to teardown/delete a iscsi bsg job
+ * @job: iscsi_bsg_job that is to be torn down
+ */
+static void
+iscsi_destroy_bsgjob(struct iscsi_bsg_job *job)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(job-job_lock, flags);
+ if (job-ref_cnt) {
+ spin_unlock_irqrestore(job-job_lock, flags);
+ return;
+ }
+ spin_unlock_irqrestore(job-job_lock, flags);
+
+ put_device(job-dev); /* release reference for the request */
+
+ kfree(job-request_payload.sg_list);
+ kfree(job-reply_payload.sg_list);
+ kfree(job);
+}
+
+/**
+ * iscsi_bsg_jobdone - completion routine for bsg requests that the LLD has
+ * completed
+ * @job: iscsi_bsg_job that is complete
+ */
+static void
+iscsi_bsg_jobdone(struct iscsi_bsg_job *job)
+{
+ struct request *req = job-req;
+ struct request *rsp = req-next_rq;
+ int err;
+
+ err = job-req-errors = job-reply-result;
+
+ if (err 0)
+ /* we're only returning the result field in the reply */
+ job-req-sense_len = sizeof(uint32_t);
+ else
+ job-req-sense_len = job-reply_len;
+
+ /* we assume all request payload was transferred, residual == 0 */
+ req-resid_len = 0;
+
+ if (rsp) {
+ WARN_ON(job-reply-reply_payload_rcv_len rsp-resid_len);
+
+ /* set reply (bidi) residual */
+ rsp-resid_len -= min(job-reply-reply_payload_rcv_len,
+ rsp-resid_len);
+ }
+ blk_complete_request(req);
+}
+
+/**
+ * iscsi_bsg_softirq_done - softirq done routine for destroying the bsg
requests
+ * @rq:BSG request that holds the job to be destroyed
+ */
+static void
+iscsi_bsg_softirq_done(struct request *rq)
+{
+ struct iscsi_bsg_job *job = rq-special;
+ unsigned long flags;
+
+ spin_lock_irqsave(job-job_lock, flags);
+ job-state_flags |= ISCSI_RQST_STATE_DONE;
+ job-ref_cnt--;
+ spin_unlock_irqrestore(job-job_lock, flags);
+
+ blk_end_request_all(rq, rq-errors);
+ iscsi_destroy_bsgjob(job);
+}
+
+/**
+ * iscsi_bsg_job_timeout - handler for when a bsg request timesout
+ * @req: request that timed out
+ */
+static enum blk_eh_timer_return
+iscsi_bsg_job_timeout(struct request *req)
+{
+ struct iscsi_bsg_job *job = (void *) req-special;
+ struct Scsi_Host *shost = job-shost;
+ struct iscsi_internal *i = to_iscsi_internal(shost-transportt);
+ unsigned long flags;
+ int err = 0, done = 0;
+
+ spin_lock_irqsave(job-job_lock, flags);
+ if (job-state_flags ISCSI_RQST_STATE_DONE)
+ done = 1;
+ else
+ job-ref_cnt++;
+ spin_unlock_irqrestore(job-job_lock, flags);
+
+ if (!done i-iscsi_transport-bsg_timeout) {
+ /* call LLDD to abort the i/o as it has timed out */
+ err = i-iscsi_transport-bsg_timeout(job);
+ if (err == -EAGAIN) {
+ job-ref_cnt--;
+ return BLK_EH_RESET_TIMER;
+ } else if (err)
+ printk(KERN_ERR ERROR: iSCSI BSG request timeout -
+ LLD abort failed with status %d\n, err);
+ }
+
+ /* the blk_end_sync_io() doesn't check the error */
+ if (done)
+ return BLK_EH_NOT_HANDLED;
+ else
+ return BLK_EH_HANDLED;
+}
+
+static int
+iscsi_bsg_map_buffer(struct iscsi_bsg_buffer *buf, struct request *req)
+{
+ size_t sz = (sizeof(struct scatterlist) * req-nr_phys_segments);
+
+ BUG_ON(!req-nr_phys_segments);
+
+ buf-sg_list = kzalloc(sz, GFP_KERNEL);
+ if (!buf-sg_list)
