[PATCH 1/2] scsi:netlink support in scsi and fc transports for hba specific messages

[PATCH 1/2] scsi:netlink support in scsi and fc transports for hba specific messages

[David Somayajulu]

This patch adds support to the scsi transport layer for passing hba specific netlink messages to a low level driver. Also support is added to the fc transport, to enable an FC hba driver to post netlink message to a specific user process.

Signed-off-by: David C Somayajulu <david.somayajulu@qlogic.com>
---
 drivers/scsi/scsi_netlink.c      |   13 +++++++
 drivers/scsi/scsi_transport_fc.c |   72 ++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_host.h         |    6 +++
 include/scsi/scsi_netlink.h      |    3 +-
 include/scsi/scsi_transport_fc.h |    2 +
 5 files changed, 95 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
index ae7ed9a..3bc85c9 100644
--- a/drivers/scsi/scsi_netlink.c
+++ b/drivers/scsi/scsi_netlink.c
@@ -24,6 +24,7 @@
 #include <net/sock.h>
 #include <net/netlink.h>
 
+#include <scsi/scsi_host.h>
 #include <scsi/scsi_netlink.h>
 #include "scsi_priv.h"
 
@@ -47,6 +48,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 	struct scsi_nl_hdr *hdr;
 	uint32_t rlen;
 	int err;
