Skip to contentSkip to conversation

scsi_netlink: Add transport and LLD recieve and event support

PATCH In review James Smart 2 participants 4 messages Aug 9, 2008 – Sep 2, 2008 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 scsi_netlink: Add transport and LLD recieve and event support

This patch adds scsi netlink recieve and event support for transport and scsi LLDD's. It is a reimplementation of the patch posted last week by David Somayajulu.
http://marc.info/?l=linux-scsi&m=121745486221819&w=2

There are a few things done differently:

- Transport support is included

- Event delivery is included

- The vendor message is now its own unique message type, considered part of the generic "SCSI Transport".

- LLDD entry points are now registered rather than included in the scsi_host_template.

Background: When I started to implement the event handler via template, I had to either: muck up scsi_add_host and scsi_remove_host; or have the event handler search all possible shosts. Neither was acceptable. Moving to a registration solves this, and also limits the scope of the changes to something that could be backported to a distro without breaking an already-released-distro kabi. However, I admit it isn't as elegant, as the passing of the LLDD host template in the registration and the complexity around dynamic add/remove shows.

- The receive path was augmented to require a unique identifier for the LLDD before the message was allowed to be handed off to the driver. Given how quickly very fatal errors occur if there's msg mismatches (which I saw in testing my own tools :), I believe this to be a very good thing. The id plays off the vendor id scheme already introduced for the vendor unique event messages used by FC. Additionally, the id use as the basis of the registration/deregistration.

- Send assist functions, for both the transport and LLDDs are included.

-- james s

---

 drivers/scsi/scsi_netlink.c |  523 +++++++++++++++++++++++++++++++++++++++++++-
 include/scsi/scsi_netlink.h |   62 +++++
 2 files changed, 576 insertions(+), 9 deletions(-)
drivers/scsi/scsi_netlink.c +515 -8
diff -upNr a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
@@ -21,6 +21,7 @@
 #include <linux/time.h>
 #include <linux/jiffies.h>
 #include <linux/security.h>
+#include <linux/delay.h>
 #include <net/sock.h>
 #include <net/netlink.h>
 
@@ -30,6 +31,39 @@
 struct sock *scsi_nl_sock = NULL;
 EXPORT_SYMBOL_GPL(scsi_nl_sock);
 
+static DEFINE_SPINLOCK(scsi_nl_lock);
+static struct list_head scsi_nl_drivers;
+
+static u32	scsi_nl_state;
+#define STATE_EHANDLER_BSY		0x00000001
+
+struct scsi_nl_transport {
+	int (*msg_handler)(struct sk_buff *);
+	void (*event_handler)(struct notifier_block *, unsigned long, void *);
+	unsigned int refcnt;
+	int flags;
+};
+
+/* flags values (bit flags) */
+#define HANDLER_DELETING		0x1
+
+static struct scsi_nl_transport transports[SCSI_NL_MAX_TRANSPORTS] =
+	{ {NULL, }, };
+
+
+struct scsi_nl_drvr {
+	struct list_head next;
+	int (*dmsg_handler)(struct Scsi_Host *shost, void *payload,
+				 u32 len, u32 pid);
+	void (*devt_handler)(struct notifier_block *nb,
+				 unsigned long event, void *notify_ptr);
+	struct scsi_host_template *hostt;
+	u64 vendor_id;
+	unsigned int refcnt;
+	int flags;
+};
+
+
 
 /**
  * scsi_nl_rcv_msg - Receive message handler.
@@ -45,8 +79,9 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 {
 	struct nlmsghdr *nlh;
 	struct scsi_nl_hdr *hdr;
-	uint32_t rlen;
-	int err;
+	unsigned long flags;
+	u32 rlen;
+	int err, tport;
 
 	while (skb->len >= NLMSG_SPACE(0)) {
 		err = 0;
@@ -65,7 +100,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 
 		if (nlh->nlmsg_type != SCSI_TRANSPORT_MSG) {
 			err = -EBADMSG;
-			return;
+			goto next_msg;
 		}
 
 		hdr = NLMSG_DATA(nlh);
@@ -83,12 +118,27 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 		if (nlh->nlmsg_len < (sizeof(*nlh) + hdr->msglen)) {
 			printk(KERN_WARNING "%s: discarding partial message\n",
 				 __func__);
-			return;
+			goto next_msg;
 		}
 
 		/*
-		 * We currently don't support anyone sending us a message
+		 * Deliver message to the appropriate transport
 		 */
+		spin_lock_irqsave(&scsi_nl_lock, flags);
+
+		tport = hdr->transport;
+		if ((tport < SCSI_NL_MAX_TRANSPORTS) &&
+		    !(transports[tport].flags & HANDLER_DELETING) &&
+		    (transports[tport].msg_handler)) {
+			transports[tport].refcnt++;
+			spin_unlock_irqrestore(&scsi_nl_lock, flags);
+			err = transports[tport].msg_handler(skb);
+			spin_lock_irqsave(&scsi_nl_lock, flags);
+			transports[tport].refcnt--;
+		} else
+			err = -ENOENT;
+
+		spin_unlock_irqrestore(&scsi_nl_lock, flags);
 
 next_msg:
 		if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
@@ -110,14 +160,42 @@ static int
 scsi_nl_rcv_event(struct notifier_block *this, unsigned long event, void *ptr)
 {
 	struct netlink_notify *n = ptr;
+	struct scsi_nl_drvr *driver;
+	unsigned long flags;
+	int tport;
 
 	if (n->protocol != NETLINK_SCSITRANSPORT)
 		return NOTIFY_DONE;
 
+	spin_lock_irqsave(&scsi_nl_lock, flags);
+	scsi_nl_state |= STATE_EHANDLER_BSY;
+
+	/*
+	 * Pass event on to any transports that may be listening
+	 */
+	for (tport = 0; tport < SCSI_NL_MAX_TRANSPORTS; tport++) {
+		if (!(transports[tport].flags & HANDLER_DELETING) &&
+		    (transports[tport].event_handler)) {
+			spin_unlock_irqrestore(&scsi_nl_lock, flags);
+			transports[tport].event_handler(this, event, ptr);
+			spin_lock_irqsave(&scsi_nl_lock, flags);
+		}
+	}
+
 	/*
-	 * Currently, we are not tracking PID's, etc. There is nothing
-	 * to handle.
+	 * Pass event on to any drivers that may be listening
 	 */
+	list_for_each_entry(driver, &scsi_nl_drivers, next) {
+		if (!(driver->flags & HANDLER_DELETING) &&
+		    (driver->devt_handler)) {
+			spin_unlock_irqrestore(&scsi_nl_lock, flags);
+			driver->devt_handler(this, event, ptr);
+			spin_lock_irqsave(&scsi_nl_lock, flags);
+		}
+	}
+
+	scsi_nl_state &= ~STATE_EHANDLER_BSY;
+	spin_unlock_irqrestore(&scsi_nl_lock, flags);
 
 	return NOTIFY_DONE;
 }
@@ -128,7 +206,281 @@ static struct notifier_block scsi_netlin
 
 
 /**
- * scsi_netlink_init - Called by SCSI subsystem to intialize the SCSI transport netlink interface
+ * GENERIC SCSI transport receive and event handlers
+ **/
+
+/**
+ * scsi_generic_msg_handler - receive message handler for GENERIC transport
+ * 			 messages
+ *
+ * @skb:		socket receive buffer
+ *
+ **/
+static int
+scsi_generic_msg_handler(struct sk_buff *skb)
+{
+	struct nlmsghdr *nlh = nlmsg_hdr(skb);
+	struct scsi_nl_hdr *snlh = NLMSG_DATA(nlh);
+	struct scsi_nl_drvr *driver;
+	struct Scsi_Host *shost;
+	unsigned long flags;
+	int err = 0, match, pid;
+
+	pid = NETLINK_CREDS(skb)->pid;
+
+	switch (snlh->msgtype) {
+	case SCSI_NL_SHOST_VENDOR:
+		{
+		struct scsi_nl_host_vendor_msg *msg = NLMSG_DATA(nlh);
+
+		/* Locate the driver that corresponds to the message */
+		spin_lock_irqsave(&scsi_nl_lock, flags);
+		match = 0;
+		list_for_each_entry(driver, &scsi_nl_drivers, next) {
+			if (driver->vendor_id == msg->vendor_id) {
+				match = 1;
+				break;
+			}
+		}
+
+		if ((!match) || (!driver->dmsg_handler)) {
+			spin_unlock_irqrestore(&scsi_nl_lock, flags);
+			err = -ESRCH;
+			goto rcv_exit;
+		}
+
+		if (driver->flags & HANDLER_DELETING) {
+			spin_unlock_irqrestore(&scsi_nl_lock, flags);
+			err = -ESHUTDOWN;
+			goto rcv_exit;
+		}
+
+		driver->refcnt++;
+		spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+
+		/* if successful, scsi_host_lookup takes a shost reference */
+		shost = scsi_host_lookup(msg->host_no);
+		if (!shost) {
+			err = -ENODEV;
+			goto driver_exit;
+		}
+
+		/* is this host owned by the vendor ? */
+		if (shost->hostt != driver->hostt) {
+			err = -EINVAL;
+			goto vendormsg_put;
+		}
+
+		/* pass message on to the driver */
+		err = driver->dmsg_handler(shost, (void *)&msg[1],
+					 msg->vmsg_datalen, pid);
+
+vendormsg_put:
+		/* release reference by scsi_host_lookup */
+		scsi_host_put(shost);
+
+driver_exit:
+		/* release our own reference on the registration object */
+		spin_lock_irqsave(&scsi_nl_lock, flags);
+		driver->refcnt--;
+		spin_unlock_irqrestore(&scsi_nl_lock, flags);
+		break;
+		}
+
+	default:
+		err = -EBADR;
+		break;
+	}
+
+rcv_exit:
+	if (err)
+		printk(KERN_WARNING "%s: Msgtype %d failed - err %d\n",
+			 __func__, snlh->msgtype, err);
+	return err;
+}
+
+
+/**
+ * scsi_nl_add_transport -
+ *    Registers message and event handlers for a transport. Enables
+ *    receipt of netlink messages and events to a transport.
+ *
+ * @tport:		transport registering handlers
+ * @msg_handler:	receive message handler callback
+ * @event_handler:	receive event handler callback
+ **/
+int
+scsi_nl_add_transport(u8 tport,
+	int (*msg_handler)(struct sk_buff *),
+	void (*event_handler)(struct notifier_block *, unsigned long, void *))
+{
+	unsigned long flags;
+	int err = 0;
+
+	if (tport >= SCSI_NL_MAX_TRANSPORTS)
+		return -EINVAL;
+
+	spin_lock_irqsave(&scsi_nl_lock, flags);
+
+	if (scsi_nl_state & STATE_EHANDLER_BSY) {
+		spin_unlock_irqrestore(&scsi_nl_lock, flags);
+		msleep(1);
+		spin_lock_irqsave(&scsi_nl_lock, flags);
+	}
+
+	if (transports[tport].msg_handler || transports[tport].event_handler) {
+		err = -EALREADY;
+		goto register_out;
+	}
+
+	transports[tport].msg_handler = msg_handler;
+	transports[tport].event_handler = event_handler;
+	transports[tport].flags = 0;
+	transports[tport].refcnt = 0;
+
+register_out:
+	spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_add_transport);
+
+
+/**
+ * scsi_nl_remove_transport -
+ *    Disable transport receiption of messages and events
+ *
+ * @tport:		transport deregistering handlers
+ *
+ **/
+void
+scsi_nl_remove_transport(u8 tport)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&scsi_nl_lock, flags);
+	if (scsi_nl_state & STATE_EHANDLER_BSY) {
+		spin_unlock_irqrestore(&scsi_nl_lock, flags);
+		msleep(1);
+		spin_lock_irqsave(&scsi_nl_lock, flags);
+	}
+
+	if (tport < SCSI_NL_MAX_TRANSPORTS) {
+		transports[tport].flags |= HANDLER_DELETING;
+
+		while (transports[tport].refcnt != 0) {
+			spin_unlock_irqrestore(&scsi_nl_lock, flags);
+			schedule_timeout_uninterruptible(HZ/4);
+			spin_lock_irqsave(&scsi_nl_lock, flags);
+		}
+		transports[tport].msg_handler = NULL;
+		transports[tport].event_handler = NULL;
+		transports[tport].flags = 0;
+	}
+
+	spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+	return;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_remove_transport);
+
+
+/**
+ * scsi_nl_add_driver -
+ *    A driver is registering its interfaces for SCSI netlink messages
+ *
+ * @vendor_id:          A unique identification value for the driver.
+ * @hostt:		address of the driver's host template. Used
+ *			to verify an shost is bound to the driver
+ * @nlmsg_handler:	receive message handler callback
+ * @nlevt_handler:	receive event handler callback
+ *
+ * Returns:
+ *   0 on Success
+ *   error result otherwise
+ **/
+int
+scsi_nl_add_driver(u64 vendor_id, struct scsi_host_template *hostt,
+	int (*nlmsg_handler)(struct Scsi_Host *shost, void *payload,
+				 u32 len, u32 pid),
+	void (*nlevt_handler)(struct notifier_block *nb,
+				 unsigned long event, void *notify_ptr))
+{
+	struct scsi_nl_drvr *driver;
+	unsigned long flags;
+
+	driver = kzalloc(sizeof(*driver), GFP_KERNEL);
+	if (unlikely(!driver)) {
+		printk(KERN_ERR "%s: allocation failure\n", __func__);
+		return -ENOMEM;
+	}
+
+	driver->dmsg_handler = nlmsg_handler;
+	driver->devt_handler = nlevt_handler;
+	driver->hostt = hostt;
+	driver->vendor_id = vendor_id;
+
+	spin_lock_irqsave(&scsi_nl_lock, flags);
+	if (scsi_nl_state & STATE_EHANDLER_BSY) {
+		spin_unlock_irqrestore(&scsi_nl_lock, flags);
+		msleep(1);
+		spin_lock_irqsave(&scsi_nl_lock, flags);
+	}
+	list_add_tail(&driver->next, &scsi_nl_drivers);
+	spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_add_driver);
+
+
+/**
+ * scsi_nl_remove_driver -
+ *    An driver is unregistering with the SCSI netlink messages
+ *
+ * @vendor_id:          The unique identification value for the driver.
+ **/
+void
+scsi_nl_remove_driver(u64 vendor_id)
+{
+	struct scsi_nl_drvr *driver;
+	unsigned long flags;
+
+	spin_lock_irqsave(&scsi_nl_lock, flags);
+	if (scsi_nl_state & STATE_EHANDLER_BSY) {
+		spin_unlock_irqrestore(&scsi_nl_lock, flags);
+		msleep(1);
+		spin_lock_irqsave(&scsi_nl_lock, flags);
+	}
+
+	list_for_each_entry(driver, &scsi_nl_drivers, next) {
+		if (driver->vendor_id == vendor_id) {
+			driver->flags |= HANDLER_DELETING;
+			while (driver->refcnt != 0) {
+				spin_unlock_irqrestore(&scsi_nl_lock, flags);
+				schedule_timeout_uninterruptible(HZ/4);
+				spin_lock_irqsave(&scsi_nl_lock, flags);
+			}
+			list_del(&driver->next);
+			kfree(driver);
+			spin_unlock_irqrestore(&scsi_nl_lock, flags);
+			return;
+		}
+	}
+
+	spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+	printk(KERN_ERR "%s: removal of driver failed - vendor_id 0x%llx\n",
+				__func__, vendor_id);
+	return;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_remove_driver);
+
+
+/**
+ * scsi_netlink_init - Called by SCSI subsystem to intialize
+ * 	the SCSI transport netlink interface
  *
  **/
 void