+	struct Scsi_Host *shost;
 
 	while (skb->len >= NLMSG_SPACE(0)) {
 		err = 0;
@@ -89,6 +91,17 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 		/*
 		 * We currently don't support anyone sending us a message
 		 */
+		if (hdr->msgtype == SCSI_NL_HOST_PRIVATE) {
+			shost = scsi_host_lookup(hdr->host_no);
+			if (!shost) {
+				printk(KERN_ERR "%s: scsi_host_lookup failed "
+					"host no %u\n", __FUNCTION__, hdr->host_no);
+			} else if (shost->hostt->netlink_rcv_msg) {
+				err = shost->hostt->netlink_rcv_msg(shost,
+					(void *)((char *)hdr+sizeof(*hdr)),
+					hdr->msglen, NETLINK_CREDS(skb)->pid);
+			}
+		}
 
 next_msg:
 		if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index cb971f0..d1b38d9 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -627,6 +627,78 @@ EXPORT_SYMBOL(fc_host_post_vendor_event);
 
 
 
+/**
+ * fc_host_post_vendor_event_to_pid - called to post a vendor unique event
+ *			on an fc_host to a specific process
+ * @shost:		host the event occurred on
+ * @event_number:	fc event number obtained from get_fc_event_number()
+ * @data_len:		amount, in bytes, of vendor unique data
+ * @data_buf:		pointer to vendor unique data
+ * @vendor_id:          Vendor id
+ * @pid:		proc id of the receiver
+ *
+ * Notes:
+ *	This routine assumes no locks are held on entry.
+ */
+void
+fc_host_post_vendor_event_to_pid(struct Scsi_Host *shost, u32 event_number,
+		u32 data_len, char * data_buf, u64 vendor_id, uint32_t pid)
+{
+	struct sk_buff *skb;
+	struct nlmsghdr	*nlh;
+	struct fc_nl_event *event;
+	u32 len, skblen;
+	int err;
+
+	if (!scsi_nl_sock) {
+		err = -ENOENT;
+		goto send_vendor_fail;
+	}
+
+	len = FC_NL_MSGALIGN(sizeof(*event) + 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;
+	}
+	event = NLMSG_DATA(nlh);
+
+	INIT_SCSI_NL_HDR(&event->snlh, SCSI_NL_TRANSPORT_FC,
+				FC_NL_ASYNC_EVENT, len);
+	event->seconds = get_seconds();
+	event->vendor_id = vendor_id;
+	event->host_no = shost->host_no;
+	event->event_datalen = data_len;	/* bytes */
+	event->event_num = event_number;
+	event->event_code = FCH_EVT_VENDOR_UNIQUE;
+	memcpy(&event->event_data, data_buf, data_len);
+
+	err = nlmsg_unicast(scsi_nl_sock, skb, pid);
+	if (err && (err != -ESRCH))	/* filter no recipient errors */
+		/* nlmsg_multicast already kfree_skb'd */
+		goto send_vendor_fail;
+
+	return;
+
+send_vendor_fail_skb:
+	kfree_skb(skb);
+send_vendor_fail:
+	printk(KERN_WARNING
+		"%s: Dropped Event : host %d vendor_unique - err %d\n",
+		__FUNCTION__, shost->host_no, err);
+	return;
+}
+EXPORT_SYMBOL(fc_host_post_vendor_event_to_pid);
+
 static __init int fc_transport_init(void)
 {
 	int error;
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 44a55d1..04afe93 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -485,6 +485,12 @@ struct scsi_host_template {
 	 * module_init/module_exit.
 	 */
 	struct list_head legacy_hosts;
+	/*
+	 * passes host specific to netlink messages received on scsi_nl_sock
+	 * to the LLD
+	 */
+	int (* netlink_rcv_msg)(struct Scsi_Host *, void *payload, uint32_t len,
+				uint32_t pid);
 };
 
 /*
diff --git a/include/scsi/scsi_netlink.h b/include/scsi/scsi_netlink.h
index 8c1470c..b32596e 100644
--- a/include/scsi/scsi_netlink.h
+++ b/include/scsi/scsi_netlink.h
@@ -42,6 +42,7 @@ struct scsi_nl_hdr {
 	uint16_t magic;
 	uint16_t msgtype;
 	uint16_t msglen;
+	uint16_t host_no;
 } __attribute__((aligned(sizeof(uint64_t))));
 
 /* scsi_nl_hdr->version value */
@@ -56,7 +57,7 @@ struct scsi_nl_hdr {
 #define SCSI_NL_MAX_TRANSPORTS			2
 
 /* scsi_nl_hdr->msgtype values are defined in each transport */
-
+#define SCSI_NL_HOST_PRIVATE			1
 
 /*
  * Vendor ID:
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index 21018a4..217fdc9 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -747,6 +747,8 @@ void fc_host_post_event(struct Scsi_Host *shost, u32 event_number,
 		enum fc_host_event_code event_code, u32 event_data);
 void fc_host_post_vendor_event(struct Scsi_Host *shost, u32 event_number,
 		u32 data_len, char * data_buf, u64 vendor_id);
+void fc_host_post_vendor_event_to_pid(struct Scsi_Host *shost, u32 event_number,
+                u32 data_len, char * data_buf, u64 vendor_id, uint32_t pid);
 	/* Note: when specifying vendor_id to fc_host_post_vendor_event()
 	 *   be sure to read the Vendor Type and ID formatting requirements
 	 *   specified in scsi_netlink.h

RE: [PATCH 1/2] scsi:netlink support in scsi and fc transports for hbaspecific messages

[James.Smart]

[-- Attachment #1: Type: text/plain, Size: 8172 bytes --]

David,

I've had a similar patch floating around now for months, but it was
more for messages destined to the transport. It should easily extend
to driver messages too.

My approach had the drivers registering with the scsi netlink module
rather than using the host template (transports don't have a template,
so its either reg/dereg or a static table). Comparing the two, yours
seems cleaner, as the registration/deregistration creates some
headaches. 
I'm a little bothered that there's nothing that qualifies the driver
to the shost before invoking the LLDD handler (e.g. driver has a
signature, message header contains signature, and the two must match).
But, perhaps that's livable. One thing yours doesn't do is communicate
netlink events to the consumers, so if they were tracking anything 
by pid, they could clean up. You could add this, but it would mean
another template entry point.

I've attached my patch for comparison, although it would need work to
do what you are doing.  Anyone else with a preference on the approach ?

-- james s


> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org 
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of David 
> Somayajulu
> Sent: Wednesday, July 30, 2008 5:54 PM
> To: linux-scsi@vger.kernel.org
> Cc: David Somayajulu; David Wagner
> Subject: [PATCH 1/2] scsi:netlink support in scsi and fc 
> transports for hbaspecific messages
> 
> This patch adds support to the scsi transport layer for 
> passing hba specific netlink messages to a low level driver. 
> Also support is added to the fc transport, to enable an FC 
> hba driver to post netlink message to a specific user process.
> 
> Signed-off-by: David C Somayajulu <david.somayajulu@qlogic.com>
> ---
>  drivers/scsi/scsi_netlink.c      |   13 +++++++
>  drivers/scsi/scsi_transport_fc.c |   72 
> ++++++++++++++++++++++++++++++++++++++
>  include/scsi/scsi_host.h         |    6 +++
>  include/scsi/scsi_netlink.h      |    3 +-
>  include/scsi/scsi_transport_fc.h |    2 +
>  5 files changed, 95 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
> index ae7ed9a..3bc85c9 100644
> --- a/drivers/scsi/scsi_netlink.c
> +++ b/drivers/scsi/scsi_netlink.c
> @@ -24,6 +24,7 @@
>  #include <net/sock.h>
>  #include <net/netlink.h>
>  
> +#include <scsi/scsi_host.h>
>  #include <scsi/scsi_netlink.h>
>  #include "scsi_priv.h"
>  
> @@ -47,6 +48,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
>  	struct scsi_nl_hdr *hdr;
>  	uint32_t rlen;
>  	int err;
> +	struct Scsi_Host *shost;
>  
>  	while (skb->len >= NLMSG_SPACE(0)) {
>  		err = 0;
> @@ -89,6 +91,17 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
>  		/*
>  		 * We currently don't support anyone sending us 
> a message
>  		 */
> +		if (hdr->msgtype == SCSI_NL_HOST_PRIVATE) {
> +			shost = scsi_host_lookup(hdr->host_no);
> +			if (!shost) {
> +				printk(KERN_ERR "%s: 
> scsi_host_lookup failed "
> +					"host no %u\n", 
> __FUNCTION__, hdr->host_no);
> +			} else if (shost->hostt->netlink_rcv_msg) {
> +				err = 
> shost->hostt->netlink_rcv_msg(shost,
> +					(void *)((char 
> *)hdr+sizeof(*hdr)),
> +					hdr->msglen, 
> NETLINK_CREDS(skb)->pid);
> +			}
> +		}
>  
>  next_msg:
>  		if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
> diff --git a/drivers/scsi/scsi_transport_fc.c 
> b/drivers/scsi/scsi_transport_fc.c
> index cb971f0..d1b38d9 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -627,6 +627,78 @@ EXPORT_SYMBOL(fc_host_post_vendor_event);
>  
>  
>  
> +/**
> + * fc_host_post_vendor_event_to_pid - called to post a 
> vendor unique event
> + *			on an fc_host to a specific process
> + * @shost:		host the event occurred on
> + * @event_number:	fc event number obtained from 
> get_fc_event_number()
> + * @data_len:		amount, in bytes, of vendor unique data
> + * @data_buf:		pointer to vendor unique data
> + * @vendor_id:          Vendor id
> + * @pid:		proc id of the receiver
> + *
> + * Notes:
> + *	This routine assumes no locks are held on entry.
> + */
> +void
> +fc_host_post_vendor_event_to_pid(struct Scsi_Host *shost, 
> u32 event_number,
> +		u32 data_len, char * data_buf, u64 vendor_id, 
> uint32_t pid)
> +{
> +	struct sk_buff *skb;
> +	struct nlmsghdr	*nlh;
> +	struct fc_nl_event *event;
> +	u32 len, skblen;
> +	int err;
> +
> +	if (!scsi_nl_sock) {
> +		err = -ENOENT;
> +		goto send_vendor_fail;
> +	}
> +
> +	len = FC_NL_MSGALIGN(sizeof(*event) + 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;
> +	}
> +	event = NLMSG_DATA(nlh);
> +
> +	INIT_SCSI_NL_HDR(&event->snlh, SCSI_NL_TRANSPORT_FC,
> +				FC_NL_ASYNC_EVENT, len);
> +	event->seconds = get_seconds();
> +	event->vendor_id = vendor_id;
> +	event->host_no = shost->host_no;
> +	event->event_datalen = data_len;	/* bytes */
> +	event->event_num = event_number;
> +	event->event_code = FCH_EVT_VENDOR_UNIQUE;
> +	memcpy(&event->event_data, data_buf, data_len);
> +
> +	err = nlmsg_unicast(scsi_nl_sock, skb, pid);
> +	if (err && (err != -ESRCH))	/* filter no recipient errors */
> +		/* nlmsg_multicast already kfree_skb'd */
> +		goto send_vendor_fail;
> +
> +	return;
> +
> +send_vendor_fail_skb:
> +	kfree_skb(skb);
> +send_vendor_fail:
> +	printk(KERN_WARNING
> +		"%s: Dropped Event : host %d vendor_unique - err %d\n",
> +		__FUNCTION__, shost->host_no, err);
> +	return;
> +}
> +EXPORT_SYMBOL(fc_host_post_vendor_event_to_pid);
> +
>  static __init int fc_transport_init(void)
>  {
>  	int error;
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 44a55d1..04afe93 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -485,6 +485,12 @@ struct scsi_host_template {
>  	 * module_init/module_exit.
>  	 */
>  	struct list_head legacy_hosts;
> +	/*
> +	 * passes host specific to netlink messages received on 
> scsi_nl_sock
> +	 * to the LLD
> +	 */
> +	int (* netlink_rcv_msg)(struct Scsi_Host *, void 
> *payload, uint32_t len,
> +				uint32_t pid);
>  };
>  
>  /*
> diff --git a/include/scsi/scsi_netlink.h b/include/scsi/scsi_netlink.h
> index 8c1470c..b32596e 100644
> --- a/include/scsi/scsi_netlink.h
> +++ b/include/scsi/scsi_netlink.h
> @@ -42,6 +42,7 @@ struct scsi_nl_hdr {
>  	uint16_t magic;
>  	uint16_t msgtype;
>  	uint16_t msglen;
> +	uint16_t host_no;
>  } __attribute__((aligned(sizeof(uint64_t))));
>  
>  /* scsi_nl_hdr->version value */
> @@ -56,7 +57,7 @@ struct scsi_nl_hdr {
>  #define SCSI_NL_MAX_TRANSPORTS			2
>  
>  /* scsi_nl_hdr->msgtype values are defined in each transport */
> -
> +#define SCSI_NL_HOST_PRIVATE			1
>  
>  /*
>   * Vendor ID:
> diff --git a/include/scsi/scsi_transport_fc.h 
> b/include/scsi/scsi_transport_fc.h
> index 21018a4..217fdc9 100644
> --- a/include/scsi/scsi_transport_fc.h
> +++ b/include/scsi/scsi_transport_fc.h
> @@ -747,6 +747,8 @@ void fc_host_post_event(struct Scsi_Host 
> *shost, u32 event_number,
>  		enum fc_host_event_code event_code, u32 event_data);
>  void fc_host_post_vendor_event(struct Scsi_Host *shost, u32 
> event_number,
>  		u32 data_len, char * data_buf, u64 vendor_id);
> +void fc_host_post_vendor_event_to_pid(struct Scsi_Host 
> *shost, u32 event_number,
> +                u32 data_len, char * data_buf, u64 
> vendor_id, uint32_t pid);
>  	/* Note: when specifying vendor_id to 
> fc_host_post_vendor_event()
>  	 *   be sure to read the Vendor Type and ID formatting 
> requirements
>  	 *   specified in scsi_netlink.h
> 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[-- Attachment #2: RFC for SCSI netlink expansion.txt --]
[-- Type: text/plain, Size: 5842 bytes --]

--- scsi_netlink.c.orig 2008-05-21 10:34:51.602084000 -0400
+++ scsi_netlink.c      2008-05-27 11:33:46.649668000 -0400
@@ -29,7 +29,10 @@
 
 struct sock *scsi_nl_sock = NULL;
 EXPORT_SYMBOL_GPL(scsi_nl_sock);
-
+static const struct nl_transport_funcs nl_func[SCSI_NL_MAX_TRANSPORTS]
= {
+       [SCSI_NL_TRANSPORT]     = { NULL, NULL },
+       [SCSI_NL_TRANSPORT_FC]  = { NULL, NULL },
+};
 
 /**
  * scsi_nl_rcv_msg - Receive message handler.
@@ -65,7 +68,7 @@
 
                if (nlh->nlmsg_type != SCSI_TRANSPORT_MSG) {
                        err = -EBADMSG;
-                       return;
+                       goto next_msg;
                }
 
                hdr = NLMSG_DATA(nlh);
@@ -86,10 +89,16 @@
                        return;
                }
 
-               /*
-                * We currently don't support anyone sending us a
message
-                */
-
+               if (hdr->transport < SCSI_NL_MAX_TRANSPORTS) {
+                       if (nl_func[hdr->transport].nl_rcv_msg)
+                               nl_func[hdr->transport].nl_rcv_msg(nlh);
+                       else
+                               printk(KERN_ERR
+                                      "%s: NULL netlink rcv function
(%d)\n",
+                                      __func__, hdr->transport);
+               } else
+                       printk(KERN_ERR "%s: Invalid transport ID:
%d\n",
+                              __func__, hdr->transport);
 next_msg:
                if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
                        netlink_ack(skb, nlh, err);
@@ -110,14 +119,15 @@
 scsi_nl_rcv_event(struct notifier_block *this, unsigned long event,
void *ptr)
 {
        struct netlink_notify *n = ptr;
+       int i;
 
        if (n->protocol != NETLINK_SCSITRANSPORT)
                return NOTIFY_DONE;
 
-       /*
-        * Currently, we are not tracking PID's, etc. There is nothing
-        * to handle.
-        */
+       for (i = 0; i < SCSI_NL_MAX_TRANSPORTS; i++) {
+               if (nl_func[i].nl_rcv_event)
+                       nl_func[i].nl_rcv_event(event, ptr);
+       }
 
        return NOTIFY_DONE;
 }
@@ -171,4 +181,66 @@
        return;
 }
 
+/**
+ * scsi_netlink_add_transport - adds a transport to the SCSI netlink
multiplexer
+ * @transport_id:      The transport ID defined in scsi_netlink.h that
+ *    corresponds to the transport. This value is used by applications
sending
+ *    netlink messages to direct them to the correct transport.
+ * @nl_rcv_msg:                The transport's netlink receive message
function.
+ * @nl_rcv_event:      The transport's netlink receive event function.
+ *
+ * Description:
+ * This function registers netlink receive functions for a transport
type. Once
+ * called the SCSI netlink code will forward all received netlink
messages with
+ * a transport value of @transport_id to @nl_rcv_func. All SCSI netlink
events
+ * will be forwarded to the @nl_rcv_event function.
+ *
+ * Return:
+ * A return value of zero indicates successful registration of netlink
receive
+ * functions. A non-zero value indicates an error with input
parameters.
+ */
+int
+scsi_netlink_add_transport(int transport_id,
+                          int (*nl_rcv_msg)(void *),
+                          int (*nl_rcv_event)(unsigned long, void *))
+{
+       if (transport_id >= SCSI_NL_MAX_TRANSPORTS) {
+               printk(KERN_ERR "%s: Invalid transport ID:%d",
+                      __func__, transport_id);
+               return EINVAL;
+       }
+       if (nl_func[transport_id].nl_rcv_msg != NULL ||
+           nl_func[transport_id].nl_rcv_event != NULL) {
+               printk(KERN_ERR "%s: Transport (%d) already registered",
+                      __func__, transport_id);
+               return EINVAL;
+       }
+       nl_func[transport_id].nl_rcv_msg = nl_rcv_msg;
+       nl_func[transport_id].nl_rcv_event = nl_rcv_event;
+       return 0;
+}
+EXPORT_SYMBOL(scsi_netlink_add_transport);
 
+/**
+ * scsi_netlink_remove_transport - removes an entry from the netlink
multiplexer
+ * @transport_id:      The transport ID defined in scsi_netlink.h that
+ *     corresponds to the transport. This value is used by applications
sending
+ *     netlink messages to direct them to the correct transport.
+ *
+ * Description:
+ * Once called the transport indicated by @transport_id will no longer
receive
+ * incoming netlink messages from the SCSI transport netlink.
+ */
+int
+scsi_netlink_remove_transport(int transport_id)
+{
+       if (transport_id >= SCSI_NL_MAX_TRANSPORTS) {
+               printk(KERN_ERR "%s: Invalid transport ID:%d",
+                      __func__, transport_id);
+               return EINVAL;
+       }
+       nl_func[transport_id].nl_rcv_msg = NULL;
+       nl_func[transport_id].nl_rcv_event = NULL;
+       return 0;
+}
+EXPORT_SYMBOL(scsi_netlink_remove_transport);

--- scsi_netlink.h.orig 2008-05-21 10:37:34.635586000 -0400
+++ scsi_netlink.h      2008-05-27 11:32:23.318744000 -0400
@@ -55,6 +55,11 @@
 #define SCSI_NL_TRANSPORT_FC                   1
 #define SCSI_NL_MAX_TRANSPORTS                 2
 
+struct nl_transport_funcs {
+       int     (*nl_rcv_msg)(void *);
+       int     (*nl_rcv_event)(unsigned long, void *);
+};
+
 /* scsi_nl_hdr->msgtype values are defined in each transport */
 
 
@@ -82,6 +87,10 @@
        (hdr)->msglen = mlen;                                   \
        }
 
+int scsi_netlink_add_transport(int transport_id,
+                              int (*)(void *),
+                              int (*)(unsigned long, void *));
+int scsi_netlink_remove_transport(int);
 
 #endif /* SCSI_NETLINK_H */

Re: [PATCH 1/2] scsi:netlink support in scsi and fc transports for hba specific messages

[David Somayajulu]

On Thu, 2008-07-31 at 14:56 -0500, Mike Christie wrote:
> David Somayajulu wrote:
> > This patch adds support to the scsi transport layer for passing hba specific netlink messages to a low level driver. Also support is added to the fc transport, to enable an FC hba driver to post netlink message to a specific user process.

> > +		if (hdr->msgtype == SCSI_NL_HOST_PRIVATE) {
> > +			shost = scsi_host_lookup(hdr->host_no);
> > +			if (!shost) {
> > +				printk(KERN_ERR "%s: scsi_host_lookup failed "
> > +					"host no %u\n", __FUNCTION__, hdr->host_no);
> > +			} else if (shost->hostt->netlink_rcv_msg) {
> > +				err = shost->hostt->netlink_rcv_msg(shost,
> > +					(void *)((char *)hdr+sizeof(*hdr)),
> > +					hdr->msglen, NETLINK_CREDS(skb)->pid);
> > +			}
> > +		}
> 
> Finally getting this done :) Thanks!
> 
> If shost is not null you need to do a scsi_host_put hen you are done 
> with the host.

Incorporating scsi_host_put() as pointed by Mike Christie. (Thanks Mike)

Signed-off-by: David C Somayajulu <david.somayajulu@qlogic.com>
---
 drivers/scsi/scsi_netlink.c      |   14 +++++++
 drivers/scsi/scsi_transport_fc.c |   72 ++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_host.h         |    6 +++
 include/scsi/scsi_netlink.h      |    3 +-
 include/scsi/scsi_transport_fc.h |    2 +
 5 files changed, 96 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
index ae7ed9a..fea328c 100644
--- a/drivers/scsi/scsi_netlink.c
+++ b/drivers/scsi/scsi_netlink.c
@@ -24,6 +24,7 @@
 #include <net/sock.h>
 #include <net/netlink.h>
 
+#include <scsi/scsi_host.h>
 #include <scsi/scsi_netlink.h>
 #include "scsi_priv.h"
 
@@ -47,6 +48,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 	struct scsi_nl_hdr *hdr;
 	uint32_t rlen;
 	int err;
+	struct Scsi_Host *shost;
 
 	while (skb->len >= NLMSG_SPACE(0)) {
 		err = 0;
@@ -89,6 +91,18 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
 		/*
 		 * We currently don't support anyone sending us a message
 		 */
+		if (hdr->msgtype == SCSI_NL_HOST_PRIVATE) {
+			shost = scsi_host_lookup(hdr->host_no);
+			if (!shost) {
+				printk(KERN_ERR "%s: scsi_host_lookup failed "
+					"host no %u\n", __FUNCTION__, hdr->host_no);
+			} else if (shost->hostt->netlink_rcv_msg) {
+				err = shost->hostt->netlink_rcv_msg(shost,
+					(void *)((char *)hdr+sizeof(*hdr)),
+					hdr->msglen, NETLINK_CREDS(skb)->pid);
+				scsi_host_put(shost);
+			}
+		}
 
 next_msg:
 		if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index cb971f0..d1b38d9 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -627,6 +627,78 @@ EXPORT_SYMBOL(fc_host_post_vendor_event);
 
 
 
+/**
+ * fc_host_post_vendor_event_to_pid - called to post a vendor unique event
+ *			on an fc_host to a specific process
+ * @shost:		host the event occurred on
+ * @event_number:	fc event number obtained from get_fc_event_number()
+ * @data_len:		amount, in bytes, of vendor unique data
+ * @data_buf:		pointer to vendor unique data
+ * @vendor_id:          Vendor id
+ * @pid:		proc id of the receiver
+ *
+ * Notes:
+ *	This routine assumes no locks are held on entry.
+ */
+void
+fc_host_post_vendor_event_to_pid(struct Scsi_Host *shost, u32 event_number,
+		u32 data_len, char * data_buf, u64 vendor_id, uint32_t pid)
+{
+	struct sk_buff *skb;
+	struct nlmsghdr	*nlh;
+	struct fc_nl_event *event;
+	u32 len, skblen;
+	int err;
+
+	if (!scsi_nl_sock) {
+		err = -ENOENT;
+		goto send_vendor_fail;
+	}
+
+	len = FC_NL_MSGALIGN(sizeof(*event) + 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;
+	}
+	event = NLMSG_DATA(nlh);
+
+	INIT_SCSI_NL_HDR(&event->snlh, SCSI_NL_TRANSPORT_FC,
+				FC_NL_ASYNC_EVENT, len);
+	event->seconds = get_seconds();
+	event->vendor_id = vendor_id;
+	event->host_no = shost->host_no;
+	event->event_datalen = data_len;	/* bytes */
+	event->event_num = event_number;
+	event->event_code = FCH_EVT_VENDOR_UNIQUE;
+	memcpy(&event->event_data, data_buf, data_len);
+
+	err = nlmsg_unicast(scsi_nl_sock, skb, pid);
+	if (err && (err != -ESRCH))	/* filter no recipient errors */
+		/* nlmsg_multicast already kfree_skb'd */
+		goto send_vendor_fail;
+
+	return;
+
+send_vendor_fail_skb:
+	kfree_skb(skb);
+send_vendor_fail:
+	printk(KERN_WARNING
+		"%s: Dropped Event : host %d vendor_unique - err %d\n",
+		__FUNCTION__, shost->host_no, err);
+	return;
+}
+EXPORT_SYMBOL(fc_host_post_vendor_event_to_pid);
+
 static __init int fc_transport_init(void)
 {
 	int error;
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 44a55d1..04afe93 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -485,6 +485,12 @@ struct scsi_host_template {
 	 * module_init/module_exit.
 	 */
 	struct list_head legacy_hosts;
+	/*
+	 * passes host specific to netlink messages received on scsi_nl_sock
+	 * to the LLD
+	 */
+	int (* netlink_rcv_msg)(struct Scsi_Host *, void *payload, uint32_t len,
+				uint32_t pid);
 };
 
 /*
diff --git a/include/scsi/scsi_netlink.h b/include/scsi/scsi_netlink.h
index 8c1470c..b32596e 100644
--- a/include/scsi/scsi_netlink.h
+++ b/include/scsi/scsi_netlink.h
@@ -42,6 +42,7 @@ struct scsi_nl_hdr {
 	uint16_t magic;
 	uint16_t msgtype;
 	uint16_t msglen;
+	uint16_t host_no;
 } __attribute__((aligned(sizeof(uint64_t))));
 
 /* scsi_nl_hdr->version value */
@@ -56,7 +57,7 @@ struct scsi_nl_hdr {
 #define SCSI_NL_MAX_TRANSPORTS			2
 
 /* scsi_nl_hdr->msgtype values are defined in each transport */
-
+#define SCSI_NL_HOST_PRIVATE			1
 
 /*
  * Vendor ID:
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index 21018a4..217fdc9 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -747,6 +747,8 @@ void fc_host_post_event(struct Scsi_Host *shost, u32 event_number,
 		enum fc_host_event_code event_code, u32 event_data);
 void fc_host_post_vendor_event(struct Scsi_Host *shost, u32 event_number,
 		u32 data_len, char * data_buf, u64 vendor_id);
+void fc_host_post_vendor_event_to_pid(struct Scsi_Host *shost, u32 event_number,
+                u32 data_len, char * data_buf, u64 vendor_id, uint32_t pid);
 	/* Note: when specifying vendor_id to fc_host_post_vendor_event()
 	 *   be sure to read the Vendor Type and ID formatting requirements
 	 *   specified in scsi_netlink.h

Re: [PATCH 1/2] scsi:netlink support in scsi and fc transports for hbaspecific messages

[David Somayajulu]

On 7/30/08 7:09 PM, "James.Smart@Emulex.Com" <James.Smart@Emulex.Com> wrote:
James,
Thanks for the feedback.
> David,
> 
> I've had a similar patch floating around now for months, but it was
> more for messages destined to the transport. It should easily extend
> to driver messages too.
> 
> My approach had the drivers registering with the scsi netlink module
> rather than using the host template (transports don't have a template,
> so its either reg/dereg or a static table). Comparing the two, yours
> seems cleaner, as the registration/deregistration creates some
> headaches. 
> I'm a little bothered that there's nothing that qualifies the driver
> to the shost before invoking the LLDD handler (e.g. driver has a
> signature, message header contains signature, and the two must match).
The idea was to let the Low Level Driver validate the message contents -
signature, etc. Since this is a private message to the LLD, I felt it was
appropriate that the signature or any other message validation scheme should
be left to the LLD. The scsi-ml should not be responsible for it.
> But, perhaps that's livable. One thing yours doesn't do is communicate
> netlink events to the consumers, so if they were tracking anything
> by pid, they could clean up. You could add this, but it would mean
> another template entry point.
I would appreciate if you can explain your comment a bit more, if my
reasoning below does not suffice. I have already provided a function
"fc_host_post_vendor_event_to_pid() [in scsi_transport_fc.c]", which enables
an LLD to post a message to a specific pid. This may be used by the LLD to
send response messages to the pid, for command messages sent by the pid. It
can also be used to post any event messages, provided the user process(pid)
has already informed the LLD to post event messages to it (in this case the
LLD can cache the pid value and use it to post event messages to it). I am
not sure why another template entry point is needed.
> 
> I've attached my patch for comparison, although it would need work to
> do what you are doing.  Anyone else with a preference on the approach ?
> 
> -- james s
> 
> 
>> -----Original Message-----
>> From: linux-scsi-owner@vger.kernel.org
>> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of David
>> Somayajulu
>> Sent: Wednesday, July 30, 2008 5:54 PM
>> To: linux-scsi@vger.kernel.org
>> Cc: David Somayajulu; David Wagner
>> Subject: [PATCH 1/2] scsi:netlink support in scsi and fc
>> transports for hbaspecific messages
>> 
>> This patch adds support to the scsi transport layer for
>> passing hba specific netlink messages to a low level driver.
>> Also support is added to the fc transport, to enable an FC
>> hba driver to post netlink message to a specific user process.
>> 
>> Signed-off-by: David C Somayajulu <david.somayajulu@qlogic.com>
>> ---
>>  drivers/scsi/scsi_netlink.c      |   13 +++++++
>>  drivers/scsi/scsi_transport_fc.c |   72
>> ++++++++++++++++++++++++++++++++++++++
>>  include/scsi/scsi_host.h         |    6 +++
>>  include/scsi/scsi_netlink.h      |    3 +-
>>  include/scsi/scsi_transport_fc.h |    2 +
>>  5 files changed, 95 insertions(+), 1 deletions(-)
>> 
>> diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
>> index ae7ed9a..3bc85c9 100644
>> --- a/drivers/scsi/scsi_netlink.c
>> +++ b/drivers/scsi/scsi_netlink.c
>> @@ -24,6 +24,7 @@
>>  #include <net/sock.h>
>>  #include <net/netlink.h>
>>  
>> +#include <scsi/scsi_host.h>
>>  #include <scsi/scsi_netlink.h>
>>  #include "scsi_priv.h"
>>  
>> @@ -47,6 +48,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
>> struct scsi_nl_hdr *hdr;
>> uint32_t rlen;
>> int err;
>> + struct Scsi_Host *shost;
>>  
>> while (skb->len >= NLMSG_SPACE(0)) {
>> err = 0;
>> @@ -89,6 +91,17 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
>> /*
>> * We currently don't support anyone sending us
>> a message
>> */
>> +  if (hdr->msgtype == SCSI_NL_HOST_PRIVATE) {
>> +   shost = scsi_host_lookup(hdr->host_no);
>> +   if (!shost) {
>> +    printk(KERN_ERR "%s:
>> scsi_host_lookup failed "
>> +     "host no %u\n",
>> __FUNCTION__, hdr->host_no);
>> +   } else if (shost->hostt->netlink_rcv_msg) {
>> +    err = 
>> shost->hostt->netlink_rcv_msg(shost,
>> +     (void *)((char
>> *)hdr+sizeof(*hdr)),
>> +     hdr->msglen,
>> NETLINK_CREDS(skb)->pid);
>> +   }
>> +  }
>>  
>>  next_msg:
>> if ((err) || (nlh->nlmsg_flags & NLM_F_ACK))
>> diff --git a/drivers/scsi/scsi_transport_fc.c
>> b/drivers/scsi/scsi_transport_fc.c
>> index cb971f0..d1b38d9 100644
>> --- a/drivers/scsi/scsi_transport_fc.c
>> +++ b/drivers/scsi/scsi_transport_fc.c
>> @@ -627,6 +627,78 @@ EXPORT_SYMBOL(fc_host_post_vendor_event);
>>  
>>  
>>  
>> +/**
>> + * fc_host_post_vendor_event_to_pid - called to post a
>> vendor unique event
>> + *   on an fc_host to a specific process
>> + * @shost:  host the event occurred on
>> + * @event_number: fc event number obtained from
>> get_fc_event_number()
>> + * @data_len:  amount, in bytes, of vendor unique data
>> + * @data_buf:  pointer to vendor unique data
>> + * @vendor_id:          Vendor id
>> + * @pid:  proc id of the receiver
>> + *
>> + * Notes:
>> + * This routine assumes no locks are held on entry.
>> + */
>> +void
>> +fc_host_post_vendor_event_to_pid(struct Scsi_Host *shost,
>> u32 event_number,
>> +  u32 data_len, char * data_buf, u64 vendor_id,
>> uint32_t pid)
>> +{
>> + struct sk_buff *skb;
>> + struct nlmsghdr *nlh;
>> + struct fc_nl_event *event;
>> + u32 len, skblen;
>> + int err;
>> +
>> + if (!scsi_nl_sock) {
>> +  err = -ENOENT;
>> +  goto send_vendor_fail;
>> + }
>> +
>> + len = FC_NL_MSGALIGN(sizeof(*event) + 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;
>> + }
>> + event = NLMSG_DATA(nlh);
>> +
>> + INIT_SCSI_NL_HDR(&event->snlh, SCSI_NL_TRANSPORT_FC,
>> +    FC_NL_ASYNC_EVENT, len);
>> + event->seconds = get_seconds();
>> + event->vendor_id = vendor_id;
>> + event->host_no = shost->host_no;
>> + event->event_datalen = data_len; /* bytes */
>> + event->event_num = event_number;
>> + event->event_code = FCH_EVT_VENDOR_UNIQUE;
>> + memcpy(&event->event_data, data_buf, data_len);
>> +
>> + err = nlmsg_unicast(scsi_nl_sock, skb, pid);
>> + if (err && (err != -ESRCH)) /* filter no recipient errors */
>> +  /* nlmsg_multicast already kfree_skb'd */
>> +  goto send_vendor_fail;
>> +
>> + return;
>> +
>> +send_vendor_fail_skb:
>> + kfree_skb(skb);
>> +send_vendor_fail:
>> + printk(KERN_WARNING
>> +  "%s: Dropped Event : host %d vendor_unique - err %d\n",
>> +  __FUNCTION__, shost->host_no, err);
>> + return;
>> +}
>> +EXPORT_SYMBOL(fc_host_post_vendor_event_to_pid);
>> +
>>  static __init int fc_transport_init(void)
>>  {
>> int error;
>> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
>> index 44a55d1..04afe93 100644
>> --- a/include/scsi/scsi_host.h
>> +++ b/include/scsi/scsi_host.h
>> @@ -485,6 +485,12 @@ struct scsi_host_template {
>> * module_init/module_exit.
>> */
>> struct list_head legacy_hosts;
>> + /*
>> +  * passes host specific to netlink messages received on
>> scsi_nl_sock
>> +  * to the LLD
>> +  */
>> + int (* netlink_rcv_msg)(struct Scsi_Host *, void
>> *payload, uint32_t len,
>> +    uint32_t pid);
>>  };
>>  
>>  /*
>> diff --git a/include/scsi/scsi_netlink.h b/include/scsi/scsi_netlink.h
>> index 8c1470c..b32596e 100644
>> --- a/include/scsi/scsi_netlink.h
>> +++ b/include/scsi/scsi_netlink.h
>> @@ -42,6 +42,7 @@ struct scsi_nl_hdr {
>> uint16_t magic;
>> uint16_t msgtype;
>> uint16_t msglen;
>> + uint16_t host_no;
>>  } __attribute__((aligned(sizeof(uint64_t))));
>>  
>>  /* scsi_nl_hdr->version value */
>> @@ -56,7 +57,7 @@ struct scsi_nl_hdr {
>>  #define SCSI_NL_MAX_TRANSPORTS   2
>>  
>>  /* scsi_nl_hdr->msgtype values are defined in each transport */
>> -
>> +#define SCSI_NL_HOST_PRIVATE   1
>>  
>>  /*
>>   * Vendor ID:
>> diff --git a/include/scsi/scsi_transport_fc.h
>> b/include/scsi/scsi_transport_fc.h
>> index 21018a4..217fdc9 100644
>> --- a/include/scsi/scsi_transport_fc.h
>> +++ b/include/scsi/scsi_transport_fc.h
>> @@ -747,6 +747,8 @@ void fc_host_post_event(struct Scsi_Host
>> *shost, u32 event_number,
>> enum fc_host_event_code event_code, u32 event_data);
>>  void fc_host_post_vendor_event(struct Scsi_Host *shost, u32
>> event_number,
>> u32 data_len, char * data_buf, u64 vendor_id);
>> +void fc_host_post_vendor_event_to_pid(struct Scsi_Host
>> *shost, u32 event_number,
>> +                u32 data_len, char * data_buf, u64
>> vendor_id, uint32_t pid);
>> /* Note: when specifying vendor_id to
>> fc_host_post_vendor_event()
>> *   be sure to read the Vendor Type and ID formatting
>> requirements
>> *   specified in scsi_netlink.h
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 

Re: [PATCH 1/2] scsi:netlink support in scsi and fc transports for hba specific messages

[Mike Christie]

David Somayajulu wrote:
> This patch adds support to the scsi transport layer for passing hba specific netlink messages to a low level driver. Also support is added to the fc transport, to enable an FC hba driver to post netlink message to a specific user process.
> 
> Signed-off-by: David C Somayajulu <david.somayajulu@qlogic.com>
> ---
>  drivers/scsi/scsi_netlink.c      |   13 +++++++
>  drivers/scsi/scsi_transport_fc.c |   72 ++++++++++++++++++++++++++++++++++++++
>  include/scsi/scsi_host.h         |    6 +++
>  include/scsi/scsi_netlink.h      |    3 +-
>  include/scsi/scsi_transport_fc.h |    2 +
>  5 files changed, 95 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
> index ae7ed9a..3bc85c9 100644
> --- a/drivers/scsi/scsi_netlink.c
> +++ b/drivers/scsi/scsi_netlink.c
> @@ -24,6 +24,7 @@
>  #include <net/sock.h>
>  #include <net/netlink.h>
>  
> +#include <scsi/scsi_host.h>
>  #include <scsi/scsi_netlink.h>
>  #include "scsi_priv.h"
>  
> @@ -47,6 +48,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
>  	struct scsi_nl_hdr *hdr;
>  	uint32_t rlen;
>  	int err;
> +	struct Scsi_Host *shost;
>  
>  	while (skb->len >= NLMSG_SPACE(0)) {
>  		err = 0;
> @@ -89,6 +91,17 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
>  		/*
>  		 * We currently don't support anyone sending us a message
>  		 */
> +		if (hdr->msgtype == SCSI_NL_HOST_PRIVATE) {
> +			shost = scsi_host_lookup(hdr->host_no);
> +			if (!shost) {
> +				printk(KERN_ERR "%s: scsi_host_lookup failed "
> +					"host no %u\n", __FUNCTION__, hdr->host_no);
> +			} else if (shost->hostt->netlink_rcv_msg) {
> +				err = shost->hostt->netlink_rcv_msg(shost,
> +					(void *)((char *)hdr+sizeof(*hdr)),
> +					hdr->msglen, NETLINK_CREDS(skb)->pid);
> +			}
> +		}

Finally getting this done :) Thanks!

If shost is not null you need to do a scsi_host_put hen you are done 
with the host.

RE: [PATCH 1/2] scsi:netlink support in scsi and fc transports for hbaspecific messages

[James.Smart]

 

David Somayajulu wrote:
> > I'm a little bothered that there's nothing that qualifies the driver
> > to the shost before invoking the LLDD handler (e.g. driver has a
> > signature, message header contains signature, and the two 
> must match).
> The idea was to let the Low Level Driver validate the message 
> contents -
> signature, etc.

But that's the point - if we have a common action and a common point,
lets
do it once. Why have all LLDs perform the same kind of thing but in
completely different manners ? And worse, what happens if one doesn't
validate ?  The check doesn't have to be extensive, and the LLD
can certainly do more checks.

> I would appreciate if you can explain your comment a bit more, if my
> reasoning below does not suffice. I have already provided a function
> "fc_host_post_vendor_event_to_pid() [in 
> scsi_transport_fc.c]", which enables
> an LLD to post a message to a specific pid. This may be used 
> by the LLD to
> send response messages to the pid, for command messages sent 
> by the pid. 

it's not about sending...

What happens if the pid unexpectedly dies ? The event notices are an
easy way to find out that it died - thus releasing the "cached pid", or
terminating partial transactions, and not abusing the netlink socket
with data destined to a dead pid.

-- james s

Re: [PATCH 1/2] scsi:netlink support in scsi and fc transports for hbaspecific messages

[David Somayajulu]

On 7/31/08 5:35 PM, "James.Smart@Emulex.Com" <James.Smart@Emulex.Com> wrote:

>  
> 
> David Somayajulu wrote:
>>> I'm a little bothered that there's nothing that qualifies the driver
>>> to the shost before invoking the LLDD handler (e.g. driver has a
>>> signature, message header contains signature, and the two
>> must match).
>> The idea was to let the Low Level Driver validate the message
>> contents -
>> signature, etc.
> 
> But that's the point - if we have a common action and a common point,
> lets
> do it once. Why have all LLDs perform the same kind of thing but in
> completely different manners ? And worse, what happens if one doesn't
> validate ?  The check doesn't have to be extensive, and the LLD
> can certainly do more checks.
In that case is it o.k to have the first 4 bytes of the LLD payload (i.e.,
following struct scsi_nl_hdr) define a signature. Let us also define the
signature to be the first 4 bytes in shost->hostt->name. This we can still
have some validation without much overhead.
> 
>> I would appreciate if you can explain your comment a bit more, if my
>> reasoning below does not suffice. I have already provided a function
>> "fc_host_post_vendor_event_to_pid() [in
>> scsi_transport_fc.c]", which enables
>> an LLD to post a message to a specific pid. This may be used
>> by the LLD to
>> send response messages to the pid, for command messages sent
>> by the pid. 
> 
> it's not about sending...
> 
> What happens if the pid unexpectedly dies ? The event notices are an
> easy way to find out that it died - thus releasing the "cached pid", or
> terminating partial transactions, and not abusing the netlink socket
> with data destined to a dead pid.
Good point. I will add a check to make sure we have a live pid.
Thanks
David S. 

RE: [PATCH 1/2] scsi:netlink support in scsi and fc transports forhba specific messages

[James.Smart]

David,

The more I look at the patch, the more it's not done correctly.

For example:
- The rcv function jumps to the msg_type, and thereby jumps over the
  transport designator. This should be a msg_type under the generic
  SCSI Transport (aka SCSI_NL_TRANSPORT). We shouldn't steal msg_types
  from the transports. There was a layering in the design, granted it
  wasn't used much yet, that was subverted by this.
- The message needs to be it's own message type - not munged into the
  FC event message.
- The send function, should not be a function under the fc transport,
  but rather a generic function exported by the scsi netlink module.
- Your recently added scsi_host_put() was very wrong. You never
  had a corresponding get().
 
I'm testing a patch which corrects the above, and adds the following:
- adds generic support for any transport to receive messages, and
  send to pid
- the shost vendor message is a now a message type of the generic
  scsi transport
- adds checks for vendor_id to the reception of vendor message
- adds event propogation to the scsi hosts too
- updates shost for the netlink rcv function, event function, vendorid
- Keeps the basic semantics you already had for entry into the shost.

I'll post it when testing is complete.

-- james s

RE: [PATCH 1/2] scsi:netlink support in scsi and fc transports forhba specific messages

[David Somayajulu]

"James.Smart@Emulex.Com" <James.Smart@Emulex.Com> wrote:
> 
> David,
> 
> The more I look at the patch, the more it's not done correctly.
> 
> For example:
> - The rcv function jumps to the msg_type, and thereby jumps over the
>   transport designator. This should be a msg_type under the generic
>   SCSI Transport (aka SCSI_NL_TRANSPORT). We shouldn't steal msg_types
>   from the transports. There was a layering in the design, granted it
>   wasn't used much yet, that was subverted by this.
Isn't this what my patch does - as msg type on SCSI_NL_TRANSPORT.
> - The message needs to be it's own message type - not munged into the
>   FC event message.
I agree on the send side, if you want everything to be under scsi
transport. For receive side my patch is fine. 
> - The send function, should not be a function under the fc transport,
>   but rather a generic function exported by the scsi netlink module.
I agree it would better if the entire solution is under the scsi
transport.
> - Your recently added scsi_host_put() was very wrong. You never
>   had a corresponding get().
Oops. The version I was looking at for cross reference had a get() in
scsi_host_lookup() and no corresponding put(). Looks like
scsi_host_lookup() has changed at some point. (I think Mike Christie was
under the same impression when he suggested this).
> 
> I'm testing a patch which corrects the above, and adds the following:
> - adds generic support for any transport to receive messages, and
>   send to pid
> - the shost vendor message is a now a message type of the generic
>   scsi transport
> - adds checks for vendor_id to the reception of vendor message
> - adds event propogation to the scsi hosts too
> - updates shost for the netlink rcv function, event function, vendorid
> - Keeps the basic semantics you already had for entry into the shost.
> 
> I'll post it when testing is complete.

Thanks
David S.

Re: [PATCH 1/2] scsi:netlink support in scsi and fc transports forhba specific messages

[Mike Christie]

David Somayajulu wrote:
>> - Your recently added scsi_host_put() was very wrong. You never
>>   had a corresponding get().
> Oops. The version I was looking at for cross reference had a get() in
> scsi_host_lookup() and no corresponding put(). Looks like
> scsi_host_lookup() has changed at some point. (I think Mike Christie was
> under the same impression when he suggested this).

I think you guys are missing the get() from class_find_device or what 
versions are you guys looking at? I have this from scsi-misc from today:

         cdev = class_find_device(&shost_class, &hostnum, 
__scsi_host_match);
         if (cdev) {
                 shost = scsi_host_get(class_to_shost(cdev));
                 put_device(cdev);
         }

If class_find_device finds a device class_find_device does a get on it. +1

If class_find_device found a device (cdev is non null) 
scsi_host_lookup() does another get() on it if the host is not being 
deleted. +1 (total =2)

scsi_host_lookup will then release the get() done from 
class_find_device. -1 (total =1)

So if scsi_host_lookup returns with a host there will be a reference on 
the host that must be released when we are done with it.