@@ -136,6 +488,8 @@ scsi_netlink_init(void)
 {
 	int error;
 
+	INIT_LIST_HEAD(&scsi_nl_drivers);
+
 	error = netlink_register_notifier(&scsi_netlink_notifier);
 	if (error) {
 		printk(KERN_ERR "%s: register of event handler failed - %d\n",
@@ -150,8 +504,15 @@ scsi_netlink_init(void)
 		printk(KERN_ERR "%s: register of recieve handler failed\n",
 				__func__);
 		netlink_unregister_notifier(&scsi_netlink_notifier);
+		return;
 	}
 
+	/* Register the entry points for the generic SCSI transport */
+	error = scsi_nl_add_transport(SCSI_NL_TRANSPORT,
+				scsi_generic_msg_handler, NULL);
+	if (error)
+		printk(KERN_ERR "%s: register of GENERIC transport handler"
+				"  failed - %d\n", __func__, error);
 	return;
 }
 
@@ -163,6 +524,8 @@ scsi_netlink_init(void)
 void
 scsi_netlink_exit(void)
 {
+	scsi_nl_remove_transport(SCSI_NL_TRANSPORT);
+
 	if (scsi_nl_sock) {
 		netlink_kernel_release(scsi_nl_sock);
 		netlink_unregister_notifier(&scsi_netlink_notifier);
@@ -172,3 +535,147 @@ scsi_netlink_exit(void)
 }
 
 
+/*
+ * Exported Interfaces
+ */
+
+/**
+ * scsi_nl_send_transport_msg -
+ *    Generic function to send a single message from a SCSI transport to
+ *    a single process
+ *
+ * @pid:		receiving pid
+ * @hdr:		message payload
+ *
+ **/
+void
+scsi_nl_send_transport_msg(u32 pid, struct scsi_nl_hdr *hdr)
+{
+	struct sk_buff *skb;
+	struct nlmsghdr	*nlh;
+	const char *fn;
+	char *datab;
+	u32 len, skblen;
+	int err;
+
+	if (!scsi_nl_sock) {
+		err = -ENOENT;
+		fn = "netlink socket";
+		goto msg_fail;
+	}
+
+	len = NLMSG_SPACE(hdr->msglen);
+	skblen = NLMSG_SPACE(len);
+
+	skb = alloc_skb(skblen, GFP_KERNEL);
+	if (!skb) {
+		err = -ENOBUFS;
+		fn = "alloc_skb";
+		goto msg_fail;
+	}
+
+	nlh = nlmsg_put(skb, pid, 0, SCSI_TRANSPORT_MSG, len - sizeof(*nlh), 0);
+	if (!nlh) {
+		err = -ENOBUFS;
+		fn = "nlmsg_put";
+		goto msg_fail_skb;
+	}
+	datab = NLMSG_DATA(nlh);
+	memcpy(datab, hdr, hdr->msglen);
+
+	err = nlmsg_unicast(scsi_nl_sock, skb, pid);
+	if (err < 0) {
+		fn = "nlmsg_unicast";
+		/* nlmsg_unicast already kfree_skb'd */
+		goto msg_fail;
+	}
+
+	return;
+
+msg_fail_skb:
+	kfree_skb(skb);
+msg_fail:
+	printk(KERN_WARNING
+		"%s: Dropped Message : pid %d Transport %d, msgtype x%x, "
+		"msglen %d: %s : err %d\n",
+		__func__, pid, hdr->transport, hdr->msgtype, hdr->msglen,
+		fn, err);
+	return;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_send_transport_msg);
+
+
+/**
+ * scsi_nl_send_vendor_msg - called to send a shost vendor unique message
+ *                      to a specific process id.
+ *
+ * @pid:		process id of the receiver
+ * @host_no:		host # sending the message
+ * @vendor_id:		unique identifier for the driver's vendor
+ * @data_len:		amount, in bytes, of vendor unique payload data
+ * @data_buf:		pointer to vendor unique data buffer
+ *
+ * Returns:
+ *   0 on succesful return
+ *   otherwise, failing error code
+ *
+ * Notes:
+ *	This routine assumes no locks are held on entry.
+ */
+int
+scsi_nl_send_vendor_msg(u32 pid, unsigned short host_no, u64 vendor_id,
+			 char *data_buf, u32 data_len)
+{
+	struct sk_buff *skb;
+	struct nlmsghdr	*nlh;
+	struct scsi_nl_host_vendor_msg *msg;
+	u32 len, skblen;
+	int err;
+
+	if (!scsi_nl_sock) {
+		err = -ENOENT;
+		goto send_vendor_fail;
+	}
+
+	len = SCSI_NL_MSGALIGN(sizeof(*msg) + data_len);
+	skblen = NLMSG_SPACE(len);
+
+	skb = alloc_skb(skblen, GFP_KERNEL);
+	if (!skb) {
+		err = -ENOBUFS;
+		goto send_vendor_fail;
+	}
+
+	nlh = nlmsg_put(skb, 0, 0, SCSI_TRANSPORT_MSG,
+				skblen - sizeof(*nlh), 0);
+	if (!nlh) {
+		err = -ENOBUFS;
+		goto send_vendor_fail_skb;
+	}
+	msg = NLMSG_DATA(nlh);
+
+	INIT_SCSI_NL_HDR(&msg->snlh, SCSI_NL_TRANSPORT,
+				SCSI_NL_SHOST_VENDOR, len);
+	msg->vendor_id = vendor_id;
+	msg->host_no = host_no;
+	msg->vmsg_datalen = data_len;	/* bytes */
+	memcpy(&msg[1], data_buf, data_len);
+
+	err = nlmsg_unicast(scsi_nl_sock, skb, pid);
+	if (err)
+		/* nlmsg_multicast already kfree_skb'd */
+		goto send_vendor_fail;
+
+	return 0;
+
+send_vendor_fail_skb:
+	kfree_skb(skb);
+send_vendor_fail:
+	printk(KERN_WARNING
+		"%s: Dropped SCSI Msg : host %d vendor_unique - err %d\n",
+		__func__, host_no, err);
+	return err;
+}
+EXPORT_SYMBOL(scsi_nl_send_vendor_msg);
+
+
include/scsi/scsi_netlink.h +61 -1

79 lines not on this page · raw message

#1 lore raw

Re: [PATCH] scsi_netlink: Add transport and LLD receive and event support

Quoted 5 lines · James Smart

This patch adds scsi netlink recieve and event support for transport and scsi LLDD's. It is a reimplementation of the patch posted last week by David Somayajulu.
http://marc.info/?l=linux-scsi&m=121745486221819&w=2

My appologies for not commenting earlier. This patch from my viewpoint essentially consolidates everything (accounts for the added complexity) under the SCSI Netlink Transport (loosely meant). It would work fine for us.

However I have one minor gripe, which is regarding the vendor unique id in the receive path. This makes it appear as if the scsi netlink layer is peeking into its payload (where each msg is treated as <hdr -containing address information><payload>). In the network stack, each layer looks at the packet it receives and consumes if it is the final destination, otherwise determines the next destination from the header and sends it. No layer peeks into the payload if it is not the consumer. The only integrity check is typically a checksum. I would suggest that we augment the " struct scsi_nl_hdr" with a bunch of reserved bytes and which are interpreted based on the scsi_nl_hdr->msgtype values, or have "struct scsi_nl_host_vendor_msg" sans "struct scsi_nl_hdr" as part of a union within "struct scsi_nl_hdr" which is interpreted based on scsi_nl_hdr->msgtype values. In all this is a minor issue which is more about details and doesn't matter either way.

Thanks
David S.

Quoted 753 lines

There are a few things done differently:

- Transport support is included

- Event delivery is included

- The vendor message is now its own unique message type, considered part of the generic "SCSI Transport".

- LLDD entry points are now registered rather than included in the scsi_host_template.

Background: When I started to implement the event handler via template, I had to either: muck up scsi_add_host and scsi_remove_host; or have the event handler search all possible shosts. Neither was acceptable. Moving to a registration solves this, and also limits the scope of the changes to something that could be backported to a distro without breaking an already-released-distro kabi. However, I admit it isn't as elegant, as the passing of the LLDD host template in the registration and the complexity around dynamic add/remove shows.

- The receive path was augmented to require a unique identifier for the LLDD before the message was allowed to be handed off to the driver. Given how quickly very fatal errors occur if there's msg mismatches (which I saw in testing my own tools :), I believe this to be a very good thing. The id plays off the vendor id scheme already introduced for the vendor unique event messages used by FC. Additionally, the id use as the basis of the registration/deregistration.

- Send assist functions, for both the transport and LLDDs are included.

-- james s

---

 drivers/scsi/scsi_netlink.c |  523
+++++++++++++++++++++++++++++++++++++++++++-
 include/scsi/scsi_netlink.h |   62 +++++
 2 files changed, 576 insertions(+), 9 deletions(-)

diff -upNr a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c

drivers/scsi/scsi_netlink.c 2008-08-08 01:35:33.000000000 -0400 renamed from drivers/scsi/scsi_netlink.c 2008-07-30 13:05:12.000000000 -0400 +85 -7
@@ -21,6 +21,7 @@
 #include <linux/time.h>
 #include <linux/jiffies.h>
 #include <linux/security.h>
+#include <linux/delay.h>
 #include <net/sock.h>
 #include <net/netlink.h>
 
@@ -30,6 +31,39 @@
 struct sock *scsi_nl_sock = NULL;
 EXPORT_SYMBOL_GPL(scsi_nl_sock);
 
+static DEFINE_SPINLOCK(scsi_nl_lock);
+static struct list_head scsi_nl_drivers;
+
+static u32 scsi_nl_state;
+#define STATE_EHANDLER_BSY  0x00000001
+
+struct scsi_nl_transport {
+ int (*msg_handler)(struct sk_buff *);
+ void (*event_handler)(struct notifier_block *, unsigned long, void *);
+ unsigned int refcnt;
+ int flags;
+};
+
+/* flags values (bit flags) */
+#define HANDLER_DELETING  0x1
+
+static struct scsi_nl_transport transports[SCSI_NL_MAX_TRANSPORTS] =
+ { {NULL, }, };
+
+
+struct scsi_nl_drvr {
+ struct list_head next;
+ int (*dmsg_handler)(struct Scsi_Host *shost, void *payload,
+     u32 len, u32 pid);
+ void (*devt_handler)(struct notifier_block *nb,
+     unsigned long event, void *notify_ptr);
+ struct scsi_host_template *hostt;
+ u64 vendor_id;
+ unsigned int refcnt;
+ int flags;
+};
+
+
 
 /**
  * scsi_nl_rcv_msg - Receive message handler.
@@ -45,8 +79,9 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 {
 struct nlmsghdr *nlh;
 struct scsi_nl_hdr *hdr;
- uint32_t rlen;
- int err;
+ unsigned long flags;
+ u32 rlen;
+ int err, tport;
 
 while (skb->len >= NLMSG_SPACE(0)) {
 err = 0;
@@ -65,7 +100,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 
 if (nlh->nlmsg_type != SCSI_TRANSPORT_MSG) {
 err = -EBADMSG;
-   return;
+   goto next_msg;
 }
 
 hdr = NLMSG_DATA(nlh);
@@ -83,12 +118,27 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 if (nlh->nlmsg_len < (sizeof(*nlh) + hdr->msglen)) {
 printk(KERN_WARNING "%s: discarding partial message\n",
 __func__);
-   return;
+   goto next_msg;
 }
 
 /*
-   * We currently don't support anyone sending us a message
+   * Deliver message to the appropriate transport
 */
+  spin_lock_irqsave(&scsi_nl_lock, flags);
+
+  tport = hdr->transport;
+  if ((tport < SCSI_NL_MAX_TRANSPORTS) &&
+      !(transports[tport].flags & HANDLER_DELETING) &&
+      (transports[tport].msg_handler)) {
+   transports[tport].refcnt++;
+   spin_unlock_irqrestore(&scsi_nl_lock, flags);
+   err = transports[tport].msg_handler(skb);
+   spin_lock_irqsave(&scsi_nl_lock, flags);
+   transports[tport].refcnt--;
+  } else
+   err = -ENOENT;
+
+  spin_unlock_irqrestore(&scsi_nl_lock, flags);
 
 next_msg:
 if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
@@ -110,14 +160,42 @@ static int
 scsi_nl_rcv_event(struct notifier_block *this, unsigned long event, void
 *ptr)
 {
 struct netlink_notify *n = ptr;
+ struct scsi_nl_drvr *driver;
+ unsigned long flags;
+ int tport;
 
 if (n->protocol != NETLINK_SCSITRANSPORT)
 return NOTIFY_DONE;
 
+ spin_lock_irqsave(&scsi_nl_lock, flags);
+ scsi_nl_state |= STATE_EHANDLER_BSY;
+
+ /*
+  * Pass event on to any transports that may be listening
+  */
+ for (tport = 0; tport < SCSI_NL_MAX_TRANSPORTS; tport++) {
+  if (!(transports[tport].flags & HANDLER_DELETING) &&
+      (transports[tport].event_handler)) {
+   spin_unlock_irqrestore(&scsi_nl_lock, flags);
+   transports[tport].event_handler(this, event, ptr);
+   spin_lock_irqsave(&scsi_nl_lock, flags);
+  }
+ }
+
 /*
-  * Currently, we are not tracking PID's, etc. There is nothing
-  * to handle.
+  * Pass event on to any drivers that may be listening
 */
+ list_for_each_entry(driver, &scsi_nl_drivers, next) {
+  if (!(driver->flags & HANDLER_DELETING) &&
+      (driver->devt_handler)) {
+   spin_unlock_irqrestore(&scsi_nl_lock, flags);
+   driver->devt_handler(this, event, ptr);
+   spin_lock_irqsave(&scsi_nl_lock, flags);
+  }
+ }
+
+ scsi_nl_state &= ~STATE_EHANDLER_BSY;
+ spin_unlock_irqrestore(&scsi_nl_lock, flags);
 
 return NOTIFY_DONE;

}

drivers/scsi/scsi_netlink.c 2008-08-08 01:35:33.000000000 -0400 renamed from drivers/scsi/scsi_netlink.c 2008-07-30 13:05:12.000000000 -0400 +0 -1
@@ -128,7 +206,281 @@ static struct notifier_block scsi_netlin
 
 
 /**
- * scsi_netlink_init - Called by SCSI subsystem to intialize the SCSI

transport netlink interface
+ * GENERIC SCSI transport receive and event handlers
+ **/

drivers/scsi/scsi_netlink.c 2008-08-08 01:35:33.000000000 -0400 renamed from drivers/scsi/scsi_netlink.c 2008-07-30 13:05:12.000000000 -0400 +428 -0
+
+/**
+ * scsi_generic_msg_handler - receive message handler for GENERIC transport
+ *     messages
+ *
+ * @skb:  socket receive buffer
+ *
+ **/
+static int
+scsi_generic_msg_handler(struct sk_buff *skb)
+{
+ struct nlmsghdr *nlh = nlmsg_hdr(skb);
+ struct scsi_nl_hdr *snlh = NLMSG_DATA(nlh);
+ struct scsi_nl_drvr *driver;
+ struct Scsi_Host *shost;
+ unsigned long flags;
+ int err = 0, match, pid;
+
+ pid = NETLINK_CREDS(skb)->pid;
+
+ switch (snlh->msgtype) {
+ case SCSI_NL_SHOST_VENDOR:
+  {
+  struct scsi_nl_host_vendor_msg *msg = NLMSG_DATA(nlh);
+
+  /* Locate the driver that corresponds to the message */
+  spin_lock_irqsave(&scsi_nl_lock, flags);
+  match = 0;
+  list_for_each_entry(driver, &scsi_nl_drivers, next) {
+   if (driver->vendor_id == msg->vendor_id) {
+    match = 1;
+    break;
+   }
+  }
+
+  if ((!match) || (!driver->dmsg_handler)) {
+   spin_unlock_irqrestore(&scsi_nl_lock, flags);
+   err = -ESRCH;
+   goto rcv_exit;
+  }
+
+  if (driver->flags & HANDLER_DELETING) {
+   spin_unlock_irqrestore(&scsi_nl_lock, flags);
+   err = -ESHUTDOWN;
+   goto rcv_exit;
+  }
+
+  driver->refcnt++;
+  spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+
+  /* if successful, scsi_host_lookup takes a shost reference */
+  shost = scsi_host_lookup(msg->host_no);
+  if (!shost) {
+   err = -ENODEV;
+   goto driver_exit;
+  }
+
+  /* is this host owned by the vendor ? */
+  if (shost->hostt != driver->hostt) {
+   err = -EINVAL;
+   goto vendormsg_put;
+  }
+
+  /* pass message on to the driver */
+  err = driver->dmsg_handler(shost, (void *)&msg[1],
+      msg->vmsg_datalen, pid);
+
+vendormsg_put:
+  /* release reference by scsi_host_lookup */
+  scsi_host_put(shost);
+
+driver_exit:
+  /* release our own reference on the registration object */
+  spin_lock_irqsave(&scsi_nl_lock, flags);
+  driver->refcnt--;
+  spin_unlock_irqrestore(&scsi_nl_lock, flags);
+  break;
+  }
+
+ default:
+  err = -EBADR;
+  break;
+ }
+
+rcv_exit:
+ if (err)
+  printk(KERN_WARNING "%s: Msgtype %d failed - err %d\n",
+    __func__, snlh->msgtype, err);
+ return err;
+}
+
+
+/**
+ * scsi_nl_add_transport -
+ *    Registers message and event handlers for a transport. Enables
+ *    receipt of netlink messages and events to a transport.
+ *
+ * @tport:  transport registering handlers
+ * @msg_handler: receive message handler callback
+ * @event_handler: receive event handler callback
+ **/
+int
+scsi_nl_add_transport(u8 tport,
+ int (*msg_handler)(struct sk_buff *),
+ void (*event_handler)(struct notifier_block *, unsigned long, void *))
+{
+ unsigned long flags;
+ int err = 0;
+
+ if (tport >= SCSI_NL_MAX_TRANSPORTS)
+  return -EINVAL;
+
+ spin_lock_irqsave(&scsi_nl_lock, flags);
+
+ if (scsi_nl_state & STATE_EHANDLER_BSY) {
+  spin_unlock_irqrestore(&scsi_nl_lock, flags);
+  msleep(1);
+  spin_lock_irqsave(&scsi_nl_lock, flags);
+ }
+
+ if (transports[tport].msg_handler || transports[tport].event_handler) {
+  err = -EALREADY;
+  goto register_out;
+ }
+
+ transports[tport].msg_handler = msg_handler;
+ transports[tport].event_handler = event_handler;
+ transports[tport].flags = 0;
+ transports[tport].refcnt = 0;
+
+register_out:
+ spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_add_transport);
+
+
+/**
+ * scsi_nl_remove_transport -
+ *    Disable transport receiption of messages and events
+ *
+ * @tport:  transport deregistering handlers
+ *
+ **/
+void
+scsi_nl_remove_transport(u8 tport)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&scsi_nl_lock, flags);
+ if (scsi_nl_state & STATE_EHANDLER_BSY) {
+  spin_unlock_irqrestore(&scsi_nl_lock, flags);
+  msleep(1);
+  spin_lock_irqsave(&scsi_nl_lock, flags);
+ }
+
+ if (tport < SCSI_NL_MAX_TRANSPORTS) {
+  transports[tport].flags |= HANDLER_DELETING;
+
+  while (transports[tport].refcnt != 0) {
+   spin_unlock_irqrestore(&scsi_nl_lock, flags);
+   schedule_timeout_uninterruptible(HZ/4);
+   spin_lock_irqsave(&scsi_nl_lock, flags);
+  }
+  transports[tport].msg_handler = NULL;
+  transports[tport].event_handler = NULL;
+  transports[tport].flags = 0;
+ }
+
+ spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+ return;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_remove_transport);
+
+
+/**
+ * scsi_nl_add_driver -
+ *    A driver is registering its interfaces for SCSI netlink messages
+ *
+ * @vendor_id:          A unique identification value for the driver.
+ * @hostt:  address of the driver's host template. Used
+ *   to verify an shost is bound to the driver
+ * @nlmsg_handler: receive message handler callback
+ * @nlevt_handler: receive event handler callback
+ *
+ * Returns:
+ *   0 on Success
+ *   error result otherwise
+ **/
+int
+scsi_nl_add_driver(u64 vendor_id, struct scsi_host_template *hostt,
+ int (*nlmsg_handler)(struct Scsi_Host *shost, void *payload,
+     u32 len, u32 pid),
+ void (*nlevt_handler)(struct notifier_block *nb,
+     unsigned long event, void *notify_ptr))
+{
+ struct scsi_nl_drvr *driver;
+ unsigned long flags;
+
+ driver = kzalloc(sizeof(*driver), GFP_KERNEL);
+ if (unlikely(!driver)) {
+  printk(KERN_ERR "%s: allocation failure\n", __func__);
+  return -ENOMEM;
+ }
+
+ driver->dmsg_handler = nlmsg_handler;
+ driver->devt_handler = nlevt_handler;
+ driver->hostt = hostt;
+ driver->vendor_id = vendor_id;
+
+ spin_lock_irqsave(&scsi_nl_lock, flags);
+ if (scsi_nl_state & STATE_EHANDLER_BSY) {
+  spin_unlock_irqrestore(&scsi_nl_lock, flags);
+  msleep(1);
+  spin_lock_irqsave(&scsi_nl_lock, flags);
+ }
+ list_add_tail(&driver->next, &scsi_nl_drivers);
+ spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_add_driver);
+
+
+/**
+ * scsi_nl_remove_driver -
+ *    An driver is unregistering with the SCSI netlink messages
+ *
+ * @vendor_id:          The unique identification value for the driver.
+ **/
+void
+scsi_nl_remove_driver(u64 vendor_id)
+{
+ struct scsi_nl_drvr *driver;
+ unsigned long flags;
+
+ spin_lock_irqsave(&scsi_nl_lock, flags);
+ if (scsi_nl_state & STATE_EHANDLER_BSY) {
+  spin_unlock_irqrestore(&scsi_nl_lock, flags);
+  msleep(1);
+  spin_lock_irqsave(&scsi_nl_lock, flags);
+ }
+
+ list_for_each_entry(driver, &scsi_nl_drivers, next) {
+  if (driver->vendor_id == vendor_id) {
+   driver->flags |= HANDLER_DELETING;
+   while (driver->refcnt != 0) {
+    spin_unlock_irqrestore(&scsi_nl_lock, flags);
+    schedule_timeout_uninterruptible(HZ/4);
+    spin_lock_irqsave(&scsi_nl_lock, flags);
+   }
+   list_del(&driver->next);
+   kfree(driver);
+   spin_unlock_irqrestore(&scsi_nl_lock, flags);
+   return;
+  }
+ }
+
+ spin_unlock_irqrestore(&scsi_nl_lock, flags);
+
+ printk(KERN_ERR "%s: removal of driver failed - vendor_id 0x%llx\n",
+    __func__, vendor_id);
+ return;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_remove_driver);
+
+
+/**
+ * scsi_netlink_init - Called by SCSI subsystem to intialize
+ *  the SCSI transport netlink interface
  *
  **/
 void
@@ -136,6 +488,8 @@ scsi_netlink_init(void)
 {
 int error;
 
+ INIT_LIST_HEAD(&scsi_nl_drivers);
+
 error = netlink_register_notifier(&scsi_netlink_notifier);
 if (error) {
 printk(KERN_ERR "%s: register of event handler failed - %d\n",
@@ -150,8 +504,15 @@ scsi_netlink_init(void)
 printk(KERN_ERR "%s: register of recieve handler failed\n",
 __func__);
 netlink_unregister_notifier(&scsi_netlink_notifier);
+  return;
 }
 
+ /* Register the entry points for the generic SCSI transport */
+ error = scsi_nl_add_transport(SCSI_NL_TRANSPORT,
+    scsi_generic_msg_handler, NULL);
+ if (error)
+  printk(KERN_ERR "%s: register of GENERIC transport handler"
+    "  failed - %d\n", __func__, error);
 return;
 }
 
@@ -163,6 +524,8 @@ scsi_netlink_init(void)
 void
 scsi_netlink_exit(void)
 {
+ scsi_nl_remove_transport(SCSI_NL_TRANSPORT);
+
 if (scsi_nl_sock) {
 netlink_kernel_release(scsi_nl_sock);
 netlink_unregister_notifier(&scsi_netlink_notifier);
@@ -172,3 +535,147 @@ scsi_netlink_exit(void)
 }
 
 
+/*
+ * Exported Interfaces
+ */
+
+/**
+ * scsi_nl_send_transport_msg -
+ *    Generic function to send a single message from a SCSI transport to
+ *    a single process
+ *
+ * @pid:  receiving pid
+ * @hdr:  message payload
+ *
+ **/
+void
+scsi_nl_send_transport_msg(u32 pid, struct scsi_nl_hdr *hdr)
+{
+ struct sk_buff *skb;
+ struct nlmsghdr *nlh;
+ const char *fn;
+ char *datab;
+ u32 len, skblen;
+ int err;
+
+ if (!scsi_nl_sock) {
+  err = -ENOENT;
+  fn = "netlink socket";
+  goto msg_fail;
+ }
+
+ len = NLMSG_SPACE(hdr->msglen);
+ skblen = NLMSG_SPACE(len);
+
+ skb = alloc_skb(skblen, GFP_KERNEL);
+ if (!skb) {
+  err = -ENOBUFS;
+  fn = "alloc_skb";
+  goto msg_fail;
+ }
+
+ nlh = nlmsg_put(skb, pid, 0, SCSI_TRANSPORT_MSG, len - sizeof(*nlh), 0);
+ if (!nlh) {
+  err = -ENOBUFS;
+  fn = "nlmsg_put";
+  goto msg_fail_skb;
+ }
+ datab = NLMSG_DATA(nlh);
+ memcpy(datab, hdr, hdr->msglen);
+
+ err = nlmsg_unicast(scsi_nl_sock, skb, pid);
+ if (err < 0) {
+  fn = "nlmsg_unicast";
+  /* nlmsg_unicast already kfree_skb'd */
+  goto msg_fail;
+ }
+
+ return;
+
+msg_fail_skb:
+ kfree_skb(skb);
+msg_fail:
+ printk(KERN_WARNING
+  "%s: Dropped Message : pid %d Transport %d, msgtype x%x, "
+  "msglen %d: %s : err %d\n",
+  __func__, pid, hdr->transport, hdr->msgtype, hdr->msglen,
+  fn, err);
+ return;
+}
+EXPORT_SYMBOL_GPL(scsi_nl_send_transport_msg);
+
+
+/**
+ * scsi_nl_send_vendor_msg - called to send a shost vendor unique message
+ *                      to a specific process id.
+ *
+ * @pid:  process id of the receiver
+ * @host_no:  host # sending the message
+ * @vendor_id:  unique identifier for the driver's vendor
+ * @data_len:  amount, in bytes, of vendor unique payload data
+ * @data_buf:  pointer to vendor unique data buffer
+ *
+ * Returns:
+ *   0 on succesful return
+ *   otherwise, failing error code
+ *
+ * Notes:
+ * This routine assumes no locks are held on entry.
+ */
+int
+scsi_nl_send_vendor_msg(u32 pid, unsigned short host_no, u64 vendor_id,
+    char *data_buf, u32 data_len)
+{
+ struct sk_buff *skb;
+ struct nlmsghdr *nlh;
+ struct scsi_nl_host_vendor_msg *msg;
+ u32 len, skblen;
+ int err;
+
+ if (!scsi_nl_sock) {
+  err = -ENOENT;
+  goto send_vendor_fail;
+ }
+
+ len = SCSI_NL_MSGALIGN(sizeof(*msg) + data_len);
+ skblen = NLMSG_SPACE(len);
+
+ skb = alloc_skb(skblen, GFP_KERNEL);
+ if (!skb) {
+  err = -ENOBUFS;
+  goto send_vendor_fail;
+ }
+
+ nlh = nlmsg_put(skb, 0, 0, SCSI_TRANSPORT_MSG,
+    skblen - sizeof(*nlh), 0);
+ if (!nlh) {
+  err = -ENOBUFS;
+  goto send_vendor_fail_skb;
+ }
+ msg = NLMSG_DATA(nlh);
+
+ INIT_SCSI_NL_HDR(&msg->snlh, SCSI_NL_TRANSPORT,
+    SCSI_NL_SHOST_VENDOR, len);
+ msg->vendor_id = vendor_id;
+ msg->host_no = host_no;
+ msg->vmsg_datalen = data_len; /* bytes */
+ memcpy(&msg[1], data_buf, data_len);
+
+ err = nlmsg_unicast(scsi_nl_sock, skb, pid);
+ if (err)
+  /* nlmsg_multicast already kfree_skb'd */
+  goto send_vendor_fail;
+
+ return 0;
+
+send_vendor_fail_skb:
+ kfree_skb(skb);
+send_vendor_fail:
+ printk(KERN_WARNING
+  "%s: Dropped SCSI Msg : host %d vendor_unique - err %d\n",
+  __func__, host_no, err);
+ return err;
+}
+EXPORT_SYMBOL(scsi_nl_send_vendor_msg);
+
+

diff -upNr a/include/scsi/scsi_netlink.h b/include/scsi/scsi_netlink.h

include/scsi/scsi_netlink.h 2008-08-08 01:32:45.000000000 -0400 renamed from include/scsi/scsi_netlink.h 2007-07-20 13:38:39.000000000 -0400 +38 -1
@@ -22,6 +22,9 @@
 #ifndef SCSI_NETLINK_H
 #define SCSI_NETLINK_H
 
+#include <linux/netlink.h>
+
+
 /*
  * This file intended to be included by both kernel and user space
  */
@@ -55,7 +58,41 @@ struct scsi_nl_hdr {
 #define SCSI_NL_TRANSPORT_FC   1
 #define SCSI_NL_MAX_TRANSPORTS   2
 
-/* scsi_nl_hdr->msgtype values are defined in each transport */
+/* Transport-based scsi_nl_hdr->msgtype values are defined in each transport
 */
+
+/*
+ * GENERIC SCSI scsi_nl_hdr->msgtype Values
+ */
+ /* kernel -> user */
+#define SCSI_NL_SHOST_VENDOR   0x0001
+ /* user -> kernel */
+/* SCSI_NL_SHOST_VENDOR msgtype is kernel->user and user->kernel */
+
+
+/*
+ * Message Structures :
+ */
+
+/* macro to round up message lengths to 8byte boundary */
+#define SCSI_NL_MSGALIGN(len)  (((len) + 7) & ~7)
+
+
+/*
+ * SCSI HOST Vendor Unique messages :
+ *   SCSI_NL_SHOST_VENDOR
+ *
+ * Note: The Vendor Unique message payload will begin directly after
+ *   this structure, with the length of the payload per vmsg_datalen.
+ *
+ * Note: When specifying vendor_id, be sure to read the Vendor Type and ID
+ *   formatting requirements specified below
+ */
+struct scsi_nl_host_vendor_msg {
+ struct scsi_nl_hdr snlh;  /* must be 1st element ! */
+ uint64_t vendor_id;
+ uint16_t host_no;
+ uint16_t vmsg_datalen;
+} __attribute__((aligned(sizeof(uint64_t))));
 
 

/*

include/scsi/scsi_netlink.h 2008-08-08 01:32:45.000000000 -0400 renamed from include/scsi/scsi_netlink.h 2007-07-20 13:38:39.000000000 -0400 +23 -1
@@ -83,5 +120,28 @@ struct scsi_nl_hdr {
 }
 
 
+#ifdef __KERNEL__
+
+#include <scsi/scsi_host.h>
+
+/* Exported Kernel Interfaces */
+int scsi_nl_add_transport(u8 tport,
+  int (*msg_handler)(struct sk_buff *),
+ void (*event_handler)(struct notifier_block *, unsigned long, void *));
+void scsi_nl_remove_transport(u8 tport);
+
+int scsi_nl_add_driver(u64 vendor_id, struct scsi_host_template *hostt,
+ int (*nlmsg_handler)(struct Scsi_Host *shost, void *payload,
+     u32 len, u32 pid),
+ void (*nlevt_handler)(struct notifier_block *nb,
+     unsigned long event, void *notify_ptr));
+void scsi_nl_remove_driver(u64 vendor_id);
+
+void scsi_nl_send_transport_msg(u32 pid, struct scsi_nl_hdr *hdr);
+int scsi_nl_send_vendor_msg(u32 pid, unsigned short host_no, u64 vendor_id,
+    char *data_buf, u32 data_len);
+
+#endif /* __KERNEL__ */
+
 #endif /* SCSI_NETLINK_H */
 
 
 
--

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

#2 lore raw

Re: [PATCH] scsi_netlink: Add transport and LLD receive and event support

Quoted 8 lines · David Somayajulu

However I have one minor gripe, which is regarding the vendor unique id in the receive path. This makes it appear as if the scsi netlink layer is peeking into its payload (where each msg is treated as <hdr -containing address information><payload>). In the network stack, each layer looks at the packet it receives and consumes if it is the final destination, otherwise determines the next destination from the header and sends it. No layer peeks into the payload if it is not the consumer. The only integrity check is typically a checksum.

I understand your intent - but view it a little differently. I break the "stack layers" into the following:
- "scsi netlink", which is the glue logic for our SCSI channel, that everything use. It does the first level of message decode (consuming scsi_nl_hdr), performing the main validity checks (length vs netlink packet), and selecting a transport.
- "transport", which could be the SAS transport, FC transport, or the catch-all SCSI (midlayer) transport. Each transport has its own message types. It may fully handle the message, or can be a multiplexer for the components that belong to it.
- The component layer - with shosts belonging to the catch-all SCSI transport. FC components would be fc_host's, fc_rports, etc; and so-on for SAS.

In this case, the vendor shost msg maps to the catch-all SCSI transport. The SCSI transport consumes scsi_nl_host_vendor_msg, performs common steps such as driver validation, then pushes the payload to LLDD/shost for the rest of the processing. So same concepts as you describe.

There is only one violation of the "don't peek into the next layer" rule, but it's around scsi_nl_header:msgtype (not vendor unique id). The field belongs proper to the transport, but is contained in the glue header.

I agree the vendor unique id initially feels odd. We needed some handle to recognize the driver entry points by for registration/deregistration, and we also needed at least 1 validation of something to ensure the driver and the message match. Given that we had it in the event messages, I used it as the handle. I couldn't think of a better alternative.

Quoted 6 lines

I would suggest that we augment the " struct
scsi_nl_hdr" with a bunch of reserved bytes and which are interpreted based on the scsi_nl_hdr->msgtype values, or have "struct scsi_nl_host_vendor_msg" sans "struct scsi_nl_hdr" as part of a union within "struct scsi_nl_hdr" which is interpreted based on scsi_nl_hdr->msgtype values. In all this is a minor issue which is more about details and doesn't matter either way.

I don't see what this buys... Unless you are just expanding the message headers for some future binary-compatible expansion.

-- james s

#3 lore raw

RE: [PATCH] scsi_netlink: Add transport and LLD receive and event support

James Smart wrote:
Thanks for the explanation. If this accepted to go upstream, I would like to modify the qla2xxx driver patch I sent earlier (see below) to confirm to this scheme.
http://marc.info/?l=linux-scsi&m=121745486321823&w=2

thanks
-david S.

Quoted 3 lines
Quoted 1 line · David Somayajulu

However I have one minor gripe, which is regarding the vendor unique

id

Quoted 2 lines

in

Quoted 1 line

the receive path. This makes it appear as if the scsi netlink layer

is

Quoted 1 line
Quoted 1 line

peeking into its payload (where each msg is treated as <hdr

-containing

Quoted 1 line
Quoted 1 line

address information><payload>). In the network stack, each layer

looks

Quoted 3 lines

at

Quoted 2 lines

the packet it receives and consumes if it is the final destination, otherwise determines the next destination from the header and sends

it.

Quoted 6 lines

No

Quoted 1 line

layer peeks into the payload if it is not the consumer. The only

integrity

Quoted 1 line

check is typically a checksum.

I understand your intent - but view it a little differently. I break

the

Quoted 7 lines

"stack layers" into the following:
- "scsi netlink", which is the glue logic for our SCSI channel, that everything use. It does the first level of message decode (consuming scsi_nl_hdr), performing the main validity checks (length vs netlink packet), and selecting a transport.
- "transport", which could be the SAS transport, FC transport, or the catch-all SCSI (midlayer) transport. Each transport has its own

message

Quoted 1 line

types. It may fully handle the message, or can be a multiplexer for

the

Quoted 6 lines

components that belong to it.
- The component layer - with shosts belonging to the catch-all SCSI transport. FC components would be fc_host's, fc_rports, etc; and so-on for SAS.

In this case, the vendor shost msg maps to the catch-all SCSI

transport.

Quoted 6 lines

The SCSI transport consumes scsi_nl_host_vendor_msg, performs common steps such as driver validation, then pushes the payload to LLDD/shost for the rest of the processing. So same concepts as you describe.

There is only one violation of the "don't peek into the next layer" rule, but it's around scsi_nl_header:msgtype (not vendor unique id).

The

Quoted 4 lines

field belongs proper to the transport, but is contained in the glue header.

I agree the vendor unique id initially feels odd. We needed some

handle

Quoted 1 line

to recognize the driver entry points by for

registration/deregistration,

Quoted 7 lines

and we also needed at least 1 validation of something to ensure the driver and the message match. Given that we had it in the event messages, I used it as the handle. I couldn't think of a better alternative.

Quoted 2 lines

I would suggest that we augment the " struct scsi_nl_hdr" with a bunch of reserved bytes and which are

interpreted

Quoted 4 lines

based

Quoted 1 line

on the scsi_nl_hdr->msgtype values, or have "struct

scsi_nl_host_vendor_msg"

Quoted 1 line

sans "struct scsi_nl_hdr" as part of a union within "struct

scsi_nl_hdr"

Quoted 1 line
Quoted 1 line

which is interpreted based on scsi_nl_hdr->msgtype values. In all

this

Quoted 2 lines

is a

Quoted 1 line

minor issue which is more about details and doesn't matter either

way.

Quoted 2 lines

I don't see what this buys... Unless you are just expanding the

message

Quoted 3 lines

headers for some future binary-compatible expansion.

-- james